Open pkhry opened 3 months ago
To summarize my understanding of the issue here:
Say you have a generic type like Foo<T>
. The Encode
( and presumably Decode
) derives will (AFAIU) scan through each of the fields of Foo<T>
and add where
bounds for any generic-containing-types found in the fields.
In order to avoid an issue, it will skip doing this when the field is recursively referencing itself (ie when Foo<T>
is mentioned) because that would never resolve. However, if a type references itself recursively using a path (eg super::foo::Foo<T>
) and not just ident (Foo<T>
), then this skip check fails and we end up spitting out where bounds like Box<Foo<T>>: Encode
, which will never work out since they are recursive.
The suggested fix above won't quite work because what if we have two types with the same name in different modules that reference eachother (eg Foo<T> { inner: Box<some::other::path::Foo<T>> }
.
Can we simplify the where
bounds that we are spitting out though to avoid this issue entirely?
@ascjones you may know something about this :)
The problem is the algorithm that tries to determine the trait bounds. It tries to be better than what we have in the standard library. However, we don't access to the type system in macros and we need to "guess".
For the situation like you are facing, you can use the attribute codec(dumb_trait_bound)
. This should fix the compilation for you.
My feeling is that we should simplify the trait bound stuff in this lib so that it is a bit dumber by default and just applies basic bounds on each generic like the std library, and then let users use #[codec(..)]
if they want to be smarter. Trying to be smarter like we are here opens the door for werid bugs like this one, because you have to do stuffl ike matching on idents.
Maybe there were good reasons to make it smarter though which I'm missing, ie maybe we did it to avoid issues where we use generic associated types like struct Foo<T: Config>
which I imagine being a bit more common in Substrate?
For the situation like you are facing, you can use the attribute codec(dumb_trait_bound). This should fix the compilation for you.
Yeah good point, we'll maybe do that in our codegen to avoid this issue :)
Oh!! #[codec(dumb_trait_bound)]
is actually an attribute! I thought that you were just meaning to use #[codec(trait_bounds(A: Decode, B: Decode))]
or whatever and "dumb_trait_bound" was just a dummy placeholder :D
Error encountered
```rust error[E0277]: the trait bound `BrakesOthersMinimal Repro:
Produces trait with extra bounds(expansion below, note extra
Containter<Self>
bounds introduced):Click me(`Encode` as the example, although same happens for `Decode` too)
```rust // Recursive expansion of DeriveEncode macro // ========================================== #[allow(deprecated)] const _: () = { #[automatically_derived] impl<_1> ::parity_scale_codec::Encode for BrakesOthers<_1> where Boxchanging this line to
seems to solve the issue of correctly detecting the recursive ty_ident.
Tests pass after the change, however it might break things not tested in the ecosystem(?) also see: paritytech/subxt#1603