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

[Eric Anholt]

Dave Airlie <airlied at gmail.com> writes:

> 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.

Right.  I was only trying to correct the statement about pragma being an
optimization in this thread.  Changing existing include guards/pragma
once isn't worth anyone's time IMO.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 800 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161005/a00a92e0/attachment.sig>