[libcxx-commits] [libcxx] 0e9a01d - [libc++] Fix modules builds when features are removed

[Louis Dionne via libcxx-commits]

Author: Louis Dionne
Date: 2022-06-08T18:48:25-04:00
New Revision: 0e9a01dcac99ccf599738cef394b840b126d5cc9

URL: https://github.com/llvm/llvm-project/commit/0e9a01dcac99ccf599738cef394b840b126d5cc9
DIFF: https://github.com/llvm/llvm-project/commit/0e9a01dcac99ccf599738cef394b840b126d5cc9.diff

LOG: [libc++] Fix modules builds when features are removed

When some headers are not available because we removed features like
localization or threads, the compiler should not try to include these
headers when building modules. To avoid that from happening, add a
requires-declaration that is never satisfied when the configuration
in use doesn't support a header.

rdar://93777687

Differential Revision: https://reviews.llvm.org/D127127

Added: 
    libcxx/include/module.modulemap.in

Modified: 
    libcxx/docs/Contributing.rst
    libcxx/include/CMakeLists.txt
    libcxx/include/csignal
    libcxx/test/libcxx/lint/lint_headers.sh.py
    libcxx/test/libcxx/lint/lint_modulemap.sh.py
    libcxx/test/libcxx/modules_include.sh.cpp
    libcxx/utils/generate_header_tests.py
    libcxx/utils/libcxx/test/dsl.py
    libcxx/utils/libcxx/test/features.py

Removed: 
    libcxx/include/module.modulemap


################################################################################
diff  --git a/libcxx/docs/Contributing.rst b/libcxx/docs/Contributing.rst
index b812b958e6a0..9806f9b445c4 100644
--- a/libcxx/docs/Contributing.rst
+++ b/libcxx/docs/Contributing.rst
@@ -37,7 +37,7 @@ sure you don't forget anything:
 - Did you mark all functions and type declarations with the :ref:`proper visibility macro <visibility-macros>`?
 - If you added a header:
 
-  - Did you add it to ``include/module.modulemap``?
+  - Did you add it to ``include/module.modulemap.in``?
   - Did you add it to ``include/CMakeLists.txt``?
   - If it's a public header, did you add a test under ``test/libcxx`` that the new header defines ``_LIBCPP_VERSION``? See ``test/libcxx/algorithms/version.pass.cpp`` for an example. NOTE: This should be automated.
   - If it's a public header, did you update ``utils/generate_header_inclusion_tests.py``?

