From b9d8d6aeeef9feacf6eb4838cdccca4c2da2a0eb Mon Sep 17 00:00:00 2001
From: Peter Kasting <pkasting@chromium.org>
Date: Tue, 8 Oct 2024 18:33:10 +0000
Subject: [PATCH] Tweak Windows header-sorting in .clang-format and
 add_header.py.

* Add <winternl.h> to the list of must-be-early Windows headers.
* Add "windows_h_disallowed.h" as "must be last".
* Minor comment/regex typo fixes.
* Alphabetize .clang-format list (cosmetic change; that was already the
  actual behavior).
* Alphabetize add_header.py change (behavior change; this order is
  the sort key). This matches the .clang-format behavior so they
  don't fight each other.

Bug: 364987728
Change-Id: Ic8560ab1cf476918c8dd9454dc7ef5ccb5aecd8f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5913362
Reviewed-by: Nico Weber <thakis@chromium.org>
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1365712}
---
 .clang-format            |  9 +++++---
 tools/add_header.py      | 47 ++++++++++++++++++----------------------
 tools/add_header_test.py | 24 ++++++++++----------
 3 files changed, 39 insertions(+), 41 deletions(-)

diff --git a/.clang-format b/.clang-format
index ce7d0ad33ef02..688253e53adbe 100644
--- a/.clang-format
+++ b/.clang-format
@@ -28,10 +28,10 @@ IncludeCategories:
   # LINT.IfChange(winheader)
   - Regex:           '^<objbase\.h>' # This has to be before initguid.h.
     Priority:        1
-  - Regex:           '^<(initguid|mmdeviceapi|windows|winsock2|ws2tcpip|shobjidl|atlbase|ole2|unknwn|tchar|ocidl)\.h>'
+  - Regex:           '^<(atlbase|initguid|mmdeviceapi|ocidl|ole2|shobjidl|tchar|unknwn|windows|winsock2|winternl|ws2tcpip)\.h>'
     Priority:        2
   # LINT.ThenChange(/tools/add_header.py:winheader)
-  # UIAutomation*.h need to be after base/win/atl.h.
+  # UIAutomation*.h needs to be after base/win/atl.h.
   # Note the low priority number.
   - Regex:           '^<UIAutomation.*\.h>'
     Priority:        6
@@ -39,8 +39,11 @@ IncludeCategories:
   - Regex:           '^<.*\.h>'
     Priority:        3
   # C++ standard library headers.
-  - Regex:           '^<.*'
+  - Regex:           '^<.*>'
     Priority:        4
+  # windows_h_disallowed.h should appear last. Note the low priority number.
+  - Regex:           '"(.*/)?windows_h_disallowed\.h"'
+    Priority:        7
   # Other libraries.
   - Regex:           '.*'
     Priority:        5
diff --git a/tools/add_header.py b/tools/add_header.py
index d4223d70f3ab4..958e6f57311d1 100755
--- a/tools/add_header.py
+++ b/tools/add_header.py
@@ -343,34 +343,29 @@ def SerializeIncludes(includes):
   source = []
 
   # LINT.IfChange(winheader)
