8369186: HotSpot Style Guide should permit some uses of the C++ Standard Library #27601

kimbarrett · 2025-10-02T07:11:51Z

Please review this change to the HotSpot Style Guide to suggest that C++
Standard Library components may be used, after appropriate vetting and
discussion, rather than just a blanket "no, don't use it" with a few very
narrow exceptions. It provides some guidance on that vetting process and
the criteria to use, along with usage patterns.

In particular, it proposes that Standard Library headers should not be
included directly, but instead through HotSpot-provided wrapper headers. This
gives us a place to document usage, provide workarounds for platform issues in
a single place, and so on.

Such wrapper headers are provided by this PR for <cstddef>, <limits>, and
<type_traits>, along with updates to use them. I have a separate change for
<new> that I plan to propose later, under JDK-8369187. There will be
additional followups for other C compatibility headers besides <cstddef>.

This PR also cleans up some nomenclature issues around forbid vs exclude and
the like.

Testing: mach5 tier1-5, GHA sanity tests

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-8369186: HotSpot Style Guide should permit some uses of the C++ Standard Library (Enhancement - P4)

Reviewers

Stefan Karlsson (@stefank - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 27601

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

Using diff file

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

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-10-02T07:13:55Z

👋 Welcome back kbarrett! 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 · 2025-10-02T07:15:42Z

@kimbarrett 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:

8369186: HotSpot Style Guide should permit some uses of the C++ Standard Library

Reviewed-by: stefank

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 no new commits pushed to the master branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential 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.

openjdk · 2025-10-02T07:16:22Z

@kimbarrett The following labels will be automatically applied to this pull request:

build
hotspot

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

kimbarrett · 2025-10-03T15:57:20Z

/label remove build

openjdk · 2025-10-03T15:58:56Z

@kimbarrett
The build label was successfully removed.

openjdk · 2025-10-07T07:14:52Z

@kimbarrett build, serviceability, shenandoah have been added to this pull request based on files touched in new commit(s).

kimbarrett · 2025-10-07T07:15:58Z

/label remove build

openjdk · 2025-10-07T07:16:40Z

@kimbarrett
The build label was successfully removed.

openjdk · 2025-10-07T09:07:10Z

@kimbarrett build has been added to this pull request based on files touched in new commit(s).

kimbarrett · 2025-10-08T03:14:32Z

/label remove build

openjdk · 2025-10-08T03:16:02Z

@kimbarrett
The build label was successfully removed.

openjdk · 2025-10-08T03:20:18Z

@kimbarrett build has been added to this pull request based on files touched in new commit(s).

kimbarrett · 2025-10-08T03:54:12Z

/label remove build

openjdk · 2025-10-08T03:56:32Z

@kimbarrett
The build label was successfully removed.

mlbridge · 2025-10-08T04:01:27Z

Webrevs

stefank · 2025-10-08T07:04:59Z

doc/hotspot-style.md

+* `<tuple>` - Prefer named access to class objects, rather than indexed access
+to anonymous heterogeneous sequences.  In particular, a standard-layout
+class is preferred to a tuple.
+


I gave this feedback offline, but I'll record it here as well. I think that the tuple section should go to the undecided section.

I understand the wish to go with named classes, and I often prefer that as well, but I also see that people often refrain from doing using them various reasons and instead use out-parameters or mix return values into one primitive. I don't want to fully close the door on this feature, and would like us to put this in the undecided (yes, still implicitly forbidden) section. To me that signals that we can at least experiment with it to see if it makes sense to sometimes use it (and if it does we can bring that back for discussion). Whereas outright forbidding it puts a stake in the ground and tells the story that we really shouldn't be looking at tuples. I think that's a too strong of a statement.

I have a different take on the distinction between forbidden and undecided. I
think of forbidden features as being those where there are good arguments
against. Whereas I think of undecided as perhaps having wishy washy arguments
in either direction, or even not seriously thought about. But good arguments
against can be overcome by better arguments in favor.

But I can see how someone else might take that distinction differently.

I also admit to being somewhat biased against tuple in particular. I've seen
a few pretty terrible uses... one was even my fault!

So okay, I'll recategorize tuple.

I'm OK with cracking the door open on tuple, but I have to say I do like API-specific names very much. (And fst/snd or whatever are not API specific; they are tuple-specific!) So the guidance that steers folks towards name-based techniques, instead of positional techniques, is still sound.

I even like out-args, personally, in cases where the out-arg is for a return value that is clearly secondary to the main return value. Example: Main value is boolean, and out-arg is some hint about why the main value is what it is, like a position. The out-arg can also be optional if a null pointer is tolerated (explicitly documented as such, of course), which is useful when the out-arg costs extra to compute. (A separate boolean arg is OK for such uses also, but null pointers are so darn useful as optionality sentinels!) Note that our TRAPS/THREAD macro system can be viewed as a giant set of out-args.

Often, we can use more "specialized" tuples like Maybe<T, Error>, in which case it's fine to have the 'generic' _value, _error slots. Having a bigger sign saying "hey, hey!! this might fail!!!!" is good, it makes bugs related to missing error handling more shallow.

stefank · 2025-10-08T07:07:21Z

src/hotspot/share/utilities/tuple.hpp

@@ -1,5 +1,5 @@
 /*
-* Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
+* Copyright (c) 2023, 2025, Oracle and/or its affiliates. All rights reserved.


So we have our own tuple ...

We have our own baby partial tuple. What's there is more like a heterogeneous
array with compile-time indexing. It's used for one thing, and provides just
enough functionality for that use. I'm not entirely convinced it's the right
tool for the job, though I haven't taken the time to work out alternatives.

I looked at the use of Tuple<...>. I think using named structs with named
members would be an improvement. Eliminates questions like is src element 0
and dst element 1, or is it the other way around.

TheShermanTanker · 2025-10-08T11:56:55Z

Such wrapper headers are provided by this PR for <cstddef>, <limits>, and
<type_info>, along with updates to use them.

Did you mean type_traits?

kimbarrett · 2025-10-08T13:09:43Z

Such wrapper headers are provided by this PR for <cstddef>, <limits>, and
<type_info>, along with updates to use them.

Did you mean type_traits?

Oops! Yes. Fixing description...

rose00

I like where this is going. Thanks for putting in the hard work to make it happen.

rose00 · 2025-10-08T18:12:44Z

doc/hotspot-style.md

+* Memory allocation. HotSpot requires explicit control over where allocations
+occur. The C++98/03 `std::allocator` class is too limited to support our
+usage. But changes to the allocator concept in more recent Standards removed
+some of the limitations, supporting statefull allocators. HotSpot may, in the


s/statefull/stateful/

Fixed locally for next push.

rose00 · 2025-10-08T18:13:48Z

doc/hotspot-style.md

+names into scope via a `using std;` directive. Doing so makes writing code
+using those names easier, since the qualifiers don't need to be included. But
+there are reasons not to do that.
+


s/reasons/several reasons/ (to better tee up the following bullet list)

Fixed locally for next push.

rose00 · 2025-10-08T18:38:38Z

doc/hotspot-style.md

+    * There is a risk of future name collisions. Additional Standard Library
+    headers may be included, adding to the list of names being used. Also,
+    future versions of the Standard Library may add currently unknown names to
+    the headers already being included.


Maybe mention the assert macro at the end of the bullet, if there is no other example of a previous name collision:

…already being included. (We already have a conflict from the HotSpot assert macro, although in that case the std:: prefix is not applicable.)

There aren't any previous examples because we don't have any examples of doing
this thing that we're warning against (using a namespace). assert is a
different problem, unrelated to namespaces. (Remember that one of the usual
knocks against macros is that they don't respect namespaces.)

rose00 · 2025-10-08T18:40:01Z

doc/hotspot-style.md

+* `<ratio>`
+
+* `<system_error>`



To continue the roll of rationale, for chrono etc., here are some suggestions which I am just pulling out of my back pocket:

"HotSpot has its own timing support APIs."
"Initializer lists are an advanced C++ API feature which have surprising interactions with other initialization syntaxes in use in HotSpot."
"HotSpot uses floating point numbers, which are more portable and have more predictable performance characteristics than rational arithmetic."
"HotSpot has its own mechanisms for managing errors."

I'll add some stuff to the PR change.

"HotSpot has its own timing support APIs."

The argument for chrono is that our existing APIs aren't serving us well.
chrono provides strong type safety. We've had multiple cases of mistakes like
a double seconds being treated as double millis or vice versa, and other
similar errors. But it would be a large effort to adopt chrono. And we'd need
to decide whether to use the predefined clocks or hook up chrono to our
clocks. It may be that using the predefined clocks is fine, but it's a
question that needs careful study.

"Initializer lists are an advanced C++ API feature which have surprising
interactions with other initialization syntaxes in use in HotSpot."

I think the trailing "in use in HotSpot" can be dropped from that. Mostly I
think they are fine, but the potential ambiguity between some forms of direct
initialization and initializer list initialization, and the resolution of that
ambiguity, is unfortunate.

"HotSpot uses floating point numbers ..."

Note that <ratio> is a compile-time rational arithmetic package. It's also
fixed (though parameterized) precision. It's not a general purpose rational
arithmetic facility. My understanding is that it started life as a detail of
chrono, and was extracted and promoted to a public facility in the belief that
it has broader utility.

"HotSpot has its own mechanisms for managing errors."

Well, no, we don't really. Or rather, we've got a plethora of bespoke ad hoc
mechanisms. There's a bunch of discussion happening around that topic lately.
I don't think we've coalesced around any particular approach yet. And even
once we decided on something it's likely to take quite a while for rollout to
really make use of whatever we settle on.

I love a good nerd sniping! Thanks.

rose00 · 2025-10-08T18:48:12Z

doc/hotspot-style.md

+* `<tuple>` - Prefer named access to class objects, rather than indexed access
+to anonymous heterogeneous sequences.  In particular, a standard-layout
+class is preferred to a tuple.
+


I'm OK with cracking the door open on tuple, but I have to say I do like API-specific names very much. (And fst/snd or whatever are not API specific; they are tuple-specific!) So the guidance that steers folks towards name-based techniques, instead of positional techniques, is still sound.

I even like out-args, personally, in cases where the out-arg is for a return value that is clearly secondary to the main return value. Example: Main value is boolean, and out-arg is some hint about why the main value is what it is, like a position. The out-arg can also be optional if a null pointer is tolerated (explicitly documented as such, of course), which is useful when the out-arg costs extra to compute. (A separate boolean arg is OK for such uses also, but null pointers are so darn useful as optionality sentinels!) Note that our TRAPS/THREAD macro system can be viewed as a giant set of out-args.

rose00 · 2025-10-08T18:48:53Z

src/hotspot/share/code/relocInfo.cpp

 #include "code/compiledIC.hpp"
 #include "code/nmethod.hpp"
 #include "code/relocInfo.hpp"
+#include "cppstdlib/type_traits.hpp"


(I really, really like this scheme of wrapper headers for std stuff. Thanks Kim!)

rose00 · 2025-10-08T18:52:20Z

src/hotspot/share/cppstdlib/cstddef.hpp

+// permitted:
+// * std::max_align_t, std::nullptr_t
+// * size_t (preferred) and std::size_t
+// * ptrdiff_t (preferred) and std::ptrdiff_t


Are we 100% sure that size_t and ptrdiff_t will always be the std versions, on all platforms?
If not, uses of std::size_t and std::ptrdiff_t might turn out to be portability problems.

…Should I worry that std::size_t and size_t might be different types on some weirdo platform??

Maybe we should do a static-assert that those two names refer to the same type.
Then we can prove that the std:: prefix will never cause the meaning of size_t to shift, on any platform.
I'm being a little paranoid here about possible failure modes specifically for size_t,
specifically because size_t is so pervasive and sensitive to type bugs.

Are we 100% sure ...

Yes.

Should I worry ...

No.

https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2340r0.html
Clarifying the status of the "C headers"

On the basis of that paper, C++23 undeprecated the C headers (they'd been
deprecated since C++98) and gave them their own section in the main body of
the standard, rather than being relegated to an appendix.

The guidance is that a C++ source file should only include C headers if the
file also needs to be valid C source. I'm planning to nudge us in that
direction, except by using our own wrapper headers, like cppstdlib/cstddef.hpp.

openjdk · 2025-10-09T09:14:24Z

@kimbarrett this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout stdlib-header-wrappers
git fetch https://git.openjdk.org/jdk.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

fweimer-rh · 2025-10-09T11:27:26Z

doc/hotspot-style.md

+Functions that may throw exceptions must not be used.  This is in accordance
+with the HotSpot policy of [not using exceptions](#error-handling).


In the GCC implementation, destructor registration may throw __gnu_cxx::recursive_init_error (via the __cxa_guard_acquire Itanium ABI function). Are global or static objects with non-trivial destructors permitted? I think there are a couple of such uses today.

We (you and me, @fweimer-rh) discussed this a couple of years ago:
https://mail.openjdk.org/pipermail/hotspot-dev/2023-December/082324.html

Quoting from here:
https://mail.openjdk.org/pipermail/hotspot-dev/2023-December/083142.html

"
Empirically, a recursive initialization attempt doesn't make any attempt to
throw. Rather, it blocks forever waiting for a futex signal from a thread that
succeeds in the initialization. Which of course will never come.

And that makes sense, now that I've looked at the code.

In __cxa_guard_acquire, with _GLIBCXX_USE_FUTEX, if the guard indicates
initialization hasn't yet been completed, then it goes into a while loop.
This while loop tries to claim initialization. Failing that, it checks
whether initialization is complete. Failing that, it does a SYS_futex
syscall, waiting for some other thread to perform the initialization. There's
nothing there to check for recursion.

throw_recursive_init_exception is only called if single-threaded (either by
configuration or at runtime).
"

It doesn't look like there have been any relevant changes in that area since
then. So I think there is still not a problem here.

dholmes-ora · 2025-10-10T05:43:08Z

Are global or static objects with non-trivial destructors permitted? I think there are a couple of such uses today.

It is discouraged but yes they exist and cause problems. I am currently fixing a bunch of issues due to static Semaphore instances. https://bugs.openjdk.org/browse/JDK-8361462

kimbarrett marked this pull request as draft October 2, 2025 07:12

kimbarrett force-pushed the stdlib-header-wrappers branch from 29d7201 to 4759e6c Compare October 2, 2025 07:12

openjdk bot added hotspot [email protected] build [email protected] labels Oct 2, 2025

kimbarrett force-pushed the stdlib-header-wrappers branch from 4759e6c to 5db1aac Compare October 2, 2025 07:44

openjdk bot removed the build [email protected] label Oct 3, 2025

kimbarrett force-pushed the stdlib-header-wrappers branch from f03908f to d6723d8 Compare October 7, 2025 07:13

openjdk bot added serviceability [email protected] build [email protected] shenandoah [email protected] labels Oct 7, 2025

openjdk bot removed the build [email protected] label Oct 7, 2025

kimbarrett force-pushed the stdlib-header-wrappers branch from d6723d8 to 1c2f151 Compare October 7, 2025 09:05

openjdk bot added the build [email protected] label Oct 7, 2025

kimbarrett force-pushed the stdlib-header-wrappers branch from 1c2f151 to f499567 Compare October 7, 2025 13:56

kimbarrett added 4 commits October 8, 2025 03:09

style guide permits some standard library facilities

1947a16

add wrapper for <limits>

05765af

add wrapper for <cstddef>

63b3d23

add wrapper for <type_traits>

98e7ccb

kimbarrett changed the title ~~Stdlib header wrappers~~ 8369186: HotSpot Style Guide should permit some uses of the C++ Standard Library Oct 8, 2025

openjdk bot removed the build [email protected] label Oct 8, 2025

kimbarrett force-pushed the stdlib-header-wrappers branch from f499567 to 98e7ccb Compare October 8, 2025 03:18

openjdk bot added the build [email protected] label Oct 8, 2025

openjdk bot removed the build [email protected] label Oct 8, 2025

kimbarrett marked this pull request as ready for review October 8, 2025 03:56

openjdk bot added the rfr Pull request is ready for review label Oct 8, 2025

stefank reviewed Oct 8, 2025

View reviewed changes

move tuple to undecided category

4723e45

rose00 suggested changes Oct 8, 2025

View reviewed changes

jrose comments

c886ec3

stefank approved these changes Oct 9, 2025

View reviewed changes

openjdk bot added the merge-conflict Pull request has merge conflict with target branch label Oct 9, 2025

fweimer-rh reviewed Oct 9, 2025

View reviewed changes

Merge branch 'master' into stdlib-header-wrappers

795f1e7

openjdk bot removed the merge-conflict Pull request has merge conflict with target branch label Oct 12, 2025

stefank approved these changes Oct 13, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Oct 13, 2025

		Functions that may throw exceptions must not be used. This is in accordance
		with the HotSpot policy of [not using exceptions](#error-handling).

8369186: HotSpot Style Guide should permit some uses of the C++ Standard Library #27601

Are you sure you want to change the base?

8369186: HotSpot Style Guide should permit some uses of the C++ Standard Library #27601

Conversation

kimbarrett commented Oct 2, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Oct 2, 2025

Uh oh!

openjdk bot commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kimbarrett commented Oct 3, 2025

Uh oh!

openjdk bot commented Oct 3, 2025

Uh oh!

openjdk bot commented Oct 7, 2025

Uh oh!

kimbarrett commented Oct 7, 2025

Uh oh!

openjdk bot commented Oct 7, 2025

Uh oh!

openjdk bot commented Oct 7, 2025

Uh oh!

kimbarrett commented Oct 8, 2025

Uh oh!

openjdk bot commented Oct 8, 2025

Uh oh!

openjdk bot commented Oct 8, 2025

Uh oh!

kimbarrett commented Oct 8, 2025

Uh oh!

openjdk bot commented Oct 8, 2025

Uh oh!

mlbridge bot commented Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheShermanTanker commented Oct 8, 2025

Uh oh!

kimbarrett commented Oct 8, 2025

Uh oh!

rose00 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kimbarrett commented Oct 2, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Oct 2, 2025 •

edited

Loading

openjdk bot commented Oct 2, 2025 •

edited

Loading

mlbridge bot commented Oct 8, 2025 •

edited

Loading