[C++] Unqualified reference to "flatbuffers" namespace should be qualified

[Quincunx271]

In the generated code, references to library namespaces (specifically flatbuffers) should be qualified rather than unqualified. Currently, if the user namespaces their .fbs file, they could run into trouble:

namespace their.flatbuffers;

// ...

Generates something like this:

namespace their {
namespace flatbuffers {

// ...

}
}

The references to flatbuffers::Table and all other functions and types will fail, since it finds the ::their::flatbuffers namespace rather than ::flatbuffers. Instead of generating flatbuffers::..., we should be generating ::flatbuffers::....

For example,

flatbuffers/src/idl_gen_cpp.cpp

Line 358 in 5b43e4b

code_ += " return flatbuffers::GetRoot<{{CPP_NAME}}>(buf);";

should look like code_ += " return ::flatbuffers::GetRoot<{{CPP_NAME}}>(buf);";

---

This applies to all other library references, including the standard library. Although it would be weird, users could conceivably have a std namespace nested inside their own namespace, which would cause std::vector to fail. For example, this case can fail:

namespace their.ns;

// ...

Generates:

namespace their {
namespace ns {

// ...

}
}

This normally works, but if the user has:

namespace their {
namespace std {
// ...
}

// ...

}

This perfectly compliant code would cause the flatc-generated code to fail to find std::vector, as the unqualified lookup for std stops at finding their::std.

Therefore, any reference to a type which is not one which flatc generates should use a fully qualified namespace with a leading :: to ensure we always get the correct type. This includes types passed in with --cpp-str-type and the like.

[aardappel]

Alternatively, we could error out on a namespace ending in flatbuffers or std (the only 2 used in generated code).

Everyone on the planet uses std::T, not ::std::T, so though we could fix this by prefixing all namespaces, I am not sure if its worth it?

[Quincunx271]

AFAIK, you can also pass namespaces types to --cpp-str-type and similar flags. These types should also have the prefixed ::, for the same reason.

IMO, it's worth doing, mainly for allowing users the option of nesting a flatbuffers namespace in their own, which seems to be a reasonable way to define a flatbuffers structure for some application. IMO, it's more correct for a code generation tool to ensure it has the correct namespace. For example, in well-written macros that I've seen, it's always ::std::thing instead of std::thing. This only affects the generated code; users can keep doing whatever they want.

---

But yes, erroring out on namespaces ending with flatbuffers or std is an option. Note that it can still be run into:

namespace their { namespace flatbuffers { namespace nested {

// ...

} } }

But error out in this case would prevent the user from making it work:

With namespace their.flatbuffers.nested;

namespace their { namespace flatbuffers { namespace nested {
    namespace flatbuffers = ::flatbuffers;
} } }

#include "flatbuffers_generated.h"

[aardappel]

I am not a fan of people using flatbuffers in their namespacing, as it seems a bit double, but who am I to judge.

Do you want to make a PR to add :: to all namespaces?

[stale]

This issue has been automatically marked as stale because it has not had activity for 1 year. It will be automatically closed if no further activity occurs. To keep it open, simply post a new comment. Maintainers will re-open on new activity. Thank you for your contributions.