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

[Stefan Karlsson]

On Fri, 28 Mar 2025 22:24:40 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:
> 
>   convert Windows path to Unix path

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.

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

Marked as reviewed by stefank (Reviewer).

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