diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h index 117ec7eef330..e9643d2dc7da 100644 --- a/clang/include/clang/AST/Decl.h +++ b/clang/include/clang/AST/Decl.h @@ -247,63 +247,45 @@ public: bool visibilityExplicit() const { return explicit_; } void setLinkage(Linkage L) { linkage_ = L; } + void mergeLinkage(Linkage L) { setLinkage(minLinkage(linkage(), L)); } - void mergeLinkage(LinkageInfo Other) { - mergeLinkage(Other.linkage()); + void mergeLinkage(LinkageInfo other) { + mergeLinkage(other.linkage()); } - // Merge the visibility V giving preference to explicit ones. - // This is used, for example, when merging the visibility of a class - // down to one of its members. If the member has no explicit visibility, - // the class visibility wins. - void mergeVisibility(Visibility V, bool E = false) { - // Never increase the visibility - if (visibility() < V) + /// Merge in the visibility 'newVis'. + void mergeVisibility(Visibility newVis, bool newExplicit) { + Visibility oldVis = visibility(); + + // Never increase visibility. + if (oldVis < newVis) return; - // If we have an explicit visibility, keep it - if (visibilityExplicit()) + // If the new visibility is the same as the old and the new + // visibility isn't explicit, we have nothing to add. + if (oldVis == newVis && !newExplicit) return; - setVisibility(V, E); + // Otherwise, we're either decreasing visibility or making our + // existing visibility explicit. + setVisibility(newVis, newExplicit); } - // Merge the visibility V, keeping the most restrictive one. - // This is used for cases like merging the visibility of a template - // argument to an instantiation. If we already have a hidden class, - // no argument should give it default visibility. - void mergeVisibilityWithMin(Visibility V, bool E = false) { - // Never increase the visibility - if (visibility() < V) - return; - - // FIXME: this - // If this visibility is explicit, keep it. - if (visibilityExplicit() && !E) - return; - - // should be replaced with this - // Don't lose the explicit bit for nothing - // if (visibility() == V && visibilityExplicit()) - // return; - - setVisibility(V, E); - } - void mergeVisibility(LinkageInfo Other) { - mergeVisibility(Other.visibility(), Other.visibilityExplicit()); - } - void mergeVisibilityWithMin(LinkageInfo Other) { - mergeVisibilityWithMin(Other.visibility(), Other.visibilityExplicit()); + void mergeVisibility(LinkageInfo other) { + mergeVisibility(other.visibility(), other.visibilityExplicit()); } - void merge(LinkageInfo Other) { - mergeLinkage(Other); - mergeVisibility(Other); + /// Merge both linkage and visibility. + void merge(LinkageInfo other) { + mergeLinkage(other); + mergeVisibility(other); } - void mergeWithMin(LinkageInfo Other) { - mergeLinkage(Other); - mergeVisibilityWithMin(Other); + + /// Merge linkage and conditionally merge visibility. + void mergeMaybeWithVisibility(LinkageInfo other, bool withVis) { + mergeLinkage(other); + if (withVis) mergeVisibility(other); } }; diff --git a/clang/include/clang/AST/DeclTemplate.h b/clang/include/clang/AST/DeclTemplate.h index 979827a52584..f50d774bf12f 100644 --- a/clang/include/clang/AST/DeclTemplate.h +++ b/clang/include/clang/AST/DeclTemplate.h @@ -84,6 +84,13 @@ public: unsigned size() const { return NumParams; } + llvm::ArrayRef asArray() { + return llvm::ArrayRef(begin(), size()); + } + llvm::ArrayRef asArray() const { + return llvm::ArrayRef(begin(), size()); + } + NamedDecl* getParam(unsigned Idx) { assert(Idx < size() && "Template parameter index out-of-range"); return begin()[Idx]; @@ -193,6 +200,11 @@ public: /// \brief Retrieve the template argument at a given index. const TemplateArgument &operator[](unsigned Idx) const { return get(Idx); } + /// \brief Produce this as an array ref. + llvm::ArrayRef asArray() const { + return llvm::ArrayRef(data(), size()); + } + /// \brief Retrieve the number of template arguments in this /// template argument list. unsigned size() const { return NumArguments; } @@ -324,6 +336,23 @@ public: return getTemplateSpecializationKind() == TSK_ExplicitSpecialization; } + /// \brief True if this declaration is an explicit specialization, + /// explicit instantiation declaration, or explicit instantiation + /// definition. + bool isExplicitInstantiationOrSpecialization() const { + switch (getTemplateSpecializationKind()) { + case TSK_ExplicitSpecialization: + case TSK_ExplicitInstantiationDeclaration: + case TSK_ExplicitInstantiationDefinition: + return true; + + case TSK_Undeclared: + case TSK_ImplicitInstantiation: + return false; + } + llvm_unreachable("bad template specialization kind"); + } + /// \brief Set the template specialization kind. void setTemplateSpecializationKind(TemplateSpecializationKind TSK) { assert(TSK != TSK_Undeclared && @@ -1433,6 +1462,23 @@ public: return getSpecializationKind() == TSK_ExplicitSpecialization; } + /// \brief True if this declaration is an explicit specialization, + /// explicit instantiation declaration, or explicit instantiation + /// definition. + bool isExplicitInstantiationOrSpecialization() const { + switch (getTemplateSpecializationKind()) { + case TSK_ExplicitSpecialization: + case TSK_ExplicitInstantiationDeclaration: + case TSK_ExplicitInstantiationDefinition: + return true; + + case TSK_Undeclared: + case TSK_ImplicitInstantiation: + return false; + } + llvm_unreachable("bad template specialization kind"); + } + void setSpecializationKind(TemplateSpecializationKind TSK) { SpecializationKind = TSK; } diff --git a/clang/include/clang/AST/TemplateBase.h b/clang/include/clang/AST/TemplateBase.h index 6899fdb7d932..da57293b087e 100644 --- a/clang/include/clang/AST/TemplateBase.h +++ b/clang/include/clang/AST/TemplateBase.h @@ -317,6 +317,12 @@ public: return Args.NumArgs; } + /// \brief Return the array of arguments in this template argument pack. + llvm::ArrayRef getPackAsArray() const { + assert(Kind == Pack); + return llvm::ArrayRef(Args.Args, Args.NumArgs); + } + /// \brief Determines whether two template arguments are superficially the /// same. bool structurallyEquals(const TemplateArgument &Other) const; diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index 66d698aec91d..0183c0b59ed9 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -37,6 +37,53 @@ using namespace clang; // NamedDecl Implementation //===----------------------------------------------------------------------===// +// Visibility rules aren't rigorously externally specified, but here +// are the basic principles behind what we implement: +// +// 1. An explicit visibility attribute is generally a direct expression +// of the user's intent and should be honored. Only the innermost +// visibility attribute applies. If no visibility attribute applies, +// global visibility settings are considered. +// +// 2. There is one caveat to the above: on or in a template pattern, +// an explicit visibility attribute is just a default rule, and +// visibility can be decreased by the visibility of template +// arguments. But this, too, has an exception: an attribute on an +// explicit specialization or instantiation causes all the visibility +// restrictions of the template arguments to be ignored. +// +// 3. A variable that does not otherwise have explicit visibility can +// be restricted by the visibility of its type. +// +// 4. A visibility restriction is explicit if it comes from an +// attribute (or something like it), not a global visibility setting. +// When emitting a reference to an external symbol, visibility +// restrictions are ignored unless they are explicit. + +/// Kinds of LV computation. The linkage side of the computation is +/// always the same, but different things can change how visibility is +/// computed. +enum LVComputationKind { + /// Do an LV computation that does everything normal for linkage but + /// ignores sources of visibility other than template arguments. + LVOnlyTemplateArguments, + + /// Do a normal LV computation for, ultimately, a type. + LVForType, + + /// Do a normal LV computation for, ultimately, a value. + LVForValue +}; + +typedef NamedDecl::LinkageInfo LinkageInfo; + +/// Is the given declaration a "type" or a "value" for the purposes of +/// visibility computation? +static bool usesTypeVisibility(const NamedDecl *D) { + return isa(D) || isa(D); +} + +/// Return the explicit visibility of the given declaration. static llvm::Optional getVisibilityOf(const Decl *D) { // If this declaration has an explicit visibility attribute, use it. if (const VisibilityAttr *A = D->getAttr()) { @@ -64,40 +111,63 @@ static llvm::Optional getVisibilityOf(const Decl *D) { return llvm::Optional(); } -typedef NamedDecl::LinkageInfo LinkageInfo; - static LinkageInfo getLVForType(QualType T) { std::pair P = T->getLinkageAndVisibility(); return LinkageInfo(P.first, P.second, T->isVisibilityExplicit()); } /// \brief Get the most restrictive linkage for the types in the given -/// template parameter list. +/// template parameter list. For visibility purposes, template +/// parameters are part of the signature of a template. static LinkageInfo -getLVForTemplateParameterList(const TemplateParameterList *Params) { - LinkageInfo LV(ExternalLinkage, DefaultVisibility, false); - for (TemplateParameterList::const_iterator P = Params->begin(), - PEnd = Params->end(); +getLVForTemplateParameterList(const TemplateParameterList *params) { + LinkageInfo LV; + for (TemplateParameterList::const_iterator P = params->begin(), + PEnd = params->end(); P != PEnd; ++P) { + + // Template type parameters are the most common and never + // contribute to visibility, pack or not. + if (isa(*P)) + continue; + + // Non-type template parameters can be restricted by the value type, e.g. + // template class A { ... }; + // We have to be careful here, though, because we can be dealing with + // dependent types. if (NonTypeTemplateParmDecl *NTTP = dyn_cast(*P)) { - if (NTTP->isExpandedParameterPack()) { - for (unsigned I = 0, N = NTTP->getNumExpansionTypes(); I != N; ++I) { - QualType T = NTTP->getExpansionType(I); - if (!T->isDependentType()) - LV.merge(getLVForType(T)); + // Handle the non-pack case first. + if (!NTTP->isExpandedParameterPack()) { + if (!NTTP->getType()->isDependentType()) { + LV.merge(getLVForType(NTTP->getType())); } continue; } - if (!NTTP->getType()->isDependentType()) { - LV.merge(getLVForType(NTTP->getType())); - continue; + // Look at all the types in an expanded pack. + for (unsigned i = 0, n = NTTP->getNumExpansionTypes(); i != n; ++i) { + QualType type = NTTP->getExpansionType(i); + if (!type->isDependentType()) + LV.merge(getLVForType(type)); } + continue; } - if (TemplateTemplateParmDecl *TTP - = dyn_cast(*P)) { + // Template template parameters can be restricted by their + // template parameters, recursively. + TemplateTemplateParmDecl *TTP = cast(*P); + + // Handle the non-pack case first. + if (!TTP->isExpandedParameterPack()) { LV.merge(getLVForTemplateParameterList(TTP->getTemplateParameters())); + continue; + } + + // Look at all expansions in an expanded pack. + for (unsigned i = 0, n = TTP->getNumExpansionTemplateParameters(); + i != n; ++i) { + LV.merge(getLVForTemplateParameterList( + TTP->getExpansionTemplateParameters(i))); } } @@ -105,67 +175,137 @@ getLVForTemplateParameterList(const TemplateParameterList *Params) { } /// getLVForDecl - Get the linkage and visibility for the given declaration. -static LinkageInfo getLVForDecl(const NamedDecl *D, bool OnlyTemplate); +static LinkageInfo getLVForDecl(const NamedDecl *D, + LVComputationKind computation); /// \brief Get the most restrictive linkage for the types and /// declarations in the given template argument list. -static LinkageInfo getLVForTemplateArgumentList(const TemplateArgument *Args, - unsigned NumArgs, - bool OnlyTemplate) { - LinkageInfo LV(ExternalLinkage, DefaultVisibility, false); +/// +/// Note that we don't take an LVComputationKind because we always +/// want to honor the visibility of template arguments in the same way. +static LinkageInfo +getLVForTemplateArgumentList(ArrayRef args) { + LinkageInfo LV; - for (unsigned I = 0; I != NumArgs; ++I) { - switch (Args[I].getKind()) { + for (unsigned i = 0, e = args.size(); i != e; ++i) { + const TemplateArgument &arg = args[i]; + switch (arg.getKind()) { case TemplateArgument::Null: case TemplateArgument::Integral: case TemplateArgument::Expression: - break; + continue; case TemplateArgument::Type: - LV.mergeWithMin(getLVForType(Args[I].getAsType())); - break; + LV.merge(getLVForType(arg.getAsType())); + continue; case TemplateArgument::Declaration: - if (NamedDecl *ND = dyn_cast(Args[I].getAsDecl())) - LV.mergeWithMin(getLVForDecl(ND, OnlyTemplate)); - break; + if (NamedDecl *ND = dyn_cast(arg.getAsDecl())) { + assert(!usesTypeVisibility(ND)); + LV.merge(getLVForDecl(ND, LVForValue)); + } + continue; case TemplateArgument::NullPtr: - LV.mergeWithMin(getLVForType(Args[I].getNullPtrType())); - break; + LV.merge(getLVForType(arg.getNullPtrType())); + continue; case TemplateArgument::Template: case TemplateArgument::TemplateExpansion: if (TemplateDecl *Template - = Args[I].getAsTemplateOrTemplatePattern().getAsTemplateDecl()) - LV.mergeWithMin(getLVForDecl(Template, OnlyTemplate)); - break; + = arg.getAsTemplateOrTemplatePattern().getAsTemplateDecl()) + LV.merge(getLVForDecl(Template, LVForValue)); + continue; case TemplateArgument::Pack: - LV.mergeWithMin(getLVForTemplateArgumentList(Args[I].pack_begin(), - Args[I].pack_size(), - OnlyTemplate)); - break; + LV.merge(getLVForTemplateArgumentList(arg.getPackAsArray())); + continue; } + llvm_unreachable("bad template argument kind"); } return LV; } static LinkageInfo -getLVForTemplateArgumentList(const TemplateArgumentList &TArgs, - bool OnlyTemplate) { - return getLVForTemplateArgumentList(TArgs.data(), TArgs.size(), OnlyTemplate); +getLVForTemplateArgumentList(const TemplateArgumentList &TArgs) { + return getLVForTemplateArgumentList(TArgs.asArray()); } -static bool shouldConsiderTemplateVis(const FunctionDecl *fn, - const FunctionTemplateSpecializationInfo *spec) { - return !fn->hasAttr() || spec->isExplicitSpecialization(); +/// Merge in template-related linkage and visibility for the given +/// function template specialization. +/// +/// We don't need a computation kind here because we can assume +/// LVForValue. +static void mergeTemplateLV(LinkageInfo &LV, const FunctionDecl *fn, + const FunctionTemplateSpecializationInfo *specInfo) { + bool hasExplicitVisibility = fn->hasAttr(); + FunctionTemplateDecl *temp = specInfo->getTemplate(); + + // Include visibility from the template parameters and arguments + // only if this is not an explicit instantiation or specialization + // with direct explicit visibility. (Implicit instantiations won't + // have a direct attribute.) + bool considerVisibility = !hasExplicitVisibility; + + // Merge information from the template parameters. + LinkageInfo tempLV = + getLVForTemplateParameterList(temp->getTemplateParameters()); + LV.mergeMaybeWithVisibility(tempLV, considerVisibility); + + // Merge information from the template arguments. + const TemplateArgumentList &templateArgs = *specInfo->TemplateArguments; + LinkageInfo argsLV = getLVForTemplateArgumentList(templateArgs); + LV.mergeMaybeWithVisibility(argsLV, considerVisibility); } -static bool -shouldConsiderTemplateVis(const ClassTemplateSpecializationDecl *d) { - return !d->hasAttr() || d->isExplicitSpecialization(); +/// Merge in template-related linkage and visibility for the given +/// class template specialization. +static void mergeTemplateLV(LinkageInfo &LV, + const ClassTemplateSpecializationDecl *spec, + LVComputationKind computation) { + // FIXME: type visibility + bool hasExplicitVisibility = spec->hasAttr(); + ClassTemplateDecl *temp = spec->getSpecializedTemplate(); + + // Include visibility from the template parameters and arguments + // only if this is not an explicit instantiation or specialization + // with direct explicit visibility (and note that implicit + // instantiations won't have a direct attribute). + // + // Furthermore, we want to ignore template parameters and arguments + // for an explicit instantiation or specialization when computing + // the visibility of a member thereof with explicit visibility. + // + // This is a bit complex; let's unpack it. + // + // An explicit class specialization is an independent, top-level + // declaration. As such, if it or any of its members has an + // explicit visibility attribute, that must directly express the + // user's intent, and we should honor it. The same logic applies to + // an explicit instantiation of a member of such a thing. + // + // That we're doing this for a member with explicit visibility + // is encoded by the computation kind being OnlyTemplateArguments. + bool considerVisibility = + !(hasExplicitVisibility || + (computation == LVOnlyTemplateArguments && + spec->isExplicitInstantiationOrSpecialization())); + + // Merge information from the template parameters, but ignore + // visibility if we're only considering template arguments. + + LinkageInfo tempLV = + getLVForTemplateParameterList(temp->getTemplateParameters()); + LV.mergeMaybeWithVisibility(tempLV, + considerVisibility && computation != LVOnlyTemplateArguments); + + // Merge information from the template arguments. We ignore + // template-argument visibility if we've got an explicit + // instantiation with a visibility attribute. + const TemplateArgumentList &templateArgs = spec->getTemplateArgs(); + LinkageInfo argsLV = getLVForTemplateArgumentList(templateArgs); + LV.mergeMaybeWithVisibility(argsLV, considerVisibility); } static bool useInlineVisibilityHidden(const NamedDecl *D) { @@ -202,7 +342,7 @@ template static bool isInExternCContext(T *D) { } static LinkageInfo getLVForNamespaceScopeDecl(const NamedDecl *D, - bool OnlyTemplate) { + LVComputationKind computation) { assert(D->getDeclContext()->getRedeclContext()->isFileContext() && "Not a name having namespace scope"); ASTContext &Context = D->getASTContext(); @@ -280,12 +420,12 @@ static LinkageInfo getLVForNamespaceScopeDecl(const NamedDecl *D, // external. LinkageInfo LV; - if (!OnlyTemplate) { + if (computation != LVOnlyTemplateArguments) { if (llvm::Optional Vis = D->getExplicitVisibility()) { LV.mergeVisibility(*Vis, true); } else { // If we're declared in a namespace with a visibility attribute, - // use that namespace's visibility, but don't call it explicit. + // use that namespace's visibility, and it still counts as explicit. for (const DeclContext *DC = D->getDeclContext(); !isa(DC); DC = DC->getParent()) { @@ -297,14 +437,19 @@ static LinkageInfo getLVForNamespaceScopeDecl(const NamedDecl *D, } } } - } - if (!OnlyTemplate) { - LV.mergeVisibility(Context.getLangOpts().getVisibilityMode()); - // If we're paying attention to global visibility, apply - // -finline-visibility-hidden if this is an inline method. - if (!LV.visibilityExplicit() && useInlineVisibilityHidden(D)) - LV.mergeVisibility(HiddenVisibility, true); + // Add in global settings if the above didn't give us direct visibility. + if (!LV.visibilityExplicit()) { + // FIXME: type visibility + LV.mergeVisibility(Context.getLangOpts().getVisibilityMode(), + /*explicit*/ false); + + + // If we're paying attention to global visibility, apply + // -finline-visibility-hidden if this is an inline method. + if (useInlineVisibilityHidden(D)) + LV.mergeVisibility(HiddenVisibility, true); + } } // C++ [basic.link]p4: @@ -340,7 +485,8 @@ static LinkageInfo getLVForNamespaceScopeDecl(const NamedDecl *D, LinkageInfo TypeLV = getLVForType(Var->getType()); if (TypeLV.linkage() != ExternalLinkage) return LinkageInfo::uniqueExternal(); - LV.mergeVisibility(TypeLV); + if (!LV.visibilityExplicit()) + LV.mergeVisibility(TypeLV); } if (Var->getStorageClass() == SC_PrivateExtern) @@ -377,17 +523,7 @@ static LinkageInfo getLVForNamespaceScopeDecl(const NamedDecl *D, // this is an explicit specialization with a visibility attribute. if (FunctionTemplateSpecializationInfo *specInfo = Function->getTemplateSpecializationInfo()) { - LinkageInfo TempLV = getLVForDecl(specInfo->getTemplate(), true); - const TemplateArgumentList &templateArgs = *specInfo->TemplateArguments; - LinkageInfo ArgsLV = getLVForTemplateArgumentList(templateArgs, - OnlyTemplate); - if (shouldConsiderTemplateVis(Function, specInfo)) { - LV.mergeWithMin(TempLV); - LV.mergeWithMin(ArgsLV); - } else { - LV.mergeLinkage(TempLV); - LV.mergeLinkage(ArgsLV); - } + mergeTemplateLV(LV, Function, specInfo); } // - a named class (Clause 9), or an unnamed class defined in a @@ -405,26 +541,13 @@ static LinkageInfo getLVForNamespaceScopeDecl(const NamedDecl *D, // linkage of the template and template arguments. if (const ClassTemplateSpecializationDecl *spec = dyn_cast(Tag)) { - // From the template. - LinkageInfo TempLV = getLVForDecl(spec->getSpecializedTemplate(), true); - - // The arguments at which the template was instantiated. - const TemplateArgumentList &TemplateArgs = spec->getTemplateArgs(); - LinkageInfo ArgsLV = getLVForTemplateArgumentList(TemplateArgs, - OnlyTemplate); - if (shouldConsiderTemplateVis(spec)) { - LV.mergeWithMin(TempLV); - LV.mergeWithMin(ArgsLV); - } else { - LV.mergeLinkage(TempLV); - LV.mergeLinkage(ArgsLV); - } + mergeTemplateLV(LV, spec, computation); } // - an enumerator belonging to an enumeration with external linkage; } else if (isa(D)) { LinkageInfo EnumLV = getLVForDecl(cast(D->getDeclContext()), - OnlyTemplate); + computation); if (!isExternalLinkage(EnumLV.linkage())) return LinkageInfo::none(); LV.merge(EnumLV); @@ -432,7 +555,11 @@ static LinkageInfo getLVForNamespaceScopeDecl(const NamedDecl *D, // - a template, unless it is a function template that has // internal linkage (Clause 14); } else if (const TemplateDecl *temp = dyn_cast(D)) { - LV.merge(getLVForTemplateParameterList(temp->getTemplateParameters())); + bool considerVisibility = (computation != LVOnlyTemplateArguments); + LinkageInfo tempLV = + getLVForTemplateParameterList(temp->getTemplateParameters()); + LV.mergeMaybeWithVisibility(tempLV, considerVisibility); + // - a namespace (7.3), unless it is declared within an unnamed // namespace. } else if (isa(D) && !D->isInAnonymousNamespace()) { @@ -456,7 +583,8 @@ static LinkageInfo getLVForNamespaceScopeDecl(const NamedDecl *D, return LV; } -static LinkageInfo getLVForClassMember(const NamedDecl *D, bool OnlyTemplate) { +static LinkageInfo getLVForClassMember(const NamedDecl *D, + LVComputationKind computation) { // Only certain class members have linkage. Note that fields don't // really have linkage, but it's convenient to say they do for the // purposes of calculating linkage of pointer-to-data-member @@ -470,7 +598,7 @@ static LinkageInfo getLVForClassMember(const NamedDecl *D, bool OnlyTemplate) { LinkageInfo LV; // If we have an explicit visibility attribute, merge that in. - if (!OnlyTemplate) { + if (computation != LVOnlyTemplateArguments) { if (llvm::Optional Vis = D->getExplicitVisibility()) LV.mergeVisibility(*Vis, true); // If we're paying attention to global visibility, apply @@ -485,15 +613,16 @@ static LinkageInfo getLVForClassMember(const NamedDecl *D, bool OnlyTemplate) { // If this class member has an explicit visibility attribute, the only // thing that can change its visibility is the template arguments, so // only look for them when processing the class. - bool ClassOnlyTemplate = LV.visibilityExplicit() ? true : OnlyTemplate; + LVComputationKind classComputation = + (LV.visibilityExplicit() ? LVOnlyTemplateArguments : computation); // If this member has an visibility attribute, ClassF will exclude // attributes on the class or command line options, keeping only information // about the template instantiation. If the member has no visibility // attributes, mergeWithMin behaves like merge, so in both cases mergeWithMin // produces the desired result. - LV.mergeWithMin(getLVForDecl(cast(D->getDeclContext()), - ClassOnlyTemplate)); + LV.merge(getLVForDecl(cast(D->getDeclContext()), + classComputation)); if (!isExternalLinkage(LV.linkage())) return LinkageInfo::none(); @@ -501,9 +630,6 @@ static LinkageInfo getLVForClassMember(const NamedDecl *D, bool OnlyTemplate) { if (LV.linkage() == UniqueExternalLinkage) return LinkageInfo::uniqueExternal(); - if (!OnlyTemplate) - LV.mergeVisibility(D->getASTContext().getLangOpts().getVisibilityMode()); - if (const CXXMethodDecl *MD = dyn_cast(D)) { // If the type of the function uses a type with unique-external // linkage, it's not legally usable from outside this translation unit. @@ -514,21 +640,7 @@ static LinkageInfo getLVForClassMember(const NamedDecl *D, bool OnlyTemplate) { // the template parameters and arguments. if (FunctionTemplateSpecializationInfo *spec = MD->getTemplateSpecializationInfo()) { - const TemplateArgumentList &TemplateArgs = *spec->TemplateArguments; - LinkageInfo ArgsLV = getLVForTemplateArgumentList(TemplateArgs, - OnlyTemplate); - TemplateParameterList *TemplateParams = - spec->getTemplate()->getTemplateParameters(); - LinkageInfo ParamsLV = getLVForTemplateParameterList(TemplateParams); - if (shouldConsiderTemplateVis(MD, spec)) { - LV.mergeWithMin(ArgsLV); - if (!OnlyTemplate) - LV.mergeWithMin(ParamsLV); - } else { - LV.mergeLinkage(ArgsLV); - if (!OnlyTemplate) - LV.mergeLinkage(ParamsLV); - } + mergeTemplateLV(LV, MD, spec); } // Note that in contrast to basically every other situation, we @@ -537,33 +649,26 @@ static LinkageInfo getLVForClassMember(const NamedDecl *D, bool OnlyTemplate) { } else if (const CXXRecordDecl *RD = dyn_cast(D)) { if (const ClassTemplateSpecializationDecl *spec = dyn_cast(RD)) { - // Merge template argument/parameter information for member - // class template specializations. - const TemplateArgumentList &TemplateArgs = spec->getTemplateArgs(); - LinkageInfo ArgsLV = getLVForTemplateArgumentList(TemplateArgs, - OnlyTemplate); - TemplateParameterList *TemplateParams = - spec->getSpecializedTemplate()->getTemplateParameters(); - LinkageInfo ParamsLV = getLVForTemplateParameterList(TemplateParams); - if (shouldConsiderTemplateVis(spec)) { - LV.mergeWithMin(ArgsLV); - if (!OnlyTemplate) - LV.mergeWithMin(ParamsLV); - } else { - LV.mergeLinkage(ArgsLV); - if (!OnlyTemplate) - LV.mergeLinkage(ParamsLV); - } + mergeTemplateLV(LV, spec, computation); } // Static data members. } else if (const VarDecl *VD = dyn_cast(D)) { // Modify the variable's linkage by its type, but ignore the // type's visibility unless it's a definition. - LinkageInfo TypeLV = getLVForType(VD->getType()); - if (TypeLV.linkage() != ExternalLinkage) + LinkageInfo typeLV = getLVForType(VD->getType()); + if (typeLV.linkage() != ExternalLinkage) LV.mergeLinkage(UniqueExternalLinkage); - LV.mergeVisibility(TypeLV); + if (!LV.visibilityExplicit()) + LV.mergeVisibility(typeLV); + + // Template members. + } else if (const TemplateDecl *temp = dyn_cast(D)) { + bool considerVisibility = + (!LV.visibilityExplicit() && computation != LVOnlyTemplateArguments); + LinkageInfo tempLV = + getLVForTemplateParameterList(temp->getTemplateParameters()); + LV.mergeMaybeWithVisibility(tempLV, considerVisibility); } return LV; @@ -618,7 +723,10 @@ Linkage NamedDecl::getLinkage() const { if (HasCachedLinkage) return Linkage(CachedLinkage); - CachedLinkage = getLVForDecl(this, true).linkage(); + // We don't care about visibility here, so suppress all the + // unnecessary explicit-visibility checks by asking for a + // template-argument-only analysis. + CachedLinkage = getLVForDecl(this, LVOnlyTemplateArguments).linkage(); HasCachedLinkage = 1; #ifndef NDEBUG @@ -629,7 +737,9 @@ Linkage NamedDecl::getLinkage() const { } LinkageInfo NamedDecl::getLinkageAndVisibility() const { - LinkageInfo LI = getLVForDecl(this, false); + LVComputationKind computation = + (usesTypeVisibility(this) ? LVForType : LVForValue); + LinkageInfo LI = getLVForDecl(this, computation); if (HasCachedLinkage) { assert(Linkage(CachedLinkage) == LI.linkage()); return LI; @@ -729,7 +839,61 @@ llvm::Optional NamedDecl::getExplicitVisibility() const { return llvm::Optional(); } -static LinkageInfo getLVForDecl(const NamedDecl *D, bool OnlyTemplate) { +static LinkageInfo getLVForLocalDecl(const NamedDecl *D, + LVComputationKind computation) { + if (const FunctionDecl *Function = dyn_cast(D)) { + if (Function->isInAnonymousNamespace() && + !Function->getDeclContext()->isExternCContext()) + return LinkageInfo::uniqueExternal(); + + // This is a "void f();" which got merged with a file static. + if (Function->getStorageClass() == SC_Static) + return LinkageInfo::internal(); + + LinkageInfo LV; + if (computation != LVOnlyTemplateArguments) { + if (llvm::Optional Vis = Function->getExplicitVisibility()) + LV.mergeVisibility(*Vis, true); + } + + // Note that Sema::MergeCompatibleFunctionDecls already takes care of + // merging storage classes and visibility attributes, so we don't have to + // look at previous decls in here. + + return LV; + } + + if (const VarDecl *Var = dyn_cast(D)) { + if (Var->getStorageClassAsWritten() == SC_Extern || + Var->getStorageClassAsWritten() == SC_PrivateExtern) { + if (Var->isInAnonymousNamespace() && + !Var->getDeclContext()->isExternCContext()) + return LinkageInfo::uniqueExternal(); + + // This is an "extern int foo;" which got merged with a file static. + if (Var->getStorageClass() == SC_Static) + return LinkageInfo::internal(); + + LinkageInfo LV; + if (Var->getStorageClass() == SC_PrivateExtern) + LV.mergeVisibility(HiddenVisibility, true); + else if (computation != LVOnlyTemplateArguments) { + if (llvm::Optional Vis = Var->getExplicitVisibility()) + LV.mergeVisibility(*Vis, true); + } + + // Note that Sema::MergeVarDecl already takes care of implementing + // C99 6.2.2p4 and propagating the visibility attribute, so we don't + // have to do it here. + return LV; + } + } + + return LinkageInfo::none(); +} + +static LinkageInfo getLVForDecl(const NamedDecl *D, + LVComputationKind computation) { // Objective-C: treat all Objective-C declarations as having external // linkage. switch (D->getKind()) { @@ -764,12 +928,11 @@ static LinkageInfo getLVForDecl(const NamedDecl *D, bool OnlyTemplate) { if (isa(ContextDecl)) DC = ContextDecl->getDeclContext()->getRedeclContext(); else - return getLVForDecl(cast(ContextDecl), - OnlyTemplate); + return getLVForDecl(cast(ContextDecl), computation); } if (const NamedDecl *ND = dyn_cast(DC)) - return getLVForDecl(ND, OnlyTemplate); + return getLVForDecl(ND, computation); return LinkageInfo::external(); } @@ -780,7 +943,7 @@ static LinkageInfo getLVForDecl(const NamedDecl *D, bool OnlyTemplate) { // Handle linkage for namespace-scope names. if (D->getDeclContext()->getRedeclContext()->isFileContext()) - return getLVForNamespaceScopeDecl(D, OnlyTemplate); + return getLVForNamespaceScopeDecl(D, computation); // C++ [basic.link]p5: // In addition, a member function, static data member, a named @@ -790,7 +953,7 @@ static LinkageInfo getLVForDecl(const NamedDecl *D, bool OnlyTemplate) { // purposes (7.1.3), has external linkage if the name of the class // has external linkage. if (D->getDeclContext()->isRecord()) - return getLVForClassMember(D, OnlyTemplate); + return getLVForClassMember(D, computation); // C++ [basic.link]p6: // The name of a function declared in block scope and the name of @@ -803,54 +966,8 @@ static LinkageInfo getLVForDecl(const NamedDecl *D, bool OnlyTemplate) { // one such matching entity, the program is ill-formed. Otherwise, // if no matching entity is found, the block scope entity receives // external linkage. - if (D->getDeclContext()->isFunctionOrMethod()) { - if (const FunctionDecl *Function = dyn_cast(D)) { - if (Function->isInAnonymousNamespace() && - !Function->getDeclContext()->isExternCContext()) - return LinkageInfo::uniqueExternal(); - - // This is a "void f();" which got merged with a file static. - if (Function->getStorageClass() == SC_Static) - return LinkageInfo::internal(); - - LinkageInfo LV; - if (!OnlyTemplate) { - if (llvm::Optional Vis = Function->getExplicitVisibility()) - LV.mergeVisibility(*Vis, true); - } - - // Note that Sema::MergeCompatibleFunctionDecls already takes care of - // merging storage classes and visibility attributes, so we don't have to - // look at previous decls in here. - - return LV; - } - - if (const VarDecl *Var = dyn_cast(D)) - if (Var->getStorageClassAsWritten() == SC_Extern || - Var->getStorageClassAsWritten() == SC_PrivateExtern) { - if (Var->isInAnonymousNamespace() && - !Var->getDeclContext()->isExternCContext()) - return LinkageInfo::uniqueExternal(); - - // This is an "extern int foo;" which got merged with a file static. - if (Var->getStorageClass() == SC_Static) - return LinkageInfo::internal(); - - LinkageInfo LV; - if (Var->getStorageClass() == SC_PrivateExtern) - LV.mergeVisibility(HiddenVisibility, true); - else if (!OnlyTemplate) { - if (llvm::Optional Vis = Var->getExplicitVisibility()) - LV.mergeVisibility(*Vis, true); - } - - // Note that Sema::MergeVarDecl already takes care of implementing - // C99 6.2.2p4 and propagating the visibility attribute, so we don't - // have to do it here. - return LV; - } - } + if (D->getDeclContext()->isFunctionOrMethod()) + return getLVForLocalDecl(D, computation); // C++ [basic.link]p6: // Names not covered by these rules have no linkage. diff --git a/clang/test/CodeGenCXX/visibility.cpp b/clang/test/CodeGenCXX/visibility.cpp index a196dda643dc..5564dc401955 100644 --- a/clang/test/CodeGenCXX/visibility.cpp +++ b/clang/test/CodeGenCXX/visibility.cpp @@ -580,9 +580,7 @@ namespace PR10113 { }; template class foo::bar; // CHECK: define weak_odr void @_ZN7PR101133foo3barINS_3zedEE3zedEv - - // FIXME: This should be hidden as zed is hidden. - // CHECK-HIDDEN: define weak_odr void @_ZN7PR101133foo3barINS_3zedEE3zedEv + // CHECK-HIDDEN: define weak_odr hidden void @_ZN7PR101133foo3barINS_3zedEE3zedEv } namespace PR11690 { @@ -613,9 +611,7 @@ namespace PR11690_2 { }; template class foo::zed; // CHECK: define weak_odr void @_ZN9PR11690_23foo3zedINS_3bazENS0_3barEE3barEv - - // FIXME: This should be hidden as baz is hidden. - // CHECK-HIDDEN: define weak_odr void @_ZN9PR11690_23foo3zedINS_3bazENS0_3barEE3barEv + // CHECK-HIDDEN: define weak_odr hidden void @_ZN9PR11690_23foo3zedINS_3bazENS0_3barEE3barEv } namespace test23 { @@ -729,10 +725,10 @@ namespace test34 { namespace test35 { // This is a really ugly testcase. GCC propagates the DEFAULT in zed's - // definition. What we do instead is be conservative about merging - // implicit visibilities. - // FIXME: Maybe the best thing to do here is error? The test at least - // makes sure we don't produce a hidden symbol for foo::bar. + // definition. It's not really clear what we can do here, because we + // produce the symbols before even seeing the DEFAULT definition of zed. + // FIXME: Maybe the best thing to do here is error? It's certainly hard + // to argue that this ought to be valid. template struct DEFAULT foo { void bar() {} @@ -742,7 +738,7 @@ namespace test35 { class DEFAULT zed { }; // CHECK: define weak_odr void @_ZN6test353fooINS_3zedEE3barEv - // CHECK-HIDDEN: define weak_odr void @_ZN6test353fooINS_3zedEE3barEv + // CHECK-HIDDEN: define weak_odr hidden void @_ZN6test353fooINS_3zedEE3barEv } namespace test36 { @@ -821,8 +817,8 @@ namespace test42 { }; void bar::zed() { } - // CHECK: define hidden void @_ZN6test423barINS_3fooEE3zedEv - // CHECK-HIDDEN: define hidden void @_ZN6test423barINS_3fooEE3zedEv + // CHECK: define void @_ZN6test423barINS_3fooEE3zedEv + // CHECK-HIDDEN: define void @_ZN6test423barINS_3fooEE3zedEv } namespace test43 { @@ -834,8 +830,8 @@ namespace test43 { template <> DEFAULT void bar() { } - // CHECK: define hidden void @_ZN6test433barINS_3fooEEEvv - // CHECK-HIDDEN: define hidden void @_ZN6test433barINS_3fooEEEvv + // CHECK: define void @_ZN6test433barINS_3fooEEEvv + // CHECK-HIDDEN: define void @_ZN6test433barINS_3fooEEEvv } namespace test44 { @@ -1156,3 +1152,21 @@ namespace test62 { // gcc issues a warning about it being unused since "the type is already // defined". We should probably do the same. } + +namespace test63 { + enum HIDDEN E { E0 }; + struct A { + template static void foo() {} + + template struct B { + static void foo() {} + }; + }; + + void test() { + A::foo(); + A::B::foo(); + } + // CHECK: define linkonce_odr hidden void @_ZN6test631A3fooILNS_1EE0EEEvv() + // CHECK: define linkonce_odr hidden void @_ZN6test631A1BILNS_1EE0EE3fooEv() +}