[Mesa-dev] Mesa include guard style. (Was: [PATCH] i965/cfg: Remove redundant #pragma once.)

[Dave Airlie]

On 13 March 2016 at 11:29, Ian Romanick <idr at freedesktop.org> wrote:
> On 03/11/2016 03:46 PM, Eric Anholt wrote:
>> Ian Romanick <idr at freedesktop.org> writes:
>>
>>> On 03/10/2016 05:53 PM, Francisco Jerez wrote:
>>>> Iago Toral <itoral at igalia.com> writes:
>>>>
>>>>> On Wed, 2016-03-09 at 19:04 -0800, Francisco Jerez wrote:
>>>>>> Matt Turner <mattst88 at gmail.com> writes:
>>>>>>
>>>>>>> On Wed, Mar 9, 2016 at 1:37 PM, Francisco Jerez <currojerez at riseup.net> wrote:
>>>>>>>> Iago Toral <itoral at igalia.com> writes:
>>>>>>>>
>>>>>>>>> On Tue, 2016-03-08 at 17:42 -0800, Francisco Jerez wrote:
>>>>>>>>>> brw_cfg.h already has include guards, remove the "#pragma once" which
>>>>>>>>>> is redundant and non-standard.
>>>>>>>>>
>>>>>>>>> FWIW, I think using both #pragma once and include guards is a way to
>>>>>>>>> keep portability while still getting the performance advantage of
>>>>>>>>> #pragma once where it is supported.
>>>>>>>>>
>>>>>>>> It's highly unlikely to make any significant difference on any
>>>>>>>> reasonably modern compiler.  I cannot measure any change in compilation
>>>>>>>> time locally from my cleanup.
>>>>>>>>
>>>>>>>>> Also it seems that we do the same thing in many other files...
>>>>>>>>>
>>>>>>>> Really?  I'm not aware of any other file where we use both.
>>>>>>>
>>>>>>> There are quite a few in glsl/
>>>>>>
>>>>>> Heh, apparently you're right.  Anyway it seems rather pointless to use
>>>>>> '#pragma once' in a bunch of scattered header files with the expectation
>>>>>> to gain some speed, the improvement from a single header file is so
>>>>>> minuscule (if it will make any difference at all on a modern compiler
>>>>>> and compilation workload, which I doubt) that we would have to use it
>>>>>> universally in order to have the chance to measure any improvement.
>>>>>>
>>>>>> Can we please just decide for one of the include guard styles and use it
>>>>>> consistently?  Given that the majority of header files in the Mesa
>>>>>> codebase use old-school define guards, that it's the only standard
>>>>>> option, that it has well-defined semantics in presence of file copies
>>>>>> and hardlinks, and that the performance argument against it is rather
>>>>>> dubious (although I definitely find '#pragma once' prettier and more
>>>>>> concise), I'd vote for using preprocessor define guards universally.
>>>>>>
>>>>>> What do other people think?
>>>>>
>>>>> I think we have to use define guards necessarily since #pragma once is
>>>>> not standard even it it has wide support. So the question is whether we
>>>>> want to use only define guards or define guards plus #pragma once. I am
>>>>> fine with doing only define guards as you propose.
>>>>
>>>> *Shrug* I have the impression that the only real advantage of '#pragma
>>>> once' is that you no longer need to do the ifndef/define dance, so I
>>>> don't think I can see much benefit in doing both.
>>>
>>> Several compilers will cache the file name where '#pragma once' occurs
>>> and never read that file again.  A #include of a file previously seen
>>> with '#pragma once' becomes a no-op.  Since the file is never read, the
>>> compiler avoids all the I/O and the parsing.  That is true of MSVC and,
>>> I thought, some versions of GCC.  As Iago points out, some compilers
>>> ignore the #pragma altogether.  Since Mesa supports (or does it?) some
>>> of these compilers, we have to have the ifdef/define/endif guards.
>>
>> Compilers have noticed that ifdef/define/endif is a thing and optimized
>> it, anyway.
>>
>> https://gcc.gnu.org/onlinedocs/cppinternals/Guard-Macros.html
>
> That's cool!  I don't think GCC did that when I looked into this in
> 2010.  It sounds like the #pragma actually breaks the GCC optimization,
> so let's get rid of them all.

Just to reignite this, I don't this statement is any way true. using #pragma
once doesn't break GCC optimisation, the optimisation isn't useful in the
presence of #pragma once, as gcc will never ever read those files again,
so there is no need to do it.

Dave.