8352645: Add tool support to check order of includes #24247

dougxc · 2025-03-26T09:21:59Z

This PR adds test/hotspot/jtreg/sources/SortIncludes.java, a tool to check that blocks of include statements in C++ files are sorted and that there's at least one blank line between user and sys includes (as per the style guide).

By virtue of using SortedSet, the tool also removes duplicate includes (e.g. "compiler/compilerDirectives.hpp" on line 37 and line 41). Sorting uses lowercased strings so that _ sorts before letters, preserving the prevailing convention in the code base. I've also updated the style guide to clarify this sort-order.

The tool does nothing about re-ordering blocks of conditional includes vs unconditional includes. I briefly looked into that but it gets very complicated, very quickly. That kind of re-ordering will have to continue to be done manually for now.

I have used the tool to fix the ordering of a subset of HotSpot sources and added a test to keep them sorted. That test can be expanded over time to keep includes sorted in other HotSpot directories.

When TestIncludesAreSorted.java fails, it tries to provide actionable advice. For example:

java.lang.RuntimeException: The unsorted includes listed below should be fixable by running:

    java /Users/dnsimon/dev/jdk-jdk/open/test/hotspot/jtreg/sources/SortIncludes.java --update /Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/c1 /Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/ci /Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/compiler /Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/jvmci

	at TestIncludesAreSorted.main(TestIncludesAreSorted.java:80)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
	at java.base/java.lang.reflect.Method.invoke(Method.java:565)
	at com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:335)
	at java.base/java.lang.Thread.run(Thread.java:1447)
Caused by: java.lang.RuntimeException: 36 files with unsorted headers found:

/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/c1/c1_Compilation.cpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/c1/c1_Runtime1.cpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/c1/c1_Optimizer.cpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/c1/c1_Compilation.hpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/c1/c1_FrameMap.hpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/c1/c1_RangeCheckElimination.cpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/c1/c1_InstructionPrinter.cpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/ci/bcEscapeAnalyzer.cpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/ci/ciInstance.cpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/ci/ciEnv.hpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/ci/ciUtilities.inline.hpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/ci/ciMethod.cpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/ci/ciUtilities.cpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/ci/ciEnv.cpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/ci/ciCallSite.cpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/ci/bcEscapeAnalyzer.hpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/ci/ciReplay.cpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/ci/ciInstanceKlass.cpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/compiler/compilationMemoryStatistic.cpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/compiler/compilationFailureInfo.cpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/compiler/compilationPolicy.cpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/compiler/directivesParser.hpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/compiler/compileBroker.cpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/compiler/directivesParser.cpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/compiler/compilerDirectives.hpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/compiler/methodMatcher.hpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/compiler/compilationMemoryStatistic.hpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/compiler/compileTask.cpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/compiler/disassembler.hpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/compiler/oopMap.inline.hpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/jvmci/jvmciRuntime.cpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/jvmci/jvmci.cpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/jvmci/jvmciCompiler.cpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/jvmci/jvmci.hpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/jvmci/jvmciEnv.cpp
/Users/dnsimon/dev/jdk-jdk/open/src/hotspot/share/jvmci/jvmciJavaClasses.cpp

Note that non-space characters after the closing " or > of an include statement
can be used to prevent re-ordering of the include. For example:

#include "e.hpp"
#include "d.hpp"
#include "c.hpp" // do not reorder
#include "b.hpp"
#include "a.hpp"

will be reformatted as:

#include "d.hpp"
#include "e.hpp"
#include "c.hpp" // do not reorder
#include "a.hpp"
#include "b.hpp"


	at SortIncludes.main(SortIncludes.java:190)
	at TestIncludesAreSorted.main(TestIncludesAreSorted.java:75)
	... 4 more

JavaTest Message: Test threw exception: java.lang.RuntimeException

This PR includes a commit with ordering suppression comments for some files I discovered needed it while playing around in #24180 .

This PR replaces #24180.

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-8352645: Add tool support to check order of includes (Enhancement - P4)

Reviewers

