Improve diagnostics for missing import / #include of module.

Fix a few bugs where we would fail to properly determine header to
module correspondence when determining whether to suggest a #include or
import, and suggest a #include more often in language modes where there
is no import syntax. Generally, if the target is in a header with
include guards or #pragma once, we should suggest either #including or
importing that header, and not importing a module that happens to
textually include it.

In passing, improve the notes we attach to the corresponding
diagnostics: calling an entity that we couldn't see "previous" is
confusing.

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -10298,11 +10298,6 @@ def err_module_unimported_use : Error<
   "explicit specialization|partial specialization}0 of %1 must be imported "
   "from module '%2' before it is required">;
 def err_module_unimported_use_header : Error<
-  "missing '#include %3'; "
-  "%select{declaration|definition|default argument|"
-  "explicit specialization|partial specialization}0 of %1 must be imported "
-  "from module '%2' before it is required">;
-def err_module_unimported_use_global_module_fragment : Error<
   "%select{missing '#include'|missing '#include %3'}2; "
   "%select{||default argument of |explicit specialization of |"
   "partial specialization of }0%1 must be "
@@ -10312,6 +10307,10 @@ def err_module_unimported_use_multiple : Error<
   "%select{declaration|definition|default argument|"
   "explicit specialization|partial specialization}0 of %1 must be imported "
   "from one of the following modules before it is required:%2">;
+def note_unreachable_entity : Note<
+  "%select{declaration|definition|default argument declared|"
+  "explicit specialization declared|partial specialization declared}0 here "
+  "is not %select{visible|reachable|reachable|reachable|reachable|reachable}0">;
 def ext_module_import_in_extern_c : ExtWarn<
   "import of C++ module '%0' appears within extern \"C\" language linkage "
   "specification">, DefaultError,

clang/include/clang/Lex/HeaderSearch.h

@@ -476,6 +476,13 @@ public:
   /// This routine does not consider the effect of \#import
   bool isFileMultipleIncludeGuarded(const FileEntry *File);
 
+  /// Determine whether the given file is known to have ever been \#imported
+  /// (or if it has been \#included and we've encountered a \#pragma once).
+  bool hasFileBeenImported(const FileEntry *File) {
+    const HeaderFileInfo *FI = getExistingFileInfo(File);
+    return FI && FI->isImport;
+  }
+
   /// This method returns a HeaderMap for the specified
   /// FileEntry, uniquing them through the 'HeaderMaps' datastructure.
   const HeaderMap *CreateHeaderMap(const FileEntry *FE);
@@ -559,6 +566,12 @@ public:
   ModuleMap::KnownHeader findModuleForHeader(const FileEntry *File,
                                              bool AllowTextual = false) const;
 
+  /// Retrieve all the modules corresponding to the given file.
+  ///
+  /// \ref findModuleForHeader should typically be used instead of this.
+  ArrayRef<ModuleMap::KnownHeader>
+  findAllModulesForHeader(const FileEntry *File) const;
+
   /// Read the contents of the given module map file.
   ///
   /// \param File The module map file.

clang/include/clang/Lex/ModuleMap.h

@@ -419,7 +419,10 @@ public:
     Callbacks.push_back(std::move(Callback));
   }
 
-  /// Retrieve the module that owns the given header file, if any.
+  /// Retrieve the module that owns the given header file, if any. Note that
+  /// this does not implicitly load module maps, except for builtin headers,
+  /// and does not consult the external source. (Those checks are the
+  /// responsibility of \ref HeaderSearch.)
   ///
   /// \param File The header file that is likely to be included.
   ///
@@ -433,13 +436,19 @@ public:
   KnownHeader findModuleForHeader(const FileEntry *File,
                                   bool AllowTextual = false);
 
-  /// Retrieve all the modules that contain the given header file. This
-  /// may not include umbrella modules, nor information from external sources,
-  /// if they have not yet been inferred / loaded.
+  /// Retrieve all the modules that contain the given header file. Note that
+  /// this does not implicitly load module maps, except for builtin headers,
+  /// and does not consult the external source. (Those checks are the
+  /// responsibility of \ref HeaderSearch.)
   ///
   /// Typically, \ref findModuleForHeader should be used instead, as it picks
   /// the preferred module for the header.
-  ArrayRef<KnownHeader> findAllModulesForHeader(const FileEntry *File) const;
+  ArrayRef<KnownHeader> findAllModulesForHeader(const FileEntry *File);
+
+  /// Like \ref findAllModulesForHeader, but do not attempt to infer module
+  /// ownership from umbrella headers if we've not already done so.
+  ArrayRef<KnownHeader>
+  findResolvedModulesForHeader(const FileEntry *File) const;
 
   /// Resolve all lazy header directives for the specified file.
   ///

clang/include/clang/Lex/Preprocessor.h

@@ -2278,20 +2278,22 @@ public:
   /// into a module, or is outside any module, returns nullptr.
   Module *getModuleForLocation(SourceLocation Loc);
 
-  /// We want to produce a diagnostic at location IncLoc concerning a
-  /// missing module import.
+  /// We want to produce a diagnostic at location IncLoc concerning an
+  /// unreachable effect at location MLoc (eg, where a desired entity was
+  /// declared or defined). Determine whether the right way to make MLoc
+  /// reachable is by #include, and if so, what header should be included.
   ///
-  /// \param IncLoc The location at which the missing import was detected.
-  /// \param M The desired module.
-  /// \param MLoc A location within the desired module at which some desired
-  ///        effect occurred (eg, where a desired entity was declared).
+  /// This is not necessarily fast, and might load unexpected module maps, so
+  /// should only be called by code that intends to produce an error.
   ///
