Hi doug.gregor,
http://llvm-reviews.chandlerc.com/D1204
CHANGE SINCE LAST DIFF
http://llvm-reviews.chandlerc.com/D1204?vs=2995&id=2996#toc
Files:
lib/Basic/Module.cpp
lib/Sema/SemaTemplateInstantiate.cpp
lib/Sema/SemaType.cpp
lib/Sema/SemaLookup.cpp
lib/Sema/Sema.cpp
test/Modules/Inputs/cxx-templates-common.h
test/Modules/Inputs/cxx-templates-b-impl.h
test/Modules/Inputs/cxx-templates-b.h
test/Modules/Inputs/module.map
test/Modules/Inputs/cxx-templates-a.h
test/Modules/cxx-templates.cpp
include/clang/Basic/Module.h
include/clang/Sema/Lookup.h
include/clang/Sema/Sema.h
include/clang/AST/DeclBase.h
Index: lib/Basic/Module.cpp
===================================================================
--- lib/Basic/Module.cpp
+++ lib/Basic/Module.cpp
@@ -242,6 +242,24 @@
}
}
+void Module::buildVisibleModulesCache() const {
+ assert(VisibleModulesCache.empty() && "cache does not need building");
+
+ // This module is visible to itself.
+ VisibleModulesCache.insert(this);
+
+ llvm::SmallVector<Module*, 4> Exported;
+ for (unsigned I = 0, N = Imports.size(); I != N; ++I) {
+ // Every imported module is visible.
+ VisibleModulesCache.insert(Imports[I]);
+
+ // Every module exported by an imported module is visible.
+ Imports[I]->getExportedModules(Exported);
+ VisibleModulesCache.insert(Exported.begin(), Exported.end());
+ Exported.clear();
+ }
+}
+
void Module::print(raw_ostream &OS, unsigned Indent) const {
OS.indent(Indent);
if (IsFramework)
Index: lib/Sema/SemaTemplateInstantiate.cpp
===================================================================
--- lib/Sema/SemaTemplateInstantiate.cpp
+++ lib/Sema/SemaTemplateInstantiate.cpp
@@ -398,6 +398,18 @@
}
SemaRef.InNonInstantiationSFINAEContext
= SavedInNonInstantiationSFINAEContext;
+
+ // Name lookup no longer looks in this template's defining module.
+ assert(SemaRef.ActiveTemplateInstantiations.size() >=
+ SemaRef.ActiveTemplateInstantiationLookupModules.size() &&
+ "forgot to remove a lookup module for a template instantiation");
+ if (SemaRef.ActiveTemplateInstantiations.size() ==
+ SemaRef.ActiveTemplateInstantiationLookupModules.size()) {
+ if (Module *M = SemaRef.ActiveTemplateInstantiationLookupModules.back())
+ SemaRef.LookupModulesCache.erase(M);
+ SemaRef.ActiveTemplateInstantiationLookupModules.pop_back();
+ }
+
SemaRef.ActiveTemplateInstantiations.pop_back();
Invalid = true;
}
Index: lib/Sema/SemaType.cpp
===================================================================
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -4901,7 +4901,7 @@
NamedDecl *Def = 0;
if (!T->isIncompleteType(&Def)) {
// If we know about the definition but it is not visible, complain.
- if (!Diagnoser.Suppressed && Def && !LookupResult::isVisible(Def)) {
+ if (!Diagnoser.Suppressed && Def && !LookupResult::isVisible(*this, Def)) {
// 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.
Index: lib/Sema/SemaLookup.cpp
===================================================================
--- lib/Sema/SemaLookup.cpp
+++ lib/Sema/SemaLookup.cpp
@@ -336,12 +336,6 @@
delete Paths;
}
-static NamedDecl *getVisibleDecl(NamedDecl *D);
-
-NamedDecl *LookupResult::getAcceptableDeclSlow(NamedDecl *D) const {
- return getVisibleDecl(D);
-}
-
/// Resolves the result kind of this lookup.
void LookupResult::resolveKind() {
unsigned N = Decls.size();
@@ -1106,26 +1100,120 @@
return !R.empty();
}
+/// \brief Find the declaration that a class temploid member specialization was
+/// instantiated from, or the member itself if it is an explicit specialization.
+static Decl *getInstantiatedFrom(Decl *D, MemberSpecializationInfo *MSInfo) {
+ return MSInfo->isExplicitSpecialization() ? D : MSInfo->getInstantiatedFrom();
+}
+
+/// \brief Find the module in which the given declaration was defined.
+static Module *getDefiningModule(Decl *Entity) {
+ if (FunctionDecl *FD = dyn_cast<FunctionDecl>(Entity)) {
+ // If this function was instantiated from a template, the defining module is
+ // the module containing the pattern.
+ if (FunctionDecl *Pattern = FD->getTemplateInstantiationPattern())
+ Entity = Pattern;
+ } else if (CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(Entity)) {
+ // If it's a class template specialization, find the template or partial
+ // specialization from which it was instantiated.
+ if (ClassTemplateSpecializationDecl *SpecRD =
+ dyn_cast<ClassTemplateSpecializationDecl>(RD)) {
+ llvm::PointerUnion<ClassTemplateDecl*,
+ ClassTemplatePartialSpecializationDecl*> From =
+ SpecRD->getInstantiatedFrom();
+ if (ClassTemplateDecl *FromTemplate = From.dyn_cast<ClassTemplateDecl*>())
+ Entity = FromTemplate->getTemplatedDecl();
+ else if (From)
+ Entity = From.get<ClassTemplatePartialSpecializationDecl*>();
+ // Otherwise, it's an explicit specialization.
+ } else if (MemberSpecializationInfo *MSInfo =
+ RD->getMemberSpecializationInfo())
+ Entity = getInstantiatedFrom(RD, MSInfo);
+ } else if (EnumDecl *ED = dyn_cast<EnumDecl>(Entity)) {
+ if (MemberSpecializationInfo *MSInfo = ED->getMemberSpecializationInfo())
+ Entity = getInstantiatedFrom(ED, MSInfo);
+ } else if (VarDecl *VD = dyn_cast<VarDecl>(Entity)) {
+ // FIXME: Map from variable template specializations back to the template.
+ if (MemberSpecializationInfo *MSInfo = VD->getMemberSpecializationInfo())
+ Entity = getInstantiatedFrom(VD, MSInfo);
+ }
+
+ // Walk up to the containing context. That might also have been instantiated
+ // from a template.
+ DeclContext *Context = Entity->getDeclContext();
+ if (Context->isFileContext())
+ return Entity->getOwningModule();
+ return getDefiningModule(cast<Decl>(Context));
+}
+
+llvm::DenseSet<Module*> &Sema::getLookupModules() {
+ unsigned N = ActiveTemplateInstantiations.size();
+ for (unsigned I = ActiveTemplateInstantiationLookupModules.size();
+ I != N; ++I) {
+ Module *M = getDefiningModule(ActiveTemplateInstantiations[I].Entity);
+ if (M && !LookupModulesCache.insert(M).second)
+ M = 0;
+ ActiveTemplateInstantiationLookupModules.push_back(M);
+ }
+ return LookupModulesCache;
+}
+
+/// \brief Determine whether a declaration is visible to name lookup.
+///
+/// This routine determines whether the declaration D is visible in the current
+/// lookup context, taking into account the current template instantiation
+/// stack. During template instantiation, a declaration is visible if it is
+/// visible from a module containing any entity on the template instantiation
+/// path (by instantiating a template, you allow it to see the declarations that
+/// your module can see, including those later on in your module).
+bool LookupResult::isVisibleSlow(Sema &SemaRef, NamedDecl *D) {
+ assert(D->isHidden() && !SemaRef.ActiveTemplateInstantiations.empty() &&
+ "should not call this: not in slow case");
+ Module *DeclModule = D->getOwningModule();
+ assert(DeclModule && "hidden decl not from a module");
+
+ // Find the extra places where we need to look.
+ llvm::DenseSet<Module*> &LookupModules = SemaRef.getLookupModules();
+ if (LookupModules.empty())
+ return false;
+
+ // If our lookup set contains the decl's module, it's visible.
+ if (LookupModules.count(DeclModule))
+ return true;
+
+ // If the declaration isn't exported, it's not visible in any other module.
+ if (D->isModulePrivate())
+ return false;
+
+ // Check whether DeclModule is transitively exported to an import of
+ // the lookup set.
+ for (llvm::DenseSet<Module *>::iterator I = LookupModules.begin(),
+ E = LookupModules.end();
+ I != E; ++I)
+ if ((*I)->isModuleVisible(DeclModule))
+ return true;
+ return false;
+}
+
/// \brief Retrieve the visible declaration corresponding to D, if any.
///
/// This routine determines whether the declaration D is visible in the current
/// module, with the current imports. If not, it checks whether any
/// redeclaration of D is visible, and if so, returns that declaration.
-///
+///
/// \returns D, or a visible previous declaration of D, whichever is more recent
/// and visible. If no declaration of D is visible, returns null.
-static NamedDecl *getVisibleDecl(NamedDecl *D) {
- if (LookupResult::isVisible(D))
- return D;
-
+NamedDecl *LookupResult::getAcceptableDeclSlow(NamedDecl *D) const {
+ assert(!isVisible(SemaRef, D) && "not in slow case");
+
for (Decl::redecl_iterator RD = D->redecls_begin(), RDEnd = D->redecls_end();
RD != RDEnd; ++RD) {
if (NamedDecl *ND = dyn_cast<NamedDecl>(*RD)) {
- if (LookupResult::isVisible(ND))
+ if (isVisible(SemaRef, ND))
return ND;
}
}
-
+
return 0;
}
@@ -1175,8 +1263,6 @@
S = S->getParent();
}
- unsigned IDNS = R.getIdentifierNamespace();
-
// Scan up the scope chain looking for a decl that matches this
// identifier that is in the appropriate namespace. This search
// should not take long, as shadowing of names is uncommon, and
@@ -1186,7 +1272,7 @@
for (IdentifierResolver::iterator I = IdResolver.begin(Name),
IEnd = IdResolver.end();
I != IEnd; ++I)
- if ((*I)->isInIdentifierNamespace(IDNS)) {
+ if (NamedDecl *D = R.getAcceptableDecl(*I)) {
if (NameKind == LookupRedeclarationWithLinkage) {
// Determine whether this (or a previous) declaration is
// out-of-scope.
@@ -1201,13 +1287,7 @@
else if (NameKind == LookupObjCImplicitSelfParam &&
!isa<ImplicitParamDecl>(*I))
continue;
-
- // If this declaration is module-private and it came from an AST
- // file, we can't see it.
- NamedDecl *D = R.isHiddenDeclarationVisible()? *I : getVisibleDecl(*I);
- if (!D)
- continue;
-
+
R.addDecl(D);
// Check whether there are any other declarations with the same name
@@ -1242,14 +1322,10 @@
if (!LastDC->Equals(DC))
break;
}
-
- // If the declaration isn't in the right namespace, skip it.
- if (!(*LastI)->isInIdentifierNamespace(IDNS))
- continue;
-
- D = R.isHiddenDeclarationVisible()? *LastI : getVisibleDecl(*LastI);
- if (D)
- R.addDecl(D);
+
+ // If the declaration is in the right namespace and visible, add it.
+ if (NamedDecl *LastD = R.getAcceptableDecl(*LastI))
+ R.addDecl(LastD);
}
R.resolveKind();
Index: lib/Sema/Sema.cpp
===================================================================
--- lib/Sema/Sema.cpp
+++ lib/Sema/Sema.cpp
@@ -551,9 +551,9 @@
if (PP.isCodeCompletionEnabled())
return;
- // Only complete translation units define vtables and perform implicit
- // instantiations.
- if (TUKind == TU_Complete) {
+ // Complete translation units and modules define vtables and perform implicit
+ // instantiations. PCH files do not.
+ if (TUKind != TU_Prefix) {
DiagnoseUseOfUnimplementedSelectors();
// If any dynamic classes have their key function defined within
@@ -582,10 +582,9 @@
// carefully keep track of the point of instantiation (C++ [temp.point]).
// This means that name lookup that occurs within the template
// instantiation will always happen at the end of the translation unit,
- // so it will find some names that should not be found. Although this is
- // common behavior for C++ compilers, it is technically wrong. In the
- // future, we either need to be able to filter the results of name lookup
- // or we need to perform template instantiations earlier.
+ // so it will find some names that are not required to be found. This is
+ // valid, but we could do better by diagnosing if an instantiation uses a
+ // name that was not visible at its first point of instantiation.
PerformPendingInstantiations();
}
Index: test/Modules/Inputs/cxx-templates-common.h
===================================================================
--- test/Modules/Inputs/cxx-templates-common.h
+++ test/Modules/Inputs/cxx-templates-common.h
@@ -1 +1,7 @@
template<typename T> struct SomeTemplate {};
+
+struct DefinedInCommon {
+ void f();
+ struct Inner {};
+ friend void FoundByADL(DefinedInCommon);
+};
Index: test/Modules/Inputs/cxx-templates-b-impl.h
===================================================================
--- test/Modules/Inputs/cxx-templates-b-impl.h
+++ test/Modules/Inputs/cxx-templates-b-impl.h
@@ -0,0 +1,5 @@
+struct DefinedInBImpl {
+ void f();
+ struct Inner {};
+ friend void FoundByADL(DefinedInBImpl);
+};
Index: test/Modules/Inputs/cxx-templates-b.h
===================================================================
--- test/Modules/Inputs/cxx-templates-b.h
+++ test/Modules/Inputs/cxx-templates-b.h
@@ -14,3 +14,26 @@
template<typename T> struct SomeTemplate<T&> {};
template<typename T> struct SomeTemplate<T&>;
typedef SomeTemplate<int&> SomeTemplateIntRef;
+
+extern DefinedInCommon &defined_in_common;
+
+@import cxx_templates_b_impl;
+
+template<typename T, typename> struct Identity { typedef T type; };
+template<typename T> void UseDefinedInBImpl() {
+ typename Identity<DefinedInBImpl, T>::type dependent;
+ FoundByADL(dependent);
+ typename Identity<DefinedInBImpl, T>::type::Inner inner;
+ dependent.f();
+}
+
+extern DefinedInBImpl &defined_in_b_impl;
+
+@import cxx_templates_a;
+template<typename T> void UseDefinedInBImplIndirectly(T &v) {
+ PerformDelayedLookup(v);
+}
+
+void TriggerInstantiation() {
+ UseDefinedInBImpl<void>();
+}
Index: test/Modules/Inputs/module.map
===================================================================
--- test/Modules/Inputs/module.map
+++ test/Modules/Inputs/module.map
@@ -196,6 +196,10 @@
header "cxx-templates-a.h"
}
+module cxx_templates_b_impl {
+ header "cxx-templates-b-impl.h"
+}
+
module cxx_templates_b {
header "cxx-templates-b.h"
}
Index: test/Modules/Inputs/cxx-templates-a.h
===================================================================
--- test/Modules/Inputs/cxx-templates-a.h
+++ test/Modules/Inputs/cxx-templates-a.h
@@ -14,3 +14,9 @@
template<typename T> struct SomeTemplate<T*>;
template<typename T> struct SomeTemplate<T*> {};
typedef SomeTemplate<int*> SomeTemplateIntPtr;
+
+template<typename T> void PerformDelayedLookup(T &t) {
+ t.f();
+ typename T::Inner inner;
+ FoundByADL(t);
+}
Index: test/Modules/cxx-templates.cpp
===================================================================
--- test/Modules/cxx-templates.cpp
+++ test/Modules/cxx-templates.cpp
@@ -24,8 +24,8 @@
N::f<double>(1.0);
N::f<int>();
N::f(); // expected-error {{no matching function}}
- // expected-note@Inputs/cxx-templates-a.h:6 {{couldn't infer template argument}}
- // expected-note@Inputs/cxx-templates-a.h:7 {{requires 1 argument, but 0 were provided}}
+ // expected-note@Inputs/cxx-templates-b.h:6 {{couldn't infer template argument}}
+ // expected-note@Inputs/cxx-templates-b.h:7 {{requires single argument 't'}}
template_param_kinds_1<0>(); // ok, from cxx-templates-a.h
template_param_kinds_1<int>(); // ok, from cxx-templates-b.h
@@ -46,6 +46,26 @@
template_param_kinds_3<Tmpl_T_T_B>(); // expected-error {{ambiguous}}
// expected-note@Inputs/cxx-templates-a.h:12 {{candidate}}
// expected-note@Inputs/cxx-templates-b.h:12 {{candidate}}
+
+ // Trigger the instantiation of a template in 'a' that uses a type defined in
+ // 'common'. That type is not visible here.
+ PerformDelayedLookup(defined_in_common);
+
+ // Trigger the instantiation of a template in 'b' that uses a type defined in
+ // 'b_impl'. That type is not visible here.
+ UseDefinedInBImpl<int>();
+
+ // Trigger the instantiation of a template in 'a' that uses a type defined in
+ // 'b_impl', via a template defined in 'b'. Since the type is visible from
+ // within 'b', the instantiation succeeds.
+ UseDefinedInBImplIndirectly(defined_in_b_impl);
+
+ // Trigger the instantiation of a template in 'a' that uses a type defined in
+ // 'b_impl'. That type is not visible here, nor in 'a'. This fails; there is
+ // no reason why DefinedInBImpl should be visible 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}}
+ PerformDelayedLookup(defined_in_b_impl); // expected-note {{in instantiation of}}
}
@import cxx_templates_common;
@@ -62,9 +82,6 @@
// CHECK-GLOBAL-NEXT: |-FunctionTemplate {{.*}} 'f'
// CHECK-GLOBAL-NEXT: `-FunctionTemplate {{.*}} 'f'
-// FIXME: There should only be two 'f's here.
// CHECK-NAMESPACE-N: DeclarationName 'f'
// CHECK-NAMESPACE-N-NEXT: |-FunctionTemplate {{.*}} 'f'
-// CHECK-NAMESPACE-N-NEXT: |-FunctionTemplate {{.*}} 'f'
-// CHECK-NAMESPACE-N-NEXT: |-FunctionTemplate {{.*}} 'f'
// CHECK-NAMESPACE-N-NEXT: `-FunctionTemplate {{.*}} 'f'
Index: include/clang/Basic/Module.h
===================================================================
--- include/clang/Basic/Module.h
+++ include/clang/Basic/Module.h
@@ -16,6 +16,7 @@
#define LLVM_CLANG_BASIC_MODULE_H
#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/PointerIntPair.h"
#include "llvm/ADT/PointerUnion.h"
#include "llvm/ADT/SetVector.h"
@@ -75,6 +76,9 @@
/// \brief top-level header filenames that aren't resolved to FileEntries yet.
std::vector<std::string> TopHeaderNames;
+ /// \brief Cache of modules visible to lookup in this module.
+ mutable llvm::DenseSet<const Module*> VisibleModulesCache;
+
public:
/// \brief The headers that are part of this module.
SmallVector<const FileEntry *, 2> NormalHeaders;
@@ -369,6 +373,14 @@
/// \returns The submodule if found, or NULL otherwise.
Module *findSubmodule(StringRef Name) const;
+ /// \brief Determine whether the specified module would be visible to
+ /// a lookup at the end of this module.
+ bool isModuleVisible(const Module *M) const {
+ if (VisibleModulesCache.empty())
+ buildVisibleModulesCache();
+ return VisibleModulesCache.count(M);
+ }
+
typedef std::vector<Module *>::iterator submodule_iterator;
typedef std::vector<Module *>::const_iterator submodule_const_iterator;
@@ -390,6 +402,9 @@
/// \brief Dump the contents of this module to the given output stream.
void dump() const;
+
+private:
+ void buildVisibleModulesCache() const;
};
} // end namespace clang
Index: include/clang/Sema/Lookup.h
===================================================================
--- include/clang/Sema/Lookup.h
+++ include/clang/Sema/Lookup.h
@@ -283,33 +283,36 @@
/// \brief Determine whether the given declaration is visible to the
/// program.
- static bool isVisible(NamedDecl *D) {
+ static bool isVisible(Sema &SemaRef, NamedDecl *D) {
// If this declaration is not hidden, it's visible.
if (!D->isHidden())
return true;
-
- // FIXME: We should be allowed to refer to a module-private name from
- // within the same module, e.g., during template instantiation.
- // This requires us know which module a particular declaration came from.
- return false;
+
+ if (SemaRef.ActiveTemplateInstantiations.empty())
+ return false;
+
+ // During template instantiation, we can refer to hidden declarations, if
+ // they were visible in any module along the path of instantiation.
+ return isVisibleSlow(SemaRef, D);
}
-
+
/// \brief Retrieve the accepted (re)declaration of the given declaration,
/// if there is one.
NamedDecl *getAcceptableDecl(NamedDecl *D) const {
if (!D->isInIdentifierNamespace(IDNS))
return 0;
-
- if (isHiddenDeclarationVisible() || isVisible(D))
+
+ if (isHiddenDeclarationVisible() || isVisible(SemaRef, D))
return D;
-
+
return getAcceptableDeclSlow(D);
}
-
+
private:
+ static bool isVisibleSlow(Sema &SemaRef, NamedDecl *D);
NamedDecl *getAcceptableDeclSlow(NamedDecl *D) const;
+
public:
-
/// \brief Returns the identifier namespace mask for this lookup.
unsigned getIdentifierNamespace() const {
return IDNS;
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -5932,6 +5932,20 @@
SmallVector<ActiveTemplateInstantiation, 16>
ActiveTemplateInstantiations;
+ /// \brief Extra modules inspected when performing a lookup during a template
+ /// instantiation. Computed lazily.
+ SmallVector<Module*, 16> ActiveTemplateInstantiationLookupModules;
+
+ /// \brief Cache of additional modules that should be used for name lookup
+ /// within the current template instantiation. Computed lazily; use
+ /// getLookupModules() to get a complete set.
+ llvm::DenseSet<Module*> LookupModulesCache;
+
+ /// \brief Get the set of additional modules that should be checked during
+ /// name lookup. A module and its imports become visible when instanting a
+ /// template defined within it.
+ llvm::DenseSet<Module*> &getLookupModules();
+
/// \brief Whether we are in a SFINAE context that is not associated with
/// template instantiation.
///
Index: include/clang/AST/DeclBase.h
===================================================================
--- include/clang/AST/DeclBase.h
+++ include/clang/AST/DeclBase.h
@@ -523,13 +523,13 @@
NextInContextAndBits.setInt(Bits);
}
-protected:
/// \brief Whether this declaration was marked as being private to the
/// module in which it was defined.
- bool isModulePrivate() const {
+ bool isModulePrivate() const {
return NextInContextAndBits.getInt() & ModulePrivateFlag;
}
-
+
+protected:
/// \brief Specify whether this declaration was marked as being private
/// to the module in which it was defined.
void setModulePrivate(bool MP = true) {
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