Stefan Karlsson (@stefank - Reviewer)
Kim Barrett (@kimbarrett - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 24247

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

Using diff file

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

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-03-26T09:22:47Z

👋 Welcome back dnsimon! 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-03-26T09:23:14Z

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

8352645: Add tool support to check order of includes

Reviewed-by: stefank, kbarrett

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 175 new commits pushed to the master branch:

cc870d4: 8352088: Call of com.sun.jdi.ThreadReference.threadGroups() can lock up target VM
d979bd8: 8344671: Few JFR streaming tests fail with application not alive error on MacOS 15
49cb7aa: 8339114: DaCapo xalan performance with -XX:+UseObjectMonitorTable
d32ff13: 8353117: Crash: assert(id >= ThreadIdentifier::initial() && id < ThreadIdentifier::current()) failed: must be reasonable)
a0677d9: 8353263: Parallel: Remove locking in PSOldGen::resize
8608b16: 8348887: Create IR framework test for JDK-8347997
23eb648: 8353545: Improve debug info for StartOptionTest
4f97c4c: 8349211: Add support for intrusive trees to the utilities red-black tree
c9baa8a: 8352418: Add verification code to check that the associated loop nodes of useless Template Assertion Predicates are dead
b80b04d: 8353329: Small memory leak when create GrowableArray with initial size 0
... and 165 more: https://git.openjdk.org/jdk/compare/df9210e6578acd53384ee1ac06601510c9a52696...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.

openjdk · 2025-03-26T09:24:12Z

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

graal
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.

…hem sorted

mlbridge · 2025-03-26T10:43:42Z

Webrevs

stefank

Thanks for updating to use the lower-case comparison. I wonder if a small tweak can fix the extra blank lines I complained about in the other PR.

The tool removes the extra blank line we have in our .inline.hpp. From the Style Guide:

All .inline.hpp files should include their corresponding .hpp file as the first include line with a blank line separating it from the rest of the include lines. Declarations needed by other files should be put in the .hpp file, and not in the .inline.hpp file. This rule exists to resolve problems with circular dependencies between .inline.hpp files.

I think this needs to be fixed, otherwise people will start to remove these.

stefank · 2025-03-26T12:19:14Z

test/hotspot/jtreg/sources/SortIncludes.java

+        if (!userIncludes.isEmpty() && !sysIncludes.isEmpty() && blankLines.isEmpty()) {
+            blankLines = List.of("");
+        }
+        result.addAll(blankLines);


If this line is removed you don't get the extra blank lines I mentioned in the previous PR. It also removes the extra blank line that you get inserted into oopMap.inline.hpp before the INCLUDE_JVMCI block.

Or, rather if the code is changed to:

if (!userIncludes.isEmpty() && !sysIncludes.isEmpty()) { result.add(""); }

stefank · 2025-03-26T12:19:51Z

src/hotspot/share/compiler/oopMap.inline.hpp

 #define SHARE_VM_COMPILER_OOPMAP_INLINE_HPP

 #include "compiler/oopMap.hpp"
-


This blank line should not be removed.

….hpp files

stefank

Thanks for doing the last two fixes. I think this looks good now, but I need a bit more time to do some deeper verification. Thanks!

kimbarrett · 2025-03-27T05:56:55Z

test/hotspot/jtreg/sources/SortIncludes.java

+import java.util.stream.Collectors;
+
+public class SortIncludes {
+    private static final String INCLUDE_LINE = "^ *#include *(<[^>]+>|\"[^\"]+\") *$\\n";


There are files that have spaces between the # and include. I'm kind of inclined to suggest we fix those
at some point (not in this PR). But the regex here needs to allow for that possibility, and perhaps (eventually)
complain about such.

Since there are no such cases in the files processed in this PR, I'd suggest not adding support for them. They can be fixed in follow up PRs as the relevant directories are added to TestIncludesAreSorted.HOTSPOT_SOURCES_TO_CHECK.

The regex needs to detect that case eventually anyway, so I think it should be done now. Either we allow that
case, in which case the regex must match to work properly where they are present. Or we forbid that case,
in which case the regex must match to detect future mistakes even after we've cleaned up existing usage.

To me it seems like a small adjustment fixes this

Suggested change

private static final String INCLUDE_LINE = "^ *#include *(<[^>]+>|\"[^\"]+\") *$\\n";

private static final String INCLUDE_LINE = "^ *# *include *(<[^>]+>|\"[^\"]+\") *$\\n";

kimbarrett · 2025-03-27T06:01:44Z

test/hotspot/jtreg/sources/SortIncludes.java

+        return String.join("\n", result) + "\n";
+    }
+
+    /// Processes the C++ source file in `path` to sort its include statements.


