[PATCH] D21508: Make friend function template definition available if class is instantiated.

[Richard Smith via cfe-commits]

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(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 struct X { friend void f() {} };
  X 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

Re: [PATCH] D21508: Make friend function template definition available if class is instantiated.

[Serge Pavlov via cfe-commits]

Ping.

Thanks,
--Serge

2016-10-18 0:09 GMT+07:00 Serge Pavlov :

> Ping.
>
> Thanks,
> --Serge
>
> 2016-10-13 11:51 GMT+07:00 Serge Pavlov :
>
>> sepavloff updated the summary for this revision.
>>
>> https://reviews.llvm.org/D21508
>>
>>
>>
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21508: Make friend function template definition available if class is instantiated.

[Serge Pavlov via cfe-commits]

Ping.

Thanks,
--Serge

2016-10-13 11:51 GMT+07:00 Serge Pavlov :

> sepavloff updated the summary for this revision.
>
> https://reviews.llvm.org/D21508
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D21508: Make friend function template definition available if class is instantiated.

[Serge Pavlov via cfe-commits]

sepavloff updated this revision to Diff 74218.
sepavloff added a comment.

Updated comments, NFC otherwise.


https://reviews.llvm.org/D21508

Files:
  include/clang/AST/DeclTemplate.h
  lib/AST/DeclTemplate.cpp
  lib/Sema/SemaDecl.cpp
  test/SemaCXX/friend2.cpp

Index: test/SemaCXX/friend2.cpp
===
--- test/SemaCXX/friend2.cpp
+++ test/SemaCXX/friend2.cpp
@@ -101,7 +101,6 @@
   friend void func_12(int x = 0);  // expected-error{{friend declaration specifying a default argument must be the only declaration}}
 };
 
-
 namespace pr22307 {
 
 struct t {
@@ -170,3 +169,70 @@
 template class Test;
 
 }
+
+// Case of template friend functions.
+
+template void func_21(T *x);
+template
+struct C21a {
+  template friend void func_21(T *x) {}
+};
+template
+struct C21b {
+  template friend void func_21(T *x) {}
+};
+
+
+template inline void func_22(T *x) {}
+template
+struct C22a {
+  template friend void func_22(T *x) {}
+};
+template
+struct C22b {
+  template friend void func_22(T *x) {}
+};
+
+
+template
+struct C23a {
+  template friend void func_23(T *x) {}
+};
+template
+struct C23b {
+  template friend void func_23(T *x) {}
+};
+
+
+template inline void func_24(T *x) {}  // expected-note{{previous definition is here}}
+template
+struct C24 {
+  template friend void func_24(T *x) {} // expected-error{{redefinition of 'func_24'}}
+};
+
+C24 v24;  // expected-note{{in instantiation of template class 'C24' requested here}}
+
+
+template inline void func_25(T *x);
+template
+struct C25a {
+  template friend void func_25(T *x) {} // expected-note{{previous definition is here}}
+};
+template
+struct C25b {
+  template friend void func_25(T *x) {} // expected-error{{redefinition of 'func_25'}}
+};
+
+C25a v25a;
+C25b v25b;  // expected-note{{in instantiation of template class 'C25b' requested here}}
+
+
+template void func_26(T *x);
+template
+struct C26 {
+  template friend void func_26(T *x) {}  // expected-error{{redefinition of 'func_26'}}
+ // expected-note@-1{{previous definition is here}}
+};
+
+C26 v26a;
+C26 v26b;  //expected-note{{in instantiation of template class 'C26' requested here}}
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -8846,10 +8846,23 @@
 
 if (FunctionTemplateDecl *OldTemplateDecl
   = dyn_cast(OldDecl)) {
-  NewFD->setPreviousDeclaration(OldTemplateDecl->getTemplatedDecl());
   FunctionTemplateDecl *NewTemplateDecl
 = NewFD->getDescribedFunctionTemplate();
   assert(NewTemplateDecl && "Template/non-template mismatch");
+  Redeclaration = shouldLinkDependentDeclWithPrevious(NewTemplateDecl,
+  OldTemplateDecl);
+  if (Redeclaration &&
+  (NewTemplateDecl->getFriendObjectKind() != Decl::FOK_None ||
+   OldTemplateDecl->getFriendObjectKind() != Decl::FOK_None))
+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;
+  }
+  if (Redeclaration)
+NewFD->setPreviousDeclaration(OldTemplateDecl->getTemplatedDecl());
   if (CXXMethodDecl *Method
 = dyn_cast(NewTemplateDecl->getTemplatedDecl())) {
 Method->setAccess(OldTemplateDecl->getAccess());
Index: lib/AST/DeclTemplate.cpp
===
--- lib/AST/DeclTemplate.cpp
+++ lib/AST/DeclTemplate.cpp
@@ -289,6 +289,27 @@
   }
 }
 
