8352645: Add tool support to check order of includes

doc/hotspot-style.html

@@ -207,27 +207,34 @@ <h3 id="source-files">Source Files</h3>
 <ul>
 <li><p>All source files must have a globally unique basename. The build
 system depends on this uniqueness.</p></li>
-<li><p>Keep the include lines within a section alphabetically sorted by their lowercase value.</p></li>
-<li><p>Put conditional inclusions (`#if ...`) at the end of the section of HotSpot
-include lines. This also applies to macro-expanded includes of platform
-dependent files.</p></li>
-<li><p>Put system includes in a section after the HotSpot include lines with a blank
-line separating the two sections.</p></li>
+<li><p>Keep the include lines within a section alphabetically sorted by
+their lowercase value. If an include must be out of order for
+correctness, suffix with it a comment such as
+<code>// do not reorder</code>. Source code processing tools can also
+use this hint.</p></li>
+<li><p>Put conditional inclusions (<code>#if ...</code>) at the end of
+the section of HotSpot include lines. This also applies to
+macro-expanded includes of platform dependent files.</p></li>
+<li><p>Put system includes in a section after the HotSpot include lines
+with a blank line separating the two sections.</p></li>
 <li><p>Do not put non-trivial function implementations in .hpp files. If
-the implementation depends on other .hpp files, put it in a .cpp or
-a .inline.hpp file.</p></li>
+the implementation depends on other .hpp files, put it in a .cpp or a
+.inline.hpp file.</p></li>
 <li><p>.inline.hpp files should only be included in .cpp or .inline.hpp
 files.</p></li>
-<li><p>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.</p></li>
-<li><p>Do not include a .hpp file if the corresponding .inline.hpp file is included.</p></li>
-<li><p>Use include guards for .hpp and .inline.hpp files. The name of the defined
-guard should be derived from the full search path of the file relative to the
-hotspot source directory. The guard should be all upper case with all paths
-separators and periods replaced by underscores.</p></li>
+<li><p>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.</p></li>
+<li><p>Do not include a .hpp file if the corresponding .inline.hpp file
+is included.</p></li>
+<li><p>Use include guards for .hpp and .inline.hpp files. The name of
+the defined guard should be derived from the full search path of the
+file relative to the hotspot source directory. The guard should be all
+upper case with all paths separators and periods replaced by
+underscores.</p></li>
 <li><p>Some build configurations use precompiled headers to speed up the
 build times. The precompiled headers are included in the precompiled.hpp
 file. Note that precompiled.hpp is just a build time optimization, so

doc/hotspot-style.md

@@ -138,7 +138,10 @@ change should be done with a "setter" accessor matched to the simple
 * All source files must have a globally unique basename. The build
 system depends on this uniqueness.
 
-* Keep the include lines within a section alphabetically sorted by their lowercase value.
+* Keep the include lines within a section alphabetically sorted by their
+lowercase value. If an include must be out of order for correctness,
+suffix with it a comment such as `// do not reorder`. Source code
+processing tools can also use this hint.
 
 * Put conditional inclusions (`#if ...`) at the end of the section of HotSpot
 include lines. This also applies to macro-expanded includes of platform

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

@@ -29,7 +29,6 @@
 
 #include "runtime/interfaceSupport.inline.hpp"
 
-
 // Add a ci native entry wrapper?
 
 // Bring the compilation thread into the VM state.

src/hotspot/share/compiler/disassembler.hpp

@@ -33,7 +33,6 @@
 #include "utilities/globalDefinitions.hpp"
 #include "utilities/macros.hpp"
 
-
 class decode_env;
 
 // The disassembler prints out assembly code annotated

test/hotspot/jtreg/sources/SortIncludes.java

@@ -33,10 +33,9 @@
 import java.util.TreeSet;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
-import java.util.stream.Collectors;
 
 public class SortIncludes {
-    private static final String INCLUDE_LINE = "^ *#include *(<[^>]+>|\"[^\"]+\") *$\\n";
+    private static final String INCLUDE_LINE = "^ *# *include *(<[^>]+>|\"[^\"]+\") *$\\n";
     private static final String BLANK_LINE = "^$\\n";
     private static final Pattern INCLUDES_RE = Pattern.compile(
                     String.format("%s(?:(?:%s)*%s)*", INCLUDE_LINE, BLANK_LINE, INCLUDE_LINE),
@@ -60,7 +59,7 @@ private static String extract(String s, char start, char end) {
 
     /// Sorts the include statements in `block`.
     ///
-    /// @param path path of source file containing `block`
+    /// @param path path of the file containing `block`
     /// @param block source code chunk containing 1 or more include statements
     /// @return `block` with the include statements sorted and a blank line between user and
     /// sys includes
@@ -85,7 +84,6 @@ private static String sortedIncludes(Path path, String block) {
 
         // Partition lines into user include and sys includes and discard blank lines
         for (String line : lines) {
-            int doubleQuote = line.indexOf('"');
             if (line.contains("\"")) {
                 if (isInlineHpp && nonInlineHpp.endsWith(extract(line, '"', '"'))) {
                     result.add(line);
@@ -146,6 +144,8 @@ public static boolean sortIncludes(Path path, boolean update) throws IOException
         return false;
     }
 
+    /// Record of the files processed by [#process(List, boolean)] and those
+    /// that had unsorted includes.
     public record Result(List<Path> files, List<Path> unsorted) {
     }
 
@@ -182,7 +182,25 @@ public static Result process(List<Path> paths, boolean update) throws IOExceptio
         return new Result(files, unsorted);
     }
 
-    public static void main(String[] args) throws IOException {
+    /// Exception thrown by [#main] if `"--update"` is in `args` and
+    /// files with unsorted includes were seen.
+    public static class UnsortedIncludesException extends Exception {
+        /// Files with unsorted includes.
+        public final List<Path> files;
+
+        public UnsortedIncludesException(List<Path> files) {
+            this.files = files;
+        }
+    }
+
+    /// Processes C++ files to check if their include statements are sorted.
+    ///
+    /// @param args `[--update] dir|file...` where `update` means the processed
+    ///        files are updated to sort any unsorted includes and `dir|file` are the
+    ///        roots to scan for the C++ files to be processed
+    /// @throws UnsortedIncludesException if `args` includes `"--update"` and
+    ///         files with unsorted includes were found
+    public static void main(String[] args) throws IOException, UnsortedIncludesException {
         boolean update = false;
         List<Path> paths = new ArrayList<>();
         for (String arg : args) {
@@ -199,32 +217,7 @@ public static void main(String[] args) throws IOException {
                             result.files.size(),
                             result.unsorted().size());
         } else if (!result.unsorted().isEmpty()) {
-            String unsorted = result.unsorted.stream().map(Path::toString).collect(Collectors.joining(System.lineSeparator()));
-            String msg = String.format("""
-                            %d files with unsorted headers found:
-
-                            %s
-
-                            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"
-
-                            """,
-                            result.unsorted().size(), unsorted);
-            throw new RuntimeException(msg);
+            throw new UnsortedIncludesException(result.unsorted);
         }
     }
 }

test/hotspot/jtreg/sources/TestIncludesAreSorted.java

@@ -33,6 +33,7 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 public class TestIncludesAreSorted {
@@ -73,11 +74,40 @@
                         .toArray(String[]::new);
         try {
             SortIncludes.main(args);
-        } catch (RuntimeException e) {
-            String msg = String.format("The unsorted includes listed below should be fixable by running:%n%n    java %s.java --update %s%n",
-                            testSrcDir.resolve(SortIncludes.class.getSimpleName()),
-                            String.join(" ", args));
-            throw new RuntimeException(msg, e);
+        } catch (SortIncludes.UnsortedIncludesException e) {
+            String unsorted = e.files.stream().map(Path::toString).collect(Collectors.joining(System.lineSeparator()));
+            String msg = String.format("""
+                            %d files with unsorted headers found:
+
+                            %s
+
+                            This should be fixable by running:
+
+                                java %s.java --update %s
+
+
+                            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"
+
+                            """,
+                    e.files.size(), unsorted,
+                    testSrcDir.resolve(SortIncludes.class.getSimpleName()),
+                    String.join(" ", args));
+            throw new RuntimeException(msg);
         }
     }
 }