If we want to apply this to hotspot jtreg test code, then C source files also come into the picture.

I think the tool will need to be updated to handle C source files. At that point, the comment should be generalized.

kimbarrett · 2025-03-27T06:03:30Z

test/hotspot/jtreg/sources/SortIncludes.java

+    }
+
+    /// Processes the C++ source files in `paths` to check if their include statements are sorted.
+    /// Include statements with any non-space characters after the closing `"` or `>` will not


Perhaps this should be mentioned in the style guide?

kimbarrett · 2025-03-27T06:06:00Z

src/hotspot/share/ci/ciUtilities.inline.hpp


 #include "runtime/interfaceSupport.inline.hpp"

+


Extra blank line inserted?

kimbarrett · 2025-03-27T06:07:25Z

src/hotspot/share/ci/ciUtilities.inline.hpp

 #define SHARE_CI_CIUTILITIES_INLINE_HPP

 #include "ci/ciUtilities.hpp"



Extra blank line not removed?

kimbarrett · 2025-03-27T06:09:29Z

src/hotspot/share/compiler/compilationFailureInfo.cpp

 #include "compiler/abstractCompiler.hpp"
 #include "compiler/compilationFailureInfo.hpp"
 #include "compiler/compileTask.hpp"
 #ifdef COMPILER2


Conditional includes are supposed to follow unconditional in a section.
Out of scope for this PR?

Yep. From the PR description:

The tool does nothing about re-ordering blocks of conditional includes vs unconditional includes. I briefly looked into that but it gets very complicated, very quickly. That kind of re-ordering will have to continue to be done manually for now.

kimbarrett · 2025-03-27T06:10:37Z

src/hotspot/share/compiler/disassembler.hpp

+#include "utilities/globalDefinitions.hpp"
 #include "utilities/macros.hpp"

+


Extra blank line inserted?

This seems to be left-overs from an earlier run. If I run the tool on this file it doesn't add this blank line.

kimbarrett · 2025-03-27T06:26:43Z

Probably we want to eventually apply this to gtests, but there might be additional rules there. The include of
unittest.hpp is (usually) last, and there may be (or may have been) a technical reason for that.

Applying it to jtreg test support files could also introduce some challenges. Or at least discover a lot of
non-conforming files. We might eventually want a mechanism for excluding directories, in addition to an inclusion
list (that might eventually be "all").

These kinds of things can be followups once we have the basic mechanism in place.

stefank

I ran the latest script over the HotSpot source and see that it messes up corner-cases with our platform includes.

diff --git a/src/hotspot/cpu/aarch64/continuationEntry_aarch64.inline.hpp b/src/hotspot/cpu/aarch64/continuationEntry_aarch64.inline.hpp
index df4d3957239..e8816767a96 100644
--- a/src/hotspot/cpu/aarch64/continuationEntry_aarch64.inline.hpp
+++ b/src/hotspot/cpu/aarch64/continuationEntry_aarch64.inline.hpp
@@ -25,10 +25,9 @@
 #ifndef CPU_AARCH64_CONTINUATIONENTRY_AARCH64_INLINE_HPP
 #define CPU_AARCH64_CONTINUATIONENTRY_AARCH64_INLINE_HPP

-#include "runtime/continuationEntry.hpp"
-
 #include "code/codeCache.hpp"
 #include "oops/method.inline.hpp"
+#include "runtime/continuationEntry.hpp"
 #include "runtime/frame.inline.hpp"
 #include "runtime/registerMap.hpp"