+FunctionTemplateDecl *FunctionTemplateDecl::getDefinition() const {
+  for (auto *R : redecls()) {
+FunctionTemplateDecl *F = cast(R);
+if (F->isThisDeclarationADefinition())
+  return F;
+
+// If template does not have a body, probably it is instantiated from
+// another template and is not used yet.
+if (FunctionTemplateDecl *P = F->getInstantiatedFromMemberTemplate()) {
+  // If we have hit a point where the user provided a specialization of
+  // this template, we're done looking.
+  if (F->isMemberSpecialization())
+return F;
+  if (FunctionTemplateDecl *Def = P->getDefinition())
+return Def;
+}
+  }
+
+  return nullptr;
+}
+
 llvm::FoldingSetVector &
 FunctionTemplateDecl::getSpecializations() const {
   LoadLazySpecializations();
Index: include/clang/AST/DeclTemplate.h
===
--- include/clang/AST/DeclTemplate.h
+++ include/clang/AST/DeclTemplate.h
@@ -933,6 +933,15 @@
 return getTemplatedDecl()->isThisDeclarationADefinition();
   

Re: [PATCH] D21508: Make friend function template definition available if class is instantiated.

[Serge Pavlov via cfe-commits]

sepavloff updated this revision to Diff 61989.
sepavloff added a comment.

Make more correct template function definition lookup

Lookup is made recursive to cover more cases.


http://reviews.llvm.org/D21508

Files:
  include/clang/AST/DeclTemplate.h
  lib/AST/DeclTemplate.cpp
  lib/Sema/SemaDecl.cpp
  test/SemaCXX/friend2.cpp

Index: test/SemaCXX/friend2.cpp
===
--- test/SemaCXX/friend2.cpp
+++ test/SemaCXX/friend2.cpp
@@ -91,6 +91,73 @@
   friend int func10(int);  // expected-error{{functions that differ only in their return type cannot be overloaded}}
 };
 
