From 6ddd583f7f9c2f63cfedbcdc59aa618b42baee14 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Thu, 5 May 2016 00:56:12 +0000 Subject: [PATCH] [modules] Enforce the rules that an explicit or partial specialization must be declared before it is used. Because we don't use normal name lookup to find these, the normal code to filter out non-visible names from name lookup results does not apply. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@268585 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/AST/Decl.h | 4 + include/clang/AST/DeclTemplate.h | 14 +- include/clang/Basic/DiagnosticSemaKinds.td | 13 +- include/clang/Sema/Sema.h | 33 +++- lib/AST/Decl.cpp | 15 ++ lib/Sema/SemaCXXScopeSpec.cpp | 31 +++- lib/Sema/SemaExpr.cpp | 87 ++++++---- lib/Sema/SemaLookup.cpp | 75 +++++++-- lib/Sema/SemaTemplate.cpp | 158 +++++++++++++++++- lib/Sema/SemaTemplateInstantiate.cpp | 15 +- lib/Sema/SemaTemplateInstantiateDecl.cpp | 8 + lib/Sema/SemaType.cpp | 20 ++- test/Modules/Inputs/cxx-templates-common.h | 17 ++ .../Modules/Inputs/cxx-templates-unimported.h | 43 +++++ test/Modules/Inputs/module.map | 2 + test/Modules/cxx-templates.cpp | 70 +++++++- 16 files changed, 527 insertions(+), 78 deletions(-) create mode 100644 test/Modules/Inputs/cxx-templates-unimported.h diff --git a/include/clang/AST/Decl.h b/include/clang/AST/Decl.h index 04c4b46c9f..8229078b50 100644 --- a/include/clang/AST/Decl.h +++ b/include/clang/AST/Decl.h @@ -3184,6 +3184,10 @@ public: return isCompleteDefinition() || isFixed(); } + /// \brief Retrieve the enum definition from which this enumeration could + /// be instantiated, if it is an instantiation (rather than a non-template). + EnumDecl *getTemplateInstantiationPattern() const; + /// \brief Returns the enumeration (declared within the template) /// from which this enumeration type was instantiated, or NULL if /// this enumeration was not instantiated from any template. diff --git a/include/clang/AST/DeclTemplate.h b/include/clang/AST/DeclTemplate.h index 7e6dfbba27..d4e9f65e91 100644 --- a/include/clang/AST/DeclTemplate.h +++ b/include/clang/AST/DeclTemplate.h @@ -1876,6 +1876,10 @@ public: cast(getFirstDecl()); return First->InstantiatedFromMember.getPointer(); } + ClassTemplatePartialSpecializationDecl * + getInstantiatedFromMemberTemplate() const { + return getInstantiatedFromMember(); + } void setInstantiatedFromMember( ClassTemplatePartialSpecializationDecl *PartialSpec) { @@ -2511,17 +2515,11 @@ public: /// it was instantiated. llvm::PointerUnion getInstantiatedFrom() const { - if (getSpecializationKind() != TSK_ImplicitInstantiation && - getSpecializationKind() != TSK_ExplicitInstantiationDefinition && - getSpecializationKind() != TSK_ExplicitInstantiationDeclaration) + if (!isTemplateInstantiation(getSpecializationKind())) return llvm::PointerUnion(); - if (SpecializedPartialSpecialization *PartialSpec = - SpecializedTemplate.dyn_cast()) - return PartialSpec->PartialSpecialization; - - return SpecializedTemplate.get(); + return getSpecializedTemplateOrPartial(); } /// \brief Retrieve the variable template or variable template partial diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 28612e6c70..413d5939ab 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -3710,6 +3710,8 @@ def err_template_spec_unknown_kind : Error< "class template">; def note_specialized_entity : Note< "explicitly specialized declaration is here">; +def note_explicit_specialization_declared_here : Note< + "explicit specialization declared here">; def err_template_spec_decl_function_scope : Error< "explicit specialization of %0 in function scope">; def err_template_spec_decl_class_scope : Error< @@ -3832,6 +3834,8 @@ def err_partial_spec_ordering_ambiguous : Error< def note_partial_spec_match : Note<"partial specialization matches %0">; def err_partial_spec_redeclared : Error< "class template partial specialization %0 cannot be redeclared">; +def note_partial_specialization_declared_here : Note< + "explicit specialization declared here">; def note_prev_partial_spec_here : Note< "previous declaration of class template partial specialization %0 is here">; def err_partial_spec_fully_specialized : Error< @@ -8262,14 +8266,17 @@ def err_module_private_local_class : Error< "local %select{struct|interface|union|class|enum}0 cannot be declared " "__module_private__">; def err_module_unimported_use : Error< - "%select{declaration|definition|default argument}0 of %1 must be imported " + "%select{declaration|definition|default argument|" + "explicit specialization|partial specialization}0 of %1 must be imported " "from module '%2' before it is required">; def err_module_unimported_use_header : Error< "missing '#include %3'; " - "%select{declaration|definition|default argument}0 of %1 must be imported " + "%select{declaration|definition|default argument|" + "explicit specialization|partial specialization}0 of %1 must be imported " "from module '%2' before it is required">; def err_module_unimported_use_multiple : Error< - "%select{declaration|definition|default argument}0 of %1 must be imported " + "%select{declaration|definition|default argument|" + "explicit specialization|partial specialization}0 of %1 must be imported " "from one of the following modules before it is required:%2">; def ext_module_import_in_extern_c : ExtWarn< "import of C++ module '%0' appears within extern \"C\" language linkage " diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index 7731569fbc..5765b9b154 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -1404,6 +1404,16 @@ public: bool isVisible(const NamedDecl *D) { return !D->isHidden() || isVisibleSlow(D); } + + /// Determine whether any declaration of an entity is visible. + bool + hasVisibleDeclaration(const NamedDecl *D, + llvm::SmallVectorImpl *Modules = nullptr) { + return isVisible(D) || hasVisibleDeclarationSlow(D, Modules); + } + bool hasVisibleDeclarationSlow(const NamedDecl *D, + llvm::SmallVectorImpl *Modules); + bool hasVisibleMergedDefinition(NamedDecl *Def); /// Determine if \p D has a visible definition. If not, suggest a declaration @@ -1420,6 +1430,11 @@ public: hasVisibleDefaultArgument(const NamedDecl *D, llvm::SmallVectorImpl *Modules = nullptr); + /// Determine if there is a visible declaration of \p D that is a member + /// specialization declaration (as opposed to an instantiated declaration). + bool hasVisibleMemberSpecialization( + const NamedDecl *D, llvm::SmallVectorImpl *Modules = nullptr); + /// Determine if \p A and \p B are equivalent internal linkage declarations /// from different modules, and thus an ambiguity error can be downgraded to /// an extension warning. @@ -1864,17 +1879,31 @@ public: enum class MissingImportKind { Declaration, Definition, - DefaultArgument + DefaultArgument, + ExplicitSpecialization, + PartialSpecialization }; /// \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); + MissingImportKind MIK, bool Recover = true); void diagnoseMissingImport(SourceLocation Loc, NamedDecl *Decl, SourceLocation DeclLoc, ArrayRef Modules, MissingImportKind MIK, bool Recover); + /// \brief We've found a use of a templated declaration that would trigger an + /// implicit instantiation. Check that any relevant explicit specializations + /// and partial specializations are visible, and diagnose if not. + /// \return Whether a problem was diagnosed. + void checkSpecializationVisibility(SourceLocation Loc, NamedDecl *Spec); + + /// \brief We've found a use of a template specialization that would select a + /// partial specialization. Check that the partial specialization is visible, + /// and diagnose if not. + void checkPartialSpecializationVisibility(SourceLocation Loc, + NamedDecl *Spec); + /// \brief Retrieve a suitable printing policy. PrintingPolicy getPrintingPolicy() const { return getPrintingPolicy(Context, PP); diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 9246f94e84..98bc36bbbd 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -3687,6 +3687,21 @@ void EnumDecl::setTemplateSpecializationKind(TemplateSpecializationKind TSK, MSI->setPointOfInstantiation(PointOfInstantiation); } +EnumDecl *EnumDecl::getTemplateInstantiationPattern() const { + if (MemberSpecializationInfo *MSInfo = getMemberSpecializationInfo()) { + if (isTemplateInstantiation(MSInfo->getTemplateSpecializationKind())) { + EnumDecl *ED = getInstantiatedFromMemberEnum(); + while (auto *NewED = ED->getInstantiatedFromMemberEnum()) + ED = NewED; + return ED; + } + } + + assert(!isTemplateInstantiation(getTemplateSpecializationKind()) && + "couldn't find pattern for enum instantiation"); + return nullptr; +} + EnumDecl *EnumDecl::getInstantiatedFromMemberEnum() const { if (SpecializationInfo) return cast(SpecializationInfo->getInstantiatedFrom()); diff --git a/lib/Sema/SemaCXXScopeSpec.cpp b/lib/Sema/SemaCXXScopeSpec.cpp index 2e774dd28c..ab0e709a97 100644 --- a/lib/Sema/SemaCXXScopeSpec.cpp +++ b/lib/Sema/SemaCXXScopeSpec.cpp @@ -117,8 +117,18 @@ DeclContext *Sema::computeDeclContext(const CXXScopeSpec &SS, // specializations, we're entering into the definition of that // class template partial specialization. if (ClassTemplatePartialSpecializationDecl *PartialSpec - = ClassTemplate->findPartialSpecialization(ContextType)) + = ClassTemplate->findPartialSpecialization(ContextType)) { + // A declaration of the partial specialization must be visible. + // We can always recover here, because this only happens when we're + // entering the context, and that can't happen in a SFINAE context. + assert(!isSFINAEContext() && + "partial specialization scope specifier in SFINAE context?"); + if (!hasVisibleDeclaration(PartialSpec)) + diagnoseMissingImport(SS.getLastQualifierNameLoc(), PartialSpec, + MissingImportKind::PartialSpecialization, + /*Recover*/true); return PartialSpec; + } } } else if (const RecordType *RecordT = NNSType->getAs()) { // The nested name specifier refers to a member of a class template. @@ -195,6 +205,8 @@ bool Sema::RequireCompleteDeclContext(CXXScopeSpec &SS, TagDecl *tag = dyn_cast(DC); // If this is a dependent type, then we consider it complete. + // FIXME: This is wrong; we should require a (visible) definition to + // exist in this case too. if (!tag || tag->isDependentContext()) return false; @@ -218,10 +230,23 @@ bool Sema::RequireCompleteDeclContext(CXXScopeSpec &SS, // Fixed enum types are complete, but they aren't valid as scopes // until we see a definition, so awkwardly pull out this special // case. - // FIXME: The definition might not be visible; complain if it is not. const EnumType *enumType = dyn_cast_or_null(tagType); - if (!enumType || enumType->getDecl()->isCompleteDefinition()) + if (!enumType) return false; + if (enumType->getDecl()->isCompleteDefinition()) { + // If we know about the definition but it is not visible, complain. + NamedDecl *SuggestedDef = nullptr; + if (!hasVisibleDefinition(enumType->getDecl(), &SuggestedDef, + /*OnlyNeedComplete*/false)) { + // If the user is going to see an error here, recover by making the + // definition visible. + bool TreatAsComplete = !isSFINAEContext(); + diagnoseMissingImport(loc, SuggestedDef, MissingImportKind::Definition, + /*Recover*/TreatAsComplete); + return !TreatAsComplete; + } + return false; + } // Try to instantiate the definition, if this is a specialization of an // enumeration temploid. diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index 6b043368db..2b89b6bd03 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -12859,39 +12859,53 @@ void Sema::MarkFunctionReferenced(SourceLocation Loc, FunctionDecl *Func, // set of overloaded functions [...]. // // We (incorrectly) mark overload resolution as an unevaluated context, so we - // can just check that here. Skip the rest of this function if we've already - // marked the function as used. + // can just check that here. bool OdrUse = MightBeOdrUse && IsPotentiallyEvaluatedContext(*this); - if (Func->isUsed(/*CheckUsedAttr=*/false) || !OdrUse) { - // C++11 [temp.inst]p3: - // Unless a function template specialization has been explicitly - // instantiated or explicitly specialized, the function template - // specialization is implicitly instantiated when the specialization is - // referenced in a context that requires a function definition to exist. - // - // We consider constexpr function templates to be referenced in a context - // that requires a definition to exist whenever they are referenced. - // - // FIXME: This instantiates constexpr functions too frequently. If this is - // really an unevaluated context (and we're not just in the definition of a - // function template or overload resolution or other cases which we - // incorrectly consider to be unevaluated contexts), and we're not in a - // subexpression which we actually need to evaluate (for instance, a - // template argument, array bound or an expression in a braced-init-list), - // we are not permitted to instantiate this constexpr function definition. - // - // FIXME: This also implicitly defines special members too frequently. They - // are only supposed to be implicitly defined if they are odr-used, but they - // are not odr-used from constant expressions in unevaluated contexts. - // However, they cannot be referenced if they are deleted, and they are - // deleted whenever the implicit definition of the special member would - // fail. - if (!Func->isConstexpr() || Func->getBody()) - return; - CXXMethodDecl *MD = dyn_cast(Func); - if (!Func->isImplicitlyInstantiable() && (!MD || MD->isUserProvided())) - return; - } + + // Determine whether we require a function definition to exist, per + // C++11 [temp.inst]p3: + // Unless a function template specialization has been explicitly + // instantiated or explicitly specialized, the function template + // specialization is implicitly instantiated when the specialization is + // referenced in a context that requires a function definition to exist. + // + // We consider constexpr function templates to be referenced in a context + // that requires a definition to exist whenever they are referenced. + // + // FIXME: This instantiates constexpr functions too frequently. If this is + // really an unevaluated context (and we're not just in the definition of a + // function template or overload resolution or other cases which we + // incorrectly consider to be unevaluated contexts), and we're not in a + // subexpression which we actually need to evaluate (for instance, a + // template argument, array bound or an expression in a braced-init-list), + // we are not permitted to instantiate this constexpr function definition. + // + // FIXME: This also implicitly defines special members too frequently. They + // are only supposed to be implicitly defined if they are odr-used, but they + // are not odr-used from constant expressions in unevaluated contexts. + // However, they cannot be referenced if they are deleted, and they are + // deleted whenever the implicit definition of the special member would + // fail (with very few exceptions). + CXXMethodDecl *MD = dyn_cast(Func); + bool NeedDefinition = + OdrUse || (Func->isConstexpr() && (Func->isImplicitlyInstantiable() || + (MD && !MD->isUserProvided()))); + + // C++14 [temp.expl.spec]p6: + // If a template [...] is explicitly specialized then that specialization + // shall be declared before the first use of that specialization that would + // cause an implicit instantiation to take place, in every translation unit + // in which such a use occurs + if (NeedDefinition && + (Func->getTemplateSpecializationKind() != TSK_Undeclared || + Func->getMemberSpecializationInfo())) + checkSpecializationVisibility(Loc, Func); + + // If we don't need to mark the function as used, and we don't need to + // try to provide a definition, there's nothing more to do. + if ((Func->isUsed(/*CheckUsedAttr=*/false) || !OdrUse) && + (!NeedDefinition || Func->getBody())) + return; // Note that this declaration has been used. if (CXXConstructorDecl *Constructor = dyn_cast(Func)) { @@ -13797,6 +13811,12 @@ static void DoMarkVarDeclReferenced(Sema &SemaRef, SourceLocation Loc, assert(!isa(Var) && "Can't instantiate a partial template specialization."); + // If this might be a member specialization of a static data member, check + // the specialization is visible. We already did the checks for variable + // template specializations when we created them. + if (TSK != TSK_Undeclared && !isa(Var)) + SemaRef.checkSpecializationVisibility(Loc, Var); + // Perform implicit instantiation of static data members, static data member // templates of class templates, and variable template specializations. Delay // instantiations of variable templates, except for those that could be used @@ -13840,7 +13860,8 @@ static void DoMarkVarDeclReferenced(Sema &SemaRef, SourceLocation Loc, } } - if(!MarkODRUsed) return; + if (!MarkODRUsed) + return; // Per C++11 [basic.def.odr], a variable is odr-used "unless it satisfies // the requirements for appearing in a constant expression (5.19) and, if diff --git a/lib/Sema/SemaLookup.cpp b/lib/Sema/SemaLookup.cpp index 66e3601f04..eaf44861d3 100644 --- a/lib/Sema/SemaLookup.cpp +++ b/lib/Sema/SemaLookup.cpp @@ -1482,6 +1482,35 @@ bool Sema::hasVisibleDefaultArgument(const NamedDecl *D, Modules); } +bool Sema::hasVisibleMemberSpecialization( + const NamedDecl *D, llvm::SmallVectorImpl *Modules) { + assert(isa(D->getDeclContext()) && + "not a member specialization"); + for (auto *Redecl : D->redecls()) { + // If the specialization is declared at namespace scope, then it's a member + // specialization declaration. If it's lexically inside the class + // definition then it was instantiated. + // + // FIXME: This is a hack. There should be a better way to determine this. + // FIXME: What about MS-style explicit specializations declared within a + // class definition? + if (Redecl->getLexicalDeclContext()->isFileContext()) { + auto *NonConstR = const_cast(cast(Redecl)); + + if (isVisible(NonConstR)) + return true; + + if (Modules) { + Modules->push_back(getOwningModule(NonConstR)); + const auto &Merged = Context.getModulesWithMergedDefinition(NonConstR); + Modules->insert(Modules->end(), Merged.begin(), Merged.end()); + } + } + } + + return false; +} + /// \brief Determine whether a declaration is visible to name lookup. /// /// This routine determines whether the declaration D is visible in the current @@ -1586,18 +1615,36 @@ static NamedDecl *findAcceptableDecl(Sema &SemaRef, NamedDecl *D) { if (RD == D) continue; - if (auto ND = dyn_cast(RD)) { - // FIXME: This is wrong in the case where the previous declaration is not - // visible in the same scope as D. This needs to be done much more - // carefully. - if (LookupResult::isVisible(SemaRef, ND)) - return ND; - } + auto ND = cast(RD); + // FIXME: This is wrong in the case where the previous declaration is not + // visible in the same scope as D. This needs to be done much more + // carefully. + if (LookupResult::isVisible(SemaRef, ND)) + return ND; } return nullptr; } +bool Sema::hasVisibleDeclarationSlow(const NamedDecl *D, + llvm::SmallVectorImpl *Modules) { + assert(!isVisible(D) && "not in slow case"); + + for (auto *Redecl : D->redecls()) { + auto *NonConstR = const_cast(cast(Redecl)); + if (isVisible(NonConstR)) + return true; + + if (Modules) { + Modules->push_back(getOwningModule(NonConstR)); + const auto &Merged = Context.getModulesWithMergedDefinition(NonConstR); + Modules->insert(Modules->end(), Merged.begin(), Merged.end()); + } + } + + return false; +} + NamedDecl *LookupResult::getAcceptableDeclSlow(NamedDecl *D) const { if (auto *ND = dyn_cast(D)) { // Namespaces are a bit of a special case: we expect there to be a lot of @@ -4904,7 +4951,7 @@ static NamedDecl *getDefinitionToImport(NamedDecl *D) { } void Sema::diagnoseMissingImport(SourceLocation Loc, NamedDecl *Decl, - bool NeedDefinition, bool Recover) { + MissingImportKind MIK, bool Recover) { assert(!isVisible(Decl) && "missing import for non-hidden decl?"); // Suggest importing a module providing the definition of this entity, if @@ -4923,9 +4970,7 @@ void Sema::diagnoseMissingImport(SourceLocation Loc, NamedDecl *Decl, auto Merged = Context.getModulesWithMergedDefinition(Decl); OwningModules.insert(OwningModules.end(), Merged.begin(), Merged.end()); - diagnoseMissingImport(Loc, Decl, Decl->getLocation(), OwningModules, - NeedDefinition ? MissingImportKind::Definition - : MissingImportKind::Declaration, + diagnoseMissingImport(Loc, Decl, Decl->getLocation(), OwningModules, MIK, Recover); } @@ -4985,6 +5030,12 @@ void Sema::diagnoseMissingImport(SourceLocation UseLoc, NamedDecl *Decl, case MissingImportKind::DefaultArgument: DiagID = diag::note_default_argument_declared_here; break; + case MissingImportKind::ExplicitSpecialization: + DiagID = diag::note_explicit_specialization_declared_here; + break; + case MissingImportKind::PartialSpecialization: + DiagID = diag::note_partial_specialization_declared_here; + break; } Diag(DeclLoc, DiagID); @@ -5020,7 +5071,7 @@ void Sema::diagnoseTypo(const TypoCorrection &Correction, assert(Decl && "import required but no declaration to import"); diagnoseMissingImport(Correction.getCorrectionRange().getBegin(), Decl, - /*NeedDefinition*/ false, ErrorRecovery); + MissingImportKind::Declaration, ErrorRecovery); return; } diff --git a/lib/Sema/SemaTemplate.cpp b/lib/Sema/SemaTemplate.cpp index 9d3bf1c160..4960a2d144 100644 --- a/lib/Sema/SemaTemplate.cpp +++ b/lib/Sema/SemaTemplate.cpp @@ -2746,9 +2746,11 @@ Sema::CheckVarTemplateId(VarTemplateDecl *Template, SourceLocation TemplateLoc, // corresponds to these arguments. void *InsertPos = nullptr; if (VarTemplateSpecializationDecl *Spec = Template->findSpecialization( - Converted, InsertPos)) + Converted, InsertPos)) { + checkSpecializationVisibility(TemplateNameLoc, Spec); // If we already have a variable template specialization, return it. return Spec; + } // This is the first time we have referenced this variable template // specialization. Create the canonical declaration and add it to @@ -2847,8 +2849,8 @@ Sema::CheckVarTemplateId(VarTemplateDecl *Template, SourceLocation TemplateLoc, } // 2. Create the canonical declaration. - // Note that we do not instantiate the variable just yet, since - // instantiation is handled in DoMarkVarDeclReferenced(). + // Note that we do not instantiate a definition until we see an odr-use + // in DoMarkVarDeclReferenced(). // FIXME: LateAttrs et al.? VarTemplateSpecializationDecl *Decl = BuildVarTemplateInstantiation( Template, InstantiationPattern, *InstantiationArgs, TemplateArgs, @@ -2876,6 +2878,8 @@ Sema::CheckVarTemplateId(VarTemplateDecl *Template, SourceLocation TemplateLoc, dyn_cast(InstantiationPattern)) Decl->setInstantiationOf(D, InstantiationArgs); + checkSpecializationVisibility(TemplateNameLoc, Decl); + assert(Decl && "No variable template specialization?"); return Decl; } @@ -3741,7 +3745,7 @@ static bool diagnoseMissingArgument(Sema &S, SourceLocation Loc, S.diagnoseMissingImport(Loc, cast(TD), D->getDefaultArgumentLoc(), Modules, Sema::MissingImportKind::DefaultArgument, - /*Recover*/ true); + /*Recover*/true); return true; } @@ -8544,3 +8548,149 @@ bool Sema::IsInsideALocalClassWithinATemplateFunction() { } return false; } + +/// \brief Walk the path from which a declaration was instantiated, and check +/// that every explicit specialization along that path is visible. This enforces +/// C++ [temp.expl.spec]/6: +/// +/// If a template, a member template or a member of a class template is +/// explicitly specialized then that specialization shall be declared before +/// the first use of that specialization that would cause an implicit +/// instantiation to take place, in every translation unit in which such a +/// use occurs; no diagnostic is required. +/// +/// and also C++ [temp.class.spec]/1: +/// +/// A partial specialization shall be declared before the first use of a +/// class template specialization that would make use of the partial +/// specialization as the result of an implicit or explicit instantiation +/// in every translation unit in which such a use occurs; no diagnostic is +/// required. +class ExplicitSpecializationVisibilityChecker { + Sema &S; + SourceLocation Loc; + llvm::SmallVector Modules; + +public: + ExplicitSpecializationVisibilityChecker(Sema &S, SourceLocation Loc) + : S(S), Loc(Loc) {} + + void check(NamedDecl *ND) { + if (auto *FD = dyn_cast(ND)) + return checkImpl(FD); + if (auto *RD = dyn_cast(ND)) + return checkImpl(RD); + if (auto *VD = dyn_cast(ND)) + return checkImpl(VD); + if (auto *ED = dyn_cast(ND)) + return checkImpl(ED); + } + +private: + void diagnose(NamedDecl *D, bool IsPartialSpec) { + auto Kind = IsPartialSpec ? Sema::MissingImportKind::PartialSpecialization + : Sema::MissingImportKind::ExplicitSpecialization; + const bool Recover = true; + + // If we got a custom set of modules (because only a subset of the + // declarations are interesting), use them, otherwise let + // diagnoseMissingImport intelligently pick some. + if (Modules.empty()) + S.diagnoseMissingImport(Loc, D, Kind, Recover); + else + S.diagnoseMissingImport(Loc, D, D->getLocation(), Modules, Kind, Recover); + } + + // Check a specific declaration. There are three problematic cases: + // + // 1) The declaration is an explicit specialization of a template + // specialization. + // 2) The declaration is an explicit specialization of a member of an + // templated class. + // 3) The declaration is an instantiation of a template, and that template + // is an explicit specialization of a member of a templated class. + // + // We don't need to go any deeper than that, as the instantiation of the + // surrounding class / etc is not triggered by whatever triggered this + // instantiation, and thus should be checked elsewhere. + template + void checkImpl(SpecDecl *Spec) { + bool IsHiddenExplicitSpecialization = false; + if (Spec->getTemplateSpecializationKind() == TSK_ExplicitSpecialization) { + IsHiddenExplicitSpecialization = + Spec->getMemberSpecializationInfo() + ? !S.hasVisibleMemberSpecialization(Spec, &Modules) + : !S.hasVisibleDeclaration(Spec); + } else { + checkInstantiated(Spec); + } + + if (IsHiddenExplicitSpecialization) + diagnose(Spec->getMostRecentDecl(), false); + } + + void checkInstantiated(FunctionDecl *FD) { + if (auto *TD = FD->getPrimaryTemplate()) + checkTemplate(TD); + } + + void checkInstantiated(CXXRecordDecl *RD) { + auto *SD = dyn_cast(RD); + if (!SD) + return; + + auto From = SD->getSpecializedTemplateOrPartial(); + if (auto *TD = From.dyn_cast()) + checkTemplate(TD); + else if (auto *TD = + From.dyn_cast()) { + if (!S.hasVisibleDeclaration(TD)) + diagnose(TD, true); + checkTemplate(TD); + } + } + + void checkInstantiated(VarDecl *RD) { + auto *SD = dyn_cast(RD); + if (!SD) + return; + + auto From = SD->getSpecializedTemplateOrPartial(); + if (auto *TD = From.dyn_cast()) + checkTemplate(TD); + else if (auto *TD = + From.dyn_cast()) { + if (!S.hasVisibleDeclaration(TD)) + diagnose(TD, true); + checkTemplate(TD); + } + } + + void checkInstantiated(EnumDecl *FD) {} + + template + void checkTemplate(TemplDecl *TD) { + if (TD->isMemberSpecialization()) { + if (!S.hasVisibleMemberSpecialization(TD, &Modules)) + diagnose(TD->getMostRecentDecl(), false); + } + } +}; + +void Sema::checkSpecializationVisibility(SourceLocation Loc, NamedDecl *Spec) { + if (!getLangOpts().Modules) + return; + + ExplicitSpecializationVisibilityChecker(*this, Loc).check(Spec); +} + +/// \brief Check whether a template partial specialization that we've discovered +/// is hidden, and produce suitable diagnostics if so. +void Sema::checkPartialSpecializationVisibility(SourceLocation Loc, + NamedDecl *Spec) { + llvm::SmallVector Modules; + if (!hasVisibleDeclaration(Spec, &Modules)) + diagnoseMissingImport(Loc, Spec, Spec->getLocation(), Modules, + MissingImportKind::PartialSpecialization, + /*Recover*/true); +} diff --git a/lib/Sema/SemaTemplateInstantiate.cpp b/lib/Sema/SemaTemplateInstantiate.cpp index 42344e487c..c9eaa243c6 100644 --- a/lib/Sema/SemaTemplateInstantiate.cpp +++ b/lib/Sema/SemaTemplateInstantiate.cpp @@ -1859,8 +1859,19 @@ static bool DiagnoseUninstantiableTemplate(Sema &S, TagDecl *PatternDef, TemplateSpecializationKind TSK, bool Complain = true) { - if (PatternDef && !PatternDef->isBeingDefined()) + if (PatternDef && !PatternDef->isBeingDefined()) { + NamedDecl *SuggestedDef = nullptr; + if (!S.hasVisibleDefinition(PatternDef, &SuggestedDef, + /*OnlyNeedComplete*/false)) { + // If we're allowed to diagnose this and recover, do so. + bool Recover = Complain && !S.isSFINAEContext(); + if (Complain) + S.diagnoseMissingImport(PointOfInstantiation, SuggestedDef, + Sema::MissingImportKind::Definition, Recover); + return !Recover; + } return false; + } if (!Complain || (PatternDef && PatternDef->isInvalidDecl())) { // Say nothing @@ -2591,7 +2602,7 @@ Sema::InstantiateClassMembers(SourceLocation PointOfInstantiation, if (Enum->getDefinition()) continue; - EnumDecl *Pattern = Enum->getInstantiatedFromMemberEnum(); + EnumDecl *Pattern = Enum->getTemplateInstantiationPattern(); assert(Pattern && "Missing instantiated-from-template information"); if (TSK == TSK_ExplicitInstantiationDefinition) { diff --git a/lib/Sema/SemaTemplateInstantiateDecl.cpp b/lib/Sema/SemaTemplateInstantiateDecl.cpp index 66b274f626..ee4a0b3331 100644 --- a/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -3585,6 +3585,10 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation, Pattern = PatternDecl->getBody(PatternDecl); } + // FIXME: Check that the definition is visible before trying to instantiate + // it. This requires us to track the instantiation stack in order to know + // which definitions should be visible. + if (!Pattern && !PatternDecl->isDefaulted()) { if (DefinitionRequired) { if (Function->getPrimaryTemplate()) @@ -4078,6 +4082,10 @@ void Sema::InstantiateVariableDefinition(SourceLocation PointOfInstantiation, Def = PatternDecl->getOutOfLineDefinition(); } + // FIXME: Check that the definition is visible before trying to instantiate + // it. This requires us to track the instantiation stack in order to know + // which definitions should be visible. + // If we don't have a definition of the variable template, we won't perform // any instantiation. Rather, we rely on the user to instantiate this // definition (or provide a specialization for it) in another translation diff --git a/lib/Sema/SemaType.cpp b/lib/Sema/SemaType.cpp index 1ab43f909a..c8c8af109b 100644 --- a/lib/Sema/SemaType.cpp +++ b/lib/Sema/SemaType.cpp @@ -6799,8 +6799,8 @@ bool Sema::hasVisibleDefinition(NamedDecl *D, NamedDecl **Suggested, RD = Pattern; D = RD->getDefinition(); } else if (auto *ED = dyn_cast(D)) { - while (auto *NewED = ED->getInstantiatedFromMemberEnum()) - ED = NewED; + if (auto *Pattern = ED->getTemplateInstantiationPattern()) + ED = Pattern; if (OnlyNeedComplete && ED->isFixed()) { // If the enum has a fixed underlying type, and we're only looking for a // complete type (not a definition), any visible declaration of it will @@ -6887,9 +6887,16 @@ bool Sema::RequireCompleteTypeImpl(SourceLocation Loc, QualType T, } } - // If we have a complete type, we're done. NamedDecl *Def = nullptr; - if (!T->isIncompleteType(&Def)) { + bool Incomplete = T->isIncompleteType(&Def); + + // Check that any necessary explicit specializations are visible. For an + // enum, we just need the declaration, so don't check this. + if (Def && !isa(Def)) + checkSpecializationVisibility(Loc, Def); + + // If we have a complete type, we're done. + if (!Incomplete) { // If we know about the definition but it is not visible, complain. NamedDecl *SuggestedDef = nullptr; if (Def && @@ -6898,7 +6905,7 @@ bool Sema::RequireCompleteTypeImpl(SourceLocation Loc, QualType T, // definition visible. bool TreatAsComplete = Diagnoser && !isSFINAEContext(); if (Diagnoser) - diagnoseMissingImport(Loc, SuggestedDef, /*NeedDefinition*/true, + diagnoseMissingImport(Loc, SuggestedDef, MissingImportKind::Definition, /*Recover*/TreatAsComplete); return !TreatAsComplete; } @@ -6991,6 +6998,9 @@ bool Sema::RequireCompleteTypeImpl(SourceLocation Loc, QualType T, } } + // FIXME: If we didn't instantiate a definition because of an explicit + // specialization declaration, check that it's visible. + if (!Diagnoser) return true; diff --git a/test/Modules/Inputs/cxx-templates-common.h b/test/Modules/Inputs/cxx-templates-common.h index a9ca624486..8e730c8a85 100644 --- a/test/Modules/Inputs/cxx-templates-common.h +++ b/test/Modules/Inputs/cxx-templates-common.h @@ -53,4 +53,21 @@ template struct WithAnonymousDecls { typedef int X; }; +namespace hidden_specializations { + template void fn() {} + + template struct cls { + static void nested_fn() {} + struct nested_cls {}; + static int nested_var; + enum class nested_enum {}; + + template static void nested_fn_t() {} + template struct nested_cls_t {}; + template static int nested_var_t; + }; + + template int var; +} + #include "cxx-templates-textual.h" diff --git a/test/Modules/Inputs/cxx-templates-unimported.h b/test/Modules/Inputs/cxx-templates-unimported.h new file mode 100644 index 0000000000..c2b6b91592 --- /dev/null +++ b/test/Modules/Inputs/cxx-templates-unimported.h @@ -0,0 +1,43 @@ +#include "cxx-templates-common.h" + +namespace hidden_specializations { + // explicit specializations + template<> void fn() {} + template<> struct cls { + void nested_fn(); + struct nested_cls; + static int nested_var; + enum nested_enum : int; + }; + template<> int var; + + // partial specializations + template struct cls { + void nested_fn(); + struct nested_cls; + static int nested_var; + enum nested_enum : int; + }; + template int var; + + // member specializations + template<> void cls::nested_fn() {} + template<> struct cls::nested_cls {}; + template<> int cls::nested_var; + template<> enum class cls::nested_enum { e }; + template<> template void cls::nested_fn_t() {} + template<> template struct cls::nested_cls_t {}; + template<> template int cls::nested_var_t; + + // specializations instantiated here are ok if their pattern is + inline void use_stuff() { + fn(); + cls(); + (void)var; + cls(); + (void)var; + cls::nested_fn_t(); + cls::nested_cls_t(); + (void)cls::nested_var_t; + } +} diff --git a/test/Modules/Inputs/module.map b/test/Modules/Inputs/module.map index 66b52e9105..4db1cca925 100644 --- a/test/Modules/Inputs/module.map +++ b/test/Modules/Inputs/module.map @@ -215,6 +215,8 @@ module cxx_linkage_cache { module cxx_templates_common { header "cxx-templates-common.h" + + explicit module unimported { header "cxx-templates-unimported.h" } } module cxx_templates_a { diff --git a/test/Modules/cxx-templates.cpp b/test/Modules/cxx-templates.cpp index ef4e4e420d..eea90774aa 100644 --- a/test/Modules/cxx-templates.cpp +++ b/test/Modules/cxx-templates.cpp @@ -1,9 +1,9 @@ // RUN: rm -rf %t -// RUN: not %clang_cc1 -x objective-c++ -fmodules -fimplicit-module-maps -fno-modules-error-recovery -fmodules-cache-path=%t -I %S/Inputs %s -std=c++11 -ast-dump-lookups | FileCheck %s --check-prefix=CHECK-GLOBAL -// RUN: not %clang_cc1 -x objective-c++ -fmodules -fimplicit-module-maps -fno-modules-error-recovery -fmodules-cache-path=%t -I %S/Inputs %s -std=c++11 -ast-dump-lookups -ast-dump-filter N | FileCheck %s --check-prefix=CHECK-NAMESPACE-N -// RUN: not %clang_cc1 -x objective-c++ -fmodules -fimplicit-module-maps -fno-modules-error-recovery -fmodules-cache-path=%t -I %S/Inputs %s -std=c++11 -ast-dump -ast-dump-filter SomeTemplate | FileCheck %s --check-prefix=CHECK-DUMP -// RUN: %clang_cc1 -x objective-c++ -fmodules -fimplicit-module-maps -fno-modules-error-recovery -fmodules-cache-path=%t -I %S/Inputs %s -verify -std=c++11 -// RUN: %clang_cc1 -x objective-c++ -fmodules -fimplicit-module-maps -fno-modules-error-recovery -fmodules-cache-path=%t -I %S/Inputs %s -verify -std=c++11 -DEARLY_IMPORT +// RUN: not %clang_cc1 -x objective-c++ -fmodules -fimplicit-module-maps -fno-modules-error-recovery -fmodules-cache-path=%t -I %S/Inputs %s -std=c++14 -ast-dump-lookups | FileCheck %s --check-prefix=CHECK-GLOBAL +// RUN: not %clang_cc1 -x objective-c++ -fmodules -fimplicit-module-maps -fno-modules-error-recovery -fmodules-cache-path=%t -I %S/Inputs %s -std=c++14 -ast-dump-lookups -ast-dump-filter N | FileCheck %s --check-prefix=CHECK-NAMESPACE-N +// RUN: not %clang_cc1 -x objective-c++ -fmodules -fimplicit-module-maps -fno-modules-error-recovery -fmodules-cache-path=%t -I %S/Inputs %s -std=c++14 -ast-dump -ast-dump-filter SomeTemplate | FileCheck %s --check-prefix=CHECK-DUMP +// RUN: %clang_cc1 -x objective-c++ -fmodules -fimplicit-module-maps -fno-modules-error-recovery -fmodules-cache-path=%t -I %S/Inputs %s -verify -std=c++14 +// RUN: %clang_cc1 -x objective-c++ -fmodules -fimplicit-module-maps -fno-modules-error-recovery -fmodules-cache-path=%t -I %S/Inputs %s -verify -std=c++14 -DEARLY_IMPORT #ifdef EARLY_IMPORT #include "cxx-templates-textual.h" @@ -105,7 +105,8 @@ void g() { TemplateInstantiationVisibility tiv1; TemplateInstantiationVisibility tiv2; - TemplateInstantiationVisibility tiv3; // expected-error 2{{must be imported from module 'cxx_templates_b_impl'}} + TemplateInstantiationVisibility tiv3; // expected-error 5{{must be imported from module 'cxx_templates_b_impl'}} + // expected-note@cxx-templates-b-impl.h:10 3{{explicit specialization declared here}} // expected-note@cxx-templates-b-impl.h:10 2{{previous definition is here}} TemplateInstantiationVisibility tiv4; @@ -172,6 +173,63 @@ bool testFriendInClassTemplate(Std::WithFriend wfi) { return wfi != wfi; } +namespace hidden_specializations { + // expected-note@cxx-templates-unimported.h:* 1+{{here}} + void test() { + // For functions, uses that would trigger instantiations of definitions are + // not allowed. + fn(); // ok + fn(); // ok + fn(); // expected-error 1+{{explicit specialization of 'fn' must be imported}} + cls::nested_fn(); // expected-error 1+{{explicit specialization of 'nested_fn' must be imported}} + cls::nested_fn_t(); // expected-error 1+{{explicit specialization of 'nested_fn_t' must be imported}} + cls::nested_fn_t(); // expected-error 1+{{explicit specialization of 'nested_fn_t' must be imported}} + + // For classes, uses that would trigger instantiations of definitions are + // not allowed. + cls *k0; // ok + cls *k1; // ok + cls *k2; // ok + cls *k3; // ok + cls::nested_cls *nk1; // ok + cls::nested_cls_t *nk2; // ok + cls::nested_cls_t *nk3; // ok + cls uk1; // expected-error 1+{{explicit specialization of 'cls' must be imported}} expected-error 1+{{definition of}} + cls uk3; // expected-error 1+{{partial specialization of 'cls' must be imported}} expected-error 1+{{definition of}} + cls uk4; // expected-error 1+{{partial specialization of 'cls' must be imported}} expected-error 1+{{definition of}} + cls::nested_cls unk1; // expected-error 1+{{explicit specialization of 'nested_cls' must be imported}} expected-error 1+{{definition of}} + cls::nested_cls_t unk2; // expected-error 1+{{explicit specialization of 'nested_cls_t' must be imported}} expected-error 1+{{definition of}} + cls::nested_cls_t unk3; // expected-error 1+{{explicit specialization of 'nested_cls_t' must be imported}} + + // For enums, uses that would trigger instantiations of definitions are not + // allowed. + cls::nested_enum e; // ok + (void)cls::nested_enum::e; // expected-error 1+{{definition of 'nested_enum' must be imported}} expected-error 1+{{declaration of 'e'}} + + // For variable template specializations, no uses are allowed because + // specializations can change the type. + (void)sizeof(var); // ok + (void)sizeof(var); // ok + (void)sizeof(var); // expected-error 1+{{explicit specialization of 'var' must be imported}} + (void)sizeof(var); // expected-error 1+{{partial specialization of 'var' must be imported}} + (void)sizeof(var); // expected-error 1+{{partial specialization of 'var' must be imported}} + (void)sizeof(cls::nested_var); // ok + (void)cls::nested_var; // expected-error 1+{{explicit specialization of 'nested_var' must be imported}} + (void)sizeof(cls::nested_var_t); // expected-error 1+{{explicit specialization of 'nested_var_t' must be imported}} + (void)sizeof(cls::nested_var_t); // expected-error 1+{{explicit specialization of 'nested_var_t' must be imported}} + } + + void cls::nested_fn() {} // expected-error 1+{{explicit specialization of 'cls' must be imported}} expected-error 1+{{definition of}} + struct cls::nested_cls {}; // expected-error 1+{{explicit specialization of 'cls' must be imported}} expected-error 1+{{definition of}} + int cls::nested_var; // expected-error 1+{{explicit specialization of 'cls' must be imported}} expected-error 1+{{definition of}} + enum cls::nested_enum : int {}; // expected-error 1+{{explicit specialization of 'cls' must be imported}} expected-error 1+{{definition of}} + + template void cls::nested_fn() {} // expected-error 1+{{partial specialization of 'cls' must be imported}} + template struct cls::nested_cls {}; // expected-error 1+{{partial specialization of 'cls' must be imported}} + template int cls::nested_var; // expected-error 1+{{partial specialization of 'cls' must be imported}} + template enum cls::nested_enum : int {}; // expected-error 1+{{partial specialization of 'cls' must be imported}} +} + namespace Std { void g(); // expected-error {{functions that differ only in their return type cannot be overloaded}} // expected-note@cxx-templates-common.h:21 {{previous}} -- 2.40.0