RFR: 8302124: HotSpot Style Guide should permit noreturn attribute

[Julian Waters]

On Mon, 13 Feb 2023 00:55:46 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Please review this change to permit the use of noreturn attributes in HotSpot
>> code.
>> 
>> http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2008/n2761.pdf
>> https://en.cppreference.com/w/cpp/language/attributes/noreturn
>> 
>> This will permit such changes as marking failed assertions and the like as
>> noreturn, permitting the compiler to generate better code in some cases.  This
>> has benefits even in product builds, since some of the relevant checks occur
>> in product code (such as `guarantee`, `fatal`, &etc).  It also provides a
>> solution to the problem described in JDK-8294031, where the potential
>> continued execution from a failed assertion leads to compiler warnings.
>> 
>> The change is written in such a way that it should be easy to add the
>> appropriate text for new attributes in the future.  There have been
>> discussions of adopting C++17, which adds several attributes.
>> 
>> The change to the Style Guide is forward looking, toward a time when more
>> attributes are available due to the adoption of a newer language standard than
>> the current C++14.  It is written in such a way that it should be easy to add
>> the appropriate text for new attributes.
>> 
>> Testing:
>> I have a prototype of making HotSpot assertions noreturn, which has been run
>> through mach5 tier1-8 for all Oracle-supported platforms.
>> 
>> This is a modification of the Style Guide, so rough consensus among the
>> HotSpot Group members is required to make this change. Only Group members
>> should vote for approval (via the github PR), though reasoned objections or
>> comments from anyone will be considered. A decision on this proposal will not
>> be made before Friday 24-Feb-2023 at 12h00 UTC.
>> 
>> Since we're piggybacking on github PRs here, please use the PR review process
>> to approve (click on Review Changes > Approve), rather than sending a "vote:
>> yes" email reply that would be normal for a CFV.
>
> doc/hotspot-style.md line 1072:
> 
>> 1070: * An attribute that appertains to a function is placed at the beginning of the
>> 1071: function's declaration, rather than between the function name and the parameter
>> 1072: list.
> 
> Sorry to be pedantic but what exactly constitutes the "function declaration"? Does it include keywords like `virtual`? What about placement wrt.  compiler-specific attributes?

As for the function declaration, it's pretty much the entire function, keywords and all, as the C++ Standard always has the attribute specifier sequence appear before everything else (alignas is also an attribute specifier, to that extent), and never between keywords. I'm in favour of adding an extra line also forbidding having the attributes placed behind the function's parameter list brackets though, since HotSpot currently has a ton of attributes that are listed behind the whole function

I think Kim has stated before that compiler specific attributes don't give much benefit and isn't really interested in having them, that said it would be nice to have them in the future with perhaps the compiler's namespace required in the attribute (msvc::noinline in C++20 guarantees no inlining of a function as compared to our current __declspec(noinline) for instance).

-------------

PR: https://git.openjdk.org/jdk/pull/12507