[Modules] no_undeclared_includes modules (Apple Darwin) don't work the clang modules #68241
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-modules @llvm/pr-subscribers-clang ChangesAll of the _Builtin_stdarg and _Builtin_stddef submodules need to be allowed from [no_undeclared_includes] modules. Split the builtin headers tests out from the compiler_builtins test so that the testing modules can be modified without affecting the other many tests that use Inputs/System/usr/include. Full diff: https://github.com/llvm/llvm-project/pull/68241.diff 17 Files Affected:
diff --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp
index 0fd9c1dca39984d..ad353e6089a8920 100644
--- a/clang/lib/Basic/Module.cpp
+++ b/clang/lib/Basic/Module.cpp
@@ -301,7 +301,7 @@ bool Module::directlyUses(const Module *Requested) {
// Anyone is allowed to use our builtin stdarg.h and stddef.h and their
// accompanying modules.
- if (!Requested->Parent && (Requested->Name == "_Builtin_stdarg" || Requested->Name == "_Builtin_stddef"))
+ if (Requested->getTopLevelModuleName() == "_Builtin_stdarg" || Requested->getTopLevelModuleName() == "_Builtin_stddef")
return true;
if (NoUndeclaredIncludes)
diff --git a/clang/test/Modules/Inputs/System/usr/include/inttypes.h b/clang/test/Modules/Inputs/System/usr/include/inttypes.h
deleted file mode 100644
index e69de29bb2d1d64..000000000000000
diff --git a/clang/test/Modules/Inputs/System/usr/include/math.h b/clang/test/Modules/Inputs/System/usr/include/math.h
deleted file mode 100644
index e69de29bb2d1d64..000000000000000
diff --git a/clang/test/Modules/Inputs/System/usr/include/module.map b/clang/test/Modules/Inputs/System/usr/include/module.map
index 5a15c70a394d2e2..1d88ca380f49a50 100644
--- a/clang/test/Modules/Inputs/System/usr/include/module.map
+++ b/clang/test/Modules/Inputs/System/usr/include/module.map
@@ -1,24 +1,9 @@
module cstd [system] {
- // Only in system headers directory
- module complex {
- header "complex.h"
- }
-
// Only in compiler support directory
module float_constants {
header "float.h"
}
- // In both directories (compiler support version wins, forwards)
- module inttypes {
- header "inttypes.h"
- }
-
- // Only in system headers directory
- module math {
- header "math.h"
- }
-
// Only in system headers directory
module stdio {
header "stdio.h"
diff --git a/clang/test/Modules/Inputs/System/usr/include/stdint.h b/clang/test/Modules/Inputs/System/usr/include/stdint.h
index 209d54cd411ad55..e8e50f90290ced7 100644
--- a/clang/test/Modules/Inputs/System/usr/include/stdint.h
+++ b/clang/test/Modules/Inputs/System/usr/include/stdint.h
@@ -1,36 +1 @@
-#ifndef STDINT_H
-#define STDINT_H
-
typedef int my_awesome_nonstandard_integer_type;
-
-// types needed by stdatomic.h
-
-typedef char int_least8_t;
-typedef short int_least16_t;
-typedef int int_least32_t;
-typedef long long int int_least64_t;
-typedef unsigned char uint_least8_t;
-typedef unsigned short uint_least16_t;
-typedef unsigned int uint_least32_t;
-typedef unsigned long long uint_least64_t;
-
-typedef char int_fast8_t;
-typedef short int_fast16_t;
-typedef int int_fast32_t;
-typedef long long int int_fast64_t;
-typedef unsigned char uint_fast8_t;
-typedef unsigned short uint_fast16_t;
-typedef unsigned int uint_fast32_t;
-typedef unsigned long long uint_fast64_t;
-
-typedef int intptr_t;
-typedef unsigned int uintptr_t;
-typedef int intmax_t;
-typedef unsigned int uintmax_t;
-
-// additional types for unwind.h
-
-typedef unsigned int uint32_t;
-typedef unsigned long long uint64_t;
-
-#endif /* STDINT_H */
diff --git a/clang/test/Modules/Inputs/builtin-headers/builtin-modules.modulemap b/clang/test/Modules/Inputs/builtin-headers/builtin-modules.modulemap
new file mode 100644
index 000000000000000..0fbb454dbb0223d
--- /dev/null
+++ b/clang/test/Modules/Inputs/builtin-headers/builtin-modules.modulemap
@@ -0,0 +1,29 @@
+module c_complex [system] {
+ header "complex.h"
+ export *
+}
+
+module c_float [system] {
+ header "float.h"
+ export *
+}
+
+module c_inttypes [system] {
+ header "inttypes.h"
+ export *
+}
+
+module c_limits [system] {
+ header "limits.h"
+ export *
+}
+
+module c_math [system] {
+ header "math.h"
+ export *
+}
+
+module c_stdint [system] {
+ header "stdint.h"
+ export *
+}
diff --git a/clang/test/Modules/Inputs/builtin-headers/c++/module.modulemap b/clang/test/Modules/Inputs/builtin-headers/c++/module.modulemap
new file mode 100644
index 000000000000000..c969f067a234f0c
--- /dev/null
+++ b/clang/test/Modules/Inputs/builtin-headers/c++/module.modulemap
@@ -0,0 +1,4 @@
+module cpp_stdint [system] {
+ header "stdint.h"
+ export *
+}
diff --git a/clang/test/Modules/Inputs/builtin-headers/c++/stdint.h b/clang/test/Modules/Inputs/builtin-headers/c++/stdint.h
new file mode 100644
index 000000000000000..22cebacfdb64e4c
--- /dev/null
+++ b/clang/test/Modules/Inputs/builtin-headers/c++/stdint.h
@@ -0,0 +1,6 @@
+#ifndef CPP_STDINT_H
+#define CPP_STDINT_H
+
+#include_next <stdint.h>
+
+#endif
diff --git a/clang/test/Modules/Inputs/builtin-headers/complex.h b/clang/test/Modules/Inputs/builtin-headers/complex.h
new file mode 100644
index 000000000000000..b6ef3c5a35b450e
--- /dev/null
+++ b/clang/test/Modules/Inputs/builtin-headers/complex.h
@@ -0,0 +1 @@
+// Required by tgmath.h
diff --git a/clang/test/Modules/Inputs/System/usr/include/complex.h b/clang/test/Modules/Inputs/builtin-headers/float.h
similarity index 100%
rename from clang/test/Modules/Inputs/System/usr/include/complex.h
rename to clang/test/Modules/Inputs/builtin-headers/float.h
diff --git a/clang/test/Modules/Inputs/builtin-headers/inttypes.h b/clang/test/Modules/Inputs/builtin-headers/inttypes.h
new file mode 100644
index 000000000000000..ddb6bb83b57238c
--- /dev/null
+++ b/clang/test/Modules/Inputs/builtin-headers/inttypes.h
@@ -0,0 +1,12 @@
+#ifndef INTTYPES_H
+#define INTTYPES_H
+
+// This creates an include cycle when inttypes.h and stdint.h
+// are both part of the cstd module. This include will resolve
+// to the C++ stdint.h, which will #include_next eventually to
+// the stdint.h in this directory, and thus create the cycle
+// cstd (inttypes.h) -> cpp_stdint (stdint.h) -> cstd (stdint.h).
+// This cycle is worked around by cstd using [no_undeclared_includes].
+#include <stdint.h>
+
+#endif
diff --git a/clang/test/Modules/Inputs/builtin-headers/limits.h b/clang/test/Modules/Inputs/builtin-headers/limits.h
new file mode 100644
index 000000000000000..8b137891791fe96
--- /dev/null
+++ b/clang/test/Modules/Inputs/builtin-headers/limits.h
@@ -0,0 +1 @@
+
diff --git a/clang/test/Modules/Inputs/builtin-headers/math.h b/clang/test/Modules/Inputs/builtin-headers/math.h
new file mode 100644
index 000000000000000..b6ef3c5a35b450e
--- /dev/null
+++ b/clang/test/Modules/Inputs/builtin-headers/math.h
@@ -0,0 +1 @@
+// Required by tgmath.h
diff --git a/clang/test/Modules/Inputs/builtin-headers/stdint.h b/clang/test/Modules/Inputs/builtin-headers/stdint.h
new file mode 100644
index 000000000000000..8f233fd7f45d6dc
--- /dev/null
+++ b/clang/test/Modules/Inputs/builtin-headers/stdint.h
@@ -0,0 +1,34 @@
+#ifndef STDINT_H
+#define STDINT_H
+
+// types needed by stdatomic.h
+
+typedef char int_least8_t;
+typedef short int_least16_t;
+typedef int int_least32_t;
+typedef long long int int_least64_t;
+typedef unsigned char uint_least8_t;
+typedef unsigned short uint_least16_t;
+typedef unsigned int uint_least32_t;
+typedef unsigned long long uint_least64_t;
+
+typedef char int_fast8_t;
+typedef short int_fast16_t;
+typedef int int_fast32_t;
+typedef long long int int_fast64_t;
+typedef unsigned char uint_fast8_t;
+typedef unsigned short uint_fast16_t;
+typedef unsigned int uint_fast32_t;
+typedef unsigned long long uint_fast64_t;
+
+typedef int intptr_t;
+typedef unsigned int uintptr_t;
+typedef int intmax_t;
+typedef unsigned int uintmax_t;
+
+// additional types for unwind.h
+
+typedef unsigned int uint32_t;
+typedef unsigned long long uint64_t;
+
+#endif
diff --git a/clang/test/Modules/Inputs/builtin-headers/system-modules.modulemap b/clang/test/Modules/Inputs/builtin-headers/system-modules.modulemap
new file mode 100644
index 000000000000000..0161ff80fe6189a
--- /dev/null
+++ b/clang/test/Modules/Inputs/builtin-headers/system-modules.modulemap
@@ -0,0 +1,71 @@
+module cstd [system] [no_undeclared_includes] {
+ module complex {
+ header "complex.h"
+ export *
+ }
+
+ module float {
+ header "float.h"
+ export *
+ }
+
+ module inttypes {
+ header "inttypes.h"
+ export *
+ }
+
+ module iso646 {
+ header "iso646.h"
+ export *
+ }
+
+ module limits {
+ header "limits.h"
+ export *
+ }
+
+ module math {
+ header "math.h"
+ export *
+ }
+
+ module stdalign {
+ header "stdalign.h"
+ export *
+ }
+
+ module stdarg {
+ header "stdarg.h"
+ export *
+ }
+
+ module stdatomic {
+ header "stdatomic.h"
+ export *
+ }
+
+ module stdbool {
+ header "stdbool.h"
+ export *
+ }
+
+ module stddef {
+ header "stddef.h"
+ export *
+ }
+
+ module stdint {
+ header "stdint.h"
+ export *
+ }
+
+ module tgmath {
+ header "tgmath.h"
+ export *
+ }
+
+ module unwind {
+ header "unwind.h"
+ export *
+ }
+}
diff --git a/clang/test/Modules/builtin-headers.mm b/clang/test/Modules/builtin-headers.mm
new file mode 100644
index 000000000000000..4c9ddec4e99c289
--- /dev/null
+++ b/clang/test/Modules/builtin-headers.mm
@@ -0,0 +1,41 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -cxx-isystem %S/Inputs/builtin-headers/c++ -internal-isystem %S/Inputs/builtin-headers -fsyntax-only -fmodules -fmodules-cache-path=%t -fmodule-map-file=%S/Inputs/builtin-headers/c++/module.modulemap -fmodule-map-file=%resource_dir/module.modulemap -fmodule-map-file=%S/Inputs/builtin-headers/system-modules.modulemap -fbuiltin-headers-in-system-modules -DSYSTEM_MODULES %s -verify
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -cxx-isystem %S/Inputs/builtin-headers/c++ -internal-isystem %S/Inputs/builtin-headers -fsyntax-only -fmodules -fmodules-cache-path=%t -fmodule-map-file=%S/Inputs/builtin-headers/c++/module.modulemap -fmodule-map-file=%resource_dir/module.modulemap -fmodule-map-file=%S/Inputs/builtin-headers/builtin-modules.modulemap %s -verify
+
+// expected-no-diagnostics
+
+@import cpp_stdint;
+
+// The builtin modules are always available, though they're mostly
+// empty if -fbuiltin-headers-in-system-modules is used.
+@import _Builtin_float;
+@import _Builtin_inttypes;
+@import _Builtin_iso646;
+@import _Builtin_limits;
+@import _Builtin_stdalign;
+@import _Builtin_stdarg;
+@import _Builtin_stdatomic;
+@import _Builtin_stdbool;
+@import _Builtin_stddef;
+@import _Builtin_stdint;
+@import _Builtin_stdnoreturn;
+@import _Builtin_tgmath;
+@import _Builtin_unwind;
+
+#ifdef SYSTEM_MODULES
+// system-modules.modulemap uses the "mega module" style with
+// -fbuiltin-headers-in-system-modules, and its modules cover
+// the clang builtin headers.
+@import cstd;
+#else
+// builtin-modules.modulemap uses top level modules for each
+// of its headers, which allows interleaving with the builtin
+// modules and libc++ modules.
+@import c_complex;
+@import c_float;
+@import c_inttypes;
+@import c_limits;
+@import c_math;
+@import c_stdint;
+#endif
diff --git a/clang/test/Modules/compiler_builtins.m b/clang/test/Modules/compiler_builtins.m
index 8eeb65daa9e8878..a5e6315cedc8e6e 100644
--- a/clang/test/Modules/compiler_builtins.m
+++ b/clang/test/Modules/compiler_builtins.m
@@ -1,7 +1,7 @@
// RUN: rm -rf %t
-// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -internal-isystem %S/Inputs/System/usr/include -verify
-// RUN: %clang_cc1 -fsyntax-only -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -internal-isystem %S/Inputs/System/usr/include -verify
-// RUN: %clang_cc1 -fsyntax-only -fmodules -fmodule-map-file=%resource_dir/module.modulemap -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -DNO_SYSTEM_MODULES -verify
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fmodule-map-file=%resource_dir/module.modulemap -fmodules-cache-path=%t %s -I%S/Inputs/System/usr/include -verify
// expected-no-diagnostics
#ifdef __SSE__
@@ -11,19 +11,3 @@
#ifdef __AVX2__
@import _Builtin_intrinsics.intel.avx2;
#endif
-
-#ifndef NO_SYSTEM_MODULES
-@import _Builtin_float;
-@import _Builtin_inttypes;
-@import _Builtin_iso646;
-@import _Builtin_limits;
-@import _Builtin_stdalign;
-@import _Builtin_stdarg;
-@import _Builtin_stdatomic;
-@import _Builtin_stdbool;
-@import _Builtin_stddef;
-@import _Builtin_stdint;
-@import _Builtin_stdnoreturn;
-@import _Builtin_tgmath;
-@import _Builtin_unwind;
-#endif
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
…e clang modules All of the _Builtin_stdarg and _Builtin_stddef submodules need to be allowed from [no_undeclared_includes] modules. Split the builtin headers tests out from the compiler_builtins test so that the testing modules can be modified without affecting the other many tests that use Inputs/System/usr/include.
|
I could also revert most of the test changes and augment no-undeclared-includes-builtins.cpp, but I liked having a dedicated test that's not necessarily specific to Apple or glibc to test the intended system module setups with |
Bigcheese
approved these changes
Oct 4, 2023
|
I think splitting out the test here is good. |
ian-twilightcoder
pushed a commit
to ian-twilightcoder/llvm-project
that referenced
this pull request
Jan 8, 2024
…s (Apple Darwin) don't work the clang modules llvm#68241 rdar://105819340
ian-twilightcoder
pushed a commit
to ian-twilightcoder/llvm-project
that referenced
this pull request
Jan 10, 2024
…s (Apple Darwin) don't work the clang modules llvm#68241 rdar://105819340
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
All of the _Builtin_stdarg and _Builtin_stddef submodules need to be allowed from [no_undeclared_includes] modules. Split the builtin headers tests out from the compiler_builtins test so that the testing modules can be modified without affecting the other many tests that use Inputs/System/usr/include.