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

[vsapsai]

Summary

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

[vsapsai]

Ping.

Also suggested clang-format linter changes make the test useless as the order of imports is important.

[vsapsai]

Ping.

[vsapsai]

Ping.

[vsapsai]

Ping.

[vsapsai]

Rebased the change to make sure it still works.

[vsapsai]

Ping.

[vsapsai]

Ping.

[bruno]

Nice catch! LGTM

[vsapsai]

Thanks for the review.

clang/lib/Lex/HeaderSearch.cpp

 /// FileEntry.
 HeaderFileInfo &HeaderSearch::getFileInfo(const FileEntry *FE) {
   if (FE->getUID() >= FileInfo.size())
     FileInfo.resize(FE->getUID() + 1);
 
   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.IsValid) {
+      HFI->Resolved = true;
       if (ExternalHFI.External)
         mergeHeaderFileInfo(*HFI, ExternalHFI);
     }
+  }
 
   HFI->IsValid = true;
   // We have local information about this header file, so it's no longer
   // strictly external.
   HFI->External = false;
   return *HFI;
 }
 
if (FE->getUID() >= FileInfo.size()) {
         return nullptr;
       FileInfo.resize(FE->getUID() + 1);
     }
 
     HFI = &FileInfo[FE->getUID()];
     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.IsValid) {
+        HFI->Resolved = true;
         if (ExternalHFI.External)
           mergeHeaderFileInfo(*HFI, ExternalHFI);
       }
+    }
   } else if (FE->getUID() >= FileInfo.size()) {
     return nullptr;
   } else {
     HFI = &FileInfo[FE->getUID()];
   }
 
   if (!HFI->IsValid || (HFI->External && !WantExternal))
     return nullptr;

clang/test/Modules/Inputs/import-once/ImportOnce.framework/Headers/ImportOnce.h

+// No header guards on purpose.
+
+enum SomeSimpleEnum {
+    SomeSimpleEnumCase,
+};

clang/test/Modules/Inputs/import-once/ImportOnce.framework/Modules/module.modulemap

+framework module ImportOnce {
+  umbrella header "ImportOnce.h"
+  export *
+}

clang/test/Modules/Inputs/import-once/IndirectImporter.framework/Headers/IndirectImporter.h

+#import <Unrelated/Unrelated.h>
+#import <ImportOnce/ImportOnce.h>

clang/test/Modules/Inputs/import-once/IndirectImporter.framework/Modules/module.modulemap

+framework module IndirectImporter {
+    umbrella header "IndirectImporter.h"
+    export *
+}

clang/test/Modules/Inputs/import-once/Unrelated.framework/Headers/Unrelated.h

+void foo(void);

clang/test/Modules/Inputs/import-once/Unrelated.framework/Modules/module.modulemap

+framework module Unrelated {
+    umbrella header "Unrelated.h"
+    export *
+}

clang/test/Modules/import-once.m

+// 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>