Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid generation of types with >1 generic #550

Closed
hussein-aitlahcen opened this issue May 26, 2022 · 14 comments · Fixed by #1023
Closed

Invalid generation of types with >1 generic #550

hussein-aitlahcen opened this issue May 26, 2022 · 14 comments · Fixed by #1023
Assignees
Labels
bug Something isn't working

Comments

@hussein-aitlahcen
Copy link

hussein-aitlahcen commented May 26, 2022

Description

(a). If a type has >1 generic, the >1 types are considered phantoms.
(b). More than considering them phantoms, ALL the fields using >1 generic in index are coerced back to the 0th one (i.e. with the below example, a, b, c, d: T. This is caused by (d).
(c). This result in non-derived Encode/Decode for the given type.

(d). Worth to mention, the TrackedSymbol ids are all the same and it looks like they are comming from TypeInfo derive macro. Not sure this is a normal behavior either.

I'm currently trying to fix the issue.

Reproducible snippet

#[allow(unused)]
#[derive(TypeInfo)]
struct Foo<T, U, V, W> {
    a: T,
    b: U,
    c: V,
    d: W
}

Test case result

Diff < left / right > :
<pub mod tests { use super :: root ; # [derive (:: subxt :: codec :: Decode , :: subxt :: codec :: Encode , Debug)] pub struct Bar { pub p : root :: subxt_codegen :: types :: tests :: Foo < :: core :: primitive :: u32 , :: core :: primitive :: u32 , :: core :: primitive :: u64 , :: core :: primitive :: u128 > , pub q : root :: subxt_codegen :: types :: tests :: Foo < :: core :: primitive :: u8 , :: core :: primitive :: u8 , :: core :: primitive :: u8 , :: core :: primitive :: u8 > , } # [derive (:: subxt :: codec :: Decode , :: subxt :: codec :: Encode , Debug)] pub struct Foo < _0 , _1 , _2 , _3 > { pub a : _0 , pub b : _0 , pub c : _0 , pub d : _0 , # [codec (skip)] pub __subxt_unused_type_params : :: core :: marker :: PhantomData < (_1 , _2 , _3) > } }
>pub mod tests { use super :: root ; # [derive (:: subxt :: codec :: Decode , :: subxt :: codec :: Encode , Debug)] pub struct Bar { pub p : root :: subxt_codegen :: types :: tests :: Foo < :: core :: primitive :: u32 , :: core :: primitive :: u32 , :: core :: primitive :: u64 , :: core :: primitive :: u128 > , pub q : root :: subxt_codegen :: types :: tests :: Foo < :: core :: primitive :: u8 , :: core :: primitive :: u8 , :: core :: primitive :: u8 , :: core :: primitive :: u8 > , } # [derive (:: subxt :: codec :: Decode , :: subxt :: codec :: Encode , Debug)] pub struct Foo < _0 , _1 , _2 , _3 > { pub a : _0 , pub b : _1 , pub c : _2 , pub d : _3 , } }

Logs from the test

[2022-05-26T20:47:33Z INFO  subxt_codegen::types::type_def] Type "Foo" fields [Field { name: Some("a"), ty: UntrackedSymbol { id: 4, marker: PhantomData }, type_name: Some("T"), docs: [] }, Field { name: Some("b"), ty: UntrackedSymbol { id: 4, marker: PhantomData }, type_name: Some("U"), docs: [] }, Field { name: Some("c"), ty: UntrackedSymbol { id: 4, marker: PhantomData }, type_name: Some("V"), docs: [] }, Field { name: Some("d"), ty: UntrackedSymbol { id: 4, marker: PhantomData }, type_name: Some("W"), docs: [] }] generics [TypeParameter { concrete_type_id: 4, original_name: "T", name: Ident(_0) }, TypeParameter { concrete_type_id: 4, original_name: "U", name: Ident(_1) }, TypeParameter { concrete_type_id: 4, original_name: "V", name: Ident(_2) }, TypeParameter { concrete_type_id: 4, original_name: "W", name: Ident(_3) }]
@hussein-aitlahcen
Copy link
Author

hussein-aitlahcen commented May 26, 2022

The more I dig, the deeper the dip. It might definitely be a scale-info issue. Currently forking and trying to add a tests there.

@jsdw
Copy link
Collaborator

jsdw commented May 27, 2022

Thanks for reporting this, and please do let me know what you find from your digging! I'm not sure I'll have much chance to look at this for a while as I'm away next week, but @ascjones might have know more about it offhand!

@ascjones
Copy link
Contributor

ascjones commented May 27, 2022

Can you provide the full test case please @hussein-aitlahcen?

@hussein-aitlahcen
Copy link
Author

Can you provide the full test case please @hussein-aitlahcen?

#[test]
fn generics() {
    #[allow(unused)]
    #[derive(TypeInfo)]
    struct Foo<T, U, V, W> {
        a: T,
        b: U,
        c: V,
        d: W
    }

    #[allow(unused)]
    #[derive(TypeInfo)]
    struct Bar {
        p: Foo<u32, u32, u64, u128>,
        q: Foo<u8, u8, u8, u8>,
    }

    env_logger::init();
    let mut registry = Registry::new();
    registry.register_type(&meta_type::<Bar>());
    log::info!("{:#?}", registry);
    let portable_types: PortableRegistry = registry.into();
    log::info!("{:#?}", portable_types);
    let type_gen = TypeGenerator::new(
        &portable_types,
        "root",
        Default::default(),
        Default::default(),
    );
    let types = type_gen.generate_types_mod();
    let tests_mod = get_mod(&types, MOD_PATH).unwrap();

    assert_eq!(
        tests_mod.into_token_stream().to_string(),
        quote! {
            pub mod tests {
                use super::root;
                #[derive(::subxt::codec::Decode, ::subxt::codec::Encode, Debug)]
                pub struct Bar {
                    pub p: root::subxt_codegen::types::tests::Foo<::core::primitive::u32, ::core::primitive::u32, ::core::primitive::u64, ::core::primitive::u128>,
                    pub q: root::subxt_codegen::types::tests::Foo<::core::primitive::u8, ::core::primitive::u8, ::core::primitive::u8, ::core::primitive::u8>,
                }
                #[derive(::subxt::codec::Decode, ::subxt::codec::Encode, Debug)]
                pub struct Foo<_0, _1, _2, _3> {
                    pub a: _0,
                    pub b: _1,
                    pub c: _2,
                    pub d: _3,
                }
            }
        }
        .to_string()
    )
}

Result

test types::tests::generics ... FAILED

failures:

---- types::tests::generics stdout ----
thread 'types::tests::generics' panicked at 'assertion failed: `(left == right)`

Diff < left / right > :
<pub mod tests { use super :: root ; # [derive (:: subxt :: codec :: Decode , :: subxt :: codec :: Encode , Debug)] pub struct Bar { pub p : root :: subxt_codegen :: types :: tests :: Foo < :: core :: primitive :: u32 , :: core :: primitive :: u32 , :: core :: primitive :: u64 , :: core :: primitive :: u128 > , pub q : root :: subxt_codegen :: types :: tests :: Foo < :: core :: primitive :: u8 , :: core :: primitive :: u8 , :: core :: primitive :: u8 , :: core :: primitive :: u8 > , } # [derive (:: subxt :: codec :: Decode , :: subxt :: codec :: Encode , Debug)] pub struct Foo < _0 , _1 , _2 , _3 > { pub a : _0 , pub b : _0 , pub c : _0 , pub d : _0 , # [codec (skip)] pub __subxt_unused_type_params : :: core :: marker :: PhantomData < (_1 , _2 , _3) > } }
>pub mod tests { use super :: root ; # [derive (:: subxt :: codec :: Decode , :: subxt :: codec :: Encode , Debug)] pub struct Bar { pub p : root :: subxt_codegen :: types :: tests :: Foo < :: core :: primitive :: u32 , :: core :: primitive :: u32 , :: core :: primitive :: u64 , :: core :: primitive :: u128 > , pub q : root :: subxt_codegen :: types :: tests :: Foo < :: core :: primitive :: u8 , :: core :: primitive :: u8 , :: core :: primitive :: u8 , :: core :: primitive :: u8 > , } # [derive (:: subxt :: codec :: Decode , :: subxt :: codec :: Encode , Debug)] pub struct Foo < _0 , _1 , _2 , _3 > { pub a : _0 , pub b : _1 , pub c : _2 , pub d : _3 , } }

@hussein-aitlahcen
Copy link
Author

@ascjones worth to mention, I figured out this using cargo expand after being unable to compile the generated code (missing Encode/Decode because of panthomization of generics)

@ascjones
Copy link
Contributor

At a glance there seems to be a bug where concrete type for all params are the same q: Foo<u8, u8, u8, u8>,. See whether it works with the one with different concrete types p: Foo<u32, u32, u64, u128>,.

@hussein-aitlahcen
Copy link
Author

hussein-aitlahcen commented May 27, 2022

At a glance there seems to be a bug where concrete type for all params are the same q: Foo<u8, u8, u8, u8>,. See whether it works with the one with different concrete types p: Foo<u32, u32, u64, u128>,.

It does work if you instantiate with distinct types, used p: Foo<u32, u8, u64, u128>

@ascjones
Copy link
Contributor

Okay thanks for confirming that. I don't have time to address that today but will be able to have a look next week.

@hussein-aitlahcen
Copy link
Author

hussein-aitlahcen commented May 27, 2022

The scale-info registry looks weird. The UntrackedSymbol id for type params match the concrete type instead of being a unique type id somehow? See

[2022-05-27T08:36:23Z INFO  subxt_codegen::types::tests] Registry {
        type_table: Interner {
            map: {
                TypeId {
                    t: 89521198653154635,
                }: 1,
                TypeId {
                    t: 10032322638265234986,
                }: 5,
                TypeId {
                    t: 13358953601680865708,
                }: 6,
                TypeId {
                    t: 14031099647136331291,
                }: 3,
                TypeId {
                    t: 14981460040544926914,
                }: 4,
                TypeId {
                    t: 16990524813833931774,
                }: 0,
                TypeId {
                    t: 18349839772473174998,
                }: 2,
            },
            vec: [
                TypeId {
                    t: 16990524813833931774,
                },
                TypeId {
                    t: 89521198653154635,
                },
                TypeId {
                    t: 18349839772473174998,
                },
                TypeId {
                    t: 14031099647136331291,
                },
                TypeId {
                    t: 14981460040544926914,
                },
                TypeId {
                    t: 10032322638265234986,
                },
                TypeId {
                    t: 13358953601680865708,
                },
            ],
        },
        types: {
            UntrackedSymbol {
                id: 0,
                marker: PhantomData,
            }: Type {
                path: Path {
                    segments: [
                        "subxt_codegen",
                        "types",
                        "tests",
                        "Bar",
                    ],
                },
                type_params: [],
                type_def: Composite(
                    TypeDefComposite {
                        fields: [
                            Field {
                                name: Some(
                                    "p",
                                ),
                                ty: UntrackedSymbol {
                                    id: 1,
                                    marker: PhantomData,
                                },
                                type_name: Some(
                                    "Foo<u32, u32, u64, u128>",
                                ),
                                docs: [],
                            },
                            Field {
                                name: Some(
                                    "q",
                                ),
                                ty: UntrackedSymbol {
                                    id: 5,
                                    marker: PhantomData,
                                },
                                type_name: Some(
                                    "Foo<u8, u8, u8, u8>",
                                ),
                                docs: [],
                            },
                        ],
                    },
                ),
                docs: [],
            },
            UntrackedSymbol {
                id: 1,
                marker: PhantomData,
            }: Type {
                path: Path {
                    segments: [
                        "subxt_codegen",
                        "types",
                        "tests",
                        "Foo",
                    ],
                },
                type_params: [
                    TypeParameter {
                        name: "T",
                        ty: Some(
                            UntrackedSymbol {
                                id: 2,
                                marker: PhantomData,
                            },
                        ),
                    },
                    TypeParameter {
                        name: "U",
                        ty: Some(
                            UntrackedSymbol {
                                id: 2,
                                marker: PhantomData,
                            },
                        ),
                    },
                    TypeParameter {
                        name: "V",
                        ty: Some(
                            UntrackedSymbol {
                                id: 3,
                                marker: PhantomData,
                            },
                        ),
                    },
                    TypeParameter {
                        name: "W",
                        ty: Some(
                            UntrackedSymbol {
                                id: 4,
                                marker: PhantomData,
                            },
                        ),
                    },
                ],
                type_def: Composite(
                    TypeDefComposite {
                        fields: [
                            Field {
                                name: Some(
                                    "a",
                                ),
                                ty: UntrackedSymbol {
                                    id: 2,
                                    marker: PhantomData,
                                },
                                type_name: Some(
                                    "T",
                                ),
                                docs: [],
                            },
                            Field {
                                name: Some(
                                    "b",
                                ),
                                ty: UntrackedSymbol {
                                    id: 2,
                                    marker: PhantomData,
                                },
                                type_name: Some(
                                    "U",
                                ),
                                docs: [],
                            },
                            Field {
                                name: Some(
                                    "c",
                                ),
                                ty: UntrackedSymbol {
                                    id: 3,
                                    marker: PhantomData,
                                },
                                type_name: Some(
                                    "V",
                                ),
                                docs: [],
                            },
                            Field {
                                name: Some(
                                    "d",
                                ),
                                ty: UntrackedSymbol {
                                    id: 4,
                                    marker: PhantomData,
                                },
                                type_name: Some(
                                    "W",
                                ),
                                docs: [],
                            },
                        ],
                    },
                ),
                docs: [],
            },
            UntrackedSymbol {
                id: 2,
                marker: PhantomData,
            }: Type {
                path: Path {
                    segments: [],
                },
                type_params: [],
                type_def: Primitive(
                    U32,
                ),
                docs: [],
            },
            UntrackedSymbol {
                id: 3,
                marker: PhantomData,
            }: Type {
                path: Path {
                    segments: [],
                },
                type_params: [],
                type_def: Primitive(
                    U64,
                ),
                docs: [],
            },
            UntrackedSymbol {
                id: 4,
                marker: PhantomData,
            }: Type {
                path: Path {
                    segments: [],
                },
                type_params: [],
                type_def: Primitive(
                    U128,
                ),
                docs: [],
            },
            UntrackedSymbol {
                id: 5,
                marker: PhantomData,
            }: Type {
                path: Path {
                    segments: [
                        "subxt_codegen",
                        "types",
                        "tests",
                        "Foo",
                    ],
                },
                type_params: [
                    TypeParameter {
                        name: "T",
                        ty: Some(
                            UntrackedSymbol {
                                id: 6,
                                marker: PhantomData,
                            },
                        ),
                    },
                    TypeParameter {
                        name: "U",
                        ty: Some(
                            UntrackedSymbol {
                                id: 6,
                                marker: PhantomData,
                            },
                        ),
                    },
                    TypeParameter {
                        name: "V",
                        ty: Some(
                            UntrackedSymbol {
                                id: 6,
                                marker: PhantomData,
                            },
                        ),
                    },
                    TypeParameter {
                        name: "W",
                        ty: Some(
                            UntrackedSymbol {
                                id: 6,
                                marker: PhantomData,
                            },
                        ),
                    },
                ],
                type_def: Composite(
                    TypeDefComposite {
                        fields: [
                            Field {
                                name: Some(
                                    "a",
                                ),
                                ty: UntrackedSymbol {
                                    id: 6,
                                    marker: PhantomData,
                                },
                                type_name: Some(
                                    "T",
                                ),
                                docs: [],
                            },
                            Field {
                                name: Some(
                                    "b",
                                ),
                                ty: UntrackedSymbol {
                                    id: 6,
                                    marker: PhantomData,
                                },
                                type_name: Some(
                                    "U",
                                ),
                                docs: [],
                            },
                            Field {
                                name: Some(
                                    "c",
                                ),
                                ty: UntrackedSymbol {
                                    id: 6,
                                    marker: PhantomData,
                                },
                                type_name: Some(
                                    "V",
                                ),
                                docs: [],
                            },
                            Field {
                                name: Some(
                                    "d",
                                ),
                                ty: UntrackedSymbol {
                                    id: 6,
                                    marker: PhantomData,
                                },
                                type_name: Some(
                                    "W",
                                ),
                                docs: [],
                            },
                        ],
                    },
                ),
                docs: [],
            },
            UntrackedSymbol {
                id: 6,
                marker: PhantomData,
            }: Type {
                path: Path {
                    segments: [],
                },
                type_params: [],
                type_def: Primitive(
                    U8,
                ),
                docs: [],
            },
        },
    }

@ascjones
Copy link
Contributor

The scale-info registry looks weird. The UntrackedSymbol id for type params match the concrete type instead of being a unique type id somehow?

That is expected because scale-info only knows about concrete types, so when all the params are e.g.u8 they will have the same type id, which is from https://doc.rust-lang.org/std/any/struct.TypeId.html.

We should however still be able to uniquely identify the params and field usage because of the type parameter names.

@hussein-aitlahcen
Copy link
Author

The scale-info registry looks weird. The UntrackedSymbol id for type params match the concrete type instead of being a unique type id somehow?

That is expected because scale-info only knows about concrete types, so when all the params are e.g.u8 they will have the same type id, which is from https://doc.rust-lang.org/std/any/struct.TypeId.html.

We should however still be able to uniquely identify the params and field usage because of the type parameter names.

Ok makes sense then. I expected the registry to both uniquely identify + provide the concrete type.
I'm able to compile by doing a slight change on the resolve_type_path, basically adding the type name in the first match.
The nested generic test is breaking but the example we used is now passing (and making our subxt client compile).
I'll open a PR once done.

@jsdw
Copy link
Collaborator

jsdw commented Jun 14, 2022

Just trying to understand this issue; is this ultimately something that needs fixing in scale-info, here, or both?

@hussein-aitlahcen
Copy link
Author

Just trying to understand this issue; is this ultimately something that needs fixing in scale-info, here, or both?

AFAIU if scale-info must provide a unique identity to each generic types (I was assuming that) then it must be added to scale-info. Otherwise we need to somehow be able to track and distinguish generics in this library.

@jsdw
Copy link
Collaborator

jsdw commented Jun 14, 2022

I get the impression that scale-info will need a tweak to support that, and then we'll need a tweak in subxt to make use of the new information, basically. But somebody pelase correct me if wrong :)

@jsdw jsdw added this to the Release 0.23 milestone Jun 14, 2022
@jsdw jsdw changed the title Invalid generation fo types with >1 generic Invalid generation of types with >1 generic Jun 14, 2022
@jsdw jsdw modified the milestones: v0.24: Tidy and Polish, v0.25 Aug 5, 2022
@jsdw jsdw modified the milestones: v0.25, v1.0 Aug 15, 2022
@jsdw jsdw added the bug Something isn't working label Sep 27, 2022
@tadeohepperle tadeohepperle self-assigned this Jun 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants