#pragma once usage

4,064 views
Skip to first unread message

Thiago Farina

unread,
Jul 9, 2012, 4:06:50 PM7/9/12
to Chromium-dev
Hi,

Today this came out in a review. Why do we use #pragma once after the
header include guards in our header files? Is there any boost for
using it? remoting/ does not make use of it at all.

--
Thiago

Daniel Cheng

unread,
Jul 9, 2012, 4:15:38 PM7/9/12
to tfa...@chromium.org, Chromium-dev
It sped up the compile: https://groups.google.com/a/chromium.org/forum/?fromgroups#!topic/chromium-dev/U3kd_jPQC9k

Daniel



--
Thiago

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
    http://groups.google.com/a/chromium.org/group/chromium-dev

Alex Pakhunov

unread,
Jul 9, 2012, 4:22:23 PM7/9/12
to dch...@chromium.org, tfa...@chromium.org, Chromium-dev
Hi,

Interesting. According to Google C++ style guide "#pragma once" should not be used: http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Windows_Code#Windows_Code:

Do not use #pragma once; instead use the standard Google include guards. The path in the include guards should be relative to the top of your project tree.

But Chromium style guide requires it to be used: http://dev.chromium.org/developers/coding-style:

When writing header (.h) files, use both standard include guards and #pragma once.  See an existing header for the appropriate layout.

Alex. 
--
Alex.

Victor Khimenko

unread,
Jul 9, 2012, 4:54:13 PM7/9/12
to alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
On Tue, Jul 10, 2012 at 12:22 AM, Alex Pakhunov <alex...@google.com> wrote:
Hi,

Interesting. According to Google C++ style guide "#pragma once" should not be used: http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Windows_Code#Windows_Code:

Do not use #pragma once; instead use the standard Google include guards. The path in the include guards should be relative to the top of your project tree.

But Chromium style guide requires it to be used: http://dev.chromium.org/developers/coding-style:

When writing header (.h) files, use both standard include guards and #pragma once.  See an existing header for the appropriate layout.

That's because "#pragma once" can introduce subtle errors and is Windows-only optimization. Google3 does not care about Windows all that much while for Chrome it's the most important platform.

Mehrdad Niknami

unread,
Jul 9, 2012, 4:58:24 PM7/9/12
to kh...@chromium.org, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
Aren't #pragma's ignored by the compiler if they're unrecognized? (Although I've heard some major compilers for other platforms apparently also support it, though I've never tried it.)

Mehrdad

Mark Mentovai

unread,
Jul 9, 2012, 4:59:48 PM7/9/12
to kh...@chromium.org, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
Victor Khimenko wrote:
That's because "#pragma once" can introduce subtle errors and is Windows-only optimization.

That’s not quite true. GCC and Clang understand #pragma once, although it’s not necessary for them because they understand the #ifndef X #define X … #endif pattern and apply a similar optimization when they find it.

On the other hand, MSVC does not recognize this pattern, so #pragma once effectively holds its hand and tells it to never #include another copy of the file. In a project that makes a lot of use of MSVC, as you point out, this optimization can be beneficial.

Victor Khimenko

unread,
Jul 9, 2012, 5:28:00 PM7/9/12
to Mark Mentovai, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
On Tue, Jul 10, 2012 at 12:59 AM, Mark Mentovai <ma...@chromium.org> wrote:
Victor Khimenko wrote:
That's because "#pragma once" can introduce subtle errors and is Windows-only optimization.

That’s not quite true.

What exactly is not true? That it can introduce errors or that it's Windows-only optimization?
 
GCC and Clang understand #pragma once, although it’s not necessary for them because they understand the #ifndef X #define X … #endif pattern and apply a similar optimization when they find it.

Yup. And they do it RIGHT. "#pragma once" does not.

Fred Akalin

unread,
Jul 9, 2012, 5:30:47 PM7/9/12
to kh...@chromium.org, Mark Mentovai, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
On Mon, Jul 9, 2012 at 2:28 PM, Victor Khimenko <kh...@chromium.org> wrote:
On Tue, Jul 10, 2012 at 12:59 AM, Mark Mentovai <ma...@chromium.org> wrote:
Victor Khimenko wrote:
That's because "#pragma once" can introduce subtle errors and is Windows-only optimization.

That’s not quite true.

What exactly is not true? That it can introduce errors or that it's Windows-only optimization?

As Mark already said, it's not Windows-only as GCC and Clang understand it. 
 
Yup. And they do it RIGHT. "#pragma once" does not.

What's wrong with #pragma once?  What subtle errors can it introduce? 

Mark Mentovai

unread,
Jul 9, 2012, 5:30:57 PM7/9/12
to Victor Khimenko, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
Victor Khimenko wrote:
On Tue, Jul 10, 2012 at 12:59 AM, Mark Mentovai <ma...@chromium.org> wrote:
Victor Khimenko wrote:
That's because "#pragma once" can introduce subtle errors and is Windows-only optimization.

That’s not quite true.

What exactly is not true? That it can introduce errors or that it's Windows-only optimization?

My next sentence should have clarified this:

Victor Khimenko

unread,
Jul 9, 2012, 5:34:39 PM7/9/12
to Mehrdad Niknami, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
On Tue, Jul 10, 2012 at 12:58 AM, Mehrdad Niknami <mnik...@chromium.org> wrote:
Aren't #pragma's ignored by the compiler if they're unrecognized? (Although I've heard some major compilers for other platforms apparently also support it, though I've never tried it.)

GCC supports it - and this is exactly why it's dangerous. It breaks the

#define __need_size_t
#include <cstddef>

