[modules] Better support for redefinitions of an entity from the same module.

Support this across module save/reload and extend the 'missing import'
diagnostics with a list of providing modules.

llvm-svn: 239750

10 files changed, 73 insertions, 47 deletions

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index d318a209282..71cba0183b8 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7637,9 +7637,11 @@ def err_module_private_local_class : Error<
   "local %select{struct|interface|union|class|enum}0 cannot be declared "
   "__module_private__">;
 def err_module_private_declaration : Error<
-  "declaration of %0 must be imported from module '%1' before it is required">;
-def err_module_private_definition : Error<
-  "definition of %0 must be imported from module '%1' before it is required">;
+  "%select{declaration|definition}0 of %1 must be imported from "
+  "module '%2' before it is required">;
+def err_module_private_declaration_multiple : Error<
+  "%select{declaration|definition}0 of %1 must be imported from "
+  "one of the following modules before it is required:%2">;
 def err_module_import_in_extern_c : Error<
   "import of C++ module '%0' appears within extern \"C\" language linkage "
   "specification">;
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 685a1ab2435..691f21d4335 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -1730,6 +1730,11 @@ public:
   void createImplicitModuleImportForErrorRecovery(SourceLocation Loc,
                                                   Module *Mod);
 
+  /// \brief Diagnose that the specified declaration needs to be visible but
+  /// isn't, and suggest a module import that would resolve the problem.
+  void diagnoseMissingImport(SourceLocation Loc, NamedDecl *Decl,
+                             bool NeedDefinition, bool Recover = true);
+
   /// \brief Retrieve a suitable printing policy.
   PrintingPolicy getPrintingPolicy() const {
     return getPrintingPolicy(Context, PP);
diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index 202f027f6f8..b5ef3a42d17 100644
--- a/clang/lib/Sema/SemaLookup.cpp
+++ b/clang/lib/Sema/SemaLookup.cpp
@@ -4661,6 +4661,50 @@ static NamedDecl *getDefinitionToImport(NamedDecl *D) {
   return nullptr;
 }
 
+void Sema::diagnoseMissingImport(SourceLocation Loc, NamedDecl *Decl,
+                                 bool NeedDefinition, bool Recover) {
+  assert(!isVisible(Decl) && "missing import for non-hidden decl?");
+
+  // Suggest importing a module providing the definition of this entity, if
+  // possible.
+  NamedDecl *Def = getDefinitionToImport(Decl);
+  if (!Def)
+    Def = Decl;
+
+  // FIXME: Add a Fix-It that imports the corresponding module or includes
+  // the header.
+  Module *Owner = getOwningModule(Decl);
+  assert(Owner && "definition of hidden declaration is not in a module");
+
+  auto Merged = Context.getModulesWithMergedDefinition(Decl);
+  if (!Merged.empty()) {
+    std::string ModuleList;
+    ModuleList += "\n        ";
+    ModuleList += Owner->getFullModuleName();
+    unsigned N = 0;
+    for (Module *M : Merged) {
+      ModuleList += "\n        ";
+      if (++N == 5 && Merged.size() != N) {
+        ModuleList += "[...]";
+        break;
+      }
+      ModuleList += M->getFullModuleName();
+    }
+
+    Diag(Loc, diag::err_module_private_declaration_multiple)
+      << NeedDefinition << Decl << ModuleList;
+  } else {
+    Diag(Loc, diag::err_module_private_declaration)
+      << NeedDefinition << Decl << Owner->getFullModuleName();
+  }
+  Diag(Decl->getLocation(), NeedDefinition ? diag::note_previous_definition
+                                           : diag::note_previous_declaration);
+
+  // Try to recover by implicitly importing this module.
+  if (Recover)
+    createImplicitModuleImportForErrorRecovery(Loc, Owner);
+}
+
 /// \brief Diagnose a successfully-corrected typo. Separated from the correction
 /// itself to allow external validation of the result, etc.
 ///
@@ -4687,23 +4731,8 @@ void Sema::diagnoseTypo(const TypoCorrection &Correction,
     NamedDecl *Decl = Correction.getCorrectionDecl();
     assert(Decl && "import required but no declaration to import");
 
-    // Suggest importing a module providing the definition of this entity, if
-    // possible.
-    NamedDecl *Def = getDefinitionToImport(Decl);
-    if (!Def)
-      Def = Decl;
-    Module *Owner = getOwningModule(Def);
-    assert(Owner && "definition of hidden declaration is not in a module");
-
-    Diag(Correction.getCorrectionRange().getBegin(),
-         diag::err_module_private_declaration)
-      << Def << Owner->getFullModuleName();
-    Diag(Def->getLocation(), diag::note_previous_declaration);
-
-    // Recover by implicitly importing this module.
-    if (ErrorRecovery)
-      createImplicitModuleImportForErrorRecovery(
-          Correction.getCorrectionRange().getBegin(), Owner);
+    diagnoseMissingImport(Correction.getCorrectionRange().getBegin(), Decl,
+                          /*NeedDefinition*/ false, ErrorRecovery);
     return;
   }
 
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 5734f3b2ed5..d3787ec44b1 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -5239,20 +5239,8 @@ bool Sema::RequireCompleteTypeImpl(SourceLocation Loc, QualType T,
     // If we know about the definition but it is not visible, complain.
     NamedDecl *SuggestedDef = nullptr;
     if (!Diagnoser.Suppressed && Def &&
-        !hasVisibleDefinition(Def, &SuggestedDef)) {
-      // Suppress this error outside of a SFINAE context if we've already
-      // emitted the error once for this type. There's no usefulness in
-      // repeating the diagnostic.
-      // FIXME: Add a Fix-It that imports the corresponding module or includes
-      // the header.
-      Module *Owner = getOwningModule(SuggestedDef);
-      Diag(Loc, diag::err_module_private_definition)
-        << T << Owner->getFullModuleName();
-      Diag(SuggestedDef->getLocation(), diag::note_previous_definition);
-
-      // Try to recover by implicitly importing this module.
-      createImplicitModuleImportForErrorRecovery(Loc, Owner);
-    }
+        !hasVisibleDefinition(Def, &SuggestedDef))
+      diagnoseMissingImport(Loc, SuggestedDef, /*NeedDefinition*/true);
 
     // We lock in the inheritance model once somebody has asked us to ensure
     // that a pointer-to-member type is complete.
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 7e1bd07f1ee..5bb0bec4f56 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -5764,8 +5764,5 @@ void ASTWriter::DeclarationMarkedOpenMPThreadPrivate(const Decl *D) {
 void ASTWriter::RedefinedHiddenDefinition(const NamedDecl *D, Module *M) {
   assert(!WritingAST && "Already writing the AST!");
   assert(D->isHidden() && "expected a hidden declaration");
-  if (!D->isFromASTFile())
-    return;
-
   DeclUpdates[D].push_back(DeclUpdate(UPD_DECL_EXPORTED, M));
 }
diff --git a/clang/test/Modules/Inputs/submodules-merge-defs/defs.h b/clang/test/Modules/Inputs/submodules-merge-defs/defs.h
index ad3711f073f..b98cbc3a764 100644
--- a/clang/test/Modules/Inputs/submodules-merge-defs/defs.h
+++ b/clang/test/Modules/Inputs/submodules-merge-defs/defs.h
@@ -31,7 +31,7 @@ template<typename T> struct F {
 template<typename T> int F<T>::f() { return 0; }
 template<typename T> template<typename U> int F<T>::g() { return 0; }
 template<typename T> int F<T>::n = 0;
-template<> template<typename U> int F<char>::g() { return 0; }
+//template<> template<typename U> int F<char>::g() { return 0; } // FIXME: Re-enable this once we support merging member specializations.
 template<> struct F<void> { int h(); };
 inline int F<void>::h() { return 0; }
 template<typename T> struct F<T *> { int i(); };
diff --git a/clang/test/Modules/Inputs/submodules-merge-defs/module.modulemap b/clang/test/Modules/Inputs/submodules-merge-defs/module.modulemap
index f8ae60fe44f..3b5493e2b8b 100644
--- a/clang/test/Modules/Inputs/submodules-merge-defs/module.modulemap
+++ b/clang/test/Modules/Inputs/submodules-merge-defs/module.modulemap
@@ -2,6 +2,7 @@ module "stuff" {
   textual header "defs.h"
   module "empty" { header "empty.h" }
   module "use" { header "use-defs.h" }
+  module "use-2" { requires use_defs_twice header "use-defs-2.h" }
 }
 
 module "redef" {
diff --git a/clang/test/Modules/Inputs/submodules-merge-defs/use-defs-2.h b/clang/test/Modules/Inputs/submodules-merge-defs/use-defs-2.h
new file mode 100644
index 00000000000..31c69c6a447
--- /dev/null
+++ b/clang/test/Modules/Inputs/submodules-merge-defs/use-defs-2.h
@@ -0,0 +1 @@
+#include "defs.h"
diff --git a/clang/test/Modules/module-private.cpp b/clang/test/Modules/module-private.cpp
index 9213a0f20cb..478d36dd4d5 100644
--- a/clang/test/Modules/module-private.cpp
+++ b/clang/test/Modules/module-private.cpp
@@ -14,7 +14,7 @@ void test() {
 int test_broken() {
   HiddenStruct hidden; // \
   // expected-error{{must use 'struct' tag to refer to type 'HiddenStruct' in this scope}} \
-  // expected-error{{definition of 'struct HiddenStruct' must be imported}}
+  // expected-error{{definition of 'HiddenStruct' must be imported}}
   // expected-note@Inputs/module_private_left.h:3 {{previous definition is here}}
 
   Integer i; // expected-error{{unknown type name 'Integer'}}
diff --git a/clang/test/Modules/submodules-merge-defs.cpp b/clang/test/Modules/submodules-merge-defs.cpp
index e942335151e..52b12ef850a 100644
--- a/clang/test/Modules/submodules-merge-defs.cpp
+++ b/clang/test/Modules/submodules-merge-defs.cpp
@@ -4,6 +4,7 @@
 // RUN: %clang_cc1 -x c++ -std=c++11 -fmodules-cache-path=%t -fmodules -I %S/Inputs/submodules-merge-defs %s -verify -fno-modules-error-recovery -fmodules-local-submodule-visibility -DTEXTUAL
 // RUN: %clang_cc1 -x c++ -std=c++11 -fmodules-cache-path=%t -fmodules -I %S/Inputs/submodules-merge-defs %s -verify -fno-modules-error-recovery -fmodules-local-submodule-visibility
 // RUN: %clang_cc1 -x c++ -std=c++11 -fmodules-cache-path=%t -fmodule-maps -I %S/Inputs/submodules-merge-defs %s -verify -fno-modules-error-recovery -fmodules-local-submodule-visibility -DTEXTUAL -DEARLY_INDIRECT_INCLUDE
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules-cache-path=%t -fmodules -I %S/Inputs/submodules-merge-defs %s -verify -fno-modules-error-recovery -fmodules-local-submodule-visibility -fmodule-feature use_defs_twice -DIMPORT_USE_2
 
 // Trigger import of definitions, but don't make them visible.
 #include "empty.h"
@@ -11,7 +12,14 @@
 #include "indirect.h"
 #endif
 
-A pre_a; // expected-error {{must be imported}} expected-error {{must use 'struct'}}
+A pre_a; // expected-error {{must use 'struct'}}
+#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-note@defs.h:1 +{{here}}
 // expected-note@defs.h:2 +{{here}}
 int pre_use_a = use_a(pre_a); // expected-error {{'A' must be imported}} expected-error {{'use_a' must be imported}}
@@ -46,6 +54,8 @@ J<> pre_j; // expected-error {{must be imported}} expected-error {{too few}}
 // Make definitions from second module visible.
 #ifdef TEXTUAL
 #include "import-and-redefine.h"
+#elif defined IMPORT_USE_2
+#include "use-defs-2.h"
 #else
 #include "merged-defs.h"
 #endif
@@ -61,13 +71,6 @@ int post_use_dx = use_dx(post_dx);
 int post_e = E(0);
 int post_ff = F<char>().f();
 int post_fg = F<char>().g<int>();
-#ifdef EARLY_INDIRECT_INCLUDE
-// FIXME: Properly track the owning module for a member specialization.
-// expected-error@defs.h:34 {{redefinition}}
-// expected-note@defs.h:34 {{previous definition}}
-// expected-error@-5 {{no matching member function}}
-// expected-note@defs.h:34 {{substitution failure}}
-#endif
 J<> post_j;
 template<typename T, int N, template<typename> class K> struct J;
 J<> post_j2;