Hello I think this is correctly observed, and I'm surprised that it hasn't yet bitten us given our propensity for short names and use of double underscores.
I think renaming the guard macros should be trivial enough that we just do it to conform to the standard. Maybe open a JIRA ticket for this specifically that someone can grab if they're inspired. This thread could devolve into bikeshedding pretty quickly, but I don't know of another way to thrash out what might be the correct approach. Renaming symbols is fairly trivial, the hard part is choosing the naming scheme[1]. I'm not a fan of numbering as I find it difficult to parse quickly. I thought the issue was only with leading underscores, but as you point out it isn't. Perhaps we should use prefix or suffixed 'c' for continuation. Example: launch -> c_launch -> cc_launch or launch -> launch_c -> launch_cc or we could be explicit in naming: launch -> launchCont -> launchContCont Thoughts? 1. there are 2 hard problems in computer science: caching, naming things, and off-by-one errors. On Sun, Oct 5, 2014 at 4:20 PM, Michael Park <[email protected]> wrote: > Hello, > > I just wanted to mention an issue with our naming convention that goes > against the C++ standard due to the rules around reserved names. > > From N3797, > > 17.6.4.3 Reserved names [reserved.names] > > > > . . . > > > > 17.6.4.3.2 Global names [global.names] > > > > 1 Certain sets of names and function signatures are always reserved to > the > > implementation: > > > > — *Each name that contains a double underscore _ _ or begins with an > > underscore followed by an uppercase letter (2.12) is reserved to the > > implementation for any use. * > > > > — *Each name that begins with an underscore is reserved to the > > implementation for use as a name in the global namespace.* > > > The biggest offender is the include guard since all of them start and end > with double underscores. A few other examples are *<stout/exit.hpp>*'s > *__Exit* struct, *src/zookeeper/zookeeper.cpp*'s *__create*, > *src/slave/containerizer/docker.cpp*'s *__launch*. > > We use leading double underscore for internal implementation sometimes, > which I think should live in the *internal* namespace instead or have names > such as *createImpl* or *createHelper*. > > In the case of *__launch*, we start out with *launch* then go onto > *_launch* (this > one is allowed because it's a member function and therefore not in the > global namespace and it started with an underscore but is not followed by > an uppercase letter.) then we get to *__launch* which is not allowed since > it contains a double underscore. We should probably give these kinds of > functions better names associated with their phase, or even just numbering > them would be ok. Changing them to be trailing underscores wouldn't help > much either since the rule around double underscores is *contains* rather > than *starts with*. > > Anyway, practically speaking it hasn't been a big issue for us yet. It's > just something I noticed and thought should bring up to the group. > > Thanks, > > MPark > -- Dominic Hamon | @mrdo | Twitter *There are no bad ideas; only good ideas that go horribly wrong.*