+// Case of template friend functions.
+
+template void func_11(T *x);
+template
+struct C11a {
+  template friend void func_11(T *x) {}
+};
+template
+struct C11b {
+  template friend void func_11(T *x) {}
+};
+
+
+template inline void func_12(T *x) {}
+template
+struct C12a {
+  template friend void func_12(T *x) {}
+};
+template
+struct C12b {
+  template friend void func_12(T *x) {}
+};
+
+
+template
+struct C13a {
+  template friend void func_13(T *x) {}
+};
+template
+struct C13b {
+  template friend void func_13(T *x) {}
+};
+
+
+template inline void func_14(T *x) {}  // expected-note{{previous definition is here}}
+template
+struct C14 {
+  template friend void func_14(T *x) {} // expected-error{{redefinition of 'func_14'}}
+};
+
+C14 v14;  // expected-note{{in instantiation of template class 'C14' requested here}}
+
+
+template inline void func_15(T *x);
+template
+struct C15a {
+  template friend void func_15(T *x) {} // expected-note{{previous definition is here}}
+};
+template
+struct C15b {
+  template friend void func_15(T *x) {} // expected-error{{redefinition of 'func_15'}}
+};
+
+C15a v15a;
+C15b v15b;  // expected-note{{in instantiation of template class 'C15b' requested here}}
+
+
+template void func_16(T *x);
+template
+struct C16 {
+  template friend void func_16(T *x) {}  // expected-error{{redefinition of 'func_16'}}
+ // expected-note@-1{{previous definition is here}}
+};
+
+C16 v16a;
+C16 v16b;  //expected-note{{in instantiation of template class 'C16' requested here}}
+
 
 namespace pr22307 {
 
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -8807,10 +8807,23 @@
 
 if (FunctionTemplateDecl *OldTemplateDecl
   = dyn_cast(OldDecl)) {
-  NewFD->setPreviousDeclaration(OldTemplateDecl->getTemplatedDecl());
   FunctionTemplateDecl *NewTemplateDecl
 = NewFD->getDescribedFunctionTemplate();
   assert(NewTemplateDecl && "Template/non-template mismatch");
+  Redeclaration = shouldLinkDependentDeclWithPrevious(NewTemplateDecl,
+  OldTemplateDecl);
+  if (Redeclaration &&
+  (NewTemplateDecl->getFriendObjectKind() != Decl::FOK_None ||
+   OldTemplateDecl->getFriendObjectKind() != Decl::FOK_None))
+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;
+  }
+  if (Redeclaration)
+NewFD->setPreviousDeclaration(OldTemplateDecl->getTemplatedDecl());
   if (CXXMethodDecl *Method
 = dyn_cast(NewTemplateDecl->getTemplatedDecl())) {
 Method->setAccess(OldTemplateDecl->getAccess());
Index: lib/AST/DeclTemplate.cpp
===
--- lib/AST/DeclTemplate.cpp
+++ lib/AST/DeclTemplate.cpp
@@ -289,6 +289,34 @@
   }
 }
 
+/// \brief Returns definition for this function template of null if no
+/// definition found.
+///
+/// This function scans not only redeclarations but also templates from which
+/// these declarations are instantiated, if the function template was
+/// instantiated from another template.
+///
+FunctionTemplateDecl *FunctionTemplateDecl::getDefinition() const {
+  for (auto *R : redecls()) {
+FunctionTemplateDecl *F = cast(R);
+if (F->isThisDeclarationADefinition())
+  return F;
+
+// If template does not have a body, probably it is instantiated from
+// another template, which is not used yet.
+if (FunctionTemplateDecl *P = F->getInstantiatedFromMemberTemplate()) {
+  // If we have hit a point where the user provided a specialization of
+  // this template, we're done looking.
+  if (F->isMemberSpecialization())
+return F;
+  if (FunctionTemplateDecl *Def = P->getDefinition())
+return Def;
+}
+  }
+
+  return nullptr;
+}
+
 llvm::FoldingSetVector &
 FunctionTemplateDecl::getSpecializations() const {
   

[PATCH] D21508: Make friend function template definition available if class is instantiated.

[Serge Pavlov via cfe-commits]

sepavloff created this revision.
sepavloff added a reviewer: rsmith.
sepavloff added a subscriber: cfe-commits.

Similar to friend functions, if a friend function template is defined in a
class template, its definition is available only if the class is instantiated.

http://reviews.llvm.org/D21508

Files:
  include/clang/AST/DeclTemplate.h
  lib/AST/DeclTemplate.cpp
  lib/Sema/SemaDecl.cpp
  test/SemaCXX/friend2.cpp

Index: test/SemaCXX/friend2.cpp
===
--- test/SemaCXX/friend2.cpp
+++ test/SemaCXX/friend2.cpp
@@ -91,6 +91,73 @@
   friend int func10(int);  // expected-error{{functions that differ only in their return type cannot be overloaded}}
 };
 