-  /// \return A file that can be #included to import a module containing MLoc.
-  ///         Null if no such file could be determined or if a #include is not
-  ///         appropriate.
-  const FileEntry *getModuleHeaderToIncludeForDiagnostics(SourceLocation IncLoc,
-                                                          Module *M,
-                                                          SourceLocation MLoc);
+  /// \param IncLoc The location at which the missing effect was detected.
+  /// \param MLoc A location within an unimported module at which the desired
+  ///        effect occurred.
+  /// \return A file that can be #included to provide the desired effect. Null
+  ///         if no such file could be determined or if a #include is not
+  ///         appropriate (eg, if a module should be imported instead).
+  const FileEntry *getHeaderToIncludeForDiagnostics(SourceLocation IncLoc,
+                                                    SourceLocation MLoc);
 
   bool isRecordingPreamble() const {
     return PreambleConditionalStack.isRecording();

clang/lib/Lex/HeaderSearch.cpp

@@ -1219,9 +1219,11 @@ HeaderSearch::getExistingFileInfo(const FileEntry *FE,
 }
 
 bool HeaderSearch::isFileMultipleIncludeGuarded(const FileEntry *File) {
-  // Check if we've ever seen this file as a header.
+  // Check if we've entered this file and found an include guard or #pragma
+  // once. Note that we dor't check for #import, because that's not a property
+  // of the file itself.
   if (auto *HFI = getExistingFileInfo(File))
-    return HFI->isPragmaOnce || HFI->isImport || HFI->ControllingMacro ||
+    return HFI->isPragmaOnce || HFI->ControllingMacro ||
            HFI->ControllingMacroID;
   return false;
 }
@@ -1399,6 +1401,16 @@ HeaderSearch::findModuleForHeader(const FileEntry *File,
   return ModMap.findModuleForHeader(File, AllowTextual);
 }
 
+ArrayRef<ModuleMap::KnownHeader>
+HeaderSearch::findAllModulesForHeader(const FileEntry *File) const {
+  if (ExternalSource) {
+    // Make sure the external source has handled header info about this file,
+    // which includes whether the file is part of a module.
+    (void)getExistingFileInfo(File);
+  }
+  return ModMap.findAllModulesForHeader(File);
+}
+
 static bool suggestModule(HeaderSearch &HS, const FileEntry *File,
                           Module *RequestingModule,
                           ModuleMap::KnownHeader *SuggestedModule) {

clang/lib/Lex/ModuleMap.cpp

@@ -662,7 +662,20 @@ ModuleMap::findOrCreateModuleForHeaderInUmbrellaDir(const FileEntry *File) {
 }
 
 ArrayRef<ModuleMap::KnownHeader>
-ModuleMap::findAllModulesForHeader(const FileEntry *File) const {
+ModuleMap::findAllModulesForHeader(const FileEntry *File) {
+  HeadersMap::iterator Known = findKnownHeader(File);
+  if (Known != Headers.end())
+    return Known->second;
+
+  if (findOrCreateModuleForHeaderInUmbrellaDir(File))
+    return Headers.find(File)->second;
+
+  return None;
+}
+
+ArrayRef<ModuleMap::KnownHeader>
+ModuleMap::findResolvedModulesForHeader(const FileEntry *File) const {
+  // FIXME: Is this necessary?
   resolveHeaderDirectives(File);
   auto It = Headers.find(File);
   if (It == Headers.end())

clang/lib/Lex/PPDirectives.cpp

@@ -646,24 +646,8 @@ Module *Preprocessor::getModuleForLocation(SourceLocation Loc) {
 }
 
 const FileEntry *
-Preprocessor::getModuleHeaderToIncludeForDiagnostics(SourceLocation IncLoc,
-                                                     Module *M,
-                                                     SourceLocation Loc) {
-  assert(M && "no module to include");
-
-  // If the context is the global module fragment of some module, we never
-  // want to return that file; instead, we want the innermost include-guarded
-  // header that it included.
-  bool InGlobalModuleFragment = M->Kind == Module::GlobalModuleFragment;
-
-  // If we have a module import syntax, we shouldn't include a header to
-  // make a particular module visible.
-  if ((getLangOpts().ObjC || getLangOpts().CPlusPlusModules ||
-       getLangOpts().ModulesTS) &&
-      !InGlobalModuleFragment)
-    return nullptr;
-
-  Module *TopM = M->getTopLevelModule();
+Preprocessor::getHeaderToIncludeForDiagnostics(SourceLocation IncLoc,
+                                               SourceLocation Loc) {
   Module *IncM = getModuleForLocation(IncLoc);
 
   // Walk up through the include stack, looking through textual headers of M
@@ -677,37 +661,50 @@ Preprocessor::getModuleHeaderToIncludeForDiagnostics(SourceLocation IncLoc,
     if (!FE)
       break;
 
-    if (InGlobalModuleFragment) {
-      if (getHeaderSearchInfo().isFileMultipleIncludeGuarded(FE))
-        return FE;
-      Loc = SM.getIncludeLoc(ID);
-      continue;
-    }
-
-    bool InTextualHeader = false;
-    for (auto Header : HeaderInfo.getModuleMap().findAllModulesForHeader(FE)) {
-      if (!Header.getModule()->isSubModuleOf(TopM))
-        continue;
-
-      if (!(Header.getRole() & ModuleMap::TextualHeader)) {
-        // If this is an accessible, non-textual header of M's top-level module
-        // that transitively includes the given location and makes the
-        // corresponding module visible, this is the thing to #include.
-        if (Header.isAccessibleFrom(IncM))
-          return FE;
+    // We want to find all possible modules that might contain this header, so
+    // search all enclosing directories for module maps and load them.
+    HeaderInfo.hasModuleMap(FE->getName(), /*Root*/ nullptr,
+                            SourceMgr.isInSystemHeader(Loc));
 
+    bool InPrivateHeader = false;
+    for (auto Header : HeaderInfo.findAllModulesForHeader(FE)) {
+      if (!Header.isAccessibleFrom(IncM)) {
         // It's in a private header; we can't #include it.
         // FIXME: If there's a public header in some module that re-exports it,
         // then we could suggest including that, but it's not clear that's the
         // expected way to make this entity visible.
+        InPrivateHeader = true;
         continue;
       }
 
-      InTextualHeader = true;
+      // We'll suggest including textual headers below if they're
+      // include-guarded.
+      if (Header.getRole() & ModuleMap::TextualHeader)
+        continue;
+
+      // If we have a module import syntax, we shouldn't include a header to
+      // make a particular module visible. Let the caller know they should
+      // suggest an import instead.
+      if (getLangOpts().ObjC || getLangOpts().CPlusPlusModules ||
+          getLangOpts().ModulesTS)
+        return nullptr;
+
+      // If this is an accessible, non-textual header of M's top-level module
+      // that transitively includes the given location and makes the
+      // corresponding module visible, this is the thing to #include.
+      return FE;
     }
 
-    if (!InTextualHeader)
-      break;
+    // FIXME: If we're bailing out due to a private header, we shouldn't suggest
+    // an import either.
+    if (InPrivateHeader)
+      return nullptr;
+
+    // If the header is includable and has an include guard, assume the
+    // intended way to expose its contents is by #include, not by importing a
+    // module that transitively includes it.
+    if (getHeaderSearchInfo().isFileMultipleIncludeGuarded(FE))
+      return FE;
 
     Loc = SM.getIncludeLoc(ID);
   }

clang/lib/Sema/SemaLookup.cpp

@@ -5350,9 +5350,8 @@ void Sema::diagnoseMissingImport(SourceLocation Loc, NamedDecl *Decl,
 
 /// Get a "quoted.h" or <angled.h> include path to use in a diagnostic
 /// suggesting the addition of a #include of the specified file.
-static std::string getIncludeStringForHeader(Preprocessor &PP,
-                                             const FileEntry *E,
-                                             llvm::StringRef IncludingFile) {
+static std::string getHeaderNameForHeader(Preprocessor &PP, const FileEntry *E,
+                                          llvm::StringRef IncludingFile) {
   bool IsSystem = false;
   auto Path = PP.getHeaderSearchInfo().suggestPathToFileForDiagnostics(
       E, IncludingFile, &IsSystem);
@@ -5366,25 +5365,10 @@ void Sema::diagnoseMissingImport(SourceLocation UseLoc, NamedDecl *Decl,
   assert(!Modules.empty());
 
   auto NotePrevious = [&] {
-    unsigned DiagID;
-    switch (MIK) {
-    case MissingImportKind::Declaration:
-      DiagID = diag::note_previous_declaration;
-      break;
-    case MissingImportKind::Definition:
-      DiagID = diag::note_previous_definition;
-      break;
-    case MissingImportKind::DefaultArgument:
-      DiagID = diag::note_default_argument_declared_here;
-      break;
-    case MissingImportKind::ExplicitSpecialization:
-      DiagID = diag::note_explicit_specialization_declared_here;
-      break;
-    case MissingImportKind::PartialSpecialization:
-      DiagID = diag::note_partial_specialization_declared_here;
-      break;
-    }
-    Diag(DeclLoc, DiagID);
+    // FIXME: Suppress the note backtrace even under
+    // -fdiagnostics-show-note-include-stack. We don't care how this
+    // declaration was previously reached.
+    Diag(DeclLoc, diag::note_unreachable_entity) << (int)MIK;
   };
 
   // Weed out duplicates from module list.
@@ -5397,26 +5381,24 @@ void Sema::diagnoseMissingImport(SourceLocation UseLoc, NamedDecl *Decl,
       UniqueModules.push_back(M);
   }
 
-  llvm::StringRef IncludingFile;
-  if (const FileEntry *FE =
-          SourceMgr.getFileEntryForID(SourceMgr.getFileID(UseLoc)))
-    IncludingFile = FE->tryGetRealPathName();
+  // Try to find a suitable header-name to #include.
+  std::string HeaderName;
+  if (const FileEntry *Header =
+          PP.getHeaderToIncludeForDiagnostics(UseLoc, DeclLoc)) {
+    if (const FileEntry *FE =
+            SourceMgr.getFileEntryForID(SourceMgr.getFileID(UseLoc)))
+      HeaderName = getHeaderNameForHeader(PP, Header, FE->tryGetRealPathName());
+  }
 
-  if (UniqueModules.empty()) {
-    // All candidates were global module fragments. Try to suggest a #include.
-    const FileEntry *E =
-        PP.getModuleHeaderToIncludeForDiagnostics(UseLoc, Modules[0], DeclLoc);
+  // If we have a #include we should suggest, or if all definition locations
+  // were in global module fragments, don't suggest an import.
+  if (!HeaderName.empty() || UniqueModules.empty()) {
     // FIXME: Find a smart place to suggest inserting a #include, and add
     // a FixItHint there.
-    Diag(UseLoc, diag::err_module_unimported_use_global_module_fragment)
-        << (int)MIK << Decl << !!E
-        << (E ? getIncludeStringForHeader(PP, E, IncludingFile) : "");
-    // Produce a "previous" note if it will point to a header rather than some
-    // random global module fragment.
-    // FIXME: Suppress the note backtrace even under
-    // -fdiagnostics-show-note-include-stack.
-    if (E)
-      NotePrevious();
+    Diag(UseLoc, diag::err_module_unimported_use_header)
+        << (int)MIK << Decl << !HeaderName.empty() << HeaderName;
+    // Produce a note showing where the entity was declared.
+    NotePrevious();
     if (Recover)
       createImplicitModuleImportForErrorRecovery(UseLoc, Modules[0]);
     return;
@@ -5438,16 +5420,6 @@ void Sema::diagnoseMissingImport(SourceLocation UseLoc, NamedDecl *Decl,
 
     Diag(UseLoc, diag::err_module_unimported_use_multiple)
       << (int)MIK << Decl << ModuleList;
-  } else if (const FileEntry *E = PP.getModuleHeaderToIncludeForDiagnostics(
-                 UseLoc, Modules[0], DeclLoc)) {
-    // The right way to make the declaration visible is to include a header;
-    // suggest doing so.
-    //
-    // FIXME: Find a smart place to suggest inserting a #include, and add
-    // a FixItHint there.
-    Diag(UseLoc, diag::err_module_unimported_use_header)
-        << (int)MIK << Decl << Modules[0]->getFullModuleName()
-        << getIncludeStringForHeader(PP, E, IncludingFile);
   } else {
     // FIXME: Add a FixItHint that imports the corresponding module.
     Diag(UseLoc, diag::err_module_unimported_use)

clang/lib/Serialization/ASTWriter.cpp

@@ -1812,7 +1812,7 @@ void ASTWriter::WriteHeaderSearch(const HeaderSearch &HS) {
       Filename, File->getSize(), getTimestampForOutput(File)
     };
     HeaderFileInfoTrait::data_type Data = {
-      *HFI, HS.getModuleMap().findAllModulesForHeader(File), {}
+      *HFI, HS.getModuleMap().findResolvedModulesForHeader(File), {}
     };
     Generator.insert(Key, Data, GeneratorTrait);
     ++NumHeaderSearchEntries;

clang/test/CXX/basic/basic.scope/basic.scope.namespace/p2.cpp

@@ -11,6 +11,8 @@
 #ifdef INTERFACE
 module;
 #include "foo.h"
+// FIXME: The following need to be moved to a header file. The global module
+// fragment is only permitted to contain preprocessor directives.
 int global_module_fragment;
 export module A;
 export int exported;
@@ -28,12 +30,13 @@ module;
 
 void test_early() {
   in_header = 1; // expected-error {{missing '#include "foo.h"'; 'in_header' must be declared before it is used}}
-  // expected-note@*{{previous}}
+  // expected-note@*{{not visible}}
 
   global_module_fragment = 1; // expected-error {{missing '#include'; 'global_module_fragment' must be declared before it is used}}
+  // expected-note@p2.cpp:16 {{not visible}}
 
   exported = 1; // expected-error {{must be imported from module 'A'}}
-  // expected-note@p2.cpp:16 {{previous}}
+  // expected-note@p2.cpp:18 {{not visible}}
 
   not_exported = 1; // expected-error {{undeclared identifier}}
 
@@ -52,16 +55,17 @@ import A;
 
 void test_late() {
   in_header = 1; // expected-error {{missing '#include "foo.h"'; 'in_header' must be declared before it is used}}
-  // expected-note@*{{previous}}
+  // expected-note@*{{not visible}}
 
   global_module_fragment = 1; // expected-error {{missing '#include'; 'global_module_fragment' must be declared before it is used}}
+  // expected-note@p2.cpp:16 {{not visible}}
 
   exported = 1;
 
   not_exported = 1;
 #ifndef IMPLEMENTATION
   // expected-error@-2 {{undeclared identifier 'not_exported'; did you mean 'exported'}}
-  // expected-note@p2.cpp:16 {{declared here}}
+  // expected-note@p2.cpp:18 {{declared here}}
 #endif
 
   internal = 1;

clang/test/CXX/module/module.unit/p8.cpp

@@ -36,5 +36,5 @@ export module foo:bar; // expected-error {{not yet supported}} expected-error {{
 int k = n;
 #ifndef IMPORTED
 // expected-error@-2 {{declaration of 'n' must be imported from module 'foo' before it is required}}
-// expected-note@* {{previous}}
+// expected-note@* {{not visible}}
 #endif

clang/test/CXX/modules-ts/dcl.dcl/dcl.module/dcl.module.import/p1.cpp

@@ -23,7 +23,7 @@ module MODULE_NAME;
 int use_1 = a;
 #if !MODULE_X
 // expected-error@-2 {{declaration of 'a' must be imported from module 'x' before it is required}}
-// expected-note@x.cppm:1 {{here}}
+// expected-note@x.cppm:1 {{not visible}}
 #endif
 
 import x;
@@ -32,7 +32,7 @@ int use_2 = b; // ok
 
 // There is no relation between module x and module x.y.
 int use_3 = c; // expected-error {{declaration of 'c' must be imported from module 'x.y'}}
-               // expected-note@x.y.cppm:1 {{here}}
+               // expected-note@x.y.cppm:1 {{not visible}}
 
 import x [[]];
 import x [[foo]]; // expected-warning {{unknown attribute 'foo' ignored}}

clang/test/Modules/auto-module-import.m

@@ -18,7 +18,7 @@
 
 #ifdef ERRORS
 Module *mod; // expected-error{{declaration of 'Module' must be imported from module 'Module' before it is required}}
-// expected-note@Inputs/Module.framework/Headers/Module.h:15 {{previous}}
+// expected-note@Inputs/Module.framework/Headers/Module.h:15 {{not visible}}
 #else
 #import <AlsoDependsOnModule/AlsoDependsOnModule.h> // expected-warning{{treating #import as an import of module 'AlsoDependsOnModule'}}
 #endif
@@ -29,7 +29,7 @@ int getDependsOther() { return depends_on_module_other; }
 void testSubframeworkOther() {
 #ifdef ERRORS
   double *sfo1 = sub_framework_other; // expected-error{{declaration of 'sub_framework_other' must be imported from module 'DependsOnModule.SubFramework.Other'}}
-  // expected-note@Inputs/DependsOnModule.framework/Frameworks/SubFramework.framework/Headers/Other.h:15 {{previous}}
+  // expected-note@Inputs/DependsOnModule.framework/Frameworks/SubFramework.framework/Headers/Other.h:15 {{not visible}}
 #endif
 }
 
@@ -73,7 +73,7 @@ int getModulePrivate() { return module_private; }
 int getNoUmbrellaAPrivate() { return no_umbrella_A_private; }
 
 int getNoUmbrellaBPrivateFail() { return no_umbrella_B_private; } // expected-error{{declaration of 'no_umbrella_B_private' must be imported from module 'NoUmbrella.Private.B_Private'}}
-// expected-note@Inputs/NoUmbrella.framework/PrivateHeaders/B_Private.h:1 {{previous}}
+// expected-note@Inputs/NoUmbrella.framework/PrivateHeaders/B_Private.h:1 {{not visible}}
 
 // Test inclusion of headers that are under an umbrella directory but
 // not actually part of the module.

clang/test/Modules/cxx-templates.cpp

@@ -83,7 +83,7 @@ void g() {
   // a big spew of errors here.
   //
   // expected-error@Inputs/cxx-templates-a.h:19 {{definition of 'DefinedInBImpl' must be imported}}
-  // expected-note@Inputs/cxx-templates-b-impl.h:1 +{{definition is here}}
+  // expected-note@Inputs/cxx-templates-b-impl.h:1 +{{definition here is not reachable}}
   // expected-error@Inputs/cxx-templates-a.h:19 +{{}}
   // expected-error@Inputs/cxx-templates-a.h:20 +{{}}
   PerformDelayedLookup(defined_in_b_impl); // expected-note {{in instantiation of}}
@@ -106,8 +106,8 @@ void g() {
   TemplateInstantiationVisibility<char[1]> tiv1;
   TemplateInstantiationVisibility<char[2]> tiv2;
   TemplateInstantiationVisibility<char[3]> tiv3; // expected-error 5{{must be imported from module 'cxx_templates_b_impl'}}
-  // expected-note@cxx-templates-b-impl.h:10 3{{explicit specialization declared here}}
-  // expected-note@cxx-templates-b-impl.h:10 2{{previous definition is here}}
+  // expected-note@cxx-templates-b-impl.h:10 3{{explicit specialization declared here is not reachable}}
+  // expected-note@cxx-templates-b-impl.h:10 2{{definition here is not reachable}}
   TemplateInstantiationVisibility<char[4]> tiv4;
 
   int &p = WithPartialSpecializationUse().f();

clang/test/Modules/decldef.m

@@ -2,7 +2,7 @@
 // RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fobjc-arc -I %S/Inputs -fmodules-cache-path=%t %s -verify -DUSE_EARLY
 // RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fobjc-arc -I %S/Inputs -fmodules-cache-path=%t %s -verify
 
-// expected-note@Inputs/def.h:5 {{previous}}
+// expected-note@Inputs/def.h:5 {{here}}
 
 @class Def;
 Def *def;
@@ -16,7 +16,7 @@ B *b1;
 // expected-error@-2{{must use 'struct' tag to refer to type 'B'}}
 #else
 // expected-error@-4{{declaration of 'B' must be imported from module 'decldef.Decl' before it is required}}
-// expected-note@Inputs/decl.h:2 {{previous}}
+// expected-note@Inputs/decl.h:2 {{not visible}}
 #endif
 @import decldef.Decl;
 

clang/test/Modules/decldef.mm

@@ -4,9 +4,9 @@
 // RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fobjc-arc -I %S/Inputs -fmodules-cache-path=%t %s -verify -DUSE_3 -DUSE_4
 // RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fobjc-arc -I %S/Inputs -fmodules-cache-path=%t %s -verify -DUSE_4
 
-// expected-note@Inputs/def.h:5 0-1{{previous}}
-// expected-note@Inputs/def.h:16 0-1{{previous}}
-// expected-note@Inputs/def-include.h:11 0-1{{previous}}
+// expected-note@Inputs/def.h:5 0-1{{here}}
+// expected-note@Inputs/def.h:16 0-1{{here}}
+// expected-note@Inputs/def-include.h:11 0-1{{here}}
 
 @class Def;
 Def *def;

clang/test/Modules/diagnose-missing-import.m

@@ -8,7 +8,7 @@ void foo() {
   XYZLogEvent(xyzRiskyCloseOpenParam, xyzRiskyCloseOpenParam); // expected-error {{implicit declaration of function 'XYZLogEvent'}} expected-error {{declaration of 'XYZLogEvent' must be imported}} expected-error {{declaration of 'xyzRiskyCloseOpenParam' must be imported from module 'NCI.A'}} expected-error {{declaration of 'xyzRiskyCloseOpenParam' must be imported from module 'NCI.A'}}
 }
 
-// expected-note@Inputs/diagnose-missing-import/a.h:5 {{previous declaration is here}}
-// expected-note@Inputs/diagnose-missing-import/a.h:5 {{previous declaration is here}}
-// expected-note@Inputs/diagnose-missing-import/a.h:6 {{previous declaration is here}}
+// expected-note@Inputs/diagnose-missing-import/a.h:5 {{declaration here is not visible}}
+// expected-note@Inputs/diagnose-missing-import/a.h:5 {{declaration here is not visible}}
+// expected-note@Inputs/diagnose-missing-import/a.h:6 {{declaration here is not visible}}
 

clang/test/Modules/interface-diagnose-missing-import.m

@@ -8,4 +8,4 @@
 @interface Buggy (MyExt) // expected-error {{definition of 'Buggy' must be imported from module 'Foo' before it is required}}
 @end
 
-// expected-note@Foo/RandoPriv.h:3{{previous definition is here}}
+// expected-note@Foo/RandoPriv.h:3{{definition here is not reachable}}

clang/test/Modules/ms-enums.cpp

@@ -2,8 +2,8 @@
 // RUN: %clang_cc1 -triple x86_64-windows-msvc -fms-extensions -fms-compatibility -x c++ -std=c++20 -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -I %S/Inputs/ms-enums %s -verify -fno-modules-error-recovery
 
 #include "B.h"
-// expected-note@A.h:1 {{previous declaration is here}}
-// expected-note@A.h:1 2 {{previous definition is here}}
+// expected-note@A.h:1 {{declaration here is not visible}}
+// expected-note@A.h:1 2{{definition here is not reachable}}
 
 fwd_enum gv_enum; // expected-error {{must be imported}}
 

clang/test/Modules/no-module-map.cpp

@@ -30,7 +30,7 @@
 #error A_H should not be defined
 #endif
 // expected-error@+3 {{must be imported from}}
-// expected-note@* {{previous declaration}}
+// expected-note@* {{declaration}}
 #endif
 void use_a() { a(); }
 
@@ -43,6 +43,6 @@ void use_a() { a(); }
 #error B_H should not be defined
 #endif
 // expected-error@+3 {{must be imported from}}
-// expected-note@* {{previous declaration}}
+// expected-note@* {{declaration}}
 #endif
 void use_b() { b(); }

clang/test/Modules/normal-module-map.cpp

@@ -25,7 +25,7 @@ int testNestedUmbrellaA() {
 int testNestedUmbrellaBFail() {
   return nested_umbrella_b;
   // expected-error@-1{{declaration of 'nested_umbrella_b' must be imported from module 'nested_umbrella.b' before it is required}}
-  // expected-note@Inputs/normal-module-map/nested_umbrella/b.h:1{{previous}}
+  // expected-note@Inputs/normal-module-map/nested_umbrella/b.h:1{{here}}
 }
 
 @import nested_umbrella.b;

clang/test/Modules/stddef.c

@@ -5,8 +5,8 @@
 
 ptrdiff_t pdt;
 
-size_t st; // expected-error {{must be imported}}
-// expected-note@stddef.h:* {{previous}}
+size_t st; // expected-error {{missing '#include "include_again.h"'; 'size_t' must be declared before it is used}}
+// expected-note@stddef.h:* {{here}}
 
 #include "include_again.h"
 

clang/test/Modules/subframeworks.m

@@ -6,7 +6,7 @@
 
 void testSubFramework() {
   float *sf1 = sub_framework; // expected-error{{declaration of 'sub_framework' must be imported from module 'DependsOnModule.SubFramework' before it is required}}
-  // expected-note@Inputs/DependsOnModule.framework/Frameworks/SubFramework.framework/Headers/SubFramework.h:2 {{previous}}
+  // expected-note@Inputs/DependsOnModule.framework/Frameworks/SubFramework.framework/Headers/SubFramework.h:2 {{here}}
 }
 
 @import DependsOnModule.SubFramework;

clang/test/Modules/submodule-visibility-cycles.cpp

@@ -3,7 +3,7 @@
 
 #include "cycle1.h"
 C1 c1;
-C2 c2; // expected-error {{must be imported}}
+C2 c2; // expected-error {{missing '#include "cycle2.h"'; 'C2' must be declared}}
 // expected-note@cycle2.h:6 {{here}}
 
 #include "cycle2.h"

clang/test/Modules/submodule-visibility.cpp

@@ -22,7 +22,7 @@
 // The use of -fmodule-name=x causes us to textually include the above headers.
 // The submodule visibility rules are still applied in this case.
 //
-// expected-error@b.h:1 {{declaration of 'n' must be imported from module 'x.a'}}
+// expected-error@b.h:1 {{missing '#include "a.h"'; 'n' must be declared}}
 // expected-note@a.h:1 {{here}}
 #endif
 

clang/test/Modules/submodules-merge-defs.cpp

@@ -15,67 +15,55 @@
 #endif
 
 A pre_a;
-#ifdef IMPORT_USE_2
-// expected-error-re@-2 {{must be imported from one of {{.*}}stuff.use{{.*}}stuff.use-2}}
-#elif EARLY_INDIRECT_INCLUDE
-// expected-error@-4 {{must be imported from module 'merged-defs'}}
-#else
-// expected-error@-6 {{must be imported from module 'stuff.use'}}
-#endif
+// expected-error-re@-1 {{missing '#include "{{.*}}-defs.h"'; 'A' must be declared}}
 // expected-note@defs.h:1 +{{here}}
 extern class A pre_a2;
-int pre_use_a = use_a(pre_a2); // expected-error 2{{'A' must be imported}} expected-error {{'use_a' must be imported}}
+int pre_use_a = use_a(pre_a2); // expected-error 2{{'A' must be defined}} expected-error {{'use_a' must be declared}}
 // expected-note@defs.h:2 +{{here}}
 
-B::Inner2 pre_bi; // expected-error +{{must be imported}}
+B::Inner2 pre_bi; // expected-error +{{must be declared}} expected-error +{{must be defined}}
 // expected-note@defs.h:4 +{{here}}
 // expected-note@defs.h:17 +{{here}}
-void pre_bfi(B b) { // expected-error +{{must be imported}}
+void pre_bfi(B b) { // expected-error +{{must be declared}}
   b.f<int>();
 }
 
-C_Base<1> pre_cb1; // expected-error +{{must be imported}}
+C_Base<1> pre_cb1; // expected-error +{{must be declared}} expected-error +{{must be defined}}
 // expected-note@defs.h:23 +{{here}}
-C1 pre_c1; // expected-error +{{must be imported}}
+C1 pre_c1; // expected-error +{{must be declared}}
 // expected-note@defs.h:25 +{{here}}
-C2 pre_c2; // expected-error +{{must be imported}}
+C2 pre_c2; // expected-error +{{must be declared}}
 // expected-note@defs.h:26 +{{here}}
 
-D::X pre_dx; // expected-error +{{must be imported}}
+D::X pre_dx; // expected-error +{{must be declared}} expected-error +{{must be defined}}
 // expected-note@defs.h:28 +{{here}}
 // expected-note@defs.h:29 +{{here}}
 int pre_use_dx = use_dx(pre_dx); // ignored; pre_dx is invalid
 
-int pre_e = E(0); // expected-error {{must be imported}}
+int pre_e = E(0); // expected-error {{must be declared}}
 // expected-note@defs.h:32 +{{here}}
 
-int pre_ff = F<int>().f(); // expected-error +{{must be imported}}
-int pre_fg = F<int>().g<int>(); // expected-error +{{must be imported}}
+int pre_ff = F<int>().f(); // expected-error +{{must be declared}}
+int pre_fg = F<int>().g<int>(); // expected-error +{{must be declared}}
 // expected-note@defs.h:34 +{{here}}
 
-G::A pre_ga // expected-error +{{must be imported}}
-  = G::a; // expected-error +{{must be imported}}
+G::A pre_ga // expected-error +{{must be declared}}
+  = G::a; // expected-error +{{must be declared}}
 // expected-note@defs.h:49 +{{here}}
 // expected-note@defs.h:50 +{{here}}
-decltype(G::h) pre_gh = G::h; // expected-error +{{must be imported}}
+decltype(G::h) pre_gh = G::h; // expected-error +{{must be declared}} expected-error +{{must be defined}}
 // expected-note@defs.h:51 +{{here}}
 
-int pre_h = H(); // expected-error +{{must be imported}}
+int pre_h = H(); // expected-error +{{must be declared}}
 // expected-note@defs.h:56 +{{here}}
-using pre_i = I<>; // expected-error +{{must be imported}}
+using pre_i = I<>; // expected-error +{{must be declared}} expected-error +{{default argument of 'I' must be defined}}
 // expected-note@defs.h:57 +{{here}}
 
-J<> pre_j; // expected-error {{declaration of 'J' must be imported}}
-#ifdef IMPORT_USE_2
-// expected-error-re@-2 {{default argument of 'J' must be imported from one of {{.*}}stuff.use-2{{.*}}stuff.use}}
-#elif EARLY_INDIRECT_INCLUDE
-// expected-error@-4 {{default argument of 'J' must be imported from module 'merged-defs'}}
-#else
-// expected-error@-6 {{default argument of 'J' must be imported from module 'stuff.use'}}
-#endif
+J<> pre_j; // expected-error {{'J' must be declared}}
+// expected-error-re@-1 {{missing '#include "{{.*}}.h"'; default argument of 'J' must be defined before it is used}}
 // expected-note@defs.h:58 +{{here}}
 
-ScopedEnum pre_scopedenum; // expected-error {{must be imported}}
+ScopedEnum pre_scopedenum; // expected-error {{must be declared}}
 // expected-note@defs.h:105 0-1{{here}}
 // expected-note@defs.h:106 0-1{{here}}
 enum ScopedEnum : int;

clang/test/Modules/submodules.cpp

@@ -8,8 +8,8 @@ vector<int> vi;
 
 // Note: remove_reference is not visible yet.
 remove_reference<int&>::type *int_ptr = 0; // expected-error{{declaration of 'remove_reference' must be imported from module 'std.type_traits' before it is required}}
-// expected-note@Inputs/submodules/type_traits.h:2{{previous}}
-// expected-note@Inputs/submodules/hash_map.h:1{{previous}}
+// expected-note@Inputs/submodules/type_traits.h:2{{not visible}}
+// expected-note@Inputs/submodules/hash_map.h:1{{not visible}}
 
 @import std.typetraits; // expected-error{{no submodule named 'typetraits' in module 'std'; did you mean 'type_traits'?}}
 

clang/test/Modules/suggest-include.cpp

@@ -3,23 +3,25 @@
 
 #include "empty.h" // import the module file
 
-// expected-note@usetextual1.h:2 {{previous}}
-// expected-note@textual2.h:1 {{previous}}
-// expected-note@textual3.h:1 {{previous}}
-// expected-note@textual4.h:1 {{previous}}
-// expected-note@textual5.h:1 {{previous}}
-// expected-note@private1.h:1 {{previous}}
-// expected-note@private2.h:1 {{previous}}
-// expected-note@private3.h:1 {{previous}}
+// expected-note@usetextual1.h:2 {{here}}
+// expected-note@textual2.h:1 {{here}}
+// expected-note@textual3.h:1 {{here}}
+// expected-note@textual4.h:1 {{here}}
+// expected-note@textual5.h:1 {{here}}
+// expected-note@private1.h:1 {{here}}
+// expected-note@private2.h:1 {{here}}
+// expected-note@private3.h:1 {{here}}
 
 void f() {
   (void)::usetextual1; // expected-error {{missing '#include "usetextual1.h"'}}
   (void)::usetextual2; // expected-error {{missing '#include "usetextual2.h"'}}
   (void)::textual3; // expected-error-re {{{{^}}missing '#include "usetextual3.h"'}}
-  // Don't suggest a #include that includes the entity via a path that leaves
-  // the module. In that case we can't be sure that we've picked the right header.
-  (void)::textual4; // expected-error-re {{{{^}}declaration of 'textual4'}}
-  (void)::textual5; // expected-error-re {{{{^}}declaration of 'textual5'}}
+  // If the declaration is in an include-guarded header, make sure we suggest
+  // including that rather than importing a module. In this case, there could
+  // be more than one module, and the module name we picked is almost certainly
+  // wrong.
+  (void)::textual4; // expected-error {{missing '#include "usetextual4.h"'; 'textual4' must be declared before it is used}}
+  (void)::textual5; // expected-error {{missing '#include "usetextual5.h"'; 'textual5' must be declared before it is used}}
 
   // Don't suggest #including a private header.
   // FIXME: We could suggest including "useprivate1.h" here, as it's the only

clang/test/Modules/tag-injection.c

@@ -14,5 +14,5 @@ void f(struct a *p);
 // "compatible types" rule.
 void g(struct b *p);
 
-struct b b; // expected-error {{definition of 'b' must be imported from module 'X.b' before it is required}}
+struct b b; // expected-error {{'b' must be defined before it is used}}
 // expected-note@b.h:1 {{here}}

clang/test/Modules/tag-injection.cpp

@@ -21,5 +21,5 @@ namespace N {
   };
 }
 
-X x; // expected-error {{definition of 'X' must be imported from module 'X.b' before it is required}}
+X x; // expected-error {{'X' must be defined before it is used}}
 // expected-note@b.h:1 {{here}}

clang/test/Modules/template-default-args.cpp

@@ -37,10 +37,10 @@ extern C<> c;
 D<> d;
 E<> e;
 F<> f;
-G<> g; // expected-error {{default argument of 'G' must be imported from module 'X.A' before it is required}}
-// expected-note@a.h:7 {{default argument declared here}}
-H<> h; // expected-error {{default argument of 'H' must be imported from module 'X.A' before it is required}}
-// expected-note@a.h:8 {{default argument declared here}}
+G<> g; // expected-error {{missing '#include "a.h"'; default argument of 'G' must be defined before it is used}}
+// expected-note@a.h:7 {{default argument declared here is not reachable}}
+H<> h; // expected-error {{missing '#include "a.h"'; default argument of 'H' must be defined before it is used}}
+// expected-note@a.h:8 {{default argument declared here is not reachable}}
 I<> i;
 L<> *l;
 END

clang/test/Modules/undefined-type-fixit1.cpp

@@ -5,8 +5,8 @@
 #include "public2.h"
 #include "public2sub.h"
 
-use_this1 client_variable1; // expected-error{{declaration of 'use_this1' must be imported from module 'public1' before it is required}}
+use_this1 client_variable1; // expected-error{{'use_this1' must be declared}}
 use_this2 client_variable2;
 use_this2sub client_variable2sub;
 
-// expected-note@Inputs/undefined-type-fixit/public1.h:4 {{previous declaration is here}}
+// expected-note@Inputs/undefined-type-fixit/public1.h:4 {{declaration here is not visible}}

clang/test/Modules/visibility-in-instantiation.cpp

@@ -47,5 +47,5 @@ void g() {
 
   ST<int>::f(); // expected-error {{must be imported from module 'M.B'}}
   foo(X<int>()); // expected-error {{must be imported from module 'M.C'}}
-  // expected-note@* 2{{previous declaration is here}}
+  // expected-note@* 2{{declaration here is not visible}}
 }

clang/test/SemaCXX/compare-modules-cxx2a.cpp

@@ -22,12 +22,12 @@ auto va = A() <=> A(); // expected-note {{required here}}
 
 #pragma clang module import compare.other
 
-// expected-note@std-compare.h:* 2+{{previous definition}}
+// expected-note@std-compare.h:* 2+{{not reachable}}
 
-void b() { void(0 <=> 0); } // expected-error 1+{{definition of 'strong_ordering' must be imported}}
+void b() { void(0 <=> 0); } // expected-error 1+{{missing '#include "std-compare.h"'; 'strong_ordering' must be defined}}
 
 struct B {
-  CC operator<=>(const B&) const = default; // expected-error 1+{{definition of 'strong_ordering' must be imported}}
+  CC operator<=>(const B&) const = default; // expected-error 1+{{missing '#include "std-compare.h"'; 'strong_ordering' must be defined}}
 };
 auto vb = B() <=> B(); // expected-note {{required here}}
 

clang/test/SemaCXX/modules-ts.cppm

@@ -43,7 +43,7 @@ namespace N {
 export struct T {} t;
 #elif TEST == 3
 int use_a = a; // expected-error {{declaration of 'a' must be imported from module 'foo' before it is required}}
-// expected-note@-13 {{previous}}
+// expected-note@-13 {{declaration here is not visible}}
 
 #undef foo
 import foo;

clang/tools/libclang/Indexing.cpp

@@ -227,7 +227,8 @@ private:
   }
 
   bool isParsedOnceInclude(const FileEntry *FE) {
-    return PP.getHeaderSearchInfo().isFileMultipleIncludeGuarded(FE);
+    return PP.getHeaderSearchInfo().isFileMultipleIncludeGuarded(FE) ||
+           PP.getHeaderSearchInfo().hasFileBeenImported(FE);
   }
 };