rsmith added a comment.
The model that the C++ standard seems to have settled on is that a friend
function declaration that's instantiated from a definition is considered to be
a definition, even if we've not instantiated its body yet. I think we should
try to directly implement that rule.
================
Comment at: lib/AST/DeclTemplate.cpp:292-311
+FunctionTemplateDecl *FunctionTemplateDecl::getDefinition() const {
+ for (auto *R : redecls()) {
+ FunctionTemplateDecl *F = cast<FunctionTemplateDecl>(R);
+ if (F->isThisDeclarationADefinition())
+ return F;
+
+ // If template does not have a body, probably it is instantiated from
----------------
It seems to me that we're getting something fundamentally wrong in the way we
model friend function definitions where we've instantiated the declaration but
not the definition. Here's a case we get wrong on the non-function-template
side:
template<typename T> struct X { friend void f() {} };
X<int> xi;
void f() {}
Line 3 should be ill-formed here due to redefinition, but we don't detect that
because we don't believe that the declaration we instantiated on line 2 is a
definition. That seems to be the heart of the problem.
Instead of adding this function, can you try changing
`FunctionDecl::isThisDeclarationADefinition` to contain the above logic (that
is: if this function is a friend that was instantiated from a definition, then
it's a definition even if we don't have a body yet)?
================
Comment at: lib/Sema/SemaDecl.cpp:8856
+ (NewTemplateDecl->getFriendObjectKind() != Decl::FOK_None ||
+ OldTemplateDecl->getFriendObjectKind() != Decl::FOK_None))
+ if (FunctionTemplateDecl *NewDef = NewTemplateDecl->getDefinition())
----------------
This doesn't look right; the `OldTemplateDecl` might be `FOK_None` but could
still have a definition as a `friend`.
I'm not convinced this is the right place for this check. There are two
different cases here:
1) The redefinition error is introduced by parsing a definition in the source
code. That should already be handled by `ActOnStartOfFunctionDef`.
2) The redefinition error is introduced by instantiating a friend function
template declaration (that has a definition).
I think check (2) belongs in the template instantiation code rather than here.
In fact, at the end of `TemplateDeclInstantiator::VisitFunctionDecl`, there is
an attempt to enforce the relevant rule; it just doesn't get all the cases
right.
================
Comment at: lib/Sema/SemaDecl.cpp:8857-8863
+ if (FunctionTemplateDecl *NewDef = NewTemplateDecl->getDefinition())
+ if (FunctionTemplateDecl *OldDef = OldTemplateDecl->getDefinition())
{
+ Diag(NewDef->getLocation(), diag::err_redefinition)
+ << NewDef->getDeclName();
+ Diag(OldDef->getLocation(), diag::note_previous_definition);
+ Redeclaration = false;
+ }
----------------
You should use `CheckForFunctionRedefinition` to catch this.
https://reviews.llvm.org/D21508
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