+  # Headers that are sorted above others to prevent inclusion order issues.
+  # NOTE: The order of these headers is the sort key and will be the order in
+  # the output file. It should be set to match whatever clang-format will do.
   special_headers = [
-      # Must be included before ws2tcpip.h.
-      # Doesn't need to be included before <windows.h> with
-      # WIN32_LEAN_AND_MEAN but why chance it?
-      '<winsock2.h>',
-      # Must be before lots of things, e.g. shellapi.h, winbase.h,
-      # versionhelpers.h, memoryapi.h, hidclass.h, ncrypt.h., ...
-      '<windows.h>',
-      # Must be before iphlpapi.h.
-      '<ws2tcpip.h>',
-      # Must be before propkey.h.
-      '<shobjidl.h>',
-      # Must be before atlapp.h.
-      '<atlbase.h>',
-      # Must be before intshcut.h.
-      '<ole2.h>',
-      # Must be before intshcut.h.
-      '<unknwn.h>',
-      # Must be before uiautomation.h.
+      # Listed first because it must be before initguid.h in the block below.
       '<objbase.h>',
-      # Must be before tpcshrd.h.
-      '<tchar.h>',
-      # Must be before functiondiscoverykeys_devpkey.h.
-      '<mmdeviceapi.h>',
-      # Must be before emi.h.
-      '<initguid.h>',
-      # Must be before commdlg.h.
-      '<ocidl.h>',
+
+      # Alphabetized block that don't matter relative to each other, but need to
+      # be included before any instance of the listed other header. These other
+      # listed headers are non-exhaustive examples.
+      '<atlbase.h>',      # atlapp.h
+      '<initguid.h>',     # emi.h
+      '<mmdeviceapi.h>',  # functiondiscoverykeys_devpkey.h
+      '<ocidl.h>',        # commdlg.h
+      '<ole2.h>',         # intshcut.h
+      '<shobjidl.h>',     # propkey.h
+      '<tchar.h>',        # tpcshrd.h
+      '<unknwn.h>',       # intshcut.h
+      '<windows.h>',      # hidclass.h, memoryapi.h, ncrypt.h, shellapi.h,
+                          # versionhelpers.h, winbase.h, etc.
+      '<winsock2.h>',     # ws2tcpip.h
+      '<winternl.h>',     # ntsecapi.h; also needs `#define _NTDEF_`
+      '<ws2tcpip.h>',     # iphlpapi.h
   ]
 
   # LINT.ThenChange(/.clang-format:winheader)
diff --git a/tools/add_header_test.py b/tools/add_header_test.py
index dd07b7e309aba..1576bc80e50f3 100755
--- a/tools/add_header_test.py
+++ b/tools/add_header_test.py
@@ -376,27 +376,27 @@ class SerializeIncludesTest(unittest.TestCase):
     primary_header = add_header.Include('"cow.h"', 'include', [], None)
     primary_header.is_primary_header = True
     includes.append(primary_header)
+    includes.append(add_header.Include('<objbase.h>', 'include', [], None))
+    includes.append(add_header.Include('<atlbase.h>', 'include', [], None))
+    includes.append(add_header.Include('<ole2.h>', 'include', [], None))
+    includes.append(add_header.Include('<shobjidl.h>', 'include', [], None))
+    includes.append(add_header.Include('<tchar.h>', 'include', [], None))
+    includes.append(add_header.Include('<unknwn.h>', 'include', [], None))
     includes.append(add_header.Include('<winsock2.h>', 'include', [], None))
     includes.append(add_header.Include('<windows.h>', 'include', [], None))
     includes.append(add_header.Include('<ws2tcpip.h>', 'include', [], None))
-    includes.append(add_header.Include('<shobjidl.h>', 'include', [], None))
-    includes.append(add_header.Include('<atlbase.h>', 'include', [], None))
-    includes.append(add_header.Include('<ole2.h>', 'include', [], None))
-    includes.append(add_header.Include('<unknwn.h>', 'include', [], None))
-    includes.append(add_header.Include('<objbase.h>', 'include', [], None))
-    includes.append(add_header.Include('<tchar.h>', 'include', [], None))
-    includes.append(add_header.Include('<string.h>', 'include', [], None))
     includes.append(add_header.Include('<stddef.h>', 'include', [], None))
     includes.append(add_header.Include('<stdio.h>', 'include', [], None))
+    includes.append(add_header.Include('<string.h>', 'include', [], None))
     includes.append(add_header.Include('"moo.h"', 'include', [], None))
     random.shuffle(includes)
     source = add_header.SerializeIncludes(includes)
     self.assertEqual(source, [
-        '#include "cow.h"', '', '#include <winsock2.h>', '#include <windows.h>',
-        '#include <ws2tcpip.h>', '#include <shobjidl.h>',
-        '#include <atlbase.h>', '#include <ole2.h>', '#include <unknwn.h>',
-        '#include <objbase.h>', '#include <tchar.h>', '#include <stddef.h>',
-        '#include <stdio.h>', '#include <string.h>', '', '#include "moo.h"'
+        '#include "cow.h"', '', '#include <objbase.h>', '#include <atlbase.h>',
+        '#include <ole2.h>', '#include <shobjidl.h>', '#include <tchar.h>',
+        '#include <unknwn.h>', '#include <windows.h>', '#include <winsock2.h>',
+        '#include <ws2tcpip.h>', '#include <stddef.h>', '#include <stdio.h>',
+        '#include <string.h>', '', '#include "moo.h"'
     ])