Author: Richard Smith Date: 2020-10-30T18:35:12-07:00 New Revision: dd8297b0669f8e69b03ba40171b195b5acf0f963
URL: https://github.com/llvm/llvm-project/commit/dd8297b0669f8e69b03ba40171b195b5acf0f963 DIFF: https://github.com/llvm/llvm-project/commit/dd8297b0669f8e69b03ba40171b195b5acf0f963.diff LOG: PR42513: Fix handling of function definitions lazily instantiated from friends. When determining whether a function has a template instantiation pattern, look for other declarations of that function that were instantiated from a friend function definition, rather than assuming that checking for member specialization information on whichever declaration name lookup found will be sufficient. Added: Modified: clang/include/clang/AST/Decl.h clang/lib/AST/Decl.cpp clang/lib/Sema/SemaDecl.cpp clang/lib/Sema/SemaDeclCXX.cpp clang/lib/Sema/SemaTemplateInstantiateDecl.cpp clang/test/SemaTemplate/friend.cpp Removed: ################################################################################ diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h index 5d4f18806ff4..71896c0db086 100644 --- a/clang/include/clang/AST/Decl.h +++ b/clang/include/clang/AST/Decl.h @@ -2050,7 +2050,14 @@ class FunctionDecl : public DeclaratorDecl, /// /// The variant that accepts a FunctionDecl pointer will set that function /// declaration to the declaration that is a definition (if there is one). - bool isDefined(const FunctionDecl *&Definition) const; + /// + /// \param CheckForPendingFriendDefinition If \c true, also check for friend + /// declarations that were instantiataed from function definitions. + /// Such a declaration behaves as if it is a definition for the + /// purpose of redefinition checking, but isn't actually a "real" + /// definition until its body is instantiated. + bool isDefined(const FunctionDecl *&Definition, + bool CheckForPendingFriendDefinition = false) const; bool isDefined() const { const FunctionDecl* Definition; @@ -2096,6 +2103,11 @@ class FunctionDecl : public DeclaratorDecl, willHaveBody() || hasDefiningAttr(); } + /// Determine whether this specific declaration of the function is a friend + /// declaration that was instantiated from a function definition. Such + /// declarations behave like definitions in some contexts. + bool isThisDeclarationInstantiatedFromAFriendDefinition() const; + /// Returns whether this specific declaration of the function has a body. bool doesThisDeclarationHaveABody() const { return (!FunctionDeclBits.HasDefaultedFunctionInfo && Body) || diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index b4f92d77dd5d..5da0f5a337bb 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -2886,10 +2886,55 @@ bool FunctionDecl::hasTrivialBody() const { return false; } -bool FunctionDecl::isDefined(const FunctionDecl *&Definition) const { - for (auto I : redecls()) { - if (I->isThisDeclarationADefinition()) { - Definition = I; +bool FunctionDecl::isThisDeclarationInstantiatedFromAFriendDefinition() const { + if (!getFriendObjectKind()) + return false; + + // Check for a friend function instantiated from a friend function + // definition in a templated class. + if (const FunctionDecl *InstantiatedFrom = + getInstantiatedFromMemberFunction()) + return InstantiatedFrom->getFriendObjectKind() && + InstantiatedFrom->isThisDeclarationADefinition(); + + // Check for a friend function template instantiated from a friend + // function template definition in a templated class. + if (const FunctionTemplateDecl *Template = getDescribedFunctionTemplate()) { + if (const FunctionTemplateDecl *InstantiatedFrom = + Template->getInstantiatedFromMemberTemplate()) + return InstantiatedFrom->getFriendObjectKind() && + InstantiatedFrom->isThisDeclarationADefinition(); + } + + return false; +} + +bool FunctionDecl::isDefined(const FunctionDecl *&Definition, + bool CheckForPendingFriendDefinition) const { + for (const FunctionDecl *FD : redecls()) { + if (FD->isThisDeclarationADefinition()) { + Definition = FD; + return true; + } + + // If this is a friend function defined in a class template, it does not + // have a body until it is used, nevertheless it is a definition, see + // [temp.inst]p2: + // + // ... for the purpose of determining whether an instantiated redeclaration + // is valid according to [basic.def.odr] and [class.mem], a declaration that + // corresponds to a definition in the template is considered to be a + // definition. + // + // The following code must produce redefinition error: + // + // template<typename T> struct C20 { friend void func_20() {} }; + // C20<int> c20i; + // void func_20() {} + // + if (CheckForPendingFriendDefinition && + FD->isThisDeclarationInstantiatedFromAFriendDefinition()) { + Definition = FD; return true; } } @@ -3639,7 +3684,13 @@ FunctionDecl::getTemplateInstantiationPattern(bool ForDefinition) const { return getDefinitionOrSelf(getPrimaryTemplate()->getTemplatedDecl()); } - if (MemberSpecializationInfo *Info = getMemberSpecializationInfo()) { + // Check for a declaration of this function that was instantiated from a + // friend definition. + const FunctionDecl *FD = nullptr; + if (!isDefined(FD, /*CheckForPendingFriendDefinition=*/true)) + FD = this; + + if (MemberSpecializationInfo *Info = FD->getMemberSpecializationInfo()) { if (ForDefinition && !clang::isTemplateInstantiation(Info->getTemplateSpecializationKind())) return nullptr; diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 10c9db073a25..1dcf7a16a897 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -2637,8 +2637,11 @@ static const NamedDecl *getDefinition(const Decl *D) { return Def; return VD->getActingDefinition(); } - if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) - return FD->getDefinition(); + if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) { + const FunctionDecl *Def = nullptr; + if (FD->isDefined(Def, true)) + return Def; + } return nullptr; } @@ -13860,69 +13863,23 @@ Sema::CheckForFunctionRedefinition(FunctionDecl *FD, const FunctionDecl *EffectiveDefinition, SkipBodyInfo *SkipBody) { const FunctionDecl *Definition = EffectiveDefinition; - if (!Definition && !FD->isDefined(Definition) && !FD->isCXXClassMember()) { - // If this is a friend function defined in a class template, it does not - // have a body until it is used, nevertheless it is a definition, see - // [temp.inst]p2: - // - // ... for the purpose of determining whether an instantiated redeclaration - // is valid according to [basic.def.odr] and [class.mem], a declaration that - // corresponds to a definition in the template is considered to be a - // definition. - // - // The following code must produce redefinition error: - // - // template<typename T> struct C20 { friend void func_20() {} }; - // C20<int> c20i; - // void func_20() {} - // - for (auto I : FD->redecls()) { - if (I != FD && !I->isInvalidDecl() && - I->getFriendObjectKind() != Decl::FOK_None) { - if (FunctionDecl *Original = I->getInstantiatedFromMemberFunction()) { - if (FunctionDecl *OrigFD = FD->getInstantiatedFromMemberFunction()) { - // A merged copy of the same function, instantiated as a member of - // the same class, is OK. - if (declaresSameEntity(OrigFD, Original) && - declaresSameEntity(cast<Decl>(I->getLexicalDeclContext()), - cast<Decl>(FD->getLexicalDeclContext()))) - continue; - } + if (!Definition && + !FD->isDefined(Definition, /*CheckForPendingFriendDefinition*/ true)) + return; - if (Original->isThisDeclarationADefinition()) { - Definition = I; - break; - } - } + if (Definition->getFriendObjectKind() != Decl::FOK_None) { + if (FunctionDecl *OrigDef = Definition->getInstantiatedFromMemberFunction()) { + if (FunctionDecl *OrigFD = FD->getInstantiatedFromMemberFunction()) { + // A merged copy of the same function, instantiated as a member of + // the same class, is OK. + if (declaresSameEntity(OrigFD, OrigDef) && + declaresSameEntity(cast<Decl>(Definition->getLexicalDeclContext()), + cast<Decl>(FD->getLexicalDeclContext()))) + return; } } } - if (!Definition) - // Similar to friend functions a friend function template may be a - // definition and do not have a body if it is instantiated in a class - // template. - if (FunctionTemplateDecl *FTD = FD->getDescribedFunctionTemplate()) { - for (auto I : FTD->redecls()) { - auto D = cast<FunctionTemplateDecl>(I); - if (D != FTD) { - assert(!D->isThisDeclarationADefinition() && - "More than one definition in redeclaration chain"); - if (D->getFriendObjectKind() != Decl::FOK_None) - if (FunctionTemplateDecl *FT = - D->getInstantiatedFromMemberTemplate()) { - if (FT->isThisDeclarationADefinition()) { - Definition = D->getTemplatedDecl(); - break; - } - } - } - } - } - - if (!Definition) - return; - if (canRedefineFunction(Definition, getLangOpts())) return; @@ -14040,9 +13997,10 @@ Decl *Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, Decl *D, FD->setInvalidDecl(); } - // See if this is a redefinition. If 'will have body' is already set, then - // these checks were already performed when it was set. - if (!FD->willHaveBody() && !FD->isLateTemplateParsed()) { + // See if this is a redefinition. If 'will have body' (or similar) is already + // set, then these checks were already performed when it was set. + if (!FD->willHaveBody() && !FD->isLateTemplateParsed() && + !FD->isThisDeclarationInstantiatedFromAFriendDefinition()) { CheckForFunctionRedefinition(FD, nullptr, SkipBody); // If we're skipping the body, we're done. Don't enter the scope. diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index f340b5229d47..2b1481efc385 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -694,6 +694,17 @@ bool Sema::MergeCXXFunctionDecl(FunctionDecl *New, FunctionDecl *Old, Invalid = true; } + // C++11 [temp.friend]p4 (DR329): + // When a function is defined in a friend function declaration in a class + // template, the function is instantiated when the function is odr-used. + // The same restrictions on multiple declarations and definitions that + // apply to non-template function declarations and definitions also apply + // to these implicit definitions. + const FunctionDecl *OldDefinition = nullptr; + if (New->isThisDeclarationInstantiatedFromAFriendDefinition() && + Old->isDefined(OldDefinition, true)) + CheckForFunctionRedefinition(New, OldDefinition); + return Invalid; } diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp index dc7c23cfce8f..458572cb6fa3 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -2040,8 +2040,11 @@ Decl *TemplateDeclInstantiator::VisitFunctionDecl( Function->setInstantiationOfMemberFunction(D, TSK_ImplicitInstantiation); } - if (isFriend) + if (isFriend) { Function->setObjectOfFriendDecl(); + if (FunctionTemplateDecl *FT = Function->getDescribedFunctionTemplate()) + FT->setObjectOfFriendDecl(); + } if (InitFunctionInstantiation(Function, D)) Function->setInvalidDecl(); @@ -2124,63 +2127,33 @@ Decl *TemplateDeclInstantiator::VisitFunctionDecl( SemaRef.CheckFunctionDeclaration(/*Scope*/ nullptr, Function, Previous, IsExplicitSpecialization); - NamedDecl *PrincipalDecl = (TemplateParams - ? cast<NamedDecl>(FunctionTemplate) - : Function); - - // If the original function was part of a friend declaration, - // inherit its namespace state and add it to the owner. - if (isFriend) { - Function->setObjectOfFriendDecl(); - if (FunctionTemplateDecl *FT = Function->getDescribedFunctionTemplate()) - FT->setObjectOfFriendDecl(); - DC->makeDeclVisibleInContext(PrincipalDecl); - - bool QueuedInstantiation = false; - - // C++11 [temp.friend]p4 (DR329): - // When a function is defined in a friend function declaration in a class - // template, the function is instantiated when the function is odr-used. - // The same restrictions on multiple declarations and definitions that - // apply to non-template function declarations and definitions also apply - // to these implicit definitions. - if (D->isThisDeclarationADefinition()) { - SemaRef.CheckForFunctionRedefinition(Function); - if (!Function->isInvalidDecl()) { - for (auto R : Function->redecls()) { - if (R == Function) - continue; - - // If some prior declaration of this function has been used, we need - // to instantiate its definition. - if (!QueuedInstantiation && R->isUsed(false)) { - if (MemberSpecializationInfo *MSInfo = - Function->getMemberSpecializationInfo()) { - if (MSInfo->getPointOfInstantiation().isInvalid()) { - SourceLocation Loc = R->getLocation(); // FIXME - MSInfo->setPointOfInstantiation(Loc); - SemaRef.PendingLocalImplicitInstantiations.push_back( - std::make_pair(Function, Loc)); - QueuedInstantiation = true; - } - } - } - } + // Check the template parameter list against the previous declaration. The + // goal here is to pick up default arguments added since the friend was + // declared; we know the template parameter lists match, since otherwise + // we would not have picked this template as the previous declaration. + if (isFriend && TemplateParams && FunctionTemplate->getPreviousDecl()) { + SemaRef.CheckTemplateParameterList( + TemplateParams, + FunctionTemplate->getPreviousDecl()->getTemplateParameters(), + Function->isThisDeclarationADefinition() + ? Sema::TPC_FriendFunctionTemplateDefinition + : Sema::TPC_FriendFunctionTemplate); + } + + // If we're introducing a friend definition after the first use, trigger + // instantiation. + // FIXME: If this is a friend function template definition, we should check + // to see if any specializations have been used. + if (isFriend && D->isThisDeclarationADefinition() && Function->isUsed(false)) { + if (MemberSpecializationInfo *MSInfo = + Function->getMemberSpecializationInfo()) { + if (MSInfo->getPointOfInstantiation().isInvalid()) { + SourceLocation Loc = D->getLocation(); // FIXME + MSInfo->setPointOfInstantiation(Loc); + SemaRef.PendingLocalImplicitInstantiations.push_back( + std::make_pair(Function, Loc)); } } - - // Check the template parameter list against the previous declaration. The - // goal here is to pick up default arguments added since the friend was - // declared; we know the template parameter lists match, since otherwise - // we would not have picked this template as the previous declaration. - if (TemplateParams && FunctionTemplate->getPreviousDecl()) { - SemaRef.CheckTemplateParameterList( - TemplateParams, - FunctionTemplate->getPreviousDecl()->getTemplateParameters(), - Function->isThisDeclarationADefinition() - ? Sema::TPC_FriendFunctionTemplateDefinition - : Sema::TPC_FriendFunctionTemplate); - } } if (D->isExplicitlyDefaulted()) { @@ -2190,7 +2163,13 @@ Decl *TemplateDeclInstantiator::VisitFunctionDecl( if (D->isDeleted()) SemaRef.SetDeclDeleted(Function, D->getLocation()); - if (Function->isLocalExternDecl() && !Function->getPreviousDecl()) + NamedDecl *PrincipalDecl = + (TemplateParams ? cast<NamedDecl>(FunctionTemplate) : Function); + + // If this declaration lives in a diff erent context from its lexical context, + // add it to the corresponding lookup table. + if (isFriend || + (Function->isLocalExternDecl() && !Function->getPreviousDecl())) DC->makeDeclVisibleInContext(PrincipalDecl); if (Function->isOverloadedOperator() && !DC->isRecord() && @@ -4672,8 +4651,7 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation, bool Recursive, bool DefinitionRequired, bool AtEndOfTU) { - if (Function->isInvalidDecl() || Function->isDefined() || - isa<CXXDeductionGuideDecl>(Function)) + if (Function->isInvalidDecl() || isa<CXXDeductionGuideDecl>(Function)) return; // Never instantiate an explicit specialization except if it is a class scope @@ -4683,6 +4661,20 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation, if (TSK == TSK_ExplicitSpecialization) return; + // Don't instantiate a definition if we already have one. + const FunctionDecl *ExistingDefn = nullptr; + if (Function->isDefined(ExistingDefn, + /*CheckForPendingFriendDefinition=*/true)) { + if (ExistingDefn->isThisDeclarationADefinition()) + return; + + // If we're asked to instantiate a function whose body comes from an + // instantiated friend declaration, attach the instantiated body to the + // corresponding declaration of the function. + assert(ExistingDefn->isThisDeclarationInstantiatedFromAFriendDefinition()); + Function = const_cast<FunctionDecl*>(ExistingDefn); + } + // Find the function body that we'll be substituting. const FunctionDecl *PatternDecl = Function->getTemplateInstantiationPattern(); assert(PatternDecl && "instantiating a non-template"); diff --git a/clang/test/SemaTemplate/friend.cpp b/clang/test/SemaTemplate/friend.cpp index 283c7732ccff..a1dcd866b9e8 100644 --- a/clang/test/SemaTemplate/friend.cpp +++ b/clang/test/SemaTemplate/friend.cpp @@ -141,3 +141,10 @@ namespace PR37556 { inline namespace N { int z1, z2; } template struct Z<int>; } + +namespace PR42513_comment3 { + template<typename X> struct T { friend auto f(X*) { return nullptr; } }; + struct X1 { friend auto f(X1*); }; + template struct T<X1>; + int n = f((X1*)nullptr); // expected-error {{cannot initialize a variable of type 'int' with an rvalue of type 'nullptr_t'}} +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