diff  --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt
index 8f2a66f86f8c..3f89fee658cb 100644
--- a/libcxx/include/CMakeLists.txt
+++ b/libcxx/include/CMakeLists.txt
@@ -618,7 +618,6 @@ set(files
   map
   math.h
   memory
-  module.modulemap
   mutex
   new
   numbers
@@ -669,9 +668,17 @@ set(files
   wctype.h
   )
 
+foreach(feature LIBCXX_ENABLE_FILESYSTEM LIBCXX_ENABLE_LOCALIZATION LIBCXX_ENABLE_THREADS LIBCXX_ENABLE_WIDE_CHARACTERS)
+  if (NOT ${${feature}})
+    set(requires_${feature} "requires LIBCXX_CONFIGURED_WITHOUT_SUPPORT_FOR_THIS_HEADER")
+  endif()
+endforeach()
+
 configure_file("__config_site.in" "${LIBCXX_GENERATED_INCLUDE_TARGET_DIR}/__config_site" @ONLY)
+configure_file("module.modulemap.in" "${LIBCXX_GENERATED_INCLUDE_DIR}/module.modulemap" @ONLY)
 
-set(_all_includes "${LIBCXX_GENERATED_INCLUDE_TARGET_DIR}/__config_site")
+set(_all_includes "${LIBCXX_GENERATED_INCLUDE_TARGET_DIR}/__config_site"
+                  "${LIBCXX_GENERATED_INCLUDE_DIR}/module.modulemap")
 foreach(f ${files})
   set(src "${CMAKE_CURRENT_SOURCE_DIR}/${f}")
   set(dst "${LIBCXX_GENERATED_INCLUDE_DIR}/${f}")

diff  --git a/libcxx/include/csignal b/libcxx/include/csignal
index 81ca89091c1b..c1b58f818b62 100644
--- a/libcxx/include/csignal
+++ b/libcxx/include/csignal
@@ -41,7 +41,10 @@ int raise(int sig);
 
 #include <__assert> // all public C++ headers provide the assertion handler
 #include <__config>
-#include <signal.h>
+
+#if __has_include(<signal.h>)
+# include <signal.h>
+#endif
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header

diff  --git a/libcxx/include/module.modulemap b/libcxx/include/module.modulemap.in
similarity index 97%
rename from libcxx/include/module.modulemap
rename to libcxx/include/module.modulemap.in
index 01946120462d..b89983bcfd9d 100644
--- a/libcxx/include/module.modulemap
+++ b/libcxx/include/module.modulemap.in
@@ -38,6 +38,7 @@ module std [system] {
     // <iso646.h> provided by compiler.
     // <limits.h> provided by compiler or C library.
     module locale_h {
+      @requires_LIBCXX_ENABLE_LOCALIZATION@
       header "locale.h"
       export *
     }
@@ -50,6 +51,7 @@ module std [system] {
       export *
     }
     module stdatomic_h {
+      @requires_LIBCXX_ENABLE_THREADS@
       requires cplusplus23
       header "stdatomic.h"
       export *
@@ -90,11 +92,13 @@ module std [system] {
     }
     // <time.h> provided by C library.
     module wchar_h {
+      @requires_LIBCXX_ENABLE_WIDE_CHARACTERS@
       // <wchar.h>'s __need_* macros require textual inclusion.
       textual header "wchar.h"
       export *
     }
     module wctype_h {
+      @requires_LIBCXX_ENABLE_WIDE_CHARACTERS@
       header "wctype.h"
       export *
     }
@@ -155,6 +159,7 @@ module std [system] {
       export *
     }
     module clocale {
+      @requires_LIBCXX_ENABLE_LOCALIZATION@
       header "clocale"
       export *
     }
@@ -215,11 +220,13 @@ module std [system] {
       export *
     }
     module cwchar {
+      @requires_LIBCXX_ENABLE_WIDE_CHARACTERS@
       header "cwchar"
       export depr.stdio_h
       export *
     }
     module cwctype {
+      @requires_LIBCXX_ENABLE_WIDE_CHARACTERS@
       header "cwctype"
       export *
     }
@@ -382,6 +389,7 @@ module std [system] {
     export *
   }
   module barrier {
+    @requires_LIBCXX_ENABLE_THREADS@
     header "barrier"
     export *
   }
@@ -425,7 +433,11 @@ module std [system] {
       module duration              { private header "__chrono/duration.h" }
       module file_clock            { private header "__chrono/file_clock.h" }
       module hh_mm_ss              { private header "__chrono/hh_mm_ss.h" }
-      module high_resolution_clock { private header "__chrono/high_resolution_clock.h" }
+      module high_resolution_clock {
+        private header "__chrono/high_resolution_clock.h"
+        export steady_clock
+        export system_clock
+      }
       module literals              { private header "__chrono/literals.h" }
       module month                 { private header "__chrono/month.h" }
       module month_weekday         { private header "__chrono/month_weekday.h" }
@@ -441,6 +453,7 @@ module std [system] {
     }
   }
   module codecvt {
+    @requires_LIBCXX_ENABLE_LOCALIZATION@
     header "codecvt"
     export *
   }
@@ -528,6 +541,7 @@ module std [system] {
     export *
   }
   module filesystem {
+    @requires_LIBCXX_ENABLE_FILESYSTEM@
     header "filesystem"
     export *
 
@@ -588,6 +602,7 @@ module std [system] {
     export *
   }
   module fstream {
+    @requires_LIBCXX_ENABLE_LOCALIZATION@
     header "fstream"
     export *
   }
@@ -626,6 +641,7 @@ module std [system] {
     }
   }
   module future {
+    @requires_LIBCXX_ENABLE_THREADS@
     header "future"
     export *
   }
@@ -634,10 +650,12 @@ module std [system] {
     export *
   }
   module iomanip {
+    @requires_LIBCXX_ENABLE_LOCALIZATION@
     header "iomanip"
     export *
   }
   module ios {
+    @requires_LIBCXX_ENABLE_LOCALIZATION@
     header "ios"
     export iosfwd
     export *
@@ -651,6 +669,7 @@ module std [system] {
     export *
   }
   module iostream {
+    @requires_LIBCXX_ENABLE_LOCALIZATION@
     header "iostream"
     export ios
     export streambuf
@@ -659,6 +678,7 @@ module std [system] {
     export *
   }
   module istream {
+    @requires_LIBCXX_ENABLE_LOCALIZATION@
     header "istream"
     // FIXME: should re-export ios, streambuf?
     export *
@@ -711,6 +731,7 @@ module std [system] {
     }
   }
   module latch {
+    @requires_LIBCXX_ENABLE_THREADS@
     header "latch"
     export *
   }
@@ -724,6 +745,7 @@ module std [system] {
     export *
   }
   module locale {
+    @requires_LIBCXX_ENABLE_LOCALIZATION@
     header "locale"
     export *
   }
@@ -761,6 +783,7 @@ module std [system] {
     }
   }
   module mutex {
+    @requires_LIBCXX_ENABLE_THREADS@
     header "mutex"
     export *
   }
@@ -797,6 +820,7 @@ module std [system] {
     export *
   }
   module ostream {
+    @requires_LIBCXX_ENABLE_LOCALIZATION@
     header "ostream"
     // FIXME: should re-export ios, streambuf?
     export *
@@ -909,6 +933,7 @@ module std [system] {
     export *
   }
   module regex {
+    @requires_LIBCXX_ENABLE_LOCALIZATION@
     header "regex"
     export initializer_list
     export *
@@ -918,6 +943,7 @@ module std [system] {
     export *
   }
   module semaphore {
+    @requires_LIBCXX_ENABLE_THREADS@
     header "semaphore"
     export *
   }
@@ -927,6 +953,7 @@ module std [system] {
     export *
   }
   module shared_mutex {
+    @requires_LIBCXX_ENABLE_THREADS@
     header "shared_mutex"
     export version
   }
@@ -937,6 +964,7 @@ module std [system] {
     module span_fwd { private header "__fwd/span.h" }
   }
   module sstream {
+    @requires_LIBCXX_ENABLE_LOCALIZATION@
     header "sstream"
     // FIXME: should re-export istream, ostream, ios, streambuf, string?
     export *
@@ -951,6 +979,7 @@ module std [system] {
     export *
   }
   module streambuf {
+    @requires_LIBCXX_ENABLE_LOCALIZATION@
     header "streambuf"
     export *
   }
@@ -969,6 +998,7 @@ module std [system] {
     module string_view_fwd { private header "__fwd/string_view.h" }
   }
   module strstream {
+    @requires_LIBCXX_ENABLE_LOCALIZATION@
     header "strstream"
     export *
   }
@@ -977,6 +1007,7 @@ module std [system] {
     export *
   }
   module thread {
+    @requires_LIBCXX_ENABLE_THREADS@
     header "thread"
     export *
 
@@ -1118,7 +1149,6 @@ module std [system] {
     export *
   }
 
-  // __config not modularised due to a bug in Clang
   // FIXME: These should be private.
   module __assert            {         header "__assert"            export * }
   module __availability      { private header "__availability"      export * }
@@ -1127,12 +1157,18 @@ module std [system] {
   module __debug             {         header "__debug"             export * }
   module __errc              { private header "__errc"              export * }
   module __hash_table        {         header "__hash_table"        export * }
-  module __locale            { private header "__locale"            export * }
+  module __locale            {
+    @requires_LIBCXX_ENABLE_LOCALIZATION@
+    private header "__locale" export *
+  }
   module __mbstate_t         { private header "__mbstate_t.h"       export * }
   module __mutex_base        { private header "__mutex_base"        export * }
   module __node_handle       { private header "__node_handle"       export * }
   module __split_buffer      { private header "__split_buffer"      export * }
-  module __std_stream        { private header "__std_stream"        export * }
+  module __std_stream        {
+    @requires_LIBCXX_ENABLE_LOCALIZATION@
+    private header "__std_stream" export *
+  }
   module __string            { private header "__string"            export * }
   module __threading_support {         header "__threading_support" export * }
   module __tree              {         header "__tree"              export * }
@@ -1184,6 +1220,7 @@ module std [system] {
       export *
     }
     module regex {
+      @requires_LIBCXX_ENABLE_LOCALIZATION@
       header "experimental/regex"
       export *
     }

diff  --git a/libcxx/test/libcxx/lint/lint_headers.sh.py b/libcxx/test/libcxx/lint/lint_headers.sh.py
index 9075044dc7df..9113b1aecf8a 100644
--- a/libcxx/test/libcxx/lint/lint_headers.sh.py
+++ b/libcxx/test/libcxx/lint/lint_headers.sh.py
@@ -11,7 +11,7 @@
 def exclude_from_consideration(path):
     return (
         path.endswith('.txt') or
-        path.endswith('.modulemap') or
+        path.endswith('.modulemap.in') or
         os.path.basename(path) == '__config' or
         os.path.basename(path) == '__config_site.in' or
         not os.path.isfile(path)

diff  --git a/libcxx/test/libcxx/lint/lint_modulemap.sh.py b/libcxx/test/libcxx/lint/lint_modulemap.sh.py
index e167813cfc09..bbb5885273b7 100755
--- a/libcxx/test/libcxx/lint/lint_modulemap.sh.py
+++ b/libcxx/test/libcxx/lint/lint_modulemap.sh.py
@@ -1,6 +1,6 @@
 # RUN: %{python} %s
 
-# Verify that each list of private submodules in libcxx/include/module.modulemap
+# Verify that each list of private submodules in libcxx/include/module.modulemap.in
 # is maintained in alphabetical order.
 
 import os
@@ -10,7 +10,7 @@
 if __name__ == '__main__':
     libcxx_test_libcxx_lint = os.path.dirname(os.path.abspath(__file__))
     libcxx = os.path.abspath(os.path.join(libcxx_test_libcxx_lint, '../../..'))
-    modulemap_name = os.path.join(libcxx, 'include/module.modulemap')
+    modulemap_name = os.path.join(libcxx, 'include/module.modulemap.in')
     assert os.path.isfile(modulemap_name)
 
     okay = True
@@ -31,12 +31,12 @@
                     pass
                 else:
                     okay = False
-                    print("LINE DOESN'T MATCH REGEX in libcxx/include/module.modulemap!")
+                    print("LINE DOESN'T MATCH REGEX in libcxx/include/module.modulemap.in!")
                     print(line)
                 # Check that these lines are alphabetized.
                 if (prevline is not None) and (line < prevline):
                     okay = False
-                    print('LINES OUT OF ORDER in libcxx/include/module.modulemap!')
+                    print('LINES OUT OF ORDER in libcxx/include/module.modulemap.in!')
                     print(prevline)
                     print(line)
                 prevline = line

diff  --git a/libcxx/test/libcxx/modules_include.sh.cpp b/libcxx/test/libcxx/modules_include.sh.cpp
index 4dd8a8f73e87..4ab12592ff1d 100644
--- a/libcxx/test/libcxx/modules_include.sh.cpp
+++ b/libcxx/test/libcxx/modules_include.sh.cpp
@@ -19,10 +19,6 @@
 // The Windows headers don't appear to be compatible with modules
 // UNSUPPORTED: windows
 
-// TODO: Some headers produce errors when we include them and the library has been
-//       configured without support for them, which breaks the modules build.
-// UNSUPPORTED: no-localization, no-filesystem, no-threads, no-wide-characters
-
 // Prevent <ext/hash_map> from generating deprecated warnings for this test.
 #if defined(__DEPRECATED)
 #    undef __DEPRECATED

diff  --git a/libcxx/utils/generate_header_tests.py b/libcxx/utils/generate_header_tests.py
index fa9ab3cd7dca..59b90489f16b 100755
--- a/libcxx/utils/generate_header_tests.py
+++ b/libcxx/utils/generate_header_tests.py
@@ -113,7 +113,7 @@ def produce(test_file, variables):
 
 def is_header(file):
     """Returns whether the given file is a header (i.e. not a directory or the modulemap file)."""
-    return not file.is_dir() and not file.name == 'module.modulemap'
+    return not file.is_dir() and not file.name == 'module.modulemap.in'
 
 def main():
     monorepo_root = pathlib.Path(os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))))

diff  --git a/libcxx/utils/libcxx/test/dsl.py b/libcxx/utils/libcxx/test/dsl.py
index f43937a9664d..e77ea8fa6004 100644
--- a/libcxx/utils/libcxx/test/dsl.py
+++ b/libcxx/utils/libcxx/test/dsl.py
@@ -276,9 +276,11 @@ def compilerMacros(config, flags=''):
   """
   with _makeConfigTest(config) as test:
     with open(test.getSourcePath(), 'w') as sourceFile:
-      # Make sure files like <__config> are included, since they can define
-      # additional macros.
-      sourceFile.write("#include <stddef.h>")
+      sourceFile.write("""
+      #if __has_include(<__config_site>)
+      #  include <__config_site>
+      #endif
+      """)
     unparsedOutput, err, exitCode, _ = _executeScriptInternal(test, [
       "%{{cxx}} %s -dM -E %{{flags}} %{{compile_flags}} {}".format(flags)
     ])

diff  --git a/libcxx/utils/libcxx/test/features.py b/libcxx/utils/libcxx/test/features.py
index d7e4c849a0fc..d805ae6e7ac3 100644
--- a/libcxx/utils/libcxx/test/features.py
+++ b/libcxx/utils/libcxx/test/features.py
@@ -82,7 +82,7 @@ def _hasSuitableClangTidy(cfg):
   # Check for a Windows UCRT bug (fixed in UCRT/Windows 10.0.20348.0):
   # https://developercommunity.visualstudio.com/t/utf-8-locales-break-ctype-functions-for-wchar-type/1653678
   Feature(name='win32-broken-utf8-wchar-ctype',
-          when=lambda cfg: '_WIN32' in compilerMacros(cfg) and not programSucceeds(cfg, """
+          when=lambda cfg: not '_LIBCPP_HAS_NO_LOCALIZATION' in compilerMacros(cfg) and '_WIN32' in compilerMacros(cfg) and not programSucceeds(cfg, """
             #include <locale.h>
             #include <wctype.h>
             int main(int, char**) {
@@ -94,7 +94,7 @@ def _hasSuitableClangTidy(cfg):
   # Check for a Windows UCRT bug (fixed in UCRT/Windows 10.0.19041.0).
   # https://developercommunity.visualstudio.com/t/printf-formatting-with-g-outputs-too/1660837
   Feature(name='win32-broken-printf-g-precision',
-          when=lambda cfg: '_WIN32' in compilerMacros(cfg) and not programSucceeds(cfg, """
+          when=lambda cfg: not '_LIBCPP_HAS_NO_LOCALIZATION' in compilerMacros(cfg) and '_WIN32' in compilerMacros(cfg) and not programSucceeds(cfg, """
             #include <stdio.h>
             #include <string.h>
             int main(int, char**) {