[clang] 8d74de9 - [clang] Always allow including builtin headers in [no_undeclared_headers] modules.
Martin Boehme via cfe-commits
cfe-commits at lists.llvm.org
Wed Jun 3 23:49:49 PDT 2020
Author: Martin Boehme
Date: 2020-06-04T08:33:39+02:00
New Revision: 8d74de9de6d6cca552d7de7d0bfd36b6dd7d58dc
URL: https://github.com/llvm/llvm-project/commit/8d74de9de6d6cca552d7de7d0bfd36b6dd7d58dc
DIFF: https://github.com/llvm/llvm-project/commit/8d74de9de6d6cca552d7de7d0bfd36b6dd7d58dc.diff
LOG: [clang] Always allow including builtin headers in [no_undeclared_headers] modules.
Previously, this would fail if the builtin headers had been "claimed" by
a different module that wraps these builtin headers. libc++ does this,
for example.
This change adds a test demonstrating this situation; the test fails
without the fix.
Added:
clang/test/Modules/Inputs/no-undeclared-includes-builtins/glibc/module.modulemap
clang/test/Modules/Inputs/no-undeclared-includes-builtins/glibc/stdio.h
clang/test/Modules/Inputs/no-undeclared-includes-builtins/libcxx/module.modulemap
clang/test/Modules/Inputs/no-undeclared-includes-builtins/libcxx/stddef.h
clang/test/Modules/Inputs/no-undeclared-includes-builtins/libcxx/stdio.h
clang/test/Modules/no-undeclared-includes-builtins.cpp
Modified:
clang/include/clang/Lex/ModuleMap.h
clang/lib/Lex/HeaderSearch.cpp
clang/lib/Lex/ModuleMap.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Lex/ModuleMap.h b/clang/include/clang/Lex/ModuleMap.h
index 805cc1f5017b..5b164039080b 100644
--- a/clang/include/clang/Lex/ModuleMap.h
+++ b/clang/include/clang/Lex/ModuleMap.h
@@ -413,6 +413,7 @@ class ModuleMap {
/// Is this a compiler builtin header?
static bool isBuiltinHeader(StringRef FileName);
+ bool isBuiltinHeader(const FileEntry *File);
/// Add a module map callback.
void addModuleMapCallbacks(std::unique_ptr<ModuleMapCallbacks> Callback) {
diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp
index 3ac1df1740c8..1df28cc07209 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -1276,14 +1276,12 @@ bool HeaderSearch::ShouldEnterIncludeFile(Preprocessor &PP,
//
// It's common that libc++ and system modules will both define such
// submodules. Make sure cached results for a builtin header won't
- // prevent other builtin modules to potentially enter the builtin header.
- // Note that builtins are header guarded and the decision to actually
- // enter them is postponed to the controlling macros logic below.
+ // prevent other builtin modules from potentially entering the builtin
+ // header. Note that builtins are header guarded and the decision to
+ // actually enter them is postponed to the controlling macros logic below.
bool TryEnterHdr = false;
if (FileInfo.isCompilingModuleHeader && FileInfo.isModuleHeader)
- TryEnterHdr = File->getDir() == ModMap.getBuiltinDir() &&
- ModuleMap::isBuiltinHeader(
- llvm::sys::path::filename(File->getName()));
+ TryEnterHdr = ModMap.isBuiltinHeader(File);
// Textual headers can be #imported from
diff erent modules. Since ObjC
// headers find in the wild might rely only on #import and do not contain
@@ -1416,20 +1414,31 @@ static bool suggestModule(HeaderSearch &HS, const FileEntry *File,
ModuleMap::KnownHeader *SuggestedModule) {
ModuleMap::KnownHeader Module =
HS.findModuleForHeader(File, /*AllowTextual*/true);
- if (SuggestedModule)
- *SuggestedModule = (Module.getRole() & ModuleMap::TextualHeader)
- ? ModuleMap::KnownHeader()
- : Module;
// If this module specifies [no_undeclared_includes], we cannot find any
// file that's in a non-dependency module.
if (RequestingModule && Module && RequestingModule->NoUndeclaredIncludes) {
- HS.getModuleMap().resolveUses(RequestingModule, /*Complain*/false);
+ HS.getModuleMap().resolveUses(RequestingModule, /*Complain*/ false);
if (!RequestingModule->directlyUses(Module.getModule())) {
+ // Builtin headers are a special case. Multiple modules can use the same
+ // builtin as a modular header (see also comment in
+ // ShouldEnterIncludeFile()), so the builtin header may have been
+ // "claimed" by an unrelated module. This shouldn't prevent us from
+ // including the builtin header textually in this module.
+ if (HS.getModuleMap().isBuiltinHeader(File)) {
+ if (SuggestedModule)
+ *SuggestedModule = ModuleMap::KnownHeader();
+ return true;
+ }
return false;
}
}
+ if (SuggestedModule)
+ *SuggestedModule = (Module.getRole() & ModuleMap::TextualHeader)
+ ? ModuleMap::KnownHeader()
+ : Module;
+
return true;
}
diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp
index 85bf93ac9949..bcdc5b8062a0 100644
--- a/clang/lib/Lex/ModuleMap.cpp
+++ b/clang/lib/Lex/ModuleMap.cpp
@@ -387,13 +387,17 @@ bool ModuleMap::isBuiltinHeader(StringRef FileName) {
.Default(false);
}
+bool ModuleMap::isBuiltinHeader(const FileEntry *File) {
+ return File->getDir() == BuiltinIncludeDir &&
+ ModuleMap::isBuiltinHeader(llvm::sys::path::filename(File->getName()));
+}
+
ModuleMap::HeadersMap::iterator
ModuleMap::findKnownHeader(const FileEntry *File) {
resolveHeaderDirectives(File);
HeadersMap::iterator Known = Headers.find(File);
if (HeaderInfo.getHeaderSearchOpts().ImplicitModuleMaps &&
- Known == Headers.end() && File->getDir() == BuiltinIncludeDir &&
- ModuleMap::isBuiltinHeader(llvm::sys::path::filename(File->getName()))) {
+ Known == Headers.end() && ModuleMap::isBuiltinHeader(File)) {
HeaderInfo.loadTopLevelSystemModules();
return Headers.find(File);
}
diff --git a/clang/test/Modules/Inputs/no-undeclared-includes-builtins/glibc/module.modulemap b/clang/test/Modules/Inputs/no-undeclared-includes-builtins/glibc/module.modulemap
new file mode 100644
index 000000000000..0816a7a9bc6f
--- /dev/null
+++ b/clang/test/Modules/Inputs/no-undeclared-includes-builtins/glibc/module.modulemap
@@ -0,0 +1,5 @@
+module glibc [system] [no_undeclared_includes] {
+ // glibc relies on the builtin stddef.h, so it has no stddef.h of its own.
+ header "stdio.h"
+ export *
+}
diff --git a/clang/test/Modules/Inputs/no-undeclared-includes-builtins/glibc/stdio.h b/clang/test/Modules/Inputs/no-undeclared-includes-builtins/glibc/stdio.h
new file mode 100644
index 000000000000..21aaf7436eb9
--- /dev/null
+++ b/clang/test/Modules/Inputs/no-undeclared-includes-builtins/glibc/stdio.h
@@ -0,0 +1 @@
+#include <stddef.h>
diff --git a/clang/test/Modules/Inputs/no-undeclared-includes-builtins/libcxx/module.modulemap b/clang/test/Modules/Inputs/no-undeclared-includes-builtins/libcxx/module.modulemap
new file mode 100644
index 000000000000..74a114bbd88c
--- /dev/null
+++ b/clang/test/Modules/Inputs/no-undeclared-includes-builtins/libcxx/module.modulemap
@@ -0,0 +1,5 @@
+module libcxx [system] {
+ header "stddef.h"
+ header "stdio.h"
+ export *
+}
diff --git a/clang/test/Modules/Inputs/no-undeclared-includes-builtins/libcxx/stddef.h b/clang/test/Modules/Inputs/no-undeclared-includes-builtins/libcxx/stddef.h
new file mode 100644
index 000000000000..921103499ada
--- /dev/null
+++ b/clang/test/Modules/Inputs/no-undeclared-includes-builtins/libcxx/stddef.h
@@ -0,0 +1 @@
+#include_next <stddef.h>
diff --git a/clang/test/Modules/Inputs/no-undeclared-includes-builtins/libcxx/stdio.h b/clang/test/Modules/Inputs/no-undeclared-includes-builtins/libcxx/stdio.h
new file mode 100644
index 000000000000..7ef168678c29
--- /dev/null
+++ b/clang/test/Modules/Inputs/no-undeclared-includes-builtins/libcxx/stdio.h
@@ -0,0 +1 @@
+#include_next <stdio.h>
diff --git a/clang/test/Modules/no-undeclared-includes-builtins.cpp b/clang/test/Modules/no-undeclared-includes-builtins.cpp
new file mode 100644
index 000000000000..c9bffc556199
--- /dev/null
+++ b/clang/test/Modules/no-undeclared-includes-builtins.cpp
@@ -0,0 +1,14 @@
+// Test that a [no_undeclared_headers] module can include builtin headers, even
+// if these have been "claimed" by a
diff erent module that wraps these builtin
+// headers. libc++ does this, for example.
+//
+// The test inputs used here replicates the relationship between libc++ and
+// glibc. When modularizing glibc, [no_undeclared_headers] must be used to
+// prevent glibc from including the libc++ versions of the C standard library
+// headers.
+
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -I %S/Inputs/no-undeclared-includes-builtins/libcxx -I %S/Inputs/no-undeclared-includes-builtins/glibc %s
+// expected-no-diagnostics
+
+#include <stddef.h>
More information about the cfe-commits
mailing list