RFR (tedious) 8216022: Use #pragma once

[Kim Barrett]

> On Jan 2, 2019, at 9:31 PM, coleen.phillimore at oracle.com wrote:
> 
> 
> Here is the webrev and bug link.
> 
> open webrev at http://cr.openjdk.java.net/~coleenp/8216022.01/webrev
> bug link https://bugs.openjdk.java.net/browse/JDK-8216022
> 
> On 1/2/19 9:16 PM, coleen.phillimore at oracle.com wrote:
>> Summary: change include guards to #pragma once, except in generated header files.
>> 
>> Tested with mach5 for linux-x64{-debug}, solaris-sparc, macosx-x64, windows-x64, built aarch64 with cross compiler, and zero.
>> 
>> Ran tier1 and 2 tests.
>> 
>> The webrev is huge but there are only 3 lines changed in each header file.  So click on the patch.
>> 
>> I'll update the copyright headers with a script with the commit. Also, will do this after the shenandoah copyright headers are fixed.
>> 
>> Adrian: I included you to check your platforms.
>> 
>> Happy New Year!
>> Coleen

I think we shouldn't make this change without considering the impact
of the following bug:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58770
GCC very slow compiling with #pragma once

According to comment 3, using "#pragma once" introduces N^2 behavior
on the number of included files, because the duplicate check uses a
list rather than a hashtable.

At the very least, a performance comparison should be made to find out
what the impact of that bug is.

A couple of comments regarding the problems with existing #include
guards, and continuing to use #include guards instead of "#pragma once":

(1) I wonder if a round trip using the following could fix the
#include guards:
https://github.com/cgmb/guardonce
Utilities for converting from C/C++ include guards to #pragma once and
back again.

(2) For maintaining #include guards, clang has -Wheader-guard, which
warns about mismatches between the #ifndef name and the #define.  Of
course, that doesn't solve all the issues with #include guards.

And now for a more general discussion of "#pragma once".

The question of whether to use "#pragma once" comes up pretty often in
various open source projects and Q&A sites.  So far, I haven't found
any large cross-platform projects that provide headers to clients that
have decided to go that way.  But most of HotSpot is self-contained,
which limits exposure to some of the issues below.

An exception to that are the C headers providing interfaces to the VM,
e.g. the files in src/hotspot/share/include.  This suggests that
perhaps these files should perhaps be excluded from the change, since
they get used in whatever build environment a client uses.  It also
suggests the #include guard names for these files need careful
namespace consideration, which clearly didn't happen with cds.h.

For many compilers there isn't a good performance argument for using
"#pragma once".  gcc (for a long time), clang (always?), VS2015+ all
do the #include guard optimization.  (I think Solaris Studio might
still not?  And I have no idea about XLC++.)

So the primary question seems to be the reduced clutter and avoidance
of mistakes in #include guards, vs the possibility of cases where
"#pragma once" doesn't work properly.

Here's a list of some of the discussions I found:

https://lists.boost.org/Archives/boost/2018/11/244423.php
https://lists.qt-project.org/pipermail/development/2018-October/067452.html
https://lists.qt-project.org/pipermail/development/2018-January/063932.html
https://stackoverflow.com/questions/1143936/pragma-once-vs-include-guards
https://www.reddit.com/r/cpp/comments/4cjjwe/come_on_guys_put_pragma_once_in_the_standard/d1j04te/

The main argument against "#pragma once" (besides being non-standard,
so possibly not sufficiently portable, though we think all platforms
supported by HotSpot have this feature) is that it is "unreliable".
Unfortunately, details are hard to come by.

I've seen claims that combining "#pragma once" with precompiled
headers can cause problems, though the fact that Visual Studio has
long supported both and they are commonly used together argues
contrary.  But perhaps there are additional factors needed for
problems to arise, and those don't happen on Windows?

My impression is that having sources spread across different file
systems might be a source of problems, possibly in conjunction with
other factors.  Before you say "multiple file systems" is not a
possible configuration for JDK builds, consider an out-of-tree build
with the source and build directory on different file systems (and
remember that our generated sources are in the build directory).

I've seen suggestions that network file systems can also mess things
up, though I didn't find details.

I think an additional factor that might be relevant is the typical
(but not specified by the standard) behavior of #include "..." first
searching with respect to the current directory.  I think this is at
least potentially a concern for JDK builds on (perhaps odd) file
system configurations.

Some examples are discussed in the following messages.  Having a
bind-mount involved can mess things up, for example.  I don't know if
that's a realistic scenario for building the JDK or HotSpot.
https://lists.qt-project.org/pipermail/development/2018-October/067467.html
https://lists.qt-project.org/pipermail/development/2018-October/067471.html

So there seems to be some risk with this change that it will result in
build failures or bad builds in someone's build environment, but it is
hard to characterize what a problematic build environment looks like,
so hard to know how "reasonable" or "sane" such a build environment
might be.  Local testing is obviously inadequate for this change.
Even running it through the Oracle build and test system doesn't seem
sufficient to me.  Having it checked by the various known build farms
(SAP, Debian, Red Hat, maybe others) seems called for.  It's good that
the RFR specifically called out Debian to be checked.