Correct header inclusion guards to comply with C++ standard

What it says on the tin: a place to discuss proposed new features.
User avatar
antred
Gnoblar
Posts: 21
Joined: Sun Nov 28, 2010 7:03 pm
Location: Germany
Contact:

Correct header inclusion guards to comply with C++ standard

Post by antred » Mon Mar 11, 2013 1:46 pm

Hello,

This is not so much a feature request as a minor clean-up request. ;) I've noticed that most Ogre header's have inclusion guards like:
#ifndef __OGRE_BLAH_BLAH_H
#define __OGRE_BLAH_BLAH_H
The thing is, the C++ standard has this to say on identifiers containing 2 consecutive underscores:
17.4.3.1.2 Global names [lib.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.11) is reserved to the implementation for any use.
So perhaps this inclusion guards could be changed to comply with the C++ standard? I know this is just a minor matter, but it wouldn't hurt to be as standard-compliant as possible, right?
0 x

User avatar
Klaim
Old One
Posts: 2565
Joined: Sun Sep 11, 2005 1:04 am
Location: Paris, France
Contact:

Re: Correct header inclusion guards to comply with C++ stand

Post by Klaim » Mon Mar 11, 2013 3:59 pm

I think it have been notified before but it wasn't worth the time to do the change. Or maybe you can take the time to provide a patch/PR?
0 x

User avatar
jacmoe
OGRE Retired Moderator
OGRE Retired Moderator
Posts: 20570
Joined: Thu Jan 22, 2004 10:13 am
Location: Denmark
Contact:

Re: Correct header inclusion guards to comply with C++ stand

Post by jacmoe » Mon Mar 11, 2013 4:30 pm

The reason for that nonstandard standard is AFAIK that Visual Studio used to hide defines starting with two underscores.
It doesn't seem to do that any more, though.

IMO, it should be changed to use #pragma once instead.

AFAIK, all compilers support #pragma once. And it doesn't pollute the global namespace.
0 x
/* Less noise. More signal. */
Ogitor Scenebuilder - powered by Ogre, presented by Qt, fueled by Passion.
OgreAddons - the Ogre code suppository.

zootlewurdle
Halfling
Posts: 91
Joined: Sat Aug 06, 2011 8:38 am
Location: United Kingdom

Re: Correct header inclusion guards to comply with C++ stand

Post by zootlewurdle » Mon Mar 11, 2013 7:15 pm

pragma once may be widely supported but it is not part of any standard. define guards are the only truly compiler agnostic solution.
0 x

User avatar
saejox
Goblin
Posts: 260
Joined: Tue Oct 25, 2011 1:07 am

Re: Correct header inclusion guards to comply with C++ stand

Post by saejox » Mon Mar 11, 2013 7:27 pm

<3 #pragma once

Only ancient compilers do not support it.
0 x
Nimet - Advanced Ogre3D Mesh/dotScene Viewer
asPEEK - Remote Angelscript debugger with html interface
ogreHTML - HTML5 user interfaces in Ogre

User avatar
masterfalcon
OGRE Team Member
OGRE Team Member
Posts: 4270
Joined: Sun Feb 25, 2007 4:56 am
Location: Bloomington, MN
Contact:

Re: Correct header inclusion guards to comply with C++ stand

Post by masterfalcon » Mon Mar 11, 2013 8:21 pm

Though it's supported in many compilers http://en.wikipedia.org/wiki/Pragma_once. I feel that it's better to maintain standard compliance, even if it's a little messier.

If you want to do a pull request to standardize the header guards that'd be great. It's a large but innocuous change and I see no reason why it would be a problem.
0 x

User avatar
jacmoe
OGRE Retired Moderator
OGRE Retired Moderator
Posts: 20570
Joined: Thu Jan 22, 2004 10:13 am
Location: Denmark
Contact:

Re: Correct header inclusion guards to comply with C++ stand

Post by jacmoe » Mon Mar 11, 2013 8:39 pm

Could you name one compiler where it's not supported? :)

#pragma once is less code. It doesn't require the programmer to come up with a name for the define constant, which could potentially clash with other constants defined. (I've had that happen lots of times).
And there isn't any risk of mismatched #ifdefs.
A couple of years ago, it probably was a good idea to stick with manual conditional include guards, but not now. IMHO.
0 x
/* Less noise. More signal. */
Ogitor Scenebuilder - powered by Ogre, presented by Qt, fueled by Passion.
OgreAddons - the Ogre code suppository.

zootlewurdle
Halfling
Posts: 91
Joined: Sat Aug 06, 2011 8:38 am
Location: United Kingdom

Re: Correct header inclusion guards to comply with C++ stand

Post by zootlewurdle » Tue Mar 12, 2013 2:16 pm

