Skip to content

Conversation

@mordante
Copy link
Member

@mordante mordante commented Feb 2, 2024

On Windows the libc++ test suite sees the MSVC STL headers and may conclude these are libc++ headers when inspecting the name. Modules guard against forgetting to export new headers. Finding MSVC STL's headers gives false positives. Since the CI tests non-Windows platforms too, the validation will be disabled on Windows.

Fixes: #79010

On Windows the libc++ test suite sees the MSVC STL headers and may
conclude these are libc++ headers when inspecting the name. Modules
guard against forgetting to export new headers. Finding MSVC STL's
headers gives false positives. Since the CI tests non-Windows platforms
too, the validation will be disabled on Windows.

Fixes: llvm#79010
@mordante mordante requested a review from a team as a code owner February 2, 2024 19:27
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Feb 2, 2024
@llvmbot
Copy link
Member

llvmbot commented Feb 2, 2024

@llvm/pr-subscribers-libcxx

Author: Mark de Wever (mordante)

Changes

On Windows the libc++ test suite sees the MSVC STL headers and may conclude these are libc++ headers when inspecting the name. Modules guard against forgetting to export new headers. Finding MSVC STL's headers gives false positives. Since the CI tests non-Windows platforms too, the validation will be disabled on Windows.

Fixes: #79010


Full diff: https://github.com/llvm/llvm-project/pull/80478.diff

3 Files Affected:

  • (modified) libcxx/modules/std.compat.cppm.in (+42-33)
  • (modified) libcxx/modules/std.cppm.in (+42-33)
  • (modified) libcxx/utils/generate_libcxx_cppm_in.py (+18-5)
diff --git a/libcxx/modules/std.compat.cppm.in b/libcxx/modules/std.compat.cppm.in
index 651d6ec7b9fe2..16363711ac762 100644
--- a/libcxx/modules/std.compat.cppm.in
+++ b/libcxx/modules/std.compat.cppm.in
@@ -46,39 +46,48 @@ module;
 #endif
 
 // *** Headers not yet available ***
-#if __has_include(<debugging>)
-#  error "please update the header information for <debugging> in headers_not_available in utils/libcxx/header_information.py"
-#endif // __has_include(<debugging>)
-#if __has_include(<flat_map>)
-#  error "please update the header information for <flat_map> in headers_not_available in utils/libcxx/header_information.py"
-#endif // __has_include(<flat_map>)
-#if __has_include(<flat_set>)
-#  error "please update the header information for <flat_set> in headers_not_available in utils/libcxx/header_information.py"
-#endif // __has_include(<flat_set>)
-#if __has_include(<generator>)
-#  error "please update the header information for <generator> in headers_not_available in utils/libcxx/header_information.py"
-#endif // __has_include(<generator>)
-#if __has_include(<hazard_pointer>)
-#  error "please update the header information for <hazard_pointer> in headers_not_available in utils/libcxx/header_information.py"
-#endif // __has_include(<hazard_pointer>)
-#if __has_include(<linalg>)
-#  error "please update the header information for <linalg> in headers_not_available in utils/libcxx/header_information.py"
-#endif // __has_include(<linalg>)
-#if __has_include(<rcu>)
-#  error "please update the header information for <rcu> in headers_not_available in utils/libcxx/header_information.py"
-#endif // __has_include(<rcu>)
-#if __has_include(<spanstream>)
-#  error "please update the header information for <spanstream> in headers_not_available in utils/libcxx/header_information.py"
-#endif // __has_include(<spanstream>)
-#if __has_include(<stacktrace>)
-#  error "please update the header information for <stacktrace> in headers_not_available in utils/libcxx/header_information.py"
-#endif // __has_include(<stacktrace>)
-#if __has_include(<stdfloat>)
-#  error "please update the header information for <stdfloat> in headers_not_available in utils/libcxx/header_information.py"
-#endif // __has_include(<stdfloat>)
-#if __has_include(<text_encoding>)
-#  error "please update the header information for <text_encoding> in headers_not_available in utils/libcxx/header_information.py"
-#endif // __has_include(<text_encoding>)
+//
+// This validation is mainly to aid libc++ developers to add modules for new
+// headers. On Windows the Windows SDK can be in the include path. This SDK
+// contains the MSVC STL headers. This may give false positives when MSVC STL
+// provides a header libc++ has not implemented yet. Therefore this validation
+// is not done on Windows.
+//
+#ifndef _WIN32
+#  if __has_include(<debugging>)
+#    error "please update the header information for <debugging> in headers_not_available in utils/libcxx/header_information.py"
+#  endif // __has_include(<debugging>)
+#  if __has_include(<flat_map>)
+#    error "please update the header information for <flat_map> in headers_not_available in utils/libcxx/header_information.py"
+#  endif // __has_include(<flat_map>)
+#  if __has_include(<flat_set>)
+#    error "please update the header information for <flat_set> in headers_not_available in utils/libcxx/header_information.py"
+#  endif // __has_include(<flat_set>)
+#  if __has_include(<generator>)
+#    error "please update the header information for <generator> in headers_not_available in utils/libcxx/header_information.py"
+#  endif // __has_include(<generator>)
+#  if __has_include(<hazard_pointer>)
+#    error "please update the header information for <hazard_pointer> in headers_not_available in utils/libcxx/header_information.py"
+#  endif // __has_include(<hazard_pointer>)
+#  if __has_include(<linalg>)
+#    error "please update the header information for <linalg> in headers_not_available in utils/libcxx/header_information.py"
+#  endif // __has_include(<linalg>)
+#  if __has_include(<rcu>)
+#    error "please update the header information for <rcu> in headers_not_available in utils/libcxx/header_information.py"
+#  endif // __has_include(<rcu>)
+#  if __has_include(<spanstream>)
+#    error "please update the header information for <spanstream> in headers_not_available in utils/libcxx/header_information.py"
+#  endif // __has_include(<spanstream>)
+#  if __has_include(<stacktrace>)
+#    error "please update the header information for <stacktrace> in headers_not_available in utils/libcxx/header_information.py"
+#  endif // __has_include(<stacktrace>)
+#  if __has_include(<stdfloat>)
+#    error "please update the header information for <stdfloat> in headers_not_available in utils/libcxx/header_information.py"
+#  endif // __has_include(<stdfloat>)
+#  if __has_include(<text_encoding>)
+#    error "please update the header information for <text_encoding> in headers_not_available in utils/libcxx/header_information.py"
+#  endif // __has_include(<text_encoding>)
+#endif // _WIN32
 
 export module std.compat;
 export import std;
