8325932: Replace ATTRIBUTE_NORETURN with direct [[noreturn]]

[TheShermanTanker]

With clang 13 being the minimum required JDK-8325878, the noreturn bug that requires the ATTRIBUTE_NORETURN workaround now vanishes, and we can use [[noreturn]] directly within HotSpot. We should remove the workaround as soon as possible, given the chance

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8325932: Replace ATTRIBUTE_NORETURN with direct [[noreturn]] (Enhancement - P4)

Reviewers

• Kim Barrett (@kimbarrett - Reviewer)
• David Holmes (@dholmes-ora - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/17868/head:pull/17868
$ git checkout pull/17868

Update a local copy of the PR:
$ git checkout pull/17868
$ git pull https://git.openjdk.org/jdk.git pull/17868/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 17868

View PR using the GUI difftool:
$ git pr show -t 17868

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/17868.diff

Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back jwaters! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@TheShermanTanker The following label will be automatically applied to this pull request:

• hotspot

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 00: Full (d03688ae)

[tstuefe]

We should remove the workaround as soon as possible, given the chance.

Why the hurry? (especially seeing that JDK-8325878 is still open and being discussed)

[forax]

I believe "as soon as possible" should be "once JDK-8325878 is resolved" :)

Now, the reason to use [[noreturn]] instead of ATTRIBUTER_NORETURN macro is that IDEs recognize [[noreturn]] when doing static analysis on the code and that ATTRIBUTER_NORETURN was added as a workaround because a bug (crash) in clang < 12.

[TheShermanTanker]

I believe "as soon as possible" should be "once JDK-8325878 is resolved" :)

Now, the reason to use [[noreturn]] instead of ATTRIBUTER_NORETURN macro is that IDEs recognize [[noreturn]] when doing static analysis on the code and that ATTRIBUTER_NORETURN was added as a workaround because a bug (crash) in clang < 12.

Yes, I forgot to mention that this is only going in once the clang 13 requirement is integrated

[tstuefe]

Thank you for the explanations.

[openjdk]

@TheShermanTanker This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8325932: Replace ATTRIBUTE_NORETURN with direct [[noreturn]]

Reviewed-by: kbarrett, dholmes

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 1193 new commits pushed to the master branch:

• 0bb5ae6: 8332248: (fc) java/nio/channels/FileChannel/BlockDeviceSize.java failed with RuntimeException
• 4d32c60: 8322008: Exclude some CDS tests from running with -Xshare:off
• e91492a: 8313674: (fc) java/nio/channels/FileChannel/BlockDeviceSize.java should test for more block devices
• 95a6013: 8332042: Move MEMFLAGS to its own include file
• 5a4415a: 8331858: [nmt] VM.native_memory statistics should work in summary mode
• 4ba7447: 8326957: Implement JEP 474: ZGC: Generational Mode by Default
• 7ce4a13: 8332130: RISC-V: remove wrong instructions of Vector Crypto Extension
• ea5eb74: 8326404: Assertion error when trying to compile switch with fallthrough with pattern
• beea530: 8331907: BigInteger and BigDecimal should use optimized division
• 440782e: 8331466: Problemlist serviceability/dcmd/gc/RunFinalizationTest.java on generic-all
• ... and 1183 more: https://git.openjdk.org/jdk/compare/1aae980c549741cf5fc5ca51f3c299285bafa49d...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[TheShermanTanker]

@kimbarrett Is this good to go now that clang's minimum version has been set to 13?

[bridgekeeper]

@TheShermanTanker This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[bridgekeeper]

@TheShermanTanker This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.

[kimbarrett]

This seems to have gotten lost while waiting for JDK-8325878. Please re-open and update.

[TheShermanTanker]

/open

[openjdk]

@TheShermanTanker This pull request is now open

[kimbarrett]

Looks good.

[TheShermanTanker]

Thanks Kim. Anyone else want to be the second reviewer?

[dholmes-ora]

If this now works fine for all compilers that is great!

Thanks

[TheShermanTanker]

Thanks David and Kim for the reviews :)

[TheShermanTanker]

/integrate

[openjdk]

Going to push as commit 7b4ba7f.
Since your change was applied there have been 1193 commits pushed to the master branch:

• 0bb5ae6: 8332248: (fc) java/nio/channels/FileChannel/BlockDeviceSize.java failed with RuntimeException
• 4d32c60: 8322008: Exclude some CDS tests from running with -Xshare:off
• e91492a: 8313674: (fc) java/nio/channels/FileChannel/BlockDeviceSize.java should test for more block devices
• 95a6013: 8332042: Move MEMFLAGS to its own include file
• 5a4415a: 8331858: [nmt] VM.native_memory statistics should work in summary mode
• 4ba7447: 8326957: Implement JEP 474: ZGC: Generational Mode by Default
• 7ce4a13: 8332130: RISC-V: remove wrong instructions of Vector Crypto Extension
• ea5eb74: 8326404: Assertion error when trying to compile switch with fallthrough with pattern
• beea530: 8331907: BigInteger and BigDecimal should use optimized division
• 440782e: 8331466: Problemlist serviceability/dcmd/gc/RunFinalizationTest.java on generic-all
• ... and 1183 more: https://git.openjdk.org/jdk/compare/1aae980c549741cf5fc5ca51f3c299285bafa49d...master

Your commit was automatically rebased without conflicts.

[openjdk]

@TheShermanTanker Pushed as commit 7b4ba7f.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.