RFR: 8352645: Add tool support to check order of includes [v2]

[Stefan Karlsson]

On Wed, 26 Mar 2025 14:23:09 GMT, Doug Simon <dnsimon at openjdk.org> wrote:

>> 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](https://github.com/openjdk/jdk/blob/master/doc/hotspot-style.md#source-files)).
>> 
>> By virtue of using `SortedSet`, the tool also removes duplicate includes (e.g. `"compiler/compilerDirectives.hpp"` on line [37](https://github.com/openjdk/jdk/blob/059f190f4b0c7836b89ca2070400529e8d33790b/src/hotspot/share/c1/c1_Compilation.cpp#L37) and line [41](https://github.com/openjdk/jdk/blob/059f190f4b0c7836b89ca2070400529e8d33790b/src/hotspot/share/c1/c1_Compilation.cpp#L41)). 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/dnsimo...
>
> Doug Simon has updated the pull request incrementally with one additional commit since the last revision:
> 
>   drop extra blank lines and preserve rule for first include in .inline.hpp files

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.

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

PR Review: https://git.openjdk.org/jdk/pull/24247#pullrequestreview-2720267338