RFR (tedious) 8216022: Use #pragma once
Erik Österlund
erik.osterlund at oracle.com
Fri Jan 4 15:59:58 UTC 2019
Hi David,
On 2019-01-04 15:59, David Lloyd wrote:
> On Fri, Jan 4, 2019 at 7:20 AM Andrew Haley <aph at redhat.com> wrote:
>> On 1/4/19 11:07 AM, Florian Weimer wrote:
>>> The guards occasionally cause bugs because they are not globally unique
>>> or not spelled correctly in both places in the header file.
>> Yeah, I know. That's the usual non-performance-related pro-#pragma
>> once argument. After GCC and a bunch of other compilers implemented
>> optimized include guards I thought that #pragma once would go away,
>> but evidently not. :-)
> Not a reviewer, but...
>
> The whole thing is starting to seem like a really bad idea to me.
> I've been asking around and I can't find anyone who thinks this kind
> of change is a good idea; while this clearly isn't a scientific poll,
> it does not on the other hand raise my confidence any. It was pointed
> out to me that the GCC documentation has been recommending against
> this practice perhaps as early as 3.4.
>
> It boils down to this: #pragma once isn't standardized; there's simply
> no guarantee it will be supported on a given compiler. This problem
> seems *far* more significant to me than the risk of forgetting to
> update a macro name here or there (something that can be caught on
> code review), on the occasion that a header file is renamed or
> relocated such that it requires a change (and how often is this likely
> to happen anyway?).
Hotspot relies on a whole bunch of implementation defined compiler
features that are not standardized, that all of our compilers support to
work. I think that seems okay as long as all compilers are covered that
we build with. If we were to insist on using only completely
standardized features, we would have decades of work to get there, if we
could do it at all.
> In addition, it was pointed out to me that if, for some reason, a
> header file ends up in more than one location on the include path,
> #pragma once will (probably, as it's not standardized) allow it to be
> included twice, which #ifdef guards avoid. This is perhaps not a real
> concern in this particular code base though.
That sounds like a bug. Just because something is implementation
defined, doesn't make it okay or expected to not work. Do you know how
to reproduce this, and on which platform/compiler/version? Obviously, if
this was an issue in our code base, one would quickly notice it doesn't
build.
> In the end though this is an example of the kind of change that I for
> one would never allow in one of my projects: it's large, potentially
> impacts portability, and yet in the end it's not really necessary,
> being really just a style issue when it comes right down to it.
> Include guards are standard and portable. '#pragma once' is not.
But it's not just style. It's getting rid of pointless day-to-day manual
(error prone) boilerplate work for something that should be automated
(adding a new file, moving an existing file, renaming an existing file).
And as I said in my reply to Andrew, I for example had to poke around at
quite a lot of barrier set files (>100 barrier set files were added for
the GC barrier interface), and rename them, and it was very tedious, and
resulted in some pointless screwups in some include guards. I would not
be insisting unless I perceived these issues as real, or if it was just
a matter of style.
So it seems to me that the to me very real problems that I do run into
on a daily basis, are not real problems to you, while the to me
hypothetical issues of portability of HotSpot in the future, to
platforms that don't have compilers available with the features that
have been around in GCC since 3.4, is a real problem (which BTW would
according to the #pragma once compatibility matrix on wikipedia the
"Cray C and C++" compiler as of today).
Strictly speaking, it wasn't "necessary" to move away from include DB
either, and that was also a large change. Yet I'm glad we did. Sometimes
we gotta change things to make our lives better.
Thanks,
/Erik
More information about the hotspot-dev
mailing list