The includes are:

.hpp --------------> _aarch64.hpp
 ^ ^
 | |
 | +------------------+
 |                    |
.inline.hpp -------> _aarch64.inline.hpp

So, continuationEntry.hpp acts like the .hpp file for continuationEntry_aarc64.inline.hpp.

Unfortunately, we don't have a fully consistent way to write our platform includes, so I don't know how to codify this in a tool without breaking things.

stefank · 2025-03-27T08:20:17Z

src/hotspot/share/compiler/disassembler.hpp

+#include "utilities/globalDefinitions.hpp"
 #include "utilities/macros.hpp"

+


This seems to be left-overs from an earlier run. If I run the tool on this file it doesn't add this blank line.

stefank · 2025-03-27T09:14:50Z

I verified that adding a comment to the end of the #include "runtime/continuationEntry.hpp" line leaves that file intact, so I think that is a good enough workaround for the problematic platform includes.

dougxc · 2025-03-27T09:40:20Z

Probably we want to eventually apply this to gtests, but there might be additional rules there. The include of unittest.hpp is (usually) last, and there may be (or may have been) a technical reason for that.

Applying it to jtreg test support files could also introduce some challenges. Or at least discover a lot of non-conforming files. We might eventually want a mechanism for excluding directories, in addition to an inclusion list (that might eventually be "all").

These kinds of things can be followups once we have the basic mechanism in place.

I would suggest someone open issue(s) for follow up enhancements to the tool. I think having something in place now and incrementally improving it and adjusting it for all the special cases makes most sense.

…ot be re-ordered

stefank

I'm happy with the capabilities of the tool now and think that it is good enough to include and promote to HotSpot devs.

One questions is where to put the tool? I don't think the test directory is the best place. Maybe somewhere in src/utils/. There is a tools dir here src/utils/src/build/tools/ but I don't know if it is appropriate to put it there. Maybe @magicus knows a good place for this?

A couple of nits:

jcheck fails because of whitespaces
The /// style comments is a style I haven't encountered before.

dougxc · 2025-03-27T10:36:38Z

A couple of nits:

jcheck fails because of whitespaces

The /// style comments is a style I haven't encountered before.

I fixed the whitespaces.
I can convert the /// comments if you want - no strong opinion.

dougxc · 2025-03-27T10:44:48Z

I just noticed that TestIncludesAreSorted is not run by GHA. How about we move test/hotspot/jtreg/sources into tier1_common:

diff --git a/test/hotspot/jtreg/TEST.groups b/test/hotspot/jtreg/TEST.groups
index 71b9e497e25..62b11e73aa0 100644
--- a/test/hotspot/jtreg/TEST.groups
+++ b/test/hotspot/jtreg/TEST.groups
@@ -139,6 +139,7 @@ serviceability_ttf_virtual = \
   -serviceability/jvmti/negative
 
 tier1_common = \
+  sources \
   sanity/BasicVMTest.java \
   gtest/GTestWrapper.java \
   gtest/LockStackGtests.java \
@@ -619,16 +620,12 @@ tier1_serviceability = \
   -serviceability/sa/TestJmapCore.java \
   -serviceability/sa/TestJmapCoreMetaspace.java
 
-tier1_sources = \
-   sources
-
 tier1 = \
   :tier1_common \
   :tier1_compiler \
   :tier1_gc \
   :tier1_runtime \
   :tier1_serviceability \
-  :tier1_sources
 
 tier2 = \
   :hotspot_tier2_runtime \

stefank · 2025-03-27T11:13:27Z

A couple of nits:

jcheck fails because of whitespaces

The /// style comments is a style I haven't encountered before.

I fixed the whitespaces. I can convert the /// comments if you want - no strong opinion.

Maybe someone else knows the preferred style for this? I don't think we need to block the integration because of this. If someone comes late with the proper comment style, we'll update it in a separate PR.

magicus · 2025-03-27T13:34:02Z

