Skip to content

Commit

Permalink
PR42513: Fix handling of function definitions lazily instantiated from
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@zygoloid
zygoloid committed Oct 31, 2020
1 parent ca55c99 commit dd8297b
Show file tree
Hide file tree
Showing 6 changed files with 159 additions and 128 deletions.
14 changes: 13 additions & 1 deletion clang/include/clang/AST/Decl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) ||
Expand Down
61 changes: 56 additions & 5 deletions clang/lib/AST/Decl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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;
Expand Down
84 changes: 21 additions & 63 deletions clang/lib/Sema/SemaDecl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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.
Expand Down
11 changes: 11 additions & 0 deletions clang/lib/Sema/SemaDeclCXX.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
110 changes: 51 additions & 59 deletions clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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()) {
Expand All @@ -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 different 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() &&
Expand Down Expand Up @@ -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
Expand All @@ -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");
Expand Down
7 changes: 7 additions & 0 deletions clang/test/SemaTemplate/friend.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'}}
}

0 comments on commit dd8297b

Please sign in to comment.