Namespace types in C++ #428
Conversation
| class AttrOpaque1Renamed { | ||
| public: | ||
|
|
||
| inline static std::unique_ptr<AttrOpaque1Renamed> totally_not_new(); | ||
| inline static std::unique_ptr<ns::AttrOpaque1Renamed> totally_not_new(); |
There was a problem hiding this comment.
Nit, here and elsewhere: You don't need to restate the namespace when you are inside a namespace block. I think how C++ resolution works is it will look in the current namespace and strip one level from the namespace until it either finds the item or fails. So, specifying ns::Foo here is potentially ambiguous because if there is a namespace ns { namespace ns { Foo } } it will find that one instead of the one at the current level.
There was a problem hiding this comment.
I disagree: I'd like the generating code to be clean and don't care as much about redundancy in this code. Tracking namespaces everywhere will be somewhat painful
As for ambiguity, my general take is "don't do that", in the long run we can patch this if desired, or use unambiguous ::ns imports (I believe that works?).
| pub fn append_forward(&mut self, def: TypeDef, ty_name_unnamespaced: &str) { | ||
| let forward = Self::forward_for(def, ty_name_unnamespaced); | ||
|
|
||
| let ns = def.attrs().namespace.clone(); |
There was a problem hiding this comment.
Suggestion: We should make a type Namespace. It shouldn't be a string, because:
- It could have multiple levels which some backends may care about
- There are so many types flying around in Diplomat that I have tried hard to use newtypes whenever possible to prevent coding errors like passing the wrong string as a positional argument
| body: String::new(), | ||
| indent_str: " ", | ||
| } | ||
| } | ||
|
|
||
| fn forward_for(def: TypeDef, ty_name_unnamespaced: &str) -> Forward { | ||
| match def { |
There was a problem hiding this comment.
Thought: I kind-of like the old style of putting the Forward type directly in gen_type_name to avoid the double match
There was a problem hiding this comment.
Yeah I wasn't sure of this but decided to go this way so that the rest of the code doesn't need to worry about the header's internal ontology of forward kinds.
First step of #427
Does not yet deal with capi namespacing, or putting the headers in the right folders.