I've been working as a professional C/C++ programmer for 21 years and I've never seen a collision with header guards.
It's not hard with a library like Ogre to construct a name which should be uncommon enough to avoid problems, e.g., HH_OGRE_<MODULE>_<FILE>_HH
If you're using identically named files within any given module you need a slap and you learn your lesson (because doing so can cause confusing issues anyway).
If you're using similar guard names outside of a module or library then similarly that's a heard lesson learned not to be so silly.

Wholescale changing headers in an open source library to use a non-standard feature in something that is intended to be compiler/platform specific is simply unwise at best.

I'm trying very hard to be polite here, and probably failing.
0 x

User avatar
jacmoe
OGRE Retired Moderator
OGRE Retired Moderator
Posts: 20570
Joined: Thu Jan 22, 2004 10:13 am
Location: Denmark
Contact:

Re: Correct header inclusion guards to comply with C++ stand

Post by jacmoe » Tue Mar 12, 2013 2:45 pm

I don't really give a fuck about politeness. I know programmers are notoriously anal about the smallest of issues.

You haven't presented a valid argument, only slanting. Like: "you need a slap", "I am an expert programmer (So shut up)", "learn your lesson", "lesson learned not to be so silly", "unwise at best", ...

#pragma once reduces the possibilities for bugs, and that's a really good reason to use it.

Non-standard or not.
0 x
/* Less noise. More signal. */
Ogitor Scenebuilder - powered by Ogre, presented by Qt, fueled by Passion.
OgreAddons - the Ogre code suppository.

Transporter
Minaton
Posts: 933
Joined: Mon Mar 05, 2012 11:37 am
Location: Germany

Re: Correct header inclusion guards to comply with C++ stand

Post by Transporter » Tue Mar 12, 2013 3:25 pm

I can do a patch for #pragma once tonight. But I still have an open pull request wich will be updated if you don't accept it :D
0 x

User avatar
jacmoe
OGRE Retired Moderator
OGRE Retired Moderator
Posts: 20570
Joined: Thu Jan 22, 2004 10:13 am
Location: Denmark
Contact:

Re: Correct header inclusion guards to comply with C++ stand

Post by jacmoe » Tue Mar 12, 2013 4:02 pm

#pragma once is unambiguous in any compiler that supports it.

It increases readability, and - even though Zootlewurdle haven't had problems with it in his 21 year long career - regular, old-fashioned include guards are error prone.
#pragma once is supported by any recent compiler worth mentioning.
Insomniac Games prefers pragma once in their code standard: http://www.insomniacgames.com/core-coding-standard/

If Ogre removes the leading underscores, then we will have issues with some of the files, like:

Code: Select all

#ifndef __GLRenderSystem_H__
#define __GLRenderSystem_H__
...
#endif
That would become GLRenderSystem_H and would probably clash with other include guards 'somewhere'.
Unless, of course, we insert a GUID into the define. Which would make it uglier, but more fail safe.

I like that #pragma once is unambiguous. ;)

I also realize that this is just as hot potato as tabs versus spaces and indentation style..

So, whatever floats the boat for the OGRE Team.
Just be careful about removing the double underscores as it might require updating to more unique defines.
0 x
/* Less noise. More signal. */
Ogitor Scenebuilder - powered by Ogre, presented by Qt, fueled by Passion.
OgreAddons - the Ogre code suppository.

User avatar
Klaim
Old One
Posts: 2565
Joined: Sun Sep 11, 2005 1:04 am
Location: Paris, France
Contact:

Re: Correct header inclusion guards to comply with C++ stand

Post by Klaim » Tue Mar 12, 2013 7:06 pm

I'm not against or for pragma once. But my understanding is that it's definition IS ambiguous, it's not certain all compilers do exactly the same with it.
See: http://stackoverflow.com/questions/1695 ... o-standard
A directive like #pragma once is not trivial to define in a fully portable way that has clear an unambiguous benefits. Some of the concepts for which it raises questions are not well defined on all systems that support C, and defining it in a simple way might provide no benefit over conventional include guards.

When the compile encounters #pragma once, how should it identify this file so that it doesn't include its contents again?

The obvious answer is the unique location of the file on the system. This is fine if the system has unique locations for all files but many systems provide links (symlinks and hardlinks) that mean that a 'file' doesn't have a unique location. Should the file be re-included just because it was found via a different name? Probably not.

But now there is a problem, how is it possible to define the behaviour of #pragma once in a way that has an exact meaning on all platforms - even those that don't even have directories, let alone symlinks - and still get the desirable behaviour on systems that do have them?

You could say that a files identity is determined by its contents, so if an included file has a #pragma once and a file is included that has exactly the same contents, then the second and subsequent #includes shall have no effect.

This is easy to define and has well defined semantics. It also has good properties such that if a project is moved from a system that supports and uses filesystem links to one that doesn't, it still behaves the same.

