We aren't very consistent about macro names throughout IMP. I would suggest IMP_CORE_BEGIN_NAMESPACE IMP_CORE_EXPORT IMP_EXPORT
in addition, the __ in the header guard names doesn't add anything and is technically illegal in C++/C.
Daniel Russel wrote: > We aren't very consistent about macro names throughout IMP. I would > suggest > IMP_CORE_BEGIN_NAMESPACE > IMP_CORE_EXPORT > IMP_EXPORT
On the contrary, I think the names are pretty consistent. Namespaces and header names are mapped to uppercase, :: is mapped to '', and CamelCase case changes are mapped to '_'. Seems that your proposal would either add an inconsistency (namespace macro IMP_CORE_BEGIN_NAMESPACE inconsistent with header guard IMPCORE_FOO_H) or a header guard ambiguity (no way to distinguish Bar.h in the foo module from FooBar.h in the kernel, since both would map to IMP_FOO_BAR_H). This is why there is no space between 'IMP' and 'CORE' in any of the macros.
> in addition, the __ in the header guard names doesn't add anything and > is technically illegal in C++/C.
If you have a source for that assertion I'd be happy to remove it. It actually appears to be quite common usage though (e.g. freetype and glib2 both do that).
Ben
On Oct 8, 2008, at 9:52 AM, Ben Webb wrote:
> Daniel Russel wrote: >> We aren't very consistent about macro names throughout IMP. I would >> suggest >> IMP_CORE_BEGIN_NAMESPACE >> IMP_CORE_EXPORT >> IMP_EXPORT > > On the contrary, I think the names are pretty consistent. Namespaces > and > header names are mapped to uppercase, :: is mapped to '', and > CamelCase > case changes are mapped to '_'. Seems that your proposal would either > add an inconsistency (namespace macro IMP_CORE_BEGIN_NAMESPACE > inconsistent with header guard IMPCORE_FOO_H) or a header guard > ambiguity (no way to distinguish Bar.h in the foo module from FooBar.h > in the kernel, since both would map to IMP_FOO_BAR_H). This is why > there > is no space between 'IMP' and 'CORE' in any of the macros. In camel case the case is the token deliminter. If you use underscores the underscore is the token delimiter. In namespaces the :: is the delimiter. We have to use _s is macros, so all the delimiters should map to that.
Of course the full path to the header needs to be in the header guard (IMP_CORE_HEADER_NAME_H) so there is no source of ambiguity unless we are silly enough to start prefixing things in IMP with names that are the same as modules.
For example IMPDLLEXPORT and IMPCOREEXPORT are clearly inconsistent - the DLL should be in both or neither - EXPORT is a different token than IMPCORE at least, so it should have an _ - IMP and CORE are different namespaces so the above comment applies.
> > >> in addition, the __ in the header guard names doesn't add anything >> and >> is technically illegal in C++/C. > > If you have a source for that assertion I'd be happy to remove it. It > actually appears to be quite common usage though (e.g. freetype and > glib2 both do that). For example: http://developer.mozilla.org/En/C___Portability_Guide According to the C++ Standard, 17.4.3.1.2 Global Names [lib.global.names], paragraph 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.11) 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.
So macros (which are always global) get hit.
Daniel Russel wrote: > For example IMPDLLEXPORT and IMPCOREEXPORT are clearly inconsistent > - the DLL should be in both or neither
Indeed - I just haven't got round to removing DLL from the kernel exports yet.
> According to the C++ Standard, 17.4.3.1.2 Global Names > [lib.global.names], paragraph 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.11) 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.
Fair enough - in that case I will remove it.
Ben
On Oct 8, 2008, at 10:19 AM, Ben Webb wrote:
> Daniel Russel wrote: >> For example IMPDLLEXPORT and IMPCOREEXPORT are clearly inconsistent >> - the DLL should be in both or neither > > Indeed - I just haven't got round to removing DLL from the kernel > exports yet. To put it another way, we are currently taking names "IMP/core/ CamelCase.h" or "IMP.core.CamelCase" or "IMP::core::CamelCase" and removing the first delimeter and replacing the second with an "_". And then IMP_RESTRAINT has the tokens separated by "_" but IMPCOREEXPORT does not. It is not a big deal, but it is a bit annoying and definitely inconsistent and I will probably continue to mistype them periodically.
Daniel Russel wrote: > To put it another way, we are currently taking names "IMP/core/ > CamelCase.h" or "IMP.core.CamelCase" or "IMP::core::CamelCase" and > removing the first delimeter and replacing the second with an "_". And > then IMP_RESTRAINT has the tokens separated by "_" but IMPCOREEXPORT > does not. It is not a big deal, but it is a bit annoying and > definitely inconsistent and I will probably continue to mistype them > periodically.
Given that it is a trivial issue but we disagree, I will bow to the consensus opinion. If you can persuade somebody else to back up your proposal, I will make the change.
Ben
> > Given that it is a trivial issue but we disagree, I will bow to the > consensus opinion. If you can persuade somebody else to back up your > proposal, I will make the change. Lets leave the existing symbols in place, but lets not impose any complicated set of policies to justify the names (and any policy more complicated than "tokens in macro names are delimited by underscores" is too complicated).
As with most of these things, someone else will complain sooner or later :-)
participants (2)
-
Ben Webb
-
Daniel Russel