mboehme created this revision.
mboehme added a project: clang.
Herald added a subscriber: cfe-commits.
mboehme added a reviewer: rsmith.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80781

Files:
  clang/include/clang/Lex/ModuleMap.h
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Lex/ModuleMap.cpp
  
clang/test/Modules/Inputs/no-undeclared-includes-builtins/system_module/module.modulemap
  
clang/test/Modules/Inputs/no-undeclared-includes-builtins/system_module/stddef.h
  
clang/test/Modules/Inputs/no-undeclared-includes-builtins/user_module/module.modulemap
  
clang/test/Modules/Inputs/no-undeclared-includes-builtins/user_module/user_module.h
  clang/test/Modules/no-undeclared-includes-builtins.cpp

Index: clang/test/Modules/no-undeclared-includes-builtins.cpp
===================================================================
--- /dev/null
+++ clang/test/Modules/no-undeclared-includes-builtins.cpp
@@ -0,0 +1,9 @@
+// Test that a [no_undeclared_headers] module can include builtin headers, even
+// if these have been "claimed" by a different module that wraps these builtin
+// headers. libc++ does this, for example.
+
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -I %S/Inputs/no-undeclared-includes-builtins/user_module -I %S/Inputs/no-undeclared-includes-builtins/system_module %s
+// expected-no-diagnostics
+
+#include <user_module.h>
Index: clang/test/Modules/Inputs/no-undeclared-includes-builtins/user_module/user_module.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/no-undeclared-includes-builtins/user_module/user_module.h
@@ -0,0 +1 @@
+#include <stddef.h>
Index: clang/test/Modules/Inputs/no-undeclared-includes-builtins/user_module/module.modulemap
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/no-undeclared-includes-builtins/user_module/module.modulemap
@@ -0,0 +1,4 @@
+module UserModule [no_undeclared_includes] {
+  header "user_module.h"
+  export *
+}
Index: clang/test/Modules/Inputs/no-undeclared-includes-builtins/system_module/stddef.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/no-undeclared-includes-builtins/system_module/stddef.h
@@ -0,0 +1 @@
+#include_next <stddef.h>
Index: clang/test/Modules/Inputs/no-undeclared-includes-builtins/system_module/module.modulemap
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/no-undeclared-includes-builtins/system_module/module.modulemap
@@ -0,0 +1,4 @@
+module stddef [system] {
+  header "stddef.h"
+  export *
+}
Index: clang/lib/Lex/ModuleMap.cpp
===================================================================
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -387,13 +387,17 @@
            .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);
   }
Index: clang/lib/Lex/HeaderSearch.cpp
===================================================================
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1276,14 +1276,12 @@
     //
     // 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 different modules. Since ObjC
     // headers find in the wild might rely only on #import and do not contain
@@ -1423,8 +1421,14 @@
 
   // 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);
+  // 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 (RequestingModule && Module && RequestingModule->NoUndeclaredIncludes &&
+      !HS.getModuleMap().isBuiltinHeader(File)) {
+    HS.getModuleMap().resolveUses(RequestingModule, /*Complain*/ false);
     if (!RequestingModule->directlyUses(Module.getModule())) {
       return false;
     }
Index: clang/include/clang/Lex/ModuleMap.h
===================================================================
--- clang/include/clang/Lex/ModuleMap.h
+++ clang/include/clang/Lex/ModuleMap.h
@@ -413,6 +413,7 @@
 
   /// 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) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to