Provide Decl::getOwningModule(), which determines the (sub)module in

which a particular declaration resides. Use this information to
customize the "definition of 'blah' must be imported from another
module" diagnostic with the module the user actually has to
import. Additionally, recover by importing that module, so we don't
complain about other names in that module.

Still TODO: coming up with decent Fix-Its for these cases, and expand
this recovery approach for other name lookup failures.



git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@172290 91177308-0d34-0410-b5e6-96231b3b80d8

15 files changed, 101 insertions, 28 deletions

diff --git a/include/clang/AST/DeclBase.h b/include/clang/AST/DeclBase.h
index 823340aeaf..c91284d54a 100644
--- a/include/clang/AST/DeclBase.h
+++ b/include/clang/AST/DeclBase.h
@@ -32,6 +32,7 @@ class DependentDiagnostic;
 class EnumDecl;
 class FunctionDecl;
 class LinkageSpecDecl;
+class Module;
 class NamedDecl;
 class NamespaceDecl;
 class ObjCCategoryDecl;
@@ -593,7 +594,18 @@ public:
     
     return 0;
   }
-  
+
+private:
+  Module *getOwningModuleSlow() const;
+
+public:
+  Module *getOwningModule() const {
+    if (!isFromASTFile())
+      return 0;
+
+    return getOwningModuleSlow();
+  }
+
   unsigned getIdentifierNamespace() const {
     return IdentifierNamespace;
   }
diff --git a/include/clang/AST/ExternalASTSource.h b/include/clang/AST/ExternalASTSource.h
index 871912ac09..501f8a796d 100644
--- a/include/clang/AST/ExternalASTSource.h
+++ b/include/clang/AST/ExternalASTSource.h
@@ -25,6 +25,7 @@ class CXXBaseSpecifier;
 class DeclarationName;
 class ExternalSemaSource; // layering violation required for downcasting
 class FieldDecl;
+class Module;
 class NamedDecl;
 class RecordDecl;
 class Selector;
@@ -131,9 +132,12 @@ public:
   /// \brief Ensures that the table of all visible declarations inside this
   /// context is up to date.
   ///
-  /// The default implementation of this functino is a no-op.
+  /// The default implementation of this function is a no-op.
   virtual void completeVisibleDeclsMap(const DeclContext *DC);
 
+  /// \brief Retrieve the module that corresponds to the given module ID.
+  virtual Module *getModule(unsigned ID) { return 0; }
+
   /// \brief Finds all declarations lexically contained within the given
   /// DeclContext, after applying an optional filter predicate.
   ///
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 0b1917a46d..7d6fc99264 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6082,7 +6082,7 @@ def err_module_private_local_class : Error<
   "local %select{struct|interface|union|class|enum}0 cannot be declared "
   "__module_private__">;
 def err_module_private_definition : Error<
-  "definition of %0 must be imported before it is required">;
+  "definition of %0 must be imported from module '%1' before it is required">;
 }
 
 let CategoryName = "Documentation Issue" in {
diff --git a/include/clang/Frontend/ASTUnit.h b/include/clang/Frontend/ASTUnit.h
index 2e929d98ce..3701cdc2e4 100644
--- a/include/clang/Frontend/ASTUnit.h
+++ b/include/clang/Frontend/ASTUnit.h
@@ -837,6 +837,10 @@ public:
     // ASTUnit doesn't know how to load modules (not that this matters).
     return ModuleLoadResult();
   }
+
+  virtual void makeModuleVisible(Module *Mod,
+                                 Module::NameVisibilityKind Visibility) { }
+
 };
 
 } // namespace clang
diff --git a/include/clang/Frontend/CompilerInstance.h b/include/clang/Frontend/CompilerInstance.h
index ec2fead280..2d6a8702dc 100644
--- a/include/clang/Frontend/CompilerInstance.h
+++ b/include/clang/Frontend/CompilerInstance.h
@@ -649,6 +649,10 @@ public:
                                       ModuleIdPath Path,
                                       Module::NameVisibilityKind Visibility,
                                       bool IsInclusionDirective);
+
+  virtual void makeModuleVisible(Module *Mod,
+                                 Module::NameVisibilityKind Visibility);
+
 };
 
 } // end namespace clang
diff --git a/include/clang/Lex/ModuleLoader.h b/include/clang/Lex/ModuleLoader.h
index 3a5f41d510..f086c082f9 100644
--- a/include/clang/Lex/ModuleLoader.h
+++ b/include/clang/Lex/ModuleLoader.h
@@ -80,6 +80,10 @@ public:
                                       ModuleIdPath Path,
                                       Module::NameVisibilityKind Visibility,
                                       bool IsInclusionDirective) = 0;
+
+  /// \brief Make the given module visible.
+  virtual void makeModuleVisible(Module *Mod,
+                                 Module::NameVisibilityKind Visibility) = 0;
 };
   
 }