On the downside, every time an include file is encountered containing a #pragma once its contents must be checked against every other file using #pragma once that has already been included so far. This implies a performance hit similar to using #include guards in any case and adds a not insignificant burden to compiler writers. Obviously, the results of this could be cached, but the same is true for conventional include guards.

Conventional include guards force the programmer to choose a macro that is the unique identifier for an include file, but at least the behaviour is well-defined and simple to implement.

Given the potential pitfalls and costs, and the fact the conventional include guards do work, it is not surprising to me that the standards committee didn't feel the need to standardize #pragma once.
I also found before another point about pragma once being a problem in a specific but rare case, can't find it yet though.

Anyway, I don't bother personally, I use either one or the other depending on the project and it would not be any problem to me (I think) if Ogre or any other of my current dependencies would switch to #pragma once.

If you provide patch doing it, it would be interesting to check the behaviour in unity build mode....
0 x

User avatar
jacmoe
OGRE Retired Moderator
OGRE Retired Moderator
Posts: 20570
Joined: Thu Jan 22, 2004 10:13 am
Location: Denmark
Contact:

Re: Correct header inclusion guards to comply with C++ stand

Post by jacmoe » Tue Mar 12, 2013 7:10 pm

Yeah, but that topic is ancient, brother! :)
The situation was quite a bit different in 2009. I would recommend against it if someone asked me back then.
These days I bet we can't find a compiler which doesn't support it.
0 x
/* Less noise. More signal. */
Ogitor Scenebuilder - powered by Ogre, presented by Qt, fueled by Passion.
OgreAddons - the Ogre code suppository.


User avatar
Wolfmanfx
OGRE Team Member
OGRE Team Member
Posts: 1525
Joined: Fri Feb 03, 2006 10:37 pm
Location: Austria - Leoben
x 1
Contact:

Re: Correct header inclusion guards to comply with C++ stand

Post by Wolfmanfx » Tue Mar 12, 2013 9:17 pm

afaik i had problems with pragma once on android compilers - so i am unsure its wise to just change and than revert again when i stumble on the issuse again.
Also the code is already there why should we change a working system?
0 x

User avatar
jacmoe
OGRE Retired Moderator
OGRE Retired Moderator
Posts: 20570
Joined: Thu Jan 22, 2004 10:13 am
Location: Denmark
Contact:

Re: Correct header inclusion guards to comply with C++ stand

Post by jacmoe » Tue Mar 12, 2013 9:25 pm

OpenFrameworks changed #ifdefs to #pragma once for their Android code 4 months ago.
And I found some Android code using #pragma once in code from Google. So I guess it works for Android.
What kind of compiler are we talking about there?
I thought Android was primarily Java.
But, you have a point there.
0 x
/* Less noise. More signal. */
Ogitor Scenebuilder - powered by Ogre, presented by Qt, fueled by Passion.
OgreAddons - the Ogre code suppository.

User avatar
Wolfmanfx
OGRE Team Member
OGRE Team Member
Posts: 1525
Joined: Fri Feb 03, 2006 10:37 pm
Location: Austria - Leoben
x 1
Contact:

Re: Correct header inclusion guards to comply with C++ stand

Post by Wolfmanfx » Tue Mar 12, 2013 9:32 pm

android is not just java thx to android ndk we have many supported compiler gcc 4.3,4.6, clang as i mentioned i had to change my code during a customer project so i am not happy with such a change. This needs testing on all platforms.
0 x

User avatar
jacmoe
OGRE Retired Moderator
OGRE Retired Moderator
Posts: 20570
Joined: Thu Jan 22, 2004 10:13 am
Location: Denmark
Contact:

Re: Correct header inclusion guards to comply with C++ stand

Post by jacmoe » Tue Mar 12, 2013 9:34 pm

You are right.
It is supported by the major compilers:
http://en.wikipedia.org/wiki/Pragma_once#Portability
GCC, Clang, and (of course) VC and 4 others.

And I knew about the ndk :)
Thank God for that.
0 x
/* Less noise. More signal. */
Ogitor Scenebuilder - powered by Ogre, presented by Qt, fueled by Passion.
OgreAddons - the Ogre code suppository.

User avatar
Klaim
Old One
Posts: 2565
Joined: Sun Sep 11, 2005 1:04 am
Location: Paris, France
Contact:

Re: Correct header inclusion guards to comply with C++ stand

Post by Klaim » Tue Mar 12, 2013 9:52 pm

What this page doesn't say is what is exactly the algorithm implemented.
0 x

User avatar
madmarx
OGRE Expert User
OGRE Expert User
Posts: 1668
Joined: Mon Jan 21, 2008 10:26 pm

Re: Correct header inclusion guards to comply with C++ stand

Post by madmarx » Wed Mar 13, 2013 12:03 am

17.4.3.1.2 Global names [lib.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.11) is reserved to the implementation for any use.