diff --git a/libcxx/modules/std.cppm.in b/libcxx/modules/std.cppm.in
index 6ce8e287737b8..3b59c28482b11 100644
--- a/libcxx/modules/std.cppm.in
+++ b/libcxx/modules/std.cppm.in
@@ -168,39 +168,48 @@ module;
 #include <version>
 
 // *** Headers not yet available ***
-#if __has_include(<debugging>)
-#  error "please update the header information for <debugging> in headers_not_available in utils/libcxx/header_information.py"
-#endif // __has_include(<debugging>)
-#if __has_include(<flat_map>)
-#  error "please update the header information for <flat_map> in headers_not_available in utils/libcxx/header_information.py"
-#endif // __has_include(<flat_map>)
-#if __has_include(<flat_set>)
-#  error "please update the header information for <flat_set> in headers_not_available in utils/libcxx/header_information.py"
-#endif // __has_include(<flat_set>)
-#if __has_include(<generator>)
-#  error "please update the header information for <generator> in headers_not_available in utils/libcxx/header_information.py"
-#endif // __has_include(<generator>)
-#if __has_include(<hazard_pointer>)
-#  error "please update the header information for <hazard_pointer> in headers_not_available in utils/libcxx/header_information.py"
-#endif // __has_include(<hazard_pointer>)
-#if __has_include(<linalg>)
-#  error "please update the header information for <linalg> in headers_not_available in utils/libcxx/header_information.py"
-#endif // __has_include(<linalg>)
-#if __has_include(<rcu>)
-#  error "please update the header information for <rcu> in headers_not_available in utils/libcxx/header_information.py"
-#endif // __has_include(<rcu>)
-#if __has_include(<spanstream>)
-#  error "please update the header information for <spanstream> in headers_not_available in utils/libcxx/header_information.py"
-#endif // __has_include(<spanstream>)
-#if __has_include(<stacktrace>)
-#  error "please update the header information for <stacktrace> in headers_not_available in utils/libcxx/header_information.py"
-#endif // __has_include(<stacktrace>)
-#if __has_include(<stdfloat>)
-#  error "please update the header information for <stdfloat> in headers_not_available in utils/libcxx/header_information.py"
-#endif // __has_include(<stdfloat>)
-#if __has_include(<text_encoding>)
-#  error "please update the header information for <text_encoding> in headers_not_available in utils/libcxx/header_information.py"
-#endif // __has_include(<text_encoding>)
+//
+// This validation is mainly to aid libc++ developers to add modules for new
+// headers. On Windows the Windows SDK can be in the include path. This SDK
+// contains the MSVC STL headers. This may give false positives when MSVC STL
+// provides a header libc++ has not implemented yet. Therefore this validation
+// is not done on Windows.
+//
+#ifndef _WIN32
+#  if __has_include(<debugging>)
+#    error "please update the header information for <debugging> in headers_not_available in utils/libcxx/header_information.py"
+#  endif // __has_include(<debugging>)
+#  if __has_include(<flat_map>)
+#    error "please update the header information for <flat_map> in headers_not_available in utils/libcxx/header_information.py"
+#  endif // __has_include(<flat_map>)
+#  if __has_include(<flat_set>)
+#    error "please update the header information for <flat_set> in headers_not_available in utils/libcxx/header_information.py"
+#  endif // __has_include(<flat_set>)
+#  if __has_include(<generator>)
+#    error "please update the header information for <generator> in headers_not_available in utils/libcxx/header_information.py"
+#  endif // __has_include(<generator>)
+#  if __has_include(<hazard_pointer>)
+#    error "please update the header information for <hazard_pointer> in headers_not_available in utils/libcxx/header_information.py"
+#  endif // __has_include(<hazard_pointer>)
+#  if __has_include(<linalg>)
+#    error "please update the header information for <linalg> in headers_not_available in utils/libcxx/header_information.py"
+#  endif // __has_include(<linalg>)
+#  if __has_include(<rcu>)
+#    error "please update the header information for <rcu> in headers_not_available in utils/libcxx/header_information.py"
+#  endif // __has_include(<rcu>)
+#  if __has_include(<spanstream>)
+#    error "please update the header information for <spanstream> in headers_not_available in utils/libcxx/header_information.py"
+#  endif // __has_include(<spanstream>)
+#  if __has_include(<stacktrace>)
+#    error "please update the header information for <stacktrace> in headers_not_available in utils/libcxx/header_information.py"
+#  endif // __has_include(<stacktrace>)
+#  if __has_include(<stdfloat>)
+#    error "please update the header information for <stdfloat> in headers_not_available in utils/libcxx/header_information.py"
+#  endif // __has_include(<stdfloat>)
+#  if __has_include(<text_encoding>)
+#    error "please update the header information for <text_encoding> in headers_not_available in utils/libcxx/header_information.py"
+#  endif // __has_include(<text_encoding>)
+#endif // _WIN32
 
 export module std;
 
