On Mon, 25 Aug 2025 09:42:45 GMT, Kim Barrett <kbarr...@openjdk.org> wrote:
>> Please review this change to use C++17 for building C++ parts of the JDK. In >> particular this affects HotSpot. This change also includes an update to the >> HotSpot Style Guide regarding C++17 features and their use in HotSpot code. >> >> Testing: mach5 tier1-8 >> >> This change includes a modification of the Style Guide. Rough consensus among >> the HotSpot Group members is required to make such a 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 5-September-2025 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. > > Kim Barrett has updated the pull request incrementally with one additional > commit since the last revision: > > dholmes tweaks Thanks for taking on this task! I read through it and added a few comments, but none of them are crucial to get addressed. I haven't followed and checked the links. doc/hotspot-style.md line 1481: > 1479: ## Excluded Features > 1480: > 1481: ### Structured Bindings FWIW, there was a recent discussions about memory APIs in `os::` that returned both a size and an error code. I'm a little bit intrigued to see if Structured Bindings would have been able to give us slightly more cleaner call-site code. I'll still my curiosity after the support for C++17 is in. doc/hotspot-style.md line 1509: > 1507: with files, and already has adequate mechanisms for its needs. > Rewriting in > 1508: terms of this new library would not be a good use of resources. Having > a mix > 1509: of the existing usage and uses of this new library would just be > confusing. I'm not sure the style guide should be opinionated about we choose to use our resources. Could this be slightly tweaked to not say that? doc/hotspot-style.md line 1567: > 1565: in HotSpot code because of the > 1566: [no implicit boolean](#avoid-implicit-conversions-to-bool) > 1567: guideline.) > (Note that conversion to `bool` isn't needed ... Pre-existing: This section sounds weird to me. It more or less says "it isn't needed because we forbid it". I think people can still feel the need to use it. Maybe this could be changed to say something like: > (Note that conversion to `bool` isn't permitted in HotSpot code because of the [no implicit boolean](#avoid-implicit-conversions-to-bool) guideline.) or something like that. (This doesn't have to be fixed in this PR if you disagree) ------------- Marked as reviewed by stefank (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/26884#pullrequestreview-3150965151 PR Review Comment: https://git.openjdk.org/jdk/pull/26884#discussion_r2297826334 PR Review Comment: https://git.openjdk.org/jdk/pull/26884#discussion_r2297814769 PR Review Comment: https://git.openjdk.org/jdk/pull/26884#discussion_r2297837433