+// Case of template friend functions.
+
+template void func_11(T *x);
+template
+struct C11a {
+  template friend void func_11(T *x) {}
+};
+template
+struct C11b {
+  template friend void func_11(T *x) {}
+};
+
+
+template inline void func_12(T *x) {}
+template
+struct C12a {
+  template friend void func_12(T *x) {}
+};
+template
+struct C12b {
+  template friend void func_12(T *x) {}
+};
+
+
+template
+struct C13a {
+  template friend void func_13(T *x) {}
+};
+template
+struct C13b {
+  template friend void func_13(T *x) {}
+};
+
+
+template inline void func_14(T *x) {}  // expected-note{{previous definition is here}}
+template
+struct C14 {
+  template friend void func_14(T *x) {} // expected-error{{redefinition of 'func_14'}}
+};
+
+C14 v14;  // expected-note{{in instantiation of template class 'C14' requested here}}
+
+
+template inline void func_15(T *x);
+template
+struct C15a {
+  template friend void func_15(T *x) {} // expected-note{{previous definition is here}}
+};
+template
+struct C15b {
+  template friend void func_15(T *x) {} // expected-error{{redefinition of 'func_15'}}
+};
+
+C15a v15a;
+C15b v15b;  // expected-note{{in instantiation of template class 'C15b' requested here}}
+
+
+template void func_16(T *x);
+template
+struct C16 {
+  template friend void func_16(T *x) {}  // expected-error{{redefinition of 'func_16'}}
+ // expected-note@-1{{previous definition is here}}
+};
+
+C16 v16a;
+C16 v16b;  //expected-note{{in instantiation of template class 'C16' requested here}}
+
 
 namespace pr22307 {
 
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -8757,10 +8757,23 @@
 
 if (FunctionTemplateDecl *OldTemplateDecl
   = dyn_cast(OldDecl)) {
-  NewFD->setPreviousDeclaration(OldTemplateDecl->getTemplatedDecl());
   FunctionTemplateDecl *NewTemplateDecl
 = NewFD->getDescribedFunctionTemplate();
   assert(NewTemplateDecl && "Template/non-template mismatch");
+  Redeclaration = shouldLinkDependentDeclWithPrevious(NewTemplateDecl,
+  OldTemplateDecl);
+  if (Redeclaration &&
+  (NewTemplateDecl->getFriendObjectKind() != Decl::FOK_None ||
+   OldTemplateDecl->getFriendObjectKind() != Decl::FOK_None))
+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;
+  }
+  if (Redeclaration)
+NewFD->setPreviousDeclaration(OldTemplateDecl->getTemplatedDecl());
   if (CXXMethodDecl *Method
 = dyn_cast(NewTemplateDecl->getTemplatedDecl())) {
 Method->setAccess(OldTemplateDecl->getAccess());
Index: lib/AST/DeclTemplate.cpp
===
--- lib/AST/DeclTemplate.cpp
+++ lib/AST/DeclTemplate.cpp
@@ -289,6 +289,31 @@
   }
 }
 
+/// \brief Returns definition for this function template of null if no
+/// definition found.
+///
+/// This function scans not only redeclarations but also templates from which
+/// these declarations are instantiated, if the function template was
+/// instantiated from another template.
+///
+FunctionTemplateDecl *FunctionTemplateDecl::getDefinition() const {
+  for (auto *R : redecls()) {
+FunctionTemplateDecl *F = cast(R);
+if (F->isThisDeclarationADefinition())
+  return F;
+
+// If template does not have a body, probably it is instantiated from
+// another template, which is not used yet.
+while (FunctionTemplateDecl *P = F->getInstantiatedFromMemberTemplate()) {
+  if (P->isThisDeclarationADefinition())
+return P;
+  F = P;
+}
+  }
+
+  return nullptr;
+}
+
 llvm::FoldingSetVector &
 FunctionTemplateDecl::getSpecializations() const {
   LoadLazySpecializations();
Index: include/clang/AST/DeclTemplate.h