One questions is where to put the tool? I don't think the test directory is the best place. Maybe somewhere in src/utils/. There is a tools dir here src/utils/src/build/tools/ but I don't know if it is appropriate to put it there. Maybe @magicus knows a good place for this?

I would actually recommend just the bin directory. This is , after all, intended to be run as a simple script (remember, it was originally a python script), in a similar vein to the already existing blessed-modifier-order.sh script.

magicus · 2025-03-27T13:35:18Z

The /// style comments is a style I haven't encountered before.

This is for the new markdown comments. Personally, I very much prefer them and have been looking forward to these for a long time. But I don't know if we have any policy for or against those in the JDK. Using them in a script like this seems fine to me, at any rate.

dougxc · 2025-03-27T14:07:08Z

I just noticed that TestIncludesAreSorted is not run by GHA. How about we move test/hotspot/jtreg/sources into tier1_common:

I went ahead and pushed this change.

dougxc · 2025-03-27T14:14:47Z

I would actually recommend just the bin directory.

Fine by me but I'm not sure how to then use bin/SortIncludes.java in test/hotspot/jtreg/sources/TestIncludesAreSorted.java.

magicus · 2025-03-31T10:02:39Z

Hm...

I know the source code is bundled with the test image, but I'm not 100% sure if it just includes src, or if the entire top-level source is included. I'll need to check that, including what is the best way to get a proper reference to the top-level directory from a test.

kimbarrett · 2025-04-01T09:04:47Z

I know the source code is bundled with the test image, but I'm not 100% sure if it just includes src, or if the entire top-level source is included. I'll need to check that, including what is the best way to get a proper reference to the top-level directory from a test.

There was some discussion of this when recently adding the sources/TestNoNULL.java test. The code used here
appears to similar in function (though different code) to the approach taken in that earlier test.

stefank

This looks good to me.

I personally would have preferred to have the tool somewhere other than in the test directory, but I've gotten feedback from other HotSpot devs that they think its better to have the tool there.

I leave the review of TEST.group to someone else.

kimbarrett · 2025-04-01T09:25:17Z

test/hotspot/jtreg/TEST.groups

  -serviceability/jvmti/negative

 tier1_common = \
+  sources \


I don't understand this change. How does this end up doing anything different than before?

This makes sources be tested in GHA:

jdk/.github/workflows/test.yml

Line 88 in a1ab1d8

test-suite: 'test/hotspot/jtreg/:tier1_common'

An alternative would be to add a separate GHA jobs just for sources:

- test-name: 'hs/tier1 sources' test-suite: 'test/hotspot/jtreg/:tier1_sources' debug-suffix: -debug

Given how small sources is (currently only 1 test), it felt like it should just be folded into common.

Ah, the workflows definition is what I was having trouble finding. I
understand now. In light of that, the proposed change to the groups looks
fine.

kimbarrett · 2025-04-02T17:07:52Z

test/hotspot/jtreg/TEST.groups

  -serviceability/jvmti/negative

 tier1_common = \
+  sources \


Ah, the workflows definition is what I was having trouble finding. I
understand now. In light of that, the proposed change to the groups looks
fine.

dougxc · 2025-04-02T22:29:51Z

Thanks for all the discussion and reviews.

/integrate

openjdk · 2025-04-02T22:30:18Z

Going to push as commit 814730e.
Since your change was applied there have been 178 commits pushed to the master branch:

d435362: 8353479: jcmd with streaming output breaks intendation
130b0cd: 8353217: Build libsleef on macos-aarch64
209e72d: 8353234: Refactor XMLSecurityPropertyManager
cc870d4: 8352088: Call of com.sun.jdi.ThreadReference.threadGroups() can lock up target VM
d979bd8: 8344671: Few JFR streaming tests fail with application not alive error on MacOS 15
49cb7aa: 8339114: DaCapo xalan performance with -XX:+UseObjectMonitorTable
d32ff13: 8353117: Crash: assert(id >= ThreadIdentifier::initial() && id < ThreadIdentifier::current()) failed: must be reasonable)
a0677d9: 8353263: Parallel: Remove locking in PSOldGen::resize
8608b16: 8348887: Create IR framework test for JDK-8347997
23eb648: 8353545: Improve debug info for StartOptionTest
... and 168 more: https://git.openjdk.org/jdk/compare/df9210e6578acd53384ee1ac06601510c9a52696...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-04-02T22:30:26Z

