Drake: Consider using #pragma once, instead of include guards

[jwnimmer-tri]

I think we should consider whether Drake should use #pragma once. (There was an embedded discussion of this in a PR with only weak participation.)

I'll post two comments to receive thumbs-up votes for what people like better. If you have pro/con discussions, post those as comments.

Google style guide on the topic:

• https://google.github.io/styleguide/cppguide.html#The__define_Guard

Wikipedia for a little background:

• https://en.wikipedia.org/wiki/Pragma_once

[jwnimmer-tri]

Vote here if you want to keep using ifndef-define-endif-closingcomment per googlestyle.

[jwnimmer-tri]

Vote here if you want to switch to pragma-once.

[jwnimmer-tri]

Continuing the thread from the PR:

I have seen inexplicable compilation errors that were ultimately caused by
someone forgetting to update the #ifndef at the top of a header they copied.

This happened earlier this week to a Drake developer. I am doubtful that cpplint will help find these kinds of errors quickly. Are we going to teach make (CMake?) (Ninja?) that it should give you cpplint output even when the file doesn't compile correctly yet? (Or possibly even _prior_ to the compiler warning in this case, so you don't rathole into the first compilation error?) Or integrate some kind of emacs cpplint-flymake mode where you get squiggles under cpplint warnings as you type?

I think the essence of my argument is that pragma-once works on all platforms without hassle, so we should just use it, and stop nursing DEFINE spelling and endif-comments with artisinally-crafted LOUD_WORDS that are easy to get wrong -- and that even with tools tell you that you've screwed up, it takes valuable seconds to fix the closing-comment and nurse the DEFINE spelling as you relocate files within the source tree, etc.

[billhoffman]

Have to add my 2 cents. Maybe I don't get around much, but I have never seen code that uses pragma-once. To me it would make drake an outlier. I have never seen the mismatched defines cause too much trouble, (i.e. I can not remember it ever causing me trouble). @bradking any thoughts on this?

[sherm1]

#pragma once is very appealing. I have seen some scary discussions of it, though. From stackoverflow:

pragma once is not standard. It is a widespread (but not universal) extension, which can be used

• if your portability concerns are limited, and
• you can be sure that all of your include files are always on a local disk.

_It was considered for standardization, but rejected because it cannot be implemented reliably._ (The problems occur when you have files accessible through several different remote mounts.)

And from Wikipedia:

Identifying the same file on a file system is not a trivial task. Symbolic links and hard links may cause the same file to be found under different names. Compilers may use a heuristic that compares file size, modification time and content. This backfires when the same file is intentionally copied into several parts of a project. With include guard based on file path these copies would be treated differently while #pragma once may arbitrarily treat them as the same file in a compiler-dependent way

Is Drake likely to be immune to these difficulties?

[rpoyner-tri]

re portability:

• drake already limits itself here by its template usage and use of c++11 features.
• the matrix of supported platforms and compilers is known and documented.
• all of the supported compilers implement '#pragma once'.

re filesystems:

• if you have multiple non-identical copies of a file in your include path, your men are already dead.
• if you have multiple identical copies of a file in your include path, your men are already dead.
• I look forward to someone explaining a use-case that is worth caring about that is not also already playing with fire w.r.t. correct, repeatable builds.

[liangfok]

The matrix of supported platforms is based on our current / anticipated amount of resources. It is not written in stone and may change as new opportunities arise. Thus, should we anticipate future changes that may result in the need to port Drake onto platforms that do not support #pragma? For example, maybe one day Drake will need to be ported onto some obscure embedded system?

[jwnimmer-tri]

When Drake needs to build on some hypothetical compiler that has sprung into existence that supports constexpr and <Eigen/Dense> and <chrono> and Polynomials-as-template-arguments and etc, but not pragma-once, I will happily write the perl script that puts back the ifndef boilerplate into the Drake tree, and reverse my position on this issue. (In other words, pragma-once is inexpensive to back away from if need be -- and will be the least of our worries if we retreat to the bad old GCC-3.4 / C++98 days.) Until then, we should allow ourselves the choice of using our toolchain capabilities as they exist now, and not worrying about compiler bogeymen.

[sherm1]

@jwnimmer-tri, you wouldn't even have to write it. See unpragma-once.

[liangfok]