diff --git a/include/clang/Sema/Lookup.h b/include/clang/Sema/Lookup.h
index 7dab0ef170..68b36e7d3c 100644
--- a/include/clang/Sema/Lookup.h
+++ b/include/clang/Sema/Lookup.h
@@ -138,7 +138,8 @@ public:
       IDNS(0),
       Redecl(Redecl != Sema::NotForRedeclaration),
       HideTags(true),
-      Diagnose(Redecl == Sema::NotForRedeclaration)
+      Diagnose(Redecl == Sema::NotForRedeclaration),
+      AllowHidden(Redecl == Sema::ForRedeclaration)
   {
     configure();
   }
@@ -158,7 +159,8 @@ public:
       IDNS(0),
       Redecl(Redecl != Sema::NotForRedeclaration),
       HideTags(true),
-      Diagnose(Redecl == Sema::NotForRedeclaration)
+      Diagnose(Redecl == Sema::NotForRedeclaration),
+      AllowHidden(Redecl == Sema::ForRedeclaration)
   {
     configure();
   }
@@ -176,7 +178,8 @@ public:
       IDNS(Other.IDNS),
       Redecl(Other.Redecl),
       HideTags(Other.HideTags),
-      Diagnose(false)
+      Diagnose(false),
+      AllowHidden(Other.AllowHidden)
   {}
 
   ~LookupResult() {
@@ -214,10 +217,16 @@ public:
     return Redecl;
   }
 
+  /// \brief Specify whether hidden declarations are visible, e.g.,
+  /// for recovery reasons.
+  void setAllowHidden(bool AH) {
+    AllowHidden = AH;
+  }
+
   /// \brief Determine whether this lookup is permitted to see hidden
   /// declarations, such as those in modules that have not yet been imported.
   bool isHiddenDeclarationVisible() const {
-    return Redecl || LookupKind == Sema::LookupTagName;
+    return AllowHidden || LookupKind == Sema::LookupTagName;
   }
   
   /// Sets whether tag declarations should be hidden by non-tag
@@ -483,6 +492,7 @@ public:
   /// \brief Change this lookup's redeclaration kind.
   void setRedeclarationKind(Sema::RedeclarationKind RK) {
     Redecl = RK;
+    AllowHidden = (RK == Sema::ForRedeclaration);
     configure();
   }
 
@@ -644,6 +654,9 @@ private:
   bool HideTags;
 
   bool Diagnose;
+
+  /// \brief True if we should allow hidden declarations to be 'visible'.
+  bool AllowHidden;
 };
 
   /// \brief Consumes visible declarations found when searching for
diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 5971ff5788..1f56acee64 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -275,12 +275,6 @@ public:
   /// This is only necessary for issuing pretty diagnostics.
   ExtVectorDeclsType ExtVectorDecls;
 
-  /// \brief The set of types for which we have already complained about the
-  /// definitions being hidden.
-  ///
-  /// This set is used to suppress redundant diagnostics.
-  llvm::SmallPtrSet<NamedDecl *, 4> HiddenDefinitions;
-
   /// FieldCollector - Collects CXXFieldDecls during parsing of C++ classes.
   OwningPtr<CXXFieldCollector> FieldCollector;
 
@@ -1456,6 +1450,14 @@ public:
   DeclResult ActOnModuleImport(SourceLocation AtLoc, SourceLocation ImportLoc,
                                ModuleIdPath Path);
 