diff --git a/libcxx/utils/generate_libcxx_cppm_in.py b/libcxx/utils/generate_libcxx_cppm_in.py
index 2d3f829847fb9..57e1f1a8bbd30 100644
--- a/libcxx/utils/generate_libcxx_cppm_in.py
+++ b/libcxx/utils/generate_libcxx_cppm_in.py
@@ -57,18 +57,31 @@ def write_file(module):
             else:
                 module_cpp_in.write(f"#include <{header}>\n")
 
-        module_cpp_in.write("\n// *** Headers not yet available ***\n")
+        module_cpp_in.write(
+            """
+// *** Headers not yet available ***
+//
+// This validation is mainly to aid libc++ developers to add modules for new
+// headers. On Windows the Windows SDK can be in the include path. This SDK
+// contains the MSVC STL headers. This may give false positives when MSVC STL
+// provides a header libc++ has not implemented yet. Therefore this validation
+// is not done on Windows.
+//
+#ifndef _WIN32
+"""
+        )
         for header in sorted(headers_not_available):
             module_cpp_in.write(
                 f"""\
-#if __has_include(<{header}>)
-#  error "please update the header information for <{header}> in headers_not_available in utils/libcxx/header_information.py"
-#endif // __has_include(<{header}>)
+#  if __has_include(<{header}>)
+#    error "please update the header information for <{header}> in headers_not_available in utils/libcxx/header_information.py"
+#  endif // __has_include(<{header}>)
 """
             )
 
         module_cpp_in.write(
-            f"""
+            f"""#endif // _WIN32
+
 export module {module};
 {'export import std;' if module == 'std.compat' else ''}
 

@mordante mordante merged commit 4bf9fa5 into llvm:main Feb 9, 2024
@mordante mordante deleted the review/modules_guard_against_msvc_stl_headers branch February 9, 2024 16:41
mordante added a commit that referenced this pull request Feb 10, 2024
After applying the review comments of
#80478
I've forgotten to update the generated files. This fixes the issue and
removes trailing whitespace.
blueboxd pushed a commit to blueboxd/libcxx that referenced this pull request May 4, 2024
After applying the review comments of
llvm/llvm-project#80478
I've forgotten to update the generated files. This fixes the issue and
removes trailing whitespace.

NOKEYCHECK=True
GitOrigin-RevId: f66f44eb0c194f6bd0b6387d778624b303b6edc1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[libc++][modules] Guard #error against MSVC headers on Windows

3 participants