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) { 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 TopHeaderNames; + /// \brief Cache of modules visible to lookup in this module. + mutable llvm::DenseSet VisibleModulesCache; + public: /// \brief The headers that are part of this module. SmallVector 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::iterator submodule_iterator; typedef std::vector::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 ActiveTemplateInstantiations; + /// \brief Extra modules inspected when performing a lookup during a template + /// instantiation. Computed lazily. + SmallVector 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 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 &getLookupModules(); + /// \brief Whether we are in a SFINAE context that is not associated with /// template instantiation. /// 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 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/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: 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(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(Entity)) { + // If it's a class template specialization, find the template or partial + // specialization from which it was instantiated. + if (ClassTemplateSpecializationDecl *SpecRD = + dyn_cast(RD)) { + llvm::PointerUnion From = + SpecRD->getInstantiatedFrom(); + if (ClassTemplateDecl *FromTemplate = From.dyn_cast()) + Entity = FromTemplate->getTemplatedDecl(); + else if (From) + Entity = From.get(); + // Otherwise, it's an explicit specialization. + } else if (MemberSpecializationInfo *MSInfo = + RD->getMemberSpecializationInfo()) + Entity = getInstantiatedFrom(RD, MSInfo); + } else if (EnumDecl *ED = dyn_cast(Entity)) { + if (MemberSpecializationInfo *MSInfo = ED->getMemberSpecializationInfo()) + Entity = getInstantiatedFrom(ED, MSInfo); + } else if (VarDecl *VD = dyn_cast(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(Context)); +} + +llvm::DenseSet &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 &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::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(*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(*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/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: 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 struct SomeTemplate; template struct SomeTemplate {}; typedef SomeTemplate SomeTemplateIntPtr; + +template void PerformDelayedLookup(T &t) { + t.f(); + typename T::Inner inner; + FoundByADL(t); +} 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 struct SomeTemplate {}; template struct SomeTemplate; typedef SomeTemplate SomeTemplateIntRef; + +extern DefinedInCommon &defined_in_common; + +@import cxx_templates_b_impl; + +template struct Identity { typedef T type; }; +template void UseDefinedInBImpl() { + typename Identity::type dependent; + FoundByADL(dependent); + typename Identity::type::Inner inner; + dependent.f(); +} + +extern DefinedInBImpl &defined_in_b_impl; + +@import cxx_templates_a; +template void UseDefinedInBImplIndirectly(T &v) { + PerformDelayedLookup(v); +} + +void TriggerInstantiation() { + UseDefinedInBImpl(); +} 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 struct SomeTemplate {}; + +struct DefinedInCommon { + void f(); + struct Inner {}; + friend void FoundByADL(DefinedInCommon); +}; 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/cxx-templates.cpp =================================================================== --- test/Modules/cxx-templates.cpp +++ test/Modules/cxx-templates.cpp @@ -24,8 +24,8 @@ N::f(1.0); N::f(); 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(); // ok, from cxx-templates-b.h @@ -46,6 +46,26 @@ template_param_kinds_3(); // 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(); + + // 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'