pattern. Which Windows uses very much, too, but there are you are supposed to just cope with the problem somehow.

Fred Akalin

unread,
Jul 9, 2012, 5:40:54 PM7/9/12
to kh...@chromium.org, Mehrdad Niknami, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
On Mon, Jul 9, 2012 at 2:34 PM, Victor Khimenko <kh...@chromium.org> wrote:
GCC supports it - and this is exactly why it's dangerous. It breaks the

#define __need_size_t
#include <cstddef>

pattern. Which Windows uses very much, too, but there are you are supposed to just cope with the problem somehow.

How does it break that pattern?  Are you saying that if cstddef uses #pragma once, that won't work properly if it has already been included?  How is that different from regular include guards? 

Peter Kasting

unread,
Jul 9, 2012, 5:41:59 PM7/9/12
to kh...@chromium.org, Mehrdad Niknami, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
On Mon, Jul 9, 2012 at 2:34 PM, Victor Khimenko <kh...@chromium.org> wrote:
On Tue, Jul 10, 2012 at 12:58 AM, Mehrdad Niknami <mnik...@chromium.org> wrote:
Aren't #pragma's ignored by the compiler if they're unrecognized? (Although I've heard some major compilers for other platforms apparently also support it, though I've never tried it.)

GCC supports it - and this is exactly why it's dangerous. It breaks the

#define __need_size_t
#include <cstddef>

pattern. Which Windows uses very much, too, but there are you are supposed to just cope with the problem somehow.

Breaks the pattern how?  We add #pragma once to user-written headers that are meant to be included once and thus also have standard #include guards.  We don't use it in isolation or to change what would otherwise be a multiple-inclusion-safe header (of which there are a couple in Chrome, in the IPC subsystem) into something else.

PK

Victor Khimenko

unread,
Jul 9, 2012, 5:44:10 PM7/9/12
to Fred Akalin, Mehrdad Niknami, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
Yup.
 
 How is that different from regular include guards? 

They work. "#pragma once" does not. Very easy to see.

$ cat test.c
#define _ONE_
#include "test.h"
#undef _ONE_

#define _TWO_
#include "test.h"
#undef _TWO_
khim@khim-glaptop:/tmp/2$ cat test.h
#ifdef _ONE_
int one;
#endif
#ifdef _TWO_
int two;
$ gcc -E test.c
# 1 "test.c"
# 1 "<built-in>"
# 1 "<command-line>"
# 1 "test.c"

# 1 "test.h" 1

int one;
# 3 "test.c" 2



# 1 "test.h" 1




int two;
# 7 "test.c" 2
$ cat test.h
#ifdef _ONE_
int one;
#endif
#ifdef _TWO_
int two;
#endif
#pragma once
$ gcc -E test.c
# 1 "test.c"
# 1 "<built-in>"
# 1 "<command-line>"
# 1 "test.c"

# 1 "test.h" 1

int one;




       
# 3 "test.c" 2

Jeffrey Yasskin

