flatbuffers
flatbuffers copied to clipboard
[C++] Namespace shadowing leads to _generated.h that won't compile
Given a namespace that contains a repeated entry, such as a.b.a.c
, the generated c++ code will contain a nested namespace that shadows an outer namespace, causing the generated code not to compile. This is a problem when the namespace corresponds to the path where the .fbs
is located and the path cannot be changed. Consider the following example test.fbs
:
namespace a.b.a.c;
table Foo {
x: int64;
}
root_type Foo;
In the generated file test_generated.h
we have many references to a::b::a::c
from within the innermost namespace definition. An example is:
namespace a {
namespace b {
namespace a {
namespace c {
// ...
inline const a::b::a::c::Foo *GetFoo(const void *buf) {
return flatbuffers::GetRoot<a::b::a::c::Foo>(buf);
}
// ...
} // namespace c
} // namespace a
} // namespace b
} // namespace a
In this code the first a
in a::b::a::c
is referring to the inner definition of the namespace a
, which only contains namespace c
due to shadowing the outer one and then a::b
does not exist. A simple fix to the code would be to always qualify the namespaces with the global namespace first, i.e., append ::
to all namespace references. I am not sure if there are any unintended consequences to this. The resulting code would then look like:
namespace a {
namespace b {
namespace a {
namespace c {
// ...
inline const ::a::b::a::c::Foo *GetFoo(const void *buf) {
return flatbuffers::GetRoot<::a::b::a::c::Foo>(buf);
}
// ...
} // namespace c
} // namespace a
} // namespace b
} // namespace a
After applying this fix to change all occurrences of a::b::a::c
to ::a::b::a::c
in test_generated.h
, the generated code will compile without issue.
Yeah, I believe protobufs does the :: preappend as well, this seems like a valid use case for justifying the increase verbosity of the generated code. Would you like to do a PR for this?
@CasperN Should we consider this part of the Namer interface as well?
I created a few commits but it seems like a simple change to src/idl_gen_cpp.cpp
is insufficient to solve this. I started exploring this further and changing [almost] every call to WrapInNameSpace(...)
to WrapInNameSpace(...).insert(0,"::")
. Changing the game test to have a silly repeated namespace seems to show that it is fixed now, but the fix seems a bit messy and I am not sure this was the best way to patch it.
Do you have any guidance on how to approach this? I don't think I will be able to dedicate too much more time to this, so if it is very involved I am not sure I can complete the change beyond what I have done already.
Should we consider this part of the Namer interface as well?
The Namer
interface does support generating namespaced types, its just a matter of using it in the cpp_genreator
@ahughes6, I haven't looked at the C++ generator in a little while, but I solved a very similar issue in C# with pull request #6767 so maybe that can help you
This issue is stale because it has been open 6 months with no activity. Please comment or label not-stale
, or this will be closed in 14 days.
This issue was automatically closed due to no activity for 6 months plus the 14 day notice period.