Nested types don't work

[adetaylor]

class A {
   class B {
       ...
   };
};

is generated by bindgen as

pub struct A {
   ...
}

pub struct A_B {
   ...
}

This then gives errors when we refer to A_B in generated C++.

[martinboehme]

I assume you closed this in favor of #124?

I'm reopening to discuss issues specific to nested types and don't want to clog #124 with the details. Hope that's OK.

I've started working on this, and I think what will be needed here is this:

• Bindgen should emit a bindgen_original_name for Rust types that were generated from nested C++ types. In the example above, the attribute would be # [bindgen_original_name ("A::B")]. (I already have a prototype implementation for this.)
• cxx needs a new attribute (say cxx::c_name) that allows us to tell it what the name of the C++ is for a type that we're passing to it. In the example above, we would put cxx::c_name("A::B") on struct A_B.
• autocxx's needs to forward the C++ type name as follows:
  • The Rust code generator needs to emit a cxx::c_name attribute where the bindgen bindings contained a bindgen_original_name
  • The C++ code generator needs to use the bindgen_original_name when emitting code that uses the types in question.

What do you think?

I've started having a look at how I would do the autocxx changes, and it seems a bit tricky to get the right information through to the C++ code generator. It looks like the change needs to happen in type_to_cpp, but it only has access to the syn::Type, whereas the attributes are attached to the syn::ItemStruct. IIUC, syn only does syntactic analysis but doesn't have a symbol table, so it's not easy to go from the former to the latter.

I assume I'll may have to build my own symbol table of sorts, maybe from the list of Apis. Any suggestions on how to go about this?

[martinboehme]

Update: I think we don't need to make any changes to cxx because it already has a cxx_name attribute that does exactly what we need.

[martinboehme]

Another update: Unfortunately, putting a qualified name like "A::B" in a cxx_name attribute causes cxx to output an "unexpected token" error. I hope this is relatively easy to fix.

[martinboehme]

Update 3: I've taken another look at this and have realized that the "A_B" occurs in another place, namely in the cxx::ExternType:

    unsafe impl cxx::ExternType for bindgen::root::A_B {
        type Id = cxx::type_id!("A_B");
        type Kind = cxx::kind::Opaque;
    }

When I saw this, I initially thought that maybe this is the only place that needs to be changed -- i.e. that the cxx::type_id should simply be changed to refer to "A::B" and that cxx would then know that this is the name by which to refer to the type in generated C++ code.

This interpretation seems to be supported by the docs for ExternType as well as by the fact that cxx, in extern_types.rs, defines a unsafe impl ExternType for CxxString with a type_id of std::string.

However, making this change to the code doesn't seem to make any difference to the type name that's used in the generated C++ code -- it's still "A_B".

I'm really just writing this down as "notes to myself" -- don't feel obliged to respond.

[adetaylor]

Yeah... you're refreshing my memory of various problems here.

cxx has a #[namespace] attribute too.

I think what we need to do is roughly what you propose:

• bindgen emits the name as A_B but indeed includes something like # [bindgen_original_name ("A::B")]
• When feeding this into cxx we use just B for the type name. I don't even know if we need the A:: at all. But if we do, we try feeding that into the namespace attribute.
• We also use those correct C++ names when generating our own code within autocxx's codegen_cpp bit.

[martinboehme]

Yeah... you're refreshing my memory of various problems here.

cxx has a #[namespace] attribute too.

I think what we need to do is roughly what you propose:

• bindgen emits the name as A_B but indeed includes something like # [bindgen_original_name ("A::B")]
• When feeding this into cxx we use just B for the type name. I don't even know if we need the A:: at all. But if we do, we try feeding that into the namespace attribute.

Ha! That's crafty.

I had seen that attribute but hadn't considered using it for this purpose. But it should work, as the syntax is exactly the same.

Not doing nested types via cxx_name has the benefit that we don't need to teach cxx how to deal with :: in a cxx_name. When I did that experimentally, the :: ended up in various places it didn't belong in generated C++ identifiers, and I was dreading having to go through all those occurrences and make sure cxx would replace the :: with a _ again. If we use the namespace attribute for that purpose, none of this is necessary -- perfect!

• We also use those correct C++ names when generating our own code within autocxx's codegen_cpp bit.

I'll give all of this a try and will let you know when I've got something that works.