vsapsai created this revision.
vsapsai added reviewers: rsmith, bruno, Bigcheese.
Herald added subscribers: ributzka, dexonsmith, jkorous.
Herald added a project: clang.
HeaderSearch was marking requested HeaderFileInfo as Resolved only based on
the presence of ExternalSource. As the result, using any module was enough
to set ExternalSource and headers unknown to this module would have
HeaderFileInfo with empty fields, including `isImport = 0`, `NumIncludes = 0`.
Such HeaderFileInfo was preserved without changes regardless of how the
header was used in other modules and caused incorrect result in
`HeaderSearch::ShouldEnterIncludeFile`.
Fix by marking HeaderFileInfo as Resolved only if ExternalSource knows
about this header.
rdar://problem/62126911
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D80263
Files:
clang/lib/Lex/HeaderSearch.cpp
clang/test/Modules/Inputs/import-once/ImportOnce.framework/Headers/ImportOnce.h
clang/test/Modules/Inputs/import-once/ImportOnce.framework/Modules/module.modulemap
clang/test/Modules/Inputs/import-once/IndirectImporter.framework/Headers/IndirectImporter.h
clang/test/Modules/Inputs/import-once/IndirectImporter.framework/Modules/module.modulemap
clang/test/Modules/Inputs/import-once/Unrelated.framework/Headers/Unrelated.h
clang/test/Modules/Inputs/import-once/Unrelated.framework/Modules/module.modulemap
clang/test/Modules/import-once.m
Index: clang/test/Modules/import-once.m
===================================================================
--- /dev/null
+++ clang/test/Modules/import-once.m
@@ -0,0 +1,15 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodule-name=ImportOnce -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs/import-once %s
+
+// Test #import-ed headers are processed only once, even without header guards.
+// Dependency graph is
+//
+// Unrelated ImportOnce
+// ^ ^ ^
+// \ / |
+// IndirectImporter |
+// ^ |
+// \ |
+// import-once.m
+#import <IndirectImporter/IndirectImporter.h>
+#import <ImportOnce/ImportOnce.h>
Index: clang/test/Modules/Inputs/import-once/Unrelated.framework/Modules/module.modulemap
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/import-once/Unrelated.framework/Modules/module.modulemap
@@ -0,0 +1,4 @@
+framework module Unrelated {
+ umbrella header "Unrelated.h"
+ export *
+}
Index: clang/test/Modules/Inputs/import-once/Unrelated.framework/Headers/Unrelated.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/import-once/Unrelated.framework/Headers/Unrelated.h
@@ -0,0 +1 @@
+void foo(void);
Index: clang/test/Modules/Inputs/import-once/IndirectImporter.framework/Modules/module.modulemap
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/import-once/IndirectImporter.framework/Modules/module.modulemap
@@ -0,0 +1,4 @@
+framework module IndirectImporter {
+ umbrella header "IndirectImporter.h"
+ export *
+}
Index: clang/test/Modules/Inputs/import-once/IndirectImporter.framework/Headers/IndirectImporter.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/import-once/IndirectImporter.framework/Headers/IndirectImporter.h
@@ -0,0 +1,2 @@
+#import <Unrelated/Unrelated.h>
+#import <ImportOnce/ImportOnce.h>
Index: clang/test/Modules/Inputs/import-once/ImportOnce.framework/Modules/module.modulemap
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/import-once/ImportOnce.framework/Modules/module.modulemap
@@ -0,0 +1,4 @@
+framework module ImportOnce {
+ umbrella header "ImportOnce.h"
+ export *
+}
Index: clang/test/Modules/Inputs/import-once/ImportOnce.framework/Headers/ImportOnce.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/import-once/ImportOnce.framework/Headers/ImportOnce.h
@@ -0,0 +1,5 @@
+// No header guards on purpose.
+
+enum SomeSimpleEnum {
+ SomeSimpleEnumCase,
+};
Index: clang/lib/Lex/HeaderSearch.cpp
===================================================================
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1167,12 +1167,12 @@
HeaderFileInfo *HFI = &FileInfo[FE->getUID()];
// FIXME: Use a generation count to check whether this is really up to date.
if (ExternalSource && !HFI->Resolved) {
- HFI->Resolved = true;
auto ExternalHFI = ExternalSource->GetHeaderFileInfo(FE);
-
- HFI = &FileInfo[FE->getUID()];
- if (ExternalHFI.External)
- mergeHeaderFileInfo(*HFI, ExternalHFI);
+ if (ExternalHFI.IsValid) {
+ HFI->Resolved = true;
+ if (ExternalHFI.External)
+ mergeHeaderFileInfo(*HFI, ExternalHFI);
+ }
}
HFI->IsValid = true;
@@ -1199,12 +1199,12 @@
if (!WantExternal && (!HFI->IsValid || HFI->External))
return nullptr;
if (!HFI->Resolved) {
- HFI->Resolved = true;
auto ExternalHFI = ExternalSource->GetHeaderFileInfo(FE);
-
- HFI = &FileInfo[FE->getUID()];
- if (ExternalHFI.External)
- mergeHeaderFileInfo(*HFI, ExternalHFI);
+ if (ExternalHFI.IsValid) {
+ HFI->Resolved = true;
+ if (ExternalHFI.External)
+ mergeHeaderFileInfo(*HFI, ExternalHFI);
+ }
}
} else if (FE->getUID() >= FileInfo.size()) {
return nullptr;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