How about let's compromise by using both? http://stackoverflow.com/questions/13339392/should-i-still-use-include-guards-and-pragma-once

[sherm1]

How about let's compromise by using both?

Ouch! That makes the problem worse. I'm going to vote for just #pragma once. One of the best things about it that I don't think anyone mentioned is that you then don't have to change the header guard symbols when the code gets refactored causing file name changes. And I think we're going to be doing a lot of that!

[liangfok]

That's a good point about #pragma once not needing to be updated every time the file name changes. How about the following compromise?

1. We first find or write a tool that allows us to easily switch between using the traditional #define style header guards and #pragma once. @jwnimmer-tri mentioned the use of a perl script above. @sherm1 mentioned unpragma-once, but we also need a tool to go in reverse.
2. Once we have the tool, we use #pragma once while developing Drake to reduce the probability of wasting developers' time due to silly header guard mistakes.
3. Whenever we publish an official release, we use the aforementioned tool to switch to traditional #define style header guards to reduce the probability of compiler-dependent behavior.

[jwnimmer-tri]

I think munging our source code for "Release" is unlikely to help anyone, is likely to confuse people, and opens the door to making a tooling mistake that our developers wouldn't see but would instead be directly inflicted on our users.

I advise waiting to solve the "compiler-dependent behavior" problem until we see an example of it, at which point we can tailor the solution to whatever specific problem it is. As rpoyner-tri notes above, any pragma-once-related error will actually just be a symptom of a more fundamental build system fault.

To my mind, the purpose of this issue is to reduce the complexity of our software -- to reduce the opportunity for error. A solution involves more tooling is contrary to that purpose.

[liangfok]

Perhaps we should look at other major open source projects based on C++ to see if they use pragma once instead of header guards?

[billhoffman]

I am not aware of any. :) I did a search on github and lots of gaming stuff showed up. The goolge style guide for Windows says: "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."

[liangfok]

A consensus will likely not be reached in the short term. Thus, I recommend that the Drake owners collectively make an executive decision.

[jamiesnape]

Personally, I do not see a compiling reason to go against the Google Style Guide. We should make exceptions to it a rarity.

[jpieper-tri]

I see Google as being behind the times here. It wasn't that long ago that the google style forbid default arguments, lambdas, and many other modern C++ features.

There are basically 0 documented downsides for any modern C++ project. The Google rationale boils down to two points, neither of which is very compelling. 1) older compilers don't support it (but as jwnimmer-tri pointed out, they also don't support nearly all of the C++11 features drake relies on). 2) if you have the same header file linked into your tree in multiple places, it may not do what you want (but as rpoyner-tri pointed out, that is already very poor practice, and likely in violation of the google style by itself).

The upside is that there is one less thing to get wrong when making new header files or copying them around. Just last week I was called in to debug a problem in drake that turned out to be a copied header file where the guards had not been updated correctly.

If you look at other major projects which actually debated this point, the only arguments which surfaced were 1) google doesn't allow it, or 2) if you want to include a header file multiple times with different preprocessor directives, it doesn't work (but neither do include guards).

Given the total lack of actual downside, and the admittedly small, but non-zero benefit, I see no particular reason why we need to wait for Google to revisit this particular decision.

[RussTedrake]

I agree, and would be inclined to stick with the style guide to break the tie.

[sherm1]

I would support switching to #pragma once for our own headers with our style guide updated to say we don't allow any tricks that would break it.

[RussTedrake]

My comments crossed Josh’s in transit. If it can prevent bugs, then that is a sufficiently good reason to me.

[amcastro-tri]

Sorry for being late to the discussion. My humble opinion on the subject. I don't like the idea of moving away from standard practices and #pragma once doesn't seem to be an accepted standard. It is not even accepted in Google's style guide (there are some warning words for Windows). In my a opinion a standard only becomes one when it's already been in use say for at least 10 years. I would just stick to our standard header guards.

[david-german-tri]

@RussTedrake has ruled, and the balance of votes is also pretty clear, so I propose we paint the bikeshed its new shade of #pragma once and end the thread. Advocates of this change: are any of you interested in volunteering for the refactor that's blocking #2120? If so, please coordinate with @liangfok there!

[jwnimmer-tri]

I will prepare the PR to switch the code over to the new style, though perhaps not until ~48 hours from now. If anyone want to ping me and steal this, be my guest.