[clang] f8536fb - [clang][HeaderSearch] Fix implicit module when using header maps

[Ivan Murashko via cfe-commits]

Author: Ivan Murashko
Date: 2023-05-28T11:49:51+01:00
New Revision: f8536fb11e3d71d009c9002b5aa2ef32983ac7dc

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

LOG: [clang][HeaderSearch] Fix implicit module when using header maps

Previously, if a header was found via in a header map, and not just remapped.
we wouldn't also find the module it maps to when using implicit modules (for
module maps that were explicitly loaded).

This diff just updates these code paths to also locate the owning module via
`findUsableModuleForHeader`.

Reviewed By: benlangmuir

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

Added: 
    clang/test/Modules/implicit-module-header-maps.cpp

Modified: 
    clang/lib/Lex/HeaderSearch.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp
index a650bbea1e488..d09d3ae12f581 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -491,7 +491,8 @@ OptionalFileEntryRef DirectoryLookup::LookupFile(
 
   IsInHeaderMap = true;
 
-  auto FixupSearchPath = [&]() {
+  auto FixupSearchPathAndFindUsableModule =
+      [&](auto File) -> OptionalFileEntryRef {
     if (SearchPath) {
       StringRef SearchPathRef(getName());
       SearchPath->clear();
@@ -501,6 +502,12 @@ OptionalFileEntryRef DirectoryLookup::LookupFile(
       RelativePath->clear();
       RelativePath->append(Filename.begin(), Filename.end());
     }
+    if (!HS.findUsableModuleForHeader(
+            &File.getFileEntry(), File.getFileEntry().getDir(),
+            RequestingModule, SuggestedModule, isSystemHeaderDirectory())) {
+      return std::nullopt;
+    }
+    return File;
   };
 
   // Check if the headermap maps the filename to a framework include
@@ -513,8 +520,7 @@ OptionalFileEntryRef DirectoryLookup::LookupFile(
   }
 
   if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest, OpenFile)) {
-    FixupSearchPath();
-    return *Res;
+    return FixupSearchPathAndFindUsableModule(*Res);
   }
 
   // Header maps need to be marked as used whenever the filename matches.

diff  --git a/clang/test/Modules/implicit-module-header-maps.cpp b/clang/test/Modules/implicit-module-header-maps.cpp
new file mode 100644
index 0000000000000..a190ff78f306f
--- /dev/null
+++ b/clang/test/Modules/implicit-module-header-maps.cpp
@@ -0,0 +1,54 @@
+// UNSUPPORTED: system-windows
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %hmaptool write a.hmap.json hmap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules -fimplicit-module-maps -fmodule-map-file=module.modulemap -fsyntax-only -I hmap -fmodules-cache-path=%t test.cpp
+//
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: sed -e "s|OUTPUTS_DIR|%t|g" b.hmap.json > hmap.json
+// RUN: %hmaptool write hmap.json hmap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules -fimplicit-module-maps -fmodule-map-file=module.modulemap -fsyntax-only -I hmap -fmodules-cache-path=%t test.cpp
+
+//--- After/Mapping.h
+#ifdef FOO
+#error foo
+#endif
+
+//--- a.hmap.json
+{
+  "mappings" :
+    {
+     "Before/Mapping.h" : "After/Mapping.h",
+     "After/Mapping.h" : "After/Mapping.h"
+    }
+}
+
+//--- b.hmap.json
+{
+  "mappings" :
+    {
+     "Before/Mapping.h" : "OUTPUTS_DIR/After/Mapping.h"
+    }
+}
+
+//--- module.modulemap
+module a {
+  header "After/Mapping.h"
+}
+
+
+//--- test.cpp
+#define FOO
+// This include will fail if:
+// 1) modules are't used, as the `FOO` define will propagate into the included
+//    header and trip a `#error`, or
+// 2) header maps aren't used, as the header name doesn't exist and relies on
+//    the header map to remap it to the real header.
+#include "Before/Mapping.h"