+  /// \brief Create an implicit import of the given module at the given
+  /// source location.
+  ///
+  /// This routine is typically used for error recovery, when the entity found
+  /// by name lookup is actually hidden within a module that we know about but
+  /// the user has forgotten to import.
+  void createImplicitModuleImport(SourceLocation Loc, Module *Mod);
+
   /// \brief Retrieve a suitable printing policy.
   PrintingPolicy getPrintingPolicy() const {
     return getPrintingPolicy(Context, PP);
diff --git a/include/clang/Serialization/ASTReader.h b/include/clang/Serialization/ASTReader.h
index 816dfef024..61d77b0a75 100644
--- a/include/clang/Serialization/ASTReader.h
+++ b/include/clang/Serialization/ASTReader.h
@@ -1558,7 +1558,12 @@ public:
   /// \brief Retrieve the submodule that corresponds to a global submodule ID.
   ///
   Module *getSubmodule(serialization::SubmoduleID GlobalID);
-  
+
+  /// \brief Retrieve the module that corresponds to the given module ID.
+  ///
+  /// Note: overrides method in ExternalASTSource
+  virtual Module *getModule(unsigned ID);
+
   /// \brief Retrieve a selector from the given module with its local ID
   /// number.
   Selector getLocalSelector(ModuleFile &M, unsigned LocalID);
diff --git a/lib/AST/DeclBase.cpp b/lib/AST/DeclBase.cpp
index 5dccaac020..d8b63611fd 100644
--- a/lib/AST/DeclBase.cpp
+++ b/lib/AST/DeclBase.cpp
@@ -59,6 +59,11 @@ void *Decl::AllocateDeserializedDecl(const ASTContext &Context,
   return Result;
 }
 
+Module *Decl::getOwningModuleSlow() const {
+  assert(isFromASTFile() && "Not from AST file?");
+  return getASTContext().getExternalSource()->getModule(getOwningModuleID());
+}
+
 const char *Decl::getDeclKindName() const {
   switch (DeclKind) {
   default: llvm_unreachable("Declaration not in DeclNodes.inc!");
diff --git a/lib/Frontend/CompilerInstance.cpp b/lib/Frontend/CompilerInstance.cpp
index f31301c6e9..5b2be91967 100644
--- a/lib/Frontend/CompilerInstance.cpp
+++ b/lib/Frontend/CompilerInstance.cpp
@@ -1201,3 +1201,9 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
   LastModuleImportResult = ModuleLoadResult(Module, false);
   return LastModuleImportResult;
 }
+
+void CompilerInstance::makeModuleVisible(Module *Mod,
+                                         Module::NameVisibilityKind Visibility){
+  ModuleManager->makeModuleVisible(Mod, Visibility);
+}
+
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 0cf1238220..c3a2ad1123 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -11173,6 +11173,18 @@ DeclResult Sema::ActOnModuleImport(SourceLocation AtLoc,
   return Import;
 }
 
+void Sema::createImplicitModuleImport(SourceLocation Loc, Module *Mod) {
+  // Create the implicit import declaration.
+  TranslationUnitDecl *TU = getASTContext().getTranslationUnitDecl();
+  ImportDecl *ImportD = ImportDecl::CreateImplicit(getASTContext(), TU,
+                                                   Loc, Mod, Loc);
+  TU->addDecl(ImportD);
+  Consumer.HandleImplicitImportDecl(ImportD);
+
+  // Make the module visible.
+  PP.getModuleLoader().makeModuleVisible(Mod, Module::AllVisible);
+}
+
 void Sema::ActOnPragmaRedefineExtname(IdentifierInfo* Name,
                                       IdentifierInfo* AliasName,
                                       SourceLocation PragmaLoc,
diff --git a/lib/Sema/SemaType.cpp b/lib/Sema/SemaType.cpp
index 60db55aff3..59c1dea8ff 100644
--- a/lib/Sema/SemaType.cpp
+++ b/lib/Sema/SemaType.cpp
@@ -4392,9 +4392,14 @@ bool Sema::RequireCompleteType(SourceLocation Loc, QualType T,
       // repeating the diagnostic.
       // FIXME: Add a Fix-It that imports the corresponding module or includes
       // the header.
-      if (isSFINAEContext() || HiddenDefinitions.insert(Def)) {
-        Diag(Loc, diag::err_module_private_definition) << T;
-        Diag(Def->getLocation(), diag::note_previous_definition);
+      Module *Owner = Def->getOwningModule();
+      Diag(Loc, diag::err_module_private_definition)
+        << T << Owner->getFullModuleName();
+      Diag(Def->getLocation(), diag::note_previous_definition);
+
+      if (!isSFINAEContext()) {
+        // Recover by implicitly importing this module.
+        createImplicitModuleImport(Loc, Owner);
       }
     }
 
diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp
index a4436d1671..986eee1c95 100644
--- a/lib/Serialization/ASTReader.cpp
+++ b/lib/Serialization/ASTReader.cpp
@@ -6200,7 +6200,11 @@ Module *ASTReader::getSubmodule(SubmoduleID GlobalID) {
   
   return SubmodulesLoaded[GlobalID - NUM_PREDEF_SUBMODULE_IDS];
 }
-                               
+
+Module *ASTReader::getModule(unsigned ID) {
+  return getSubmodule(ID);
+}
+
 Selector ASTReader::getLocalSelector(ModuleFile &M, unsigned LocalID) {
   return DecodeSelector(getGlobalSelectorID(M, LocalID));
 }
diff --git a/test/Modules/decldef.mm b/test/Modules/decldef.mm
index c99fdea0d8..e164aa7e08 100644
--- a/test/Modules/decldef.mm
+++ b/test/Modules/decldef.mm
@@ -2,13 +2,7 @@
 // RUN: %clang_cc1 -fmodules -I %S/Inputs -fmodule-cache-path %t %s -verify
 
 
-// in other file: expected-note{{previous definition is here}}
-
-
-
-
-
-// in other file: expected-note{{previous definition is here}}
+// In other file: expected-note {{previous definition is here}}
 
 @import decldef;
 A *a1; // expected-error{{unknown type name 'A'}}
@@ -19,10 +13,9 @@ A *a2;
 B *b;
 
 void testA(A *a) {
-  a->ivar = 17; // expected-error{{definition of 'A' must be imported before it is required}}
+  a->ivar = 17; // expected-error{{definition of 'A' must be imported from module 'decldef.Def' before it is required}}
 }
 
 void testB() {
-  B b; // expected-error{{definition of 'B' must be imported before it is required}}
-  B b2; // Note: the reundant error was silenced.
+  B b; // Note: redundant error silenced
 }