Code guidelines: anonymous namespace - enhancement #1034
Conversation
GuyWi
requested review from
AnotherButler,
davidsaada,
kjbracey,
SenRamakri and
bulislaw
| @@ -30,6 +30,7 @@ The Arm Mbed OS codebase is organized into conceptual submodules to limit the sc | |||
| - As an entry point for the module (from the user space), we suggest a single header file. For example: `mbed.h`, `rtos.h`. | |||
| - Header files should limit external includes to avoid indirectly exposing unrelated APIs. Header files should not expand namespaces. | |||
| - In C++ modules, the API should be contained in a namespace that matches the module’s name. For example: `mbed::Ticker`, `rtos::Thread`, `netsocket::Socket`. | |||
| - Wrap the content of each C++ module inside an anonymous namespace. The `GCC_ARM` toolchain automatically gives data structures in different anonymous namespaces unique names. This prevents build failures caused by link time optimization (LTO) in the `GCC_ARM` toolchain when you have local data types with the same name in different files. | |||
There was a problem hiding this comment.
You can't wrap the whole module - it just needs to be the private stuff. And this isn't a GCC-specific thing. My suggestion:
Private class types defined in a C++source file should be contained in an anonymous namespace to ensure their names are globally unique. In C++ it is not permitted to have two different definitions of the same class name in different source files, even if they are not externally visible. In practice a name collision often doesn't cause a problem, but it can cause a build failure when link time optimization (LTO) is enabled, or a run-time failure if templates are instantiated using the private classes.
(I don't think we've seen the template problem, but it could happen - the compiler might instantiate Vector<foo> for two different foos in different files, and end up trying to share the same template instantiation).
There was a problem hiding this comment.
Thanks @kjbracey-arm !
I have updated the PR based on your feedback.
There was a problem hiding this comment.
Looks good - I'm just a little nervous about the use of the word "private", as we're not talking about the C++ keyword private here. Maybe "internal"?
There was a problem hiding this comment.
Thanks again, @kjbracey-arm :)
Updated.
Remove hyphen to fit our style guide.
This PR addresses https://jira.arm.com/browse/IOTDOCS-869, and is related to ARMmbed/mbed-os#9158 and issue #884.