[c++2a] Improve diagnostic for use of declaration from another TU's

global module fragment.

We know that the declaration in question should have been introduced by
a '#include', so try to figure out which one and suggest it. Don't
suggest importing the global module fragment itself!

llvm-svn: 358631

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -9238,6 +9238,12 @@ def err_module_unimported_use_header : Error<
   "%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 "
+  "%select{declared|defined|defined|declared|declared}0 "
+  "before it is used">;
 def err_module_unimported_use_multiple : Error<
   "%select{declaration|definition|default argument|"
   "explicit specialization|partial specialization}0 of %1 must be imported "

clang/lib/Lex/PPDirectives.cpp

@@ -614,9 +614,16 @@ Preprocessor::getModuleHeaderToIncludeForDiagnostics(SourceLocation IncLoc,
                                                      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)
+  if ((getLangOpts().ObjC || getLangOpts().CPlusPlusModules ||
+       getLangOpts().ModulesTS) &&
+      !InGlobalModuleFragment)
     return nullptr;
 
   Module *TopM = M->getTopLevelModule();
@@ -633,6 +640,13 @@ 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))

clang/lib/Sema/SemaLookup.cpp

@@ -5073,7 +5073,7 @@ void Sema::diagnoseMissingImport(SourceLocation Loc, NamedDecl *Decl,
   auto Merged = Context.getModulesWithMergedDefinition(Def);
   OwningModules.insert(OwningModules.end(), Merged.begin(), Merged.end());
 
-  diagnoseMissingImport(Loc, Decl, Decl->getLocation(), OwningModules, MIK,
+  diagnoseMissingImport(Loc, Def, Def->getLocation(), OwningModules, MIK,
                         Recover);
 }
 
@@ -5093,12 +5093,58 @@ void Sema::diagnoseMissingImport(SourceLocation UseLoc, NamedDecl *Decl,
                                  MissingImportKind MIK, bool Recover) {
   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);
+  };
+
   // Weed out duplicates from module list.
   llvm::SmallVector<Module*, 8> UniqueModules;
   llvm::SmallDenseSet<Module*, 8> UniqueModuleSet;
-  for (auto *M : Modules)
+  for (auto *M : Modules) {
+    if (M->Kind == Module::GlobalModuleFragment)
+      continue;
     if (UniqueModuleSet.insert(M).second)
       UniqueModules.push_back(M);
+  }
+
+  if (UniqueModules.empty()) {
+    // All candidates were global module fragments. Try to suggest a #include.
+    const FileEntry *E =
+        PP.getModuleHeaderToIncludeForDiagnostics(UseLoc, Modules[0], DeclLoc);
+    // 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) : "");
+    // 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();
+    if (Recover)
+      createImplicitModuleImportForErrorRecovery(UseLoc, Modules[0]);
+    return;
+  }
+
   Modules = UniqueModules;
 
   if (Modules.size() > 1) {
@@ -5131,25 +5177,7 @@ void Sema::diagnoseMissingImport(SourceLocation UseLoc, NamedDecl *Decl,
       << (int)MIK << Decl << Modules[0]->getFullModuleName();
   }
 
-  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);
+  NotePrevious();
 
   // Try to recover by implicitly importing this module.
   if (Recover)

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

@@ -0,0 +1,86 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: echo '#ifndef FOO_H' > %t/foo.h
+// RUN: echo '#define FOO_H' >> %t/foo.h
+// RUN: echo 'extern int in_header;' >> %t/foo.h
+// RUN: echo '#endif' >> %t/foo.h
+// RUN: %clang_cc1 -std=c++2a -I%t -emit-module-interface -DINTERFACE %s -o %t.pcm
+// RUN: %clang_cc1 -std=c++2a -I%t -fmodule-file=%t.pcm -DIMPLEMENTATION %s -verify -fno-modules-error-recovery
+// RUN: %clang_cc1 -std=c++2a -I%t -fmodule-file=%t.pcm %s -verify -fno-modules-error-recovery
+
+#ifdef INTERFACE
+module;
+#include "foo.h"
+int global_module_fragment;
+export module A;
+export int exported;
+int not_exported;
+static int internal;
+
+module :private;
+int not_exported_private;
+static int internal_private;
+#else
+
+#ifdef IMPLEMENTATION
+module;
+#endif
+
+void test_early() {
+  in_header = 1; // expected-error {{missing '#include "foo.h"'; 'in_header' must be declared before it is used}}
+  // expected-note@*{{previous}}
+
+  global_module_fragment = 1; // expected-error {{missing '#include'; 'global_module_fragment' must be declared before it is used}}
+
+  exported = 1; // expected-error {{must be imported from module 'A'}}
+  // expected-note@p2.cpp:16 {{previous}}
+
+  not_exported = 1; // expected-error {{undeclared identifier}}
+
+  internal = 1; // expected-error {{undeclared identifier}}
+
+  not_exported_private = 1; // expected-error {{undeclared identifier}}
+
+  internal_private = 1; // expected-error {{undeclared identifier}}
+}
+
+#ifdef IMPLEMENTATION
+module A;
+#else
+import A;
+#endif
+
+void test_late() {
+  in_header = 1; // expected-error {{missing '#include "foo.h"'; 'in_header' must be declared before it is used}}
+  // expected-note@*{{previous}}
+
+  global_module_fragment = 1; // expected-error {{missing '#include'; 'global_module_fragment' must be declared before it is used}}
+
+  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}}
+#endif
+
+  internal = 1;
+#ifndef IMPLEMENTATION
+  // FIXME: should not be visible here
+  // expected-error@-3 {{undeclared identifier}}
+#endif
+
+  not_exported_private = 1;
+#ifndef IMPLEMENTATION
+  // FIXME: should not be visible here
+  // expected-error@-3 {{undeclared identifier}}
+#endif
+
+  internal_private = 1;
+#ifndef IMPLEMENTATION
+  // FIXME: should not be visible here
+  // expected-error@-3 {{undeclared identifier}}
+#endif
+}
+
+#endif