Skip to content

Commit

Permalink
[libc++] Fix modules builds when features are removed
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ldionne committed Jun 8, 2022
1 parent 6504b15 commit 0e9a01d
Show file tree
Hide file tree
Showing 10 changed files with 68 additions and 23 deletions.
2 changes: 1 addition & 1 deletion libcxx/docs/Contributing.rst
Expand Up @@ -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``?
Expand Down
11 changes: 9 additions & 2 deletions libcxx/include/CMakeLists.txt
Expand Up @@ -618,7 +618,6 @@ set(files
map
math.h
memory
module.modulemap
mutex
new
numbers
Expand Down Expand Up @@ -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}")
Expand Down
5 changes: 4 additions & 1 deletion libcxx/include/csignal
Expand Up @@ -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
Expand Down
Expand Up @@ -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 *
}
Expand All @@ -50,6 +51,7 @@ module std [system] {
export *
}
module stdatomic_h {
@requires_LIBCXX_ENABLE_THREADS@
requires cplusplus23
header "stdatomic.h"
export *
Expand Down Expand Up @@ -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 *
}
Expand Down Expand Up @@ -155,6 +159,7 @@ module std [system] {
export *
}
module clocale {
@requires_LIBCXX_ENABLE_LOCALIZATION@
header "clocale"
export *
}
Expand Down Expand Up @@ -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 *
}
Expand Down Expand Up @@ -382,6 +389,7 @@ module std [system] {
export *
}
module barrier {
@requires_LIBCXX_ENABLE_THREADS@
header "barrier"
export *
}
Expand Down Expand Up @@ -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" }
Expand All @@ -441,6 +453,7 @@ module std [system] {
}
}
module codecvt {
@requires_LIBCXX_ENABLE_LOCALIZATION@
header "codecvt"
export *
}
Expand Down Expand Up @@ -528,6 +541,7 @@ module std [system] {
export *
}
module filesystem {
@requires_LIBCXX_ENABLE_FILESYSTEM@
header "filesystem"
export *

Expand Down Expand Up @@ -588,6 +602,7 @@ module std [system] {
export *
}
module fstream {
@requires_LIBCXX_ENABLE_LOCALIZATION@
header "fstream"
export *
}
Expand Down Expand Up @@ -626,6 +641,7 @@ module std [system] {
}
}
module future {
@requires_LIBCXX_ENABLE_THREADS@
header "future"
export *
}
Expand All @@ -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 *
Expand All @@ -651,6 +669,7 @@ module std [system] {
export *
}
module iostream {
@requires_LIBCXX_ENABLE_LOCALIZATION@
header "iostream"
export ios
export streambuf
Expand All @@ -659,6 +678,7 @@ module std [system] {
export *
}
module istream {
@requires_LIBCXX_ENABLE_LOCALIZATION@
header "istream"
// FIXME: should re-export ios, streambuf?
export *
Expand Down Expand Up @@ -711,6 +731,7 @@ module std [system] {
}
}
module latch {
@requires_LIBCXX_ENABLE_THREADS@
header "latch"
export *
}
Expand All @@ -724,6 +745,7 @@ module std [system] {
export *
}
module locale {
@requires_LIBCXX_ENABLE_LOCALIZATION@
header "locale"
export *
}
Expand Down Expand Up @@ -761,6 +783,7 @@ module std [system] {
}
}
module mutex {
@requires_LIBCXX_ENABLE_THREADS@
header "mutex"
export *
}
Expand Down Expand Up @@ -797,6 +820,7 @@ module std [system] {
export *
}
module ostream {
@requires_LIBCXX_ENABLE_LOCALIZATION@
header "ostream"
// FIXME: should re-export ios, streambuf?
export *
Expand Down Expand Up @@ -909,6 +933,7 @@ module std [system] {
export *
}
module regex {
@requires_LIBCXX_ENABLE_LOCALIZATION@
header "regex"
export initializer_list
export *
Expand All @@ -918,6 +943,7 @@ module std [system] {
export *
}
module semaphore {
@requires_LIBCXX_ENABLE_THREADS@
header "semaphore"
export *
}
Expand All @@ -927,6 +953,7 @@ module std [system] {
export *
}
module shared_mutex {
@requires_LIBCXX_ENABLE_THREADS@
header "shared_mutex"
export version
}
Expand All @@ -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 *
Expand All @@ -951,6 +979,7 @@ module std [system] {
export *
}
module streambuf {
@requires_LIBCXX_ENABLE_LOCALIZATION@
header "streambuf"
export *
}
Expand All @@ -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 *
}
Expand All @@ -977,6 +1007,7 @@ module std [system] {
export *
}
module thread {
@requires_LIBCXX_ENABLE_THREADS@
header "thread"
export *

Expand Down Expand Up @@ -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 * }
Expand All @@ -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 * }
Expand Down Expand Up @@ -1184,6 +1220,7 @@ module std [system] {
export *
}
module regex {
@requires_LIBCXX_ENABLE_LOCALIZATION@
header "experimental/regex"
export *
}
Expand Down
2 changes: 1 addition & 1 deletion libcxx/test/libcxx/lint/lint_headers.sh.py
Expand Up @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions 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
Expand All @@ -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
Expand All @@ -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
Expand Down
4 changes: 0 additions & 4 deletions libcxx/test/libcxx/modules_include.sh.cpp
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion libcxx/utils/generate_header_tests.py
Expand Up @@ -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__)))))
Expand Down
8 changes: 5 additions & 3 deletions libcxx/utils/libcxx/test/dsl.py
Expand Up @@ -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)
])
Expand Down

0 comments on commit 0e9a01d

Please sign in to comment.