Add StaticTypeInfo convenience trait

[ascjones]

Adds a convenience trait StaticTypeInfo to avoid requiring the user to add TypeInfo + 'static to generic bounds everywhere.

This is done instead of adding it directly to TypeInfo to leave that trait open for a future where the 'static constraint is no longer required.

[dvdplm]

Looks great. Gut reaction is "Why on earth didn't we do this sooner?", but maybe there was a reason? One since overcome?

[gui1117]

it also makes sense to me

[Robbepop]

One con is that this design would disallow support for Encode-only types that hold references.
An example of such a type is ink! event types. However, they are not (yet) implemented in ink!, so this PR wouldn't break anything. It would just be a showstopper for this kind of type.

Another solution is to introduce another trait for dependencies that has scale_info::TypeInfo + 'static as super traits with an auto impl.

pub trait StaticTypeInfo: scale_info::TypeInfo + 'static {}
impl<T> StaticTypeInfo for T
where
    T: scale_info::TypeInfo + 'static,
{}

In my opinion it is a good design principle to keep traits as open and non-restricted as possible. Introducing 'static bound is an artificial constraint used to make some code simpler to read but preventing use cases and thus being less general and less applicable.

got convinced by Robbepop message above, didn't tought about type with lifetimes (I only thought about dyn stuff)

[ascjones]

One con is that this design would disallow support for Encode-only types that hold references.

How so? Do you mean e.g.

    #[derive(TypeInfo)]
    struct S<'a, T> {
        f: &'a T,
    }

If so, that case is already handled because we restrict all lifetimes to 'static for the derived impl (expanded):

 #[allow(unused)]
    struct S<'a, T> {
        f: &'a T,
    }
    #[allow(non_upper_case_globals, unused_attributes, unused_qualifications)]
    const _: () = {
        impl<'a, T> ::scale_info::TypeInfo for S<'a, T>
        where
            'a: 'static, // <-------------------------------- lifetime bound to 'static
            &'a T: ::scale_info::TypeInfo,
            T: ::scale_info::TypeInfo,
        {
            type Identity = Self;
            fn type_info() -> ::scale_info::Type {
                ::scale_info::Type::builder()
                    .path(::scale_info::Path::new("S", "derive"))
                    .type_params(<[_]>::into_vec(box [::scale_info::meta_type::<T>()]))
                    .composite(
                        ::scale_info::build::Fields::named()
                            .field_of::<&'static T>("f", "&\'static T"),
                    )
            }
        };
    };

Note that the current restriction to 'static already exists because of the bounds on any::TypeId so this change is just making it explicit.

Another solution is to introduce another trait for dependencies that has scale_info::TypeInfo + 'static as super traits with an auto impl.

That could work, it would leave TypeInfo open for possible utilization of non static lifetimes if we ever do need that information and if TypeId removes the static bound (which the docs says it might do in the future). Though I would say that from a SCALE description perspective lifetimes are not a concept since &T is just encoded as T. And even when generating types from the metadata we would generate owned fields only for decoding purposes.

[Robbepop]

Note that the current restriction to 'static already exists because of the bounds on any::TypeId so this change is just making it explicit.

This is just an implementation detail of how we currently implement this. If we go down the path to support references we could do much better than we do at the moment. The question is: Is it worthwhile. I think it isn't but our design should also not prevent future use cases in my opinion. Since this PR tackles a quality of life improvement I doubt that it will be a really good design decision in the long run. Rather I'd prefer the mentioned solution to be implemented by dependencies if they think it is useful to them but keep the base functionality clean and simple.

If we stick to the notion of scale_info being a meta-level on top of SCALE codec, then having a 'static bound on TypeInfo trait would only really make sense (imo) if both scale::Encode and scale::Decode would have this same bound.

[dvdplm]

I think this PR is a little more than QoL and reducing noise (which is not bad per se). As @ascjones example illustrates there is an underlying assumption of things being 'static already and making this explicit is good imo.

That said, I do agree with the design principle of keeping traits as open as they can be, but I don't think that this should trump the ergonomics of the library. If we can make pub trait StaticTypeInfo: scale_info::TypeInfo + 'static {} work well then I'm all for it; if it becomes a hassle to use, then not so much.

We don't have to make a 1.0 release that stays backwards compatible indefinitely, it is ok to make breaking changes when the circumstances warrant it, so I'm not overly concerned by "wrong" design decisions tbf.

[Robbepop]

As @ascjones example illustrates there is an underlying assumption of things being 'static already and making this explicit is good imo.

This is not an assumption resulting from the essence of what this trait represents but a mere implementation detail that could change. For example this is induced by the fact that the current implementaiton of meta_type demands such a bound. However, as stated in another discussion this might change in the future and also for reference types we could provide meta_type_ref and meta_type_mut that each drop this bound:

fn meta_type_ref<'a, T>(ty: &'a T) -> TypeInfo
where
    T: 'static, // Required for Rust's TypeInfo
{}

With Rust specialization we wouldn't even need such a new function.

[ascjones]

@Robbepop changed PR to just add this StaticTypeInfo trait.

[Robbepop]

@Robbepop changed PR to just add this StaticTypeInfo trait.

What is the gain of adding this trait in scale_info crate instead of adding it in a dependency that has a need for it?

[gui1117]

I think a lot of dependency which implements TypeInfo on some types will like to make use of it, no ?

[ascjones]

Yeah just convenience really, because at the moment you need it everywhere (just search TypeInfo + 'static in ink! for example).

Also imagine if in the future we remove the 'static requirement we can do a breaking upgrade which removes this trait, forcing users to replace with plain TypeInfo.

[Robbepop]

I am not convinced to put this convenience trait into this crate but it sounds like a compromise to me to move forward. So I approve.