From ca2ab45341c448284cf93770018c717810575f86 Mon Sep 17 00:00:00 2001
From: Douglas Gregor <dgregor@apple.com>
Date: Sat, 12 Jan 2013 01:29:50 +0000
Subject: [PATCH] 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
---
include/clang/AST/DeclBase.h | 14 +++++++++++++-
include/clang/AST/ExternalASTSource.h | 6 +++++-
include/clang/Basic/DiagnosticSemaKinds.td | 2 +-
include/clang/Frontend/ASTUnit.h | 4 ++++
include/clang/Frontend/CompilerInstance.h | 4 ++++
include/clang/Lex/ModuleLoader.h | 4 ++++
include/clang/Sema/Lookup.h | 21 +++++++++++++++++----
include/clang/Sema/Sema.h | 14 ++++++++------
include/clang/Serialization/ASTReader.h | 7 ++++++-
lib/AST/DeclBase.cpp | 5 +++++
lib/Frontend/CompilerInstance.cpp | 6 ++++++
lib/Sema/SemaDecl.cpp | 12 ++++++++++++
lib/Sema/SemaType.cpp | 11 ++++++++---
lib/Serialization/ASTReader.cpp | 6 +++++-
test/Modules/decldef.mm | 13 +++----------
15 files changed, 101 insertions(+), 28 deletions(-)
diff --git a/include/clang/AST/DeclBase.h b/include/clang/AST/DeclBase.h
index 823340aeaf0..c91284d54a0 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 871912ac09d..501f8a796df 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 0b1917a46d2..7d6fc992644 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 2e929d98ce5..3701cdc2e44 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 ec2fead280a..2d6a8702dc6 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 3a5f41d510b..f086c082f9d 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 7dab0ef170f..68b36e7d3ca 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 5971ff57884..1f56acee64f 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 816dfef024f..61d77b0a751 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 5dccaac0205..d8b63611fd1 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 f31301c6e9f..5b2be919677 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 0cf12382207..c3a2ad11230 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 60db55aff33..59c1dea8ff5 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 a4436d16716..986eee1c951 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 c99fdea0d84..e164aa7e080 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
}
--
GitLab