Author: Volodymyr Sapsai
Date: 2020-08-20T17:41:28-07:00
New Revision: 7ac737e56bee721fb3535006140362c6e08726bb

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

LOG: [HeaderSearch] Fix processing #import-ed headers multiple times with 
modules enabled.

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

Reviewed By: bruno

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

Added: 
    
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

Modified: 
    clang/lib/Lex/HeaderSearch.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp
index 1df28cc07209..3fd43e0e3aad 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -1167,12 +1167,12 @@ HeaderFileInfo &HeaderSearch::getFileInfo(const 
FileEntry *FE) {
   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 @@ HeaderSearch::getExistingFileInfo(const FileEntry *FE,
     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;

diff  --git 
a/clang/test/Modules/Inputs/import-once/ImportOnce.framework/Headers/ImportOnce.h
 
b/clang/test/Modules/Inputs/import-once/ImportOnce.framework/Headers/ImportOnce.h
new file mode 100644
index 000000000000..02e3e76bf9e8
--- /dev/null
+++ 
b/clang/test/Modules/Inputs/import-once/ImportOnce.framework/Headers/ImportOnce.h
@@ -0,0 +1,5 @@
+// No header guards on purpose.
+
+enum SomeSimpleEnum {
+    SomeSimpleEnumCase,
+};

diff  --git 
a/clang/test/Modules/Inputs/import-once/ImportOnce.framework/Modules/module.modulemap
 
b/clang/test/Modules/Inputs/import-once/ImportOnce.framework/Modules/module.modulemap
new file mode 100644
index 000000000000..df12c237818e
--- /dev/null
+++ 
b/clang/test/Modules/Inputs/import-once/ImportOnce.framework/Modules/module.modulemap
@@ -0,0 +1,4 @@
+framework module ImportOnce {
+  umbrella header "ImportOnce.h"
+  export *
+}

diff  --git 
a/clang/test/Modules/Inputs/import-once/IndirectImporter.framework/Headers/IndirectImporter.h
 
b/clang/test/Modules/Inputs/import-once/IndirectImporter.framework/Headers/IndirectImporter.h
new file mode 100644
index 000000000000..46b989659457
--- /dev/null
+++ 
b/clang/test/Modules/Inputs/import-once/IndirectImporter.framework/Headers/IndirectImporter.h
@@ -0,0 +1,2 @@
+#import <Unrelated/Unrelated.h>
+#import <ImportOnce/ImportOnce.h>

diff  --git 
a/clang/test/Modules/Inputs/import-once/IndirectImporter.framework/Modules/module.modulemap
 
b/clang/test/Modules/Inputs/import-once/IndirectImporter.framework/Modules/module.modulemap
new file mode 100644
index 000000000000..ab5108077605
--- /dev/null
+++ 
b/clang/test/Modules/Inputs/import-once/IndirectImporter.framework/Modules/module.modulemap
@@ -0,0 +1,4 @@
+framework module IndirectImporter {
+    umbrella header "IndirectImporter.h"
+    export *
+}

diff  --git 
a/clang/test/Modules/Inputs/import-once/Unrelated.framework/Headers/Unrelated.h 
b/clang/test/Modules/Inputs/import-once/Unrelated.framework/Headers/Unrelated.h
new file mode 100644
index 000000000000..ee83a3dec7b0
--- /dev/null
+++ 
b/clang/test/Modules/Inputs/import-once/Unrelated.framework/Headers/Unrelated.h
@@ -0,0 +1 @@
+void foo(void);

diff  --git 
a/clang/test/Modules/Inputs/import-once/Unrelated.framework/Modules/module.modulemap
 
b/clang/test/Modules/Inputs/import-once/Unrelated.framework/Modules/module.modulemap
new file mode 100644
index 000000000000..3661df1eb41b
--- /dev/null
+++ 
b/clang/test/Modules/Inputs/import-once/Unrelated.framework/Modules/module.modulemap
@@ -0,0 +1,4 @@
+framework module Unrelated {
+    umbrella header "Unrelated.h"
+    export *
+}

diff  --git a/clang/test/Modules/import-once.m 
b/clang/test/Modules/import-once.m
new file mode 100644
index 000000000000..738855cb9de0
--- /dev/null
+++ b/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>


        
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to