unread,
Jul 9, 2012, 5:46:02 PM7/9/12
to kh...@chromium.org, Mehrdad Niknami, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
On Mon, Jul 9, 2012 at 2:34 PM, Victor Khimenko <kh...@chromium.org> wrote:
>
>
>
> On Tue, Jul 10, 2012 at 12:58 AM, Mehrdad Niknami <mnik...@chromium.org>
> wrote:
>>
>> Aren't #pragma's ignored by the compiler if they're unrecognized?
>> (Although I've heard some major compilers for other platforms apparently
>> also support it, though I've never tried it.)
>>
> GCC supports it - and this is exactly why it's dangerous. It breaks the
>
> #define __need_size_t
> #include <cstddef>
>
> pattern. Which Windows uses very much, too, but there are you are supposed
> to just cope with the problem somehow.

I don't know a whole lot about the Windows side of this, but every
time I've interacted with the __need_size_t bits of stddef.h, it's
been a disaster. stddef.h should have been implemented with a
collection of small headers that had real include guards, instead of
trying to be clever with the preprocessor. I hope you're not
advocating writing more headers like that.

Jeffrey

Victor Khimenko

unread,
Jul 9, 2012, 5:47:08 PM7/9/12
to Peter Kasting, Mehrdad Niknami, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
On Tue, Jul 10, 2012 at 1:41 AM, Peter Kasting <pkas...@chromium.org> wrote:
On Mon, Jul 9, 2012 at 2:34 PM, Victor Khimenko <kh...@chromium.org> wrote:
On Tue, Jul 10, 2012 at 12:58 AM, Mehrdad Niknami <mnik...@chromium.org> wrote:
Aren't #pragma's ignored by the compiler if they're unrecognized? (Although I've heard some major compilers for other platforms apparently also support it, though I've never tried it.)

GCC supports it - and this is exactly why it's dangerous. It breaks the

#define __need_size_t
#include <cstddef>

pattern. Which Windows uses very much, too, but there are you are supposed to just cope with the problem somehow.

Breaks the pattern how?  We add #pragma once to user-written headers that are meant to be included once and thus also have standard #include guards.

Right. If you know how "#pragma once" works then you can use it and produce correct can working program. Still it can turn correct program to uncompileable or, even worse, incorrect program. And that's dangerous.
 
We don't use it in isolation or to change what would otherwise be a multiple-inclusion-safe header (of which there are a couple in Chrome, in the IPC subsystem) into something else.

Sure. But if it does not improve the compilation speed (and it does not on non-Windows platforms) then it's simpler to just not use it.

 
PK

Alex Pakhunov

unread,
Jul 9, 2012, 5:47:57 PM7/9/12
to Victor Khimenko, Fred Akalin, Mehrdad Niknami, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
Hi,

That is exactly what Peter and Fred are talking about. One shouldn't put "#pragma once" into a header that supposed to be included multiple times.

Alex.
--
Alex.

Fred Akalin

unread,
Jul 9, 2012, 5:48:09 PM7/9/12
to Victor Khimenko, Mehrdad Niknami, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
On Mon, Jul 9, 2012 at 2:44 PM, Victor Khimenko <kh...@chromium.org> wrote:
They work. "#pragma once" does not. Very easy to see.

Those are something other than include guards.  Include guards are:

#ifndef FOO_H
#define FOO_H
...
#endif

What you have is just preprocessor trickery.  It is an error for a header with such trickery to define #pragma once, just as it is an error for it to have include guards (as above).  There's nothing inherently wrong with #pragma once that I can see.

Victor Khimenko

unread,
Jul 9, 2012, 5:48:53 PM7/9/12
to Jeffrey Yasskin, Mehrdad Niknami, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
It's not the question of "good" or "bad". Such headers usually are result of long evolution and they are bad enough by themselves, but when you combine them with "#pragma once" the end result is disaster.

Mehrdad Niknami

unread,
Jul 9, 2012, 5:48:24 PM7/9/12
to Victor Khimenko, Fred Akalin, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
On Mon, Jul 9, 2012 at 2:44 PM, Victor Khimenko <kh...@chromium.org> wrote:
They work. "#pragma once" does not. Very easy to see.

$ cat test.c
#define _ONE_
#include "test.h"
#undef _ONE_

#define _TWO_
#include "test.h"
#undef _TWO_

Why is the header testing for something defined in the C file?

Victor Khimenko

unread,
Jul 9, 2012, 5:50:33 PM7/9/12
to Mehrdad Niknami, Fred Akalin, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
Why <windows.h> is testing for something defined in the C file?

More often then not such defines are used not in C files, but in headers which include other headers.

Peter Kasting

unread,
Jul 9, 2012, 5:53:09 PM7/9/12
to Victor Khimenko, Mehrdad Niknami, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
On Mon, Jul 9, 2012 at 2:47 PM, Victor Khimenko <kh...@chromium.org> wrote:
On Tue, Jul 10, 2012 at 1:41 AM, Peter Kasting <pkas...@chromium.org> wrote:
On Mon, Jul 9, 2012 at 2:34 PM, Victor Khimenko <kh...@chromium.org> wrote:
On Tue, Jul 10, 2012 at 12:58 AM, Mehrdad Niknami <mnik...@chromium.org> wrote:
Aren't #pragma's ignored by the compiler if they're unrecognized? (Although I've heard some major compilers for other platforms apparently also support it, though I've never tried it.)

GCC supports it - and this is exactly why it's dangerous. It breaks the

#define __need_size_t
#include <cstddef>

pattern. Which Windows uses very much, too, but there are you are supposed to just cope with the problem somehow.

Breaks the pattern how?  We add #pragma once to user-written headers that are meant to be included once and thus also have standard #include guards.

Right. If you know how "#pragma once" works then you can use it and produce correct can working program. Still it can turn correct program to uncompileable or, even worse, incorrect program. And that's dangerous.

There are a hundred trillion ways to shoot yourself in the foot using C++.  Saying that it's possible to do it by misusing this is neither interesting nor useful.

The rule in Chrome is that we use #pragma once and include guards (not the trickery you were demonstrating -- those aren't #include guards and are not relevant here; see Fred's message) in headers we write that are meant to be #included once.

No one is discussing using #pragma once in any other way.  No one but you is discussing library headers (or ANY headers) that are meant for multiple inclusion, conditionalized inclusion, etc.  You're muddying the waters.

PK

Mark Mentovai

unread,
Jul 9, 2012, 5:54:30 PM7/9/12
to kh...@chromium.org, Jeffrey Yasskin, Mehrdad Niknami, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
That’s fine. If such a header winds up with #pragma once written into it, it’s an error and a bug, and the #pragma once should be removed immediately. That doesn’t render #pragma once useless. We still use it in Chrome where it’s helpful to work around MSVC’s failure to optimize the #ifndef X #define X … #endif pattern as Clang and GCC do.

<stddef.h> (and <cstddef>) do not mention #pragma once.

Headers that expect to be included multiple times do not mention #pragma once, and do not follow the #ifndef X #define X … #endif pattern, at least not without permitting the user (#includer) to #undef X. In the bulk of Chrome headers, X in this case is owned by the header itself and it is an error for the #includer to #undef X, therefore the #ifndef X #define X … #endif pattern is effective, and #pragma once adds no peril.


Victor Khimenko

unread,
Jul 9, 2012, 5:54:35 PM7/9/12
to Fred Akalin, Mehrdad Niknami, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
I never said it was somehow "wrong". You can take perfectly valid 100% correct ANSI C program, add "#pragma once" to one file and the end result will be incorrect program. That's dangerous. If you perceive it as "right" or "wrong" is another question.

Mehrdad Niknami

unread,
Jul 9, 2012, 5:54:19 PM7/9/12
to Victor Khimenko, Fred Akalin, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
On Mon, Jul 9, 2012 at 2:50 PM, Victor Khimenko <kh...@chromium.org> wrote:
Why <windows.h> is testing for something defined in the C file?

It does? Which .c files are #included by the Windows SDK?

More often then not such defines are used not in C files, but in headers which include other headers.

But these #define's are themselves the problem, not #pragma once.

First, #define followed by #undef is just code smell. For properly managing them like that you would need push_macro and pop_macro. Of course, since those don't exist, you run into some nasty bugs.

That said, if you have header 1 and it's including header 2, and they have a circular #define dependency, that's also a code smell. It's completely irrelevant whether or not you're using #pragma once, because it's going to break with regular include guards anyway. (Which are not used in your example.)

Ryan Sleevi

unread,
Jul 9, 2012, 5:55:51 PM7/9/12
to kh...@chromium.org, Mehrdad Niknami, Fred Akalin, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
For <windows.h>, we explicitly try to put these settings in the .gyp files (either via build/common.gypi or via the include settings). This ensures that everywhere the particular symbol is seen, it will maintain its correct size, alignment, etc.

For general user headers, if you're relying on such pre-processor trickery (and really, you should try to avoid it - but exceptions exist, such as ipc/ code), you should not be using include guards - either via the #ifndef+#define+endif or via #pragma once.

Victor Khimenko

unread,
Jul 9, 2012, 5:56:21 PM7/9/12
to Mark Mentovai, Jeffrey Yasskin, Mehrdad Niknami, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
On Tue, Jul 10, 2012 at 1:54 AM, Mark Mentovai <ma...@chromium.org> wrote:
That’s fine. If such a header winds up with #pragma once written into it, it’s an error and a bug, and the #pragma once should be removed immediately. That doesn’t render #pragma once useless.

What makes it useless for Google3 is the fact that it does not improve compilation speed. Add to this the fact that it's dangerous and it's [rightfully] banned.

Situation with Chrome (which cares about MSVC) is different.

Fred Akalin

unread,
Jul 9, 2012, 5:58:38 PM7/9/12
to kh...@chromium.org, Mark Mentovai, Jeffrey Yasskin, Mehrdad Niknami, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
On Mon, Jul 9, 2012 at 2:56 PM, Victor Khimenko <kh...@chromium.org> wrote:

On Tue, Jul 10, 2012 at 1:54 AM, Mark Mentovai <ma...@chromium.org> wrote:
That’s fine. If such a header winds up with #pragma once written into it, it’s an error and a bug, and the #pragma once should be removed immediately. That doesn’t render #pragma once useless.

What makes it useless for Google3 is the fact that it does not improve compilation speed. Add to this the fact that it's dangerous and it's [rightfully] banned.

You keep calling it "dangerous" and "error-prone" (although I guess you refrain from calling it "wrong"), although we've just shown that it's no more dangerous than regular include guards.  Why do you persist? :)