@dougxc Pushed as commit 814730e.

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

do not reorder certain includes

a76d4f9

openjdk bot added graal graal-dev@openjdk.org hotspot hotspot-dev@openjdk.org labels Mar 26, 2025

dougxc force-pushed the JDK-8352645_java branch from ee585f1 to c82e6a0 Compare March 26, 2025 10:31

dougxc added 2 commits March 26, 2025 11:33

added tool to sort includes

d4d367a

sort includes in subset of HotSpot sources and added a test to keep t…

6277947

…hem sorted

dougxc force-pushed the JDK-8352645_java branch from c82e6a0 to 6277947 Compare March 26, 2025 10:33

dougxc marked this pull request as ready for review March 26, 2025 10:40

openjdk bot added the rfr Pull request is ready for review label Mar 26, 2025

dougxc mentioned this pull request Mar 26, 2025

8352645: Add tool support to check order of includes #24180

Closed

3 tasks

stefank suggested changes Mar 26, 2025

View reviewed changes

drop extra blank lines and preserve rule for first include in .inline…

18e2a1d

….hpp files

stefank reviewed Mar 26, 2025

View reviewed changes

kimbarrett suggested changes Mar 27, 2025

View reviewed changes

stefank reviewed Mar 27, 2025

View reviewed changes

dougxc added 3 commits March 27, 2025 10:41

update style guide with advice on how to label includes that should n…

807374e

…ot be re-ordered

removed extra blank lines

da06409

moved some logic out of SortIncludes into TestIncludesAreSorted

632ed38

openjdk bot removed the rfr Pull request is ready for review label Mar 27, 2025

allow spaces between # and include

cada0df

stefank reviewed Mar 27, 2025

View reviewed changes

remove trailing spaces

3819f86

moved test/hotspot/jtreg/sources into tier1_common

93770e7

openjdk bot added the rfr Pull request is ready for review label Mar 27, 2025

moved error message into UnsortedIncludesException

c93e664

convert Windows path to Unix path

921e325

stefank approved these changes Apr 1, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Apr 1, 2025

kimbarrett reviewed Apr 1, 2025

View reviewed changes

kimbarrett approved these changes Apr 2, 2025

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Apr 2, 2025

openjdk bot closed this Apr 2, 2025

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Apr 2, 2025

		#define SHARE_VM_COMPILER_OOPMAP_INLINE_HPP

		#include "compiler/oopMap.hpp"

	private static final String INCLUDE_LINE = "^ #include (<[^>]+>\|\"[^\"]+\") *$\\n";
	private static final String INCLUDE_LINE = "^ # include (<[^>]+>\|\"[^\"]+\") $\\n";

		#define SHARE_CI_CIUTILITIES_INLINE_HPP

		#include "ci/ciUtilities.hpp"

		#include "utilities/globalDefinitions.hpp"
		#include "utilities/macros.hpp"

8352645: Add tool support to check order of includes #24247

8352645: Add tool support to check order of includes #24247

Uh oh!

Conversation

dougxc commented Mar 26, 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 Mar 26, 2025

Uh oh!

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

Uh oh!

Uh oh!

openjdk bot commented Mar 26, 2025

Uh oh!

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

Uh oh!

Webrevs

Uh oh!

stefank left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stefank Mar 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stefank 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!

stefank Mar 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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 Mar 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stefank left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stefank commented Mar 27, 2025

Uh oh!

dougxc commented Mar 27, 2025

Uh oh!

dougxc commented Mar 26, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Mar 26, 2025 •

edited

Loading

mlbridge bot commented Mar 26, 2025 •

edited

Loading

stefank Mar 26, 2025 •

edited

Loading

stefank Mar 27, 2025 •

edited

Loading

kimbarrett commented Mar 27, 2025 •

edited

Loading