So perhaps this inclusion guards could be changed to comply with the C++ standard? I know this is just a minor matter, but it wouldn't hurt to be as standard-compliant as possible, right?
This is standard. The quote is about the compiler, not the precompiler. #ifdef __ANYTHING is for the precompiler, while the quote is adressing only the compiler.

Concerning those who have never seen names collisions, I remember having such with directx with the directshow samples for example. Microsoft hid 2 files error.h with __ERROR__ defines inside. That was in 2009, and it could still be. Also I remember once or two copy paste errors on these (well it was quick to correct anyway, no big deal).

My point of view :
During long time I used #defines, but for the last 2 years I began using pragma because there are more arguments for it than against it :
1/ less error prone.
2/ vastly faster to preprocess (i remember a thread about these tests on the boost nabble). It is REALLY faster. Because most of the time, the precompiler knows that he does not need to re-open the .h (yes, that's the algorithm). Otherwise the precompiler opens the .h (<=> minimum 2 hdd accesses), and read that the precompiler-variable is already defined and does nothing with the file.
3/ easier to setup.
The case where pragma once is a problem are : when you use preprocessor programming with recurssive self inclusion of files. For example like with some of Boost.PP macros.

Now, if everything already work with #define in Ogre, I am not convinced there is so much to gain by suddenly changing the existing code base to use #pragma once; apart of course to gain some compilation speed.

Best regards,

Pierre
0 x
Tutorials + Ogre searchable API + more for Ogre1.7 : http://sourceforge.net/projects/so3dtools/
Corresponding thread : http://www.ogre3d.org/forums/viewtopic. ... 93&start=0

User avatar
Klaim
Old One
Posts: 2565
Joined: Sun Sep 11, 2005 1:04 am
Location: Paris, France
Contact:

Re: Correct header inclusion guards to comply with C++ stand

Post by Klaim » Wed Mar 13, 2013 9:48 am

madmarx wrote: Now, if everything already work with #define in Ogre, I am not convinced there is so much to gain by suddenly changing the existing code base to use #pragma once; apart of course to gain some compilation speed.
If it does reduce compilation time significantly, it would be a huge improvement.
0 x

CABAListic
OGRE Retired Team Member
OGRE Retired Team Member
Posts: 2903
Joined: Thu Jan 18, 2007 2:48 pm
Contact:

Re: Correct header inclusion guards to comply with C++ stand

Post by CABAListic » Wed Mar 13, 2013 9:54 am

That is unlikely. Most compilers do have special logic to deal with inclusion guards, so the difference should not be that noticable. And I'd bet you won't notice any when doing a Unity build.
0 x

User avatar
Klaim
Old One
Posts: 2565
Joined: Sun Sep 11, 2005 1:04 am
Location: Paris, France
Contact:

Re: Correct header inclusion guards to comply with C++ stand

Post by Klaim » Wed Mar 13, 2013 10:05 am

That is my understanding too. That being said, it would be interesting to check the fact.
I will not be online for some days so I can't test this PR yet but will later, see if I find a difference, but as you say, I doubt it.
0 x

User avatar
madmarx
OGRE Expert User
OGRE Expert User
Posts: 1668
Joined: Mon Jan 21, 2008 10:26 pm

Re: Correct header inclusion guards to comply with C++ stand

Post by madmarx » Wed Mar 13, 2013 12:51 pm

And I'd bet you won't notice any when doing a Unity build.
Yes, I completely agree with you.
Most compilers do have special logic to deal with inclusion guards, so the difference should not be that noticable.
I didn't know that, but that would make sense.

Also the "include precompilation time acceleration" is very unlikely to be a major part of the whole compilation time, that is why I don't think there could be much gain anyway.
The figures where on the Boost mailing list 2-3 years ago.
0 x
Tutorials + Ogre searchable API + more for Ogre1.7 : http://sourceforge.net/projects/so3dtools/
Corresponding thread : http://www.ogre3d.org/forums/viewtopic. ... 93&start=0

User avatar
jacmoe
OGRE Retired Moderator
OGRE Retired Moderator
Posts: 20570
Joined: Thu Jan 22, 2004 10:13 am
Location: Denmark
Contact:

Re: Correct header inclusion guards to comply with C++ stand

Post by jacmoe » Wed Mar 13, 2013 3:43 pm

So, the bottom line is that there is very little difference between pragma once and include guards, except that VS handles pragma once slightly more efficient than include guards (although I don't know if that's changed).
But it's definitely cleaner, and less error prone.
Include guards feels like a hack to me - I mean: it's introducing symbols for no real reason. Then again, C/C++ is full of hacks. :)
0 x
/* Less noise. More signal. */
Ogitor Scenebuilder - powered by Ogre, presented by Qt, fueled by Passion.
OgreAddons - the Ogre code suppository.

Post Reply