Victor Khimenko

unread,
Jul 9, 2012, 6:05:26 PM7/9/12
to Fred Akalin, Mark Mentovai, Jeffrey Yasskin, Mehrdad Niknami, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
Where have I said that include guards are NOT "dangerous" or "error-prone"? They sure are: the whole "include files" strategy of C/C++ is a disaster (that's why there are talks about proper module system). But they are also necessary evil: it's impossible to program in C/C++ without using include guards OR #pragma once.

Include guards are well-known portable solution, there are no need to add another one if there are no tangible benefits. And in absence of MSVC there are no such benefits.

Mehrdad Niknami

unread,
Jul 9, 2012, 6:06:57 PM7/9/12
to Victor Khimenko, Fred Akalin, Mark Mentovai, Jeffrey Yasskin, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
Where have I said that include guards are NOT "dangerous" or "error-prone"?


On Mon, Jul 9, 2012 at 2:44 PM, Victor Khimenko <kh...@chromium.org> wrote:
 How is that different from regular include guards? 
They work. "#pragma once" does not. Very easy to see.

Peter Kasting

unread,
Jul 9, 2012, 6:07:57 PM7/9/12
to kh...@chromium.org, Mark Mentovai, Jeffrey Yasskin, Mehrdad Niknami, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
On Mon, Jul 9, 2012 at 2:56 PM, Victor Khimenko <kh...@chromium.org> wrote:
On Tue, Jul 10, 2012 at 1:54 AM, Mark Mentovai <ma...@chromium.org> wrote:
That’s fine. If such a header winds up with #pragma once written into it, it’s an error and a bug, and the #pragma once should be removed immediately. That doesn’t render #pragma once useless.

What makes it useless for Google3 is the fact that it does not improve compilation speed. Add to this the fact that it's dangerous and it's [rightfully] banned.

I suspect that in google3 it's not used because it's not effective and people who see it might think they can remove their normal #include guards and just use it instead, and THAT'S the danger.

The "dangerous" pattern you're trying to talk about isn't dangerous.  It's just broken.  So is misusing templates and supplying the wrong number of arguments to a function and all sorts of other things we coders can do by mistake or ignorance.  "If you insert #pragma once you can break compilation" -- yes, just like you can break compilation by inserting a semicolon.  We don't ban semicolons.

Situation with Chrome (which cares about MSVC) is different.
 
Indeed, so let's get back to being productive.

If you're working in Chrome code, and your header is meant to be included only once, please use normal #include guards as well as #pragma once, until such time as someone definitively shows that newer versions of MSVC get no compile speedup from doing so.  Folks who own remoting/, if you can fix this about your code, that'd be great.  Thanks.

PK

Victor Khimenko

unread,
Jul 9, 2012, 6:11:58 PM7/9/12
to Mehrdad Niknami, Fred Akalin, Mark Mentovai, Jeffrey Yasskin, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
That's not question of "dangerous" or "error-prone". It's another problem: versatility. If you insist on saying that include guards must ALWAYS cover the whole file then they, of course, work in the same way as "#pragma once". But they can cover part of the file, too - and such use in incompatible with "#pragma once".

Thiago Farina

unread,
Jul 9, 2012, 6:14:03 PM7/9/12
to Peter Kasting, kh...@chromium.org, Mark Mentovai, Jeffrey Yasskin, Mehrdad Niknami, alex...@google.com, dch...@chromium.org, Chromium-dev
Victor,

If it isn't clear yet, just to clarify, this issue was about:

foo.h

#ifndef FOO_H_
#define FOO_H_
#pragma once

...

#endif // FOO_H_

And what I was saying is that it was requested to me to remove the
#pragma once from the header files.

--
Thiago

Peter Kasting

unread,
Jul 9, 2012, 6:14:34 PM7/9/12
to kh...@chromium.org, Mehrdad Niknami, Fred Akalin, Mark Mentovai, Jeffrey Yasskin, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
On Mon, Jul 9, 2012 at 3:11 PM, Victor Khimenko <kh...@chromium.org> wrote:
It's another problem: versatility. If you insist on saying that include guards must ALWAYS cover the whole file then they, of course, work in the same way as "#pragma once". But they can cover part of the file, too - and such use in incompatible with "#pragma once".

Seriously, please stop.

This is not a thread about all possible preprocessor manipulations, whether #pragma once corresponds to each, the design flaws of C, etc.  It is a specific question about a specific kind of usage, which has been answered.

PK 

Mehrdad Niknami

unread,
Jul 9, 2012, 6:14:10 PM7/9/12
to Victor Khimenko, Fred Akalin, Mark Mentovai, Jeffrey Yasskin, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
On Mon, Jul 9, 2012 at 3:11 PM, Victor Khimenko <kh...@chromium.org> wrote:
But they can cover part of the file, too

No. If you do that then they're not called "include guards".

Mehrdad Niknami

unread,
Jul 9, 2012, 6:15:12 PM7/9/12
to Mehrdad Niknami, Victor Khimenko, Fred Akalin, Mark Mentovai, Jeffrey Yasskin, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
I think what you're misunderstanding is that #include guards guard against the #inclusion of current file, not other files.

As such, they are only called "include guards" if they cover the entire file.

Alex Pakhunov

unread,
Jul 9, 2012, 6:20:37 PM7/9/12
to Peter Kasting, kh...@chromium.org, Mark Mentovai, Jeffrey Yasskin, Mehrdad Niknami, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
Hi,

Folks who own remoting/, if you can fix this about your code, that'd be great. 

Sure thing. Will do.

Alex. 
--
Alex.

Ami Fischman

unread,
Jul 9, 2012, 6:20:42 PM7/9/12
to pkas...@google.com, kh...@chromium.org, Mark Mentovai, Jeffrey Yasskin, Mehrdad Niknami, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
If you're working in Chrome code, and your header is meant to be included only once, please use normal #include guards as well as #pragma once, until such time as someone definitively shows that newer versions of MSVC get no compile speedup from doing so.  Folks who own remoting/, if you can fix this about your code, that'd be great.  Thanks.

Nononono.

I brought this up in private mail w/ Peter & thakis@ almost a year ago because (quoting/paraphrasing from the thread):
- the originally-proposing thread petered out without confirmation that the change was worthwhile (doubt was cast on the reproducibility of the benefit).  The proposed PRESUBMIT http://codereview.chromium.org/3038036 ends with Nico saying he's thinking of backing out the change.
- as of a year ago, the codebase had ~67% adherence to the rule (and 88% in the dirs Nico did in his initial pass), implying people were at least partially unaware of the rule
- Nico thought there was no rule, but Peter pointed out that there was
Nico's conclusion (speaking of the rule) was: "Remove it. There's no data that supports it makes sense."
Peter's response was: "Then we should remove all the declarations as well.  Having declarations with no rule is an incoherent state for the codebase to be in, which is precisely why the rule got added in the first place."
The thread died there.

So, instead of adding more #pragma's which nobody is willing to stand up for (and measure), let's *remove* the existing ones, as well as the style guide rule.

-a

Victor Khimenko

unread,
Jul 9, 2012, 6:20:54 PM7/9/12
to Mehrdad Niknami, Fred Akalin, Mark Mentovai, Jeffrey Yasskin, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
On Tue, Jul 10, 2012 at 2:15 AM, Mehrdad Niknami <mnik...@chromium.org> wrote:
I think what you're misunderstanding is that #include guards guard against the #inclusion of current file, not other files.

And if they are designed to work with only part of the file then why they are not include guards? Sure, my example omitted include guards for the simplicity, but it's easy to do:

#ifdef _ONE_
#ifndef TEST_H_ONE
#define TEST_H_ONE
int one;
#endif
#endif
#ifdef _TWO_
#ifndef TEST_H_TWO
#define TEST_H_TWO
int two;
#endif
#endif
 
As such, they are only called "include guards" if they cover the entire file.

Why not?

Victor Khimenko

unread,
Jul 9, 2012, 6:28:06 PM7/9/12
to Fred Akalin, Mark Mentovai, Jeffrey Yasskin, Mehrdad Niknami, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
On Tue, Jul 10, 2012 at 1:58 AM, Fred Akalin <aka...@chromium.org> wrote:
They are MORE dangerous then include guards BTW. Especially in multilayered projects.

Simple example:
$ cat test.c
#include "test1/test.h"
#include "test2/test.h"
$ cat test1/test.h 
#pragma once

int x;
$ cat test2/test.h 
#pragma once

int x;
$ gcc -E test.c 
# 1 "test.c"
# 1 "<built-in>"
# 1 "<command-line>"
# 1 "test.c"
# 1 "test1/test.h" 1
       

int x;
# 2 "test.c" 2

Everything works just fine. Now let's imagine that someone moved file around using some questionable means:
$ todos test2/test.h
$ gcc -E test.c 
# 1 "test.c"
# 1 "<built-in>"
# 1 "<command-line>"
# 1 "test.c"
# 1 "test1/test.h" 1
       

int x;
# 2 "test.c" 2
# 1 "test2/test.h" 1
       

int x;
# 2 "test.c" 2

Oops?

Problems with version skew are quite real although I must admit that both Google3 and Chrome build systems don't usually have this problem: they don't use usual cycle of "./configure ; make ; make install"...

Peter Kasting

unread,
Jul 9, 2012, 6:29:43 PM7/9/12
to Ami Fischman, kh...@chromium.org, Mark Mentovai, Jeffrey Yasskin, Mehrdad Niknami, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
On Mon, Jul 9, 2012 at 3:20 PM, Ami Fischman <fisc...@chromium.org> wrote:
If you're working in Chrome code, and your header is meant to be included only once, please use normal #include guards as well as #pragma once, until such time as someone definitively shows that newer versions of MSVC get no compile speedup from doing so.  Folks who own remoting/, if you can fix this about your code, that'd be great.  Thanks.

So, instead of adding more #pragma's which nobody is willing to stand up for (and measure), let's *remove* the existing ones, as well as the style guide rule.

Either way is acceptable, but being half-in and half-out is not fine.  If you really care to see this gone, then make it happen.  Since it hadn't happened as of the start of this thread, the current style guide rule and the prevailing code pattern is to put these in, and that is what we should consistently do until this is fixed.  I am opposed to people checking in new code without these guards if we are also not simultaneously removing all the existing ones and changing the style rule.

(Personally, I'm also opposed to making any changes without running another test, which is comprehensive enough to give us complete confidence that these guards do or do not help.  We should do this anyway as perhaps MSVC 2010 performs differently than 2008.)

PK

Mehrdad Niknami

unread,
Jul 9, 2012, 6:29:21 PM7/9/12
to Victor Khimenko
-others
+bcc:others

On Mon, Jul 9, 2012 at 3:20 PM, Victor Khimenko <kh...@chromium.org> wrote:
And if they are designed to work with only part of the file then why they are not include guards?

Yes. That's simply not what "include guard" means. It does not mean "a guard against some arbitrary #include".

Sure, my example omitted include guards for the simplicity, but it's easy to do:

#ifdef _ONE_
#ifndef TEST_H_ONE

Your guard is not covering the line before it.


As such, they are only called "include guards" if they cover the entire file.

Why not?

(1) By definition. The term has a specific meaning, and it's not what you're describing.
(2) It's logically impossible to guard against the inclusion of the current file without simultaneously guarding against the inclusion of every line.

Ben Rudiak-Gould

unread,
Jul 9, 2012, 7:30:09 PM7/9/12
to aka...@chromium.org, kh...@chromium.org, Chromium-dev
On Mon, Jul 9, 2012 at 2:30 PM, Fred Akalin <aka...@chromium.org> wrote:
> What's wrong with #pragma once? What subtle errors can it introduce?

The real problem with #pragma once, as far as I know, is that the same
header can be included via different paths because of hardlinks or
symlinks or an actual copy in the source tree or whatever. #pragma
once says "don't include this file again", but it's not always clear
what "this file" means. Include guards say "don't include any file
with the FOO_H include guard again", which avoids that problem. Of
course, it introduces the problem of having to make sure that you
don't accidentally reuse a symbol.

-- Ben

Peter Kasting

unread,
Jul 9, 2012, 10:03:25 PM7/9/12
to be...@chromium.org, aka...@chromium.org, kh...@chromium.org, Chromium-dev
This is not a problem for Chromium code in general; the Google style guide mandates the use of identifiers that contain something akin to the full repository path to the file, which should guarantee that the identifier chosen for the include guard is globally unique.

PK

Wez

unread,
Jul 10, 2012, 2:28:01 PM7/10/12
to pkas...@google.com, Ami Fischman, kh...@chromium.org, Mark Mentovai, Jeffrey Yasskin, Mehrdad Niknami, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
In the absence of new data to support or refute the performance gain with pragma once, I think it makes sense to add the pragma to remoting/ et al for consistency, as Petter suggests, and change the style-guide if we can show that there really is no benefit.


--

John Abd-El-Malek

unread,
Jul 10, 2012, 6:34:25 PM7/10/12
to w...@chromium.org, pkas...@google.com, Ami Fischman, kh...@chromium.org, Mark Mentovai, Jeffrey Yasskin, Mehrdad Niknami, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
Here's some data. Both times are from the second clean build that I do in that configuration, using ninja VS2008 debug builds on Windows with a Z600 and SSD.

trunk: 29:40
removing all "#pragma once" in base/chrome/content/net/ui: 29:30

So it seems that it's not helping locally. From the previous thread, it doesn't help on the bots. #pragma once also masks against incorrect include guards, i.e. http://codereview.chromium.org/10736016/

Sounds to me like we should take it out and update the wiki.

Peter Kasting

unread,
Jul 10, 2012, 6:39:53 PM7/10/12
to John Abd-El-Malek, w...@chromium.org, Ami Fischman, kh...@chromium.org, Mark Mentovai, Jeffrey Yasskin, Mehrdad Niknami, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
On Tue, Jul 10, 2012 at 3:34 PM, John Abd-El-Malek <j...@chromium.org> wrote:
Sounds to me like we should take it out and update the wiki.

As I said before, I have no problem with someone doing this, but I'd like us to go ahead and remove all the existing cases at the same time we tell people to stop adding them.  If you already have a checkout where you removed all these, can you go ahead and check in the removals?  You have LGTM in advance from me to TBR all this :)

PK

John Abd-El-Malek

unread,
Jul 10, 2012, 7:00:12 PM7/10/12
to Peter Kasting, w...@chromium.org, Ami Fischman, kh...@chromium.org, Mark Mentovai, Jeffrey Yasskin, Mehrdad Niknami, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
I'm in the middle of a few things right now, so if others have the time to do this, it would be better.
 

PK

Albert J. Wong (王重傑)

unread,
Jul 10, 2012, 7:01:11 PM7/10/12
to jabde...@google.com, Peter Kasting, w...@chromium.org, Ami Fischman, kh...@chromium.org, Mark Mentovai, Jeffrey Yasskin, Mehrdad Niknami, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
I'll do it.

Ryosuke Niwa

unread,
Jul 10, 2012, 7:08:32 PM7/10/12
to ajw...@chromium.org, jabde...@google.com, Peter Kasting, w...@chromium.org, Ami Fischman, kh...@chromium.org, Mark Mentovai, Jeffrey Yasskin, Mehrdad Niknami, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
Can we get some data point on machines without SSD before going ahead with the removal? Obviously, this optimization wouldn't benefit us much if disk access was sufficiently fast.

- Ryosuke

John Abd-El-Malek

unread,
Jul 10, 2012, 7:15:09 PM7/10/12
to Ryosuke Niwa, ajw...@chromium.org, Peter Kasting, w...@chromium.org, Ami Fischman, kh...@chromium.org, Mark Mentovai, Jeffrey Yasskin, Mehrdad Niknami, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
On Tue, Jul 10, 2012 at 4:08 PM, Ryosuke Niwa <rn...@google.com> wrote:
Can we get some data point on machines without SSD before going ahead with the removal? Obviously, this optimization wouldn't benefit us much if disk access was sufficiently fast.

The buildbots don't have SSDs afaik, and they didn't see a speedup. Also, the next developer machines will all have SSDs.
 

- Ryosuke


On Tue, Jul 10, 2012 at 4:01 PM, Albert J. Wong (王重傑) <ajw...@chromium.org> wrote:
I'll do it.

Thanks!

Ryosuke Niwa

unread,
Jul 10, 2012, 7:18:34 PM7/10/12
to John Abd-El-Malek, ajw...@chromium.org, Peter Kasting, w...@chromium.org, Ami Fischman, kh...@chromium.org, Mark Mentovai, Jeffrey Yasskin, Mehrdad Niknami, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
On Tue, Jul 10, 2012 at 4:15 PM, John Abd-El-Malek <jabde...@google.com> wrote:
On Tue, Jul 10, 2012 at 4:08 PM, Ryosuke Niwa <rn...@google.com> wrote:
Can we get some data point on machines without SSD before going ahead with the removal? Obviously, this optimization wouldn't benefit us much if disk access was sufficiently fast.

The buildbots don't have SSDs afaik, and they didn't see a speedup.

I see. Fair enough.
 
Also, the next developer machines will all have SSDs.

I don't think we should completely disregard non-Google contributors.

- Ryosuke

Victor Khimenko

unread,
Jul 10, 2012, 7:19:12 PM7/10/12
to Ryosuke Niwa, ajw...@chromium.org, jabde...@google.com, Peter Kasting, w...@chromium.org, Ami Fischman, Mark Mentovai, Jeffrey Yasskin, Mehrdad Niknami, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
On Wed, Jul 11, 2012 at 3:08 AM, Ryosuke Niwa <rn...@google.com> wrote:
Can we get some data point on machines without SSD before going ahead with the removal? Obviously, this optimization wouldn't benefit us much if disk access was sufficiently fast.

On the contrary: this optimization can be of benefit ONLY if disk access is fast. First access comes from HDD/SSD, all others come from disk cache.

The only situation where HDD may be worse is when you don't have enough RAM to keep include headers in disk cache before first access and the second one - and this implies such a memory pressure that I don't think we care all that much about such usecase.

Albert J. Wong (王重傑)

unread,
Jul 10, 2012, 7:54:31 PM7/10/12
to Victor Khimenko, Ryosuke Niwa, jabde...@google.com, Peter Kasting, w...@chromium.org, Ami Fischman, Mark Mentovai, Jeffrey Yasskin, Mehrdad Niknami, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
If anyone feels like playing with benchmarks, I've attached the patch that kills it all.

As this code actually increases complexity in some places, I think the burden of proof falls on showing this is actually a real optimization.  So unless someone says something definitive, this change will go in once I manage to beat it through rietveltd, etc.

FYI, this patch was generated it via (thanks scherkus for the sed suggestion!):

   git grep -l --cached 'pragma once' | xargs sed -i '/^#pragma once.*/d'
    
Followed up with a manual inspection of the result of 

  git grep -l --cached 'pragma once'.

to fix some python auto-generation scripts.

I'm trying to upload to rietveltd now, but am not sure if it'll succeed.

-Albert
no_pragma_once.patch.bz2

Peter Kasting

unread,
Jul 10, 2012, 8:30:46 PM7/10/12
to Albert J. Wong (王重傑), Victor Khimenko, Ryosuke Niwa, jabde...@google.com, w...@chromium.org, Ami Fischman, Mark Mentovai, Jeffrey Yasskin, Mehrdad Niknami, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
On Tue, Jul 10, 2012 at 4:54 PM, Albert J. Wong (王重傑) <ajw...@chromium.org> wrote:
As this code actually increases complexity in some places,

Could you summarize an example or two?

(I'm still fine with landing this patch, FYI.)

PK

Albert J. Wong (王重傑)

unread,
Jul 10, 2012, 8:35:14 PM7/10/12
to Peter Kasting, Victor Khimenko, Ryosuke Niwa, jabde...@google.com, w...@chromium.org, Ami Fischman, Mark Mentovai, Jeffrey Yasskin, Mehrdad Niknami, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
On Tue, Jul 10, 2012 at 5:30 PM, Peter Kasting <pkas...@google.com> wrote:
On Tue, Jul 10, 2012 at 4:54 PM, Albert J. Wong (王重傑) <ajw...@chromium.org> wrote:
As this code actually increases complexity in some places,

Could you summarize an example or two?

Peter Kasting

unread,
Jul 10, 2012, 8:38:48 PM7/10/12
to Albert J. Wong (王重傑), Victor Khimenko, Ryosuke Niwa, jabde...@google.com, w...@chromium.org, Ami Fischman, Mark Mentovai, Jeffrey Yasskin, Mehrdad Niknami, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
On Tue, Jul 10, 2012 at 5:35 PM, Albert J. Wong (王重傑) <ajw...@chromium.org> wrote:
Especially the first one of those confuses me... if the file is supposed to support multiple inclusion why does it have an include guard?  Are users expected to manually undef the guard?  That seems wrong.  Not really your responsibility to fix this -- it's just weird.

(Also, I assume you nuked the portions of comments about #pragma once as part of your patch?)

PK 

Albert J. Wong (王重傑)

unread,
Jul 10, 2012, 8:45:15 PM7/10/12
to Peter Kasting, Victor Khimenko, Ryosuke Niwa, jabde...@google.com, w...@chromium.org, Ami Fischman, Mark Mentovai, Jeffrey Yasskin, Mehrdad Niknami, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
On Tue, Jul 10, 2012 at 5:38 PM, Peter Kasting <pkas...@google.com> wrote:
On Tue, Jul 10, 2012 at 5:35 PM, Albert J. Wong (王重傑) <ajw...@chromium.org> wrote:
Especially the first one of those confuses me... if the file is supposed to support multiple inclusion why does it have an include guard?  Are users expected to manually undef the guard?  That seems wrong.  Not really your responsibility to fix this -- it's just weird.

Yeah, I did't get it either and having to worry about #pragma once confused me further.

 
(Also, I assume you nuked the portions of comments about #pragma once as part of your patch?)

I've nuked most of them, though I left the ipc_messages_macros.h header alone since I didn't know how to fix that one and the extra statement about not allowing #pragma onces didn't seem to be completely out of place even if we removed them from all other headers.

-Albert

John Abd-El-Malek

unread,
Jul 10, 2012, 8:55:32 PM7/10/12
to Albert J. Wong (王重傑), Peter Kasting, Victor Khimenko, Ryosuke Niwa, w...@chromium.org, Ami Fischman, Mark Mentovai, Jeffrey Yasskin, Mehrdad Niknami, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
On Tue, Jul 10, 2012 at 5:45 PM, Albert J. Wong (王重傑) <ajw...@chromium.org> wrote:
On Tue, Jul 10, 2012 at 5:38 PM, Peter Kasting <pkas...@google.com> wrote:
On Tue, Jul 10, 2012 at 5:35 PM, Albert J. Wong (王重傑) <ajw...@chromium.org> wrote:
Especially the first one of those confuses me... if the file is supposed to support multiple inclusion why does it have an include guard?  Are users expected to manually undef the guard?  That seems wrong.  Not really your responsibility to fix this -- it's just weird.

Yeah, I did't get it either and having to worry about #pragma once confused me further.

the revision that checked this in has more info, but basically (might want to hold your nose, these are IPC macros after all, but trust me this is better) the include guard is undef'd in one cc file.

Peter Kasting

unread,
Jul 10, 2012, 9:45:42 PM7/10/12
to John Abd-El-Malek, Albert J. Wong (王重傑), Victor Khimenko, Ryosuke Niwa, w...@chromium.org, Ami Fischman, Mark Mentovai, Jeffrey Yasskin, Mehrdad Niknami, alex...@google.com, dch...@chromium.org, tfa...@chromium.org, Chromium-dev
On Tue, Jul 10, 2012 at 5:55 PM, John Abd-El-Malek <jabde...@google.com> wrote:
the revision that checked this in has more info, but basically (might want to hold your nose, these are IPC macros after all, but trust me this is better) the include guard is undef'd in one cc file.

Maybe it should be something like

if !defined(HEADER_GUARD_H) && !defined(MULTIPLY_INCLUDE_XXX) ...

?  That's at least safe against renaming the guard and a little more clear that callers could multiply include this (and how).

PK 

Peter Kasting

unread,
Jul 10, 2012, 9:52:39 PM7/10/12
to tfa...@chromium.org, Chromium-dev
I've updated the style guide to ask people to avoid #pragma once, and given a brief justification in case the issue comes up again in the future.

Thanks to Ami for raising the issue that these might not help, John for getting a data point, and Albert for going to the trouble of removing these!

Any takers on updating the presubmit checks to flag anyone who adds #pragma once back?

PK

Daniel Cheng

unread,
Jul 11, 2012, 1:14:42 AM7/11/12
to pkas...@google.com, tfa...@chromium.org, Chromium-dev
I'll do it (hopefully resent from the right email this time).

Daniel


--
Reply all
Reply to author
Forward
0 new messages