RFR (tedious) 8216022: Use #pragma once

[Erik Österlund]

Hi Thomas,

On 2019-01-04 18:24, Thomas Stüfe wrote:
> On Fri, Jan 4, 2019 at 4:01 PM David Lloyd <david.lloyd at redhat.com> wrote:
> 
>> 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.
>>
>> --
>> - DML
> 
> FWIW, I agree with David on this. Include guards are a simple
> mechanism, while the potential troubles surrounding #pragma once worry
> me. Include guard errors are easy to find and fix. But the potential
> issues with pragma once sound difficult to analyze and almost
> impossible to fix if the compiler turns out to be the culprit.

So the conversation moved on a bit since you sent this email. Sorry for 
the late reply. Are you still worried about this? In that case, what are 
your worries?

> Note that at SAP people tend to build out-of-tree and often across
> file system borders, with sources on a shared file system and a local
> output directory. So yes, that is a common usage scenario.

Doesn't sound like that would matter, as long as you don't have multiple 
include paths from different file systems pointing at the same header files.

> That said, I can understand Erik's pain when creating/changing so many
> includes. But how common is this scenario? Changing so many include
> files happens usually in the course of major rewrites which I would
> hope do not occur so often that we need to optimize our workflow for
> them. After all, these changes also bring other disruption: file
> history gets broken, it is more difficult to compare code across JDK
> versions etc.

Okay.

> Bottomline I would prefer keeping include guards and maybe add a tool
> to generate include guards automatically.

The external tool for fixing incorrect include guards does not solve the 
problem IMO, unless everyone is forced to use it. Here is why:

1) As a reviewer, you would still have to try to spot errors in include 
guards, as everyone will not be using the tool, or forget to use the tool.

2) If somebody else has a slightly different include guard to you, and 
you apply the tool for your change, it will cause a seemingly unrelated 
change to include guards of random files in your change. That means that 
you would have to look through your webrev for spurious include guard 
changes to files that did not belong to you. And reviewer would have to 
look out for that too.

3) Sometimes we don't want #pragma once our include guards, for example 
when we include stuff straight into some shared class (rather than 
having includes at the top of the header file). We would have to teach 
the external tool to understand that some headers should and some should 
not have guards. With #pragma once that is not a problem.

It also seems to me that the external tool would be problematic iff 
using #pragma once is problematic, for similar reasons. If the external 
tool can trivially determine the file identity and generate an include 
guard in hotspot based on that, then I don't see how #pragma once could 
mess it up either. They both automatically determine file identities.

Please let me know if you still feel uneasy about this change. Perhaps 
you could take the patch for a spin in your setup and see if you have 
any trouble?

Thanks,
/Erik

> Thanks, Thomas
>