[PATCH] D16579: Warn if friend function depends on template parameters.

[Serge Pavlov via Phabricator via cfe-commits]

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

Updated patch

- Remove changes that are not needed anymore,
- Function matching is implemented on recursive function instead of 
TypeVisitor. It support subset of checks, which should be enough for practical 
use.
- Added more comments.


https://reviews.llvm.org/D16579

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/Sema/Sema.cpp
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaDecl.cpp
  test/CXX/drs/dr3xx.cpp
  test/CXX/drs/dr5xx.cpp
  test/CXX/temp/temp.res/p3.cpp
  test/SemaCXX/friend.cpp
  test/SemaCXX/overload-call.cpp

Index: test/SemaCXX/overload-call.cpp
===
--- test/SemaCXX/overload-call.cpp
+++ test/SemaCXX/overload-call.cpp
@@ -574,13 +574,17 @@
   // Ensure that overload resolution attempts to complete argument types when
   // performing ADL.
   template struct S {
-friend int f(const S&);
+friend int f(const S&);  // expected-warning{{friend declaration 'IncompleteArg::f' depends on template parameter but is not a function template}}
+ // expected-note@-1{{declare function outside class template to suppress this warning}}
+ // expected-note@-2{{to befriend a template specialization, make sure the function template has already been declared and use '<>'}}
   };
   extern S s;
   int k = f(s);
 
   template struct Op {
-friend bool operator==(const Op &, const Op &);
+friend bool operator==(const Op &, const Op &);  // expected-warning{{friend declaration 'IncompleteArg::operator==' depends on template parameter but is not a function template}}
+ // expected-note@-1{{declare function outside class template to suppress this warning}}
+ // expected-note@-2{{to befriend a template specialization, make sure the function template has already been declared and use '<>'}}
   };
   extern Op op;
   bool b = op == op;
Index: test/SemaCXX/friend.cpp
===
--- test/SemaCXX/friend.cpp
+++ test/SemaCXX/friend.cpp
@@ -388,3 +388,116 @@
 friend void f(void *p = 0) {} // expected-error {{must be the only}}
   };
 }
+
+template void pr23342_func(T x);
+template
+struct pr23342_C1 {
+  friend void pr23342_func<>(T x);
+  friend bool func(T x);  // expected-warning{{friend declaration 'func' depends on template parameter but is not a function template}}
+  // expected-note@-1{{declare function outside class template to suppress this warning}}
+  // expected-note@-2{{to befriend a template specialization, make sure the function template has already been declared and use '<>'}}
+  friend bool func2(int x);
+  template friend bool func3(T2 x);
+  friend T func4();  // expected-warning{{friend declaration 'func4' depends on template parameter but is not a function template}}
+ // expected-note@-1{{declare function outside class template to suppress this warning}}
+ // expected-note@-2{{to befriend a template specialization, make sure the function template has already been declared and use '<>'}}
+};
+
+namespace pr23342 {
+
+template
+struct C1 {
+  friend void pr23342_func<>(T x);
+  friend bool func(T x);  // expected-warning{{friend declaration 'pr23342::func' depends on template parameter but is not a function template}}
+  // expected-note@-1{{declare function outside class template to suppress this warning}}
+  // expected-note@-2{{to befriend a template specialization, make sure the function template has already been declared and use '<>'}}
+  friend bool func2(int x);
+  template friend bool func3(T2 x);
+  friend T func4();// expected-warning{{friend declaration 'pr23342::func4' depends on template parameter but is not a function template}}
+   // expected-note@-1{{declare function outside class template to suppress this warning}}
+   // expected-note@-2{{to befriend a template specialization, make sure the function template has already been declared and use '<>'}}
+};
+
+template 
+struct Arg {
+  friend bool operator==(const Arg& lhs, T rhs) {
+   return false;
+  }
+  friend bool operator!=(const Arg& lhs, T rhs);  // expected-warning{{friend declaration 'pr23342::operator!=' depends on template parameter but is not a function template}}
+   // expected-note@-1{{to befriend a template specialization, make sure the function template has already been declared and use '<>'}}
+};
+template 
+bool operator!=(const Arg& lhs, T rhs) {
+  return true;
+}
+bool foo() {
+  Arg arg;
+  return (arg == 42) || (arg != 42);
+}
+
+
+template class C0 {
+  friend void func0(C0 &);  // expected-warning{{friend declaration 

Re: [PATCH] D16579: Warn if friend function depends on template parameters.

[Serge Pavlov via cfe-commits]

sepavloff marked 6 inline comments as done.


Comment at: include/clang/Sema/Sema.h:9469
@@ +9468,3 @@
+  /// of proper templates, but they are needed for checks.
+  SmallVector FriendsOfTemplates;
+

Do we really need it? When compiler is compiling a module containing enclosing 
class, it must emit these warning, as it does for separate file. The advice it 
issues is valid in this case also. When compiler compiles code that uses the 
module, warning is not needed as it must already been issued. So, it looks like 
serializing this array is not needed.


Comment at: lib/Sema/SemaChecking.cpp:10936
@@ +10935,3 @@
+/// function types.
+/*
+class FunctionMatcher : public TypeMatcher {

The visitor class is split into two auxiliary classes. Although amount of code 
becomes larger, maintainability should be better, as large amount of code is 
obtained from RecursiveASTVisitor in standard way/ the other is not specific to 
friend functions.


http://reviews.llvm.org/D16579



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D16579: Warn if friend function depends on template parameters.

[Serge Pavlov via cfe-commits]

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

Repared file modes.


http://reviews.llvm.org/D16579

Files:
  include/clang/AST/TypeInstantiationMatcher.h
  include/clang/AST/TypeMatcher.h
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/Sema/Sema.cpp
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaTemplate.cpp
  test/CXX/drs/dr3xx.cpp
  test/CXX/drs/dr5xx.cpp
  test/SemaCXX/friend.cpp
  test/SemaCXX/overload-call.cpp

Index: test/SemaCXX/overload-call.cpp
===
--- test/SemaCXX/overload-call.cpp
+++ test/SemaCXX/overload-call.cpp
@@ -574,13 +574,17 @@
   // Ensure that overload resolution attempts to complete argument types when
   // performing ADL.
   template struct S {
-friend int f(const S&);
+friend int f(const S&);  // expected-warning{{friend declaration 'IncompleteArg::f' depends on template parameter but is not a function template}}
+ // expected-note@-1{{declare function outside class template to suppress this warning}}
+ // expected-note@-2{{to befriend a template specialization, make sure the function template has already been declared and use '<>'}}
   };
   extern S s;
   int k = f(s);
 
   template struct Op {
-friend bool operator==(const Op &, const Op &);
+friend bool operator==(const Op &, const Op &);  // expected-warning{{friend declaration 'IncompleteArg::operator==' depends on template parameter but is not a function template}}
+ // expected-note@-1{{declare function outside class template to suppress this warning}}
+ // expected-note@-2{{to befriend a template specialization, make sure the function template has already been declared and use '<>'}}
   };
   extern Op op;
   bool b = op == op;
Index: test/SemaCXX/friend.cpp
===
--- test/SemaCXX/friend.cpp
+++ test/SemaCXX/friend.cpp
@@ -379,3 +379,111 @@
 X *q = p;
   }
 }
+
+
+template void pr23342_func(T x);
+template
+struct pr23342_C1 {
+  friend void pr23342_func<>(T x);
+  friend bool func(T x);  // expected-warning{{friend declaration 'func' depends on template parameter but is not a function template}}
+  // expected-note@-1{{declare function outside class template to suppress this warning}}
+  // expected-note@-2{{to befriend a template specialization, make sure the function template has already been declared and use '<>'}}
+  friend bool func2(int x);
+  template friend bool func3(T2 x);
+  friend T func4();  // expected-warning{{friend declaration 'func4' depends on template parameter but is not a function template}}
+ // expected-note@-1{{declare function outside class template to suppress this warning}}
+ // expected-note@-2{{to befriend a template specialization, make sure the function template has already been declared and use '<>'}}
+};
+
+namespace pr23342 {
+
+template
+struct C1 {
+  friend void pr23342_func<>(T x);
+  friend bool func(T x);  // expected-warning{{friend declaration 'pr23342::func' depends on template parameter but is not a function template}}
+  // expected-note@-1{{declare function outside class template to suppress this warning}}
+  // expected-note@-2{{to befriend a template specialization, make sure the function template has already been declared and use '<>'}}
+  friend bool func2(int x);
+  template friend bool func3(T2 x);
+  friend T func4();// expected-warning{{friend declaration 'pr23342::func4' depends on template parameter but is not a function template}}
+   // expected-note@-1{{declare function outside class template to suppress this warning}}
+   // expected-note@-2{{to befriend a template specialization, make sure the function template has already been declared and use '<>'}}
+};
+
+template 
+struct Arg {
+  friend bool operator==(const Arg& lhs, T rhs) {
+   return false;
+  }
+  friend bool operator!=(const Arg& lhs, T rhs);  // expected-warning{{friend declaration 'pr23342::operator!=' depends on template parameter but is not a function template}}
+   // expected-note@-1{{to befriend a template specialization, make sure the function template has already been declared and use '<>'}}
+};
+template 
+bool operator!=(const Arg& lhs, T rhs) {
+  return true;
+}
+bool foo() {
+  Arg arg;
+  return (arg == 42) || (arg != 42);
+}
+
+
+template class C0 {
+  friend void func0(C0 &);  // expected-warning{{friend declaration 'pr23342::func0' depends on template parameter but is not a function template}}
+   // expected-note@-1{{declare function outside class template 

Re: [PATCH] D16579: Warn if friend function depends on template parameters.

[Serge Pavlov via cfe-commits]

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

Updated patch


http://reviews.llvm.org/D16579

Files:
  include/clang/AST/TypeInstantiationMatcher.h
  include/clang/AST/TypeMatcher.h
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/Sema/Sema.cpp
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaTemplate.cpp
  test/CXX/drs/dr3xx.cpp
  test/CXX/drs/dr5xx.cpp
  test/SemaCXX/friend.cpp
  test/SemaCXX/overload-call.cpp

Index: test/SemaCXX/overload-call.cpp
===
--- test/SemaCXX/overload-call.cpp
+++ test/SemaCXX/overload-call.cpp
@@ -574,13 +574,17 @@
   // Ensure that overload resolution attempts to complete argument types when
   // performing ADL.
   template struct S {
-friend int f(const S&);
+friend int f(const S&);  // expected-warning{{friend declaration 'IncompleteArg::f' depends on template parameter but is not a function template}}
+ // expected-note@-1{{declare function outside class template to suppress this warning}}
+ // expected-note@-2{{to befriend a template specialization, make sure the function template has already been declared and use '<>'}}
   };
   extern S s;
   int k = f(s);
 
   template struct Op {
-friend bool operator==(const Op &, const Op &);
+friend bool operator==(const Op &, const Op &);  // expected-warning{{friend declaration 'IncompleteArg::operator==' depends on template parameter but is not a function template}}
+ // expected-note@-1{{declare function outside class template to suppress this warning}}
+ // expected-note@-2{{to befriend a template specialization, make sure the function template has already been declared and use '<>'}}
   };
   extern Op op;
   bool b = op == op;
Index: test/SemaCXX/friend.cpp
===
--- test/SemaCXX/friend.cpp
+++ test/SemaCXX/friend.cpp
@@ -379,3 +379,111 @@
 X *q = p;
   }
 }
+
+
+template void pr23342_func(T x);
+template
+struct pr23342_C1 {
+  friend void pr23342_func<>(T x);
+  friend bool func(T x);  // expected-warning{{friend declaration 'func' depends on template parameter but is not a function template}}
+  // expected-note@-1{{declare function outside class template to suppress this warning}}
+  // expected-note@-2{{to befriend a template specialization, make sure the function template has already been declared and use '<>'}}
+  friend bool func2(int x);
+  template friend bool func3(T2 x);
+  friend T func4();  // expected-warning{{friend declaration 'func4' depends on template parameter but is not a function template}}
+ // expected-note@-1{{declare function outside class template to suppress this warning}}
+ // expected-note@-2{{to befriend a template specialization, make sure the function template has already been declared and use '<>'}}
+};
+
+namespace pr23342 {
+
+template
+struct C1 {
+  friend void pr23342_func<>(T x);
+  friend bool func(T x);  // expected-warning{{friend declaration 'pr23342::func' depends on template parameter but is not a function template}}
+  // expected-note@-1{{declare function outside class template to suppress this warning}}
+  // expected-note@-2{{to befriend a template specialization, make sure the function template has already been declared and use '<>'}}
+  friend bool func2(int x);
+  template friend bool func3(T2 x);
+  friend T func4();// expected-warning{{friend declaration 'pr23342::func4' depends on template parameter but is not a function template}}
+   // expected-note@-1{{declare function outside class template to suppress this warning}}
+   // expected-note@-2{{to befriend a template specialization, make sure the function template has already been declared and use '<>'}}
+};
+
+template 
+struct Arg {
+  friend bool operator==(const Arg& lhs, T rhs) {
+   return false;
+  }
+  friend bool operator!=(const Arg& lhs, T rhs);  // expected-warning{{friend declaration 'pr23342::operator!=' depends on template parameter but is not a function template}}
+   // expected-note@-1{{to befriend a template specialization, make sure the function template has already been declared and use '<>'}}
+};
+template 
+bool operator!=(const Arg& lhs, T rhs) {
+  return true;
+}
+bool foo() {
+  Arg arg;
+  return (arg == 42) || (arg != 42);
+}
+
+
+template class C0 {
+  friend void func0(C0 &);  // expected-warning{{friend declaration 'pr23342::func0' depends on template parameter but is not a function template}}
+   // expected-note@-1{{declare function outside class template to 

Re: [PATCH] D16579: Warn if friend function depends on template parameters.

[Richard Smith via cfe-commits]

rsmith added inline comments.


Comment at: include/clang/Basic/DiagnosticSemaKinds.td:1153
@@ +1152,3 @@
+def warn_non_template_friend : Warning<"friend declaration %q0 depends on "
+  "template parameter but is not a template function">,
+   InGroup;

template function -> function template


Comment at: include/clang/Basic/DiagnosticSemaKinds.td:1155-1157
@@ +1154,5 @@
+   InGroup;
+def note_non_template_friend : Note<"if this is intentional, add function "
+  "declaration to suppress the warning, if not - make sure the function "
+  "template has already been declared and use '<>'">;
+def note_befriend_template : Note<"to befriend a template specialization, "

Rather than this long conditional note, how about producing two notes here:

  note: declare function outside class template to suppress this warning
  note: use '<>' to befriend a template specialization


Comment at: include/clang/Basic/DiagnosticSemaKinds.td:1158-1159
@@ -1152,1 +1157,4 @@
+  "template has already been declared and use '<>'">;
+def note_befriend_template : Note<"to befriend a template specialization, "
+  "use '<>'">;
 

This note should also point out that you need to declare the function template 
prior to the class template.


Comment at: include/clang/Sema/Sema.h:9342
@@ +9341,3 @@
+  /// of proper templates, but they are needed for checks.
+  llvm::DenseSet FriendsOfTemplates;
+

This will produce diagnostics in a nondeterministic order, and you have no need 
of a set because there can never be any duplicates. Just use a `SmallVector`?


Comment at: include/clang/Sema/Sema.h:9342
@@ +9341,3 @@
+  /// of proper templates, but they are needed for checks.
+  llvm::DenseSet FriendsOfTemplates;
+

rsmith wrote:
> This will produce diagnostics in a nondeterministic order, and you have no 
> need of a set because there can never be any duplicates. Just use a 
> `SmallVector`?
You're missing serialization code for this for the PCH / preamble case.


Comment at: include/clang/Sema/Sema.h:9344-9345
@@ +9343,4 @@
+
+  /// \brief Check dependent friends functions for misinterpretation as 
template
+  /// ones.
+  void CheckDependentFriends();

friends -> friend
template ones -> function templates


Comment at: lib/Sema/SemaChecking.cpp:10468
@@ +10467,3 @@
+/// function types.
+class FunctionMatcher : public TypeVisitor {
+  const Type *InstType;

This is a huge amount of complexity and maintenance burden for this check, and 
it still seems to some important cases wrong. Is there a simpler way we can get 
a largely-equivalent result? Or maybe we can just unconditionally produce both 
notes?


Comment at: lib/Sema/SemaChecking.cpp:10700
@@ +10699,2 @@
+  }
+}

`FriendsOfTemplates.clear()` here?


Comment at: lib/Sema/SemaDecl.cpp:8502-8507
@@ -8501,1 +8501,8 @@
 
+  if (isFriend && !NewFD->isInvalidDecl()) {
+if (TemplateParamLists.empty() && !DC->isRecord() &&
+!isFunctionTemplateSpecialization) {
+  FriendsOfTemplates.insert(NewFD);
+}
+  }
+

This should also be conditioned on the function having a dependent type and 
this declaration not being a definition -- this will add all non-template 
friends to `FriendsOfTemplates`, whether they're friends of a class template or 
not.


http://reviews.llvm.org/D16579



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D16579: Warn if friend function depends on template parameters.

[Serge Pavlov via cfe-commits]

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

Changed implementation of friend function set.

Friend functions declared in template classes are not added to redeclaration 
chains,
but access to them is needed to make some checks. This change allows using the
set of friend functions not only to emit particular warning but also to serve 
as a
registry of file-level friend functions.


http://reviews.llvm.org/D16579

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/Sema/Sema.cpp
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaDecl.cpp
  test/CXX/drs/dr3xx.cpp
  test/CXX/drs/dr5xx.cpp
  test/SemaCXX/friend.cpp
  test/SemaCXX/overload-call.cpp

Index: test/SemaCXX/overload-call.cpp
===
--- test/SemaCXX/overload-call.cpp
+++ test/SemaCXX/overload-call.cpp
@@ -574,13 +574,15 @@
   // Ensure that overload resolution attempts to complete argument types when
   // performing ADL.
   template struct S {
-friend int f(const S&);
+friend int f(const S&);  // expected-warning{{friend declaration 'IncompleteArg::f' depends on template parameter but is not a template function}}
+ // expected-note@-1{{if this is intentional, add function declaration to suppress the warning, if not - make sure the function template has already been declared and use '<>'}}
   };
   extern S s;
   int k = f(s);
 
   template struct Op {
-friend bool operator==(const Op &, const Op &);
+friend bool operator==(const Op &, const Op &);  // expected-warning{{friend declaration 'IncompleteArg::operator==' depends on template parameter but is not a template function}}
+ // expected-note@-1{{if this is intentional, add function declaration to suppress the warning, if not - make sure the function template has already been declared and use '<>'}}
   };
   extern Op op;
   bool b = op == op;
Index: test/SemaCXX/friend.cpp
===
--- test/SemaCXX/friend.cpp
+++ test/SemaCXX/friend.cpp
@@ -363,3 +363,102 @@
   f_pr6954(5); // expected-error{{undeclared identifier 'f_pr6954'}}
 }
 
+
+template void pr23342_func(T x);
+template
+struct pr23342_C1 {
+  friend void pr23342_func<>(T x);
+  friend bool func(T x);  // expected-warning{{friend declaration 'func' depends on template parameter but is not a template function}}
+  // expected-note@-1{{if this is intentional, add function declaration to suppress the warning, if not - make sure the function template has already been declared and use '<>'}}
+  friend bool func2(int x);
+  template friend bool func3(T2 x);
+  friend T func4();  // expected-warning{{friend declaration 'func4' depends on template parameter but is not a template function}}
+ // expected-note@-1{{if this is intentional, add function declaration to suppress the warning, if not - make sure the function template has already been declared and use '<>'}}
+};
+
+namespace pr23342 {
+
+template
+struct C1 {
+  friend void pr23342_func<>(T x);
+  friend bool func(T x);  // expected-warning{{friend declaration 'pr23342::func' depends on template parameter but is not a template function}}
+  // expected-note@-1{{if this is intentional, add function declaration to suppress the warning, if not - make sure the function template has already been declared and use '<>'}}
+  friend bool func2(int x);
+  template friend bool func3(T2 x);
+  friend T func4();// expected-warning{{friend declaration 'pr23342::func4' depends on template parameter but is not a template function}}
+   // expected-note@-1{{if this is intentional, add function declaration to suppress the warning, if not - make sure the function template has already been declared and use '<>'}}
+};
+
+template 
+struct Arg {
+  friend bool operator==(const Arg& lhs, T rhs) {
+   return false;
+  }
+  friend bool operator!=(const Arg& lhs, T rhs);  // expected-warning{{friend declaration 'pr23342::operator!=' depends on template parameter but is not a template function}}
+   // expected-note@-1{{to befriend a template specialization, use '<>'}}
+};
+template 
+bool operator!=(const Arg& lhs, T rhs) {
+  return true;
+}
+bool foo() {
+  Arg arg;
+  return (arg == 42) || (arg != 42);
+}
+
+
+template class C0 {
+  friend void func0(C0 &);  // expected-warning{{friend declaration 'pr23342::func0' depends on template parameter but is not a template function}}
+   // expected-note@-1{{if this is intentional, add function declaration to suppress the warning, if not - make sure the function template has already been declared and use '<>'}}
+};
+
+template class C0a {
+  friend void func0a(C0a &);  // expected-warning{{friend declaration 

Re: [PATCH] D16579: Warn if friend function depends on template parameters.

[Serge Pavlov via cfe-commits]

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

Updated patch

Added a check for presence of template declaration corresponding to the friend
function. Presence of similar functions but with specializations as argument
types is also checked now.

Made an atempt to make messages more clear to end user, not sure however if
they become better.


http://reviews.llvm.org/D16579

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/Sema/Sema.cpp
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaDecl.cpp
  test/CXX/drs/dr3xx.cpp
  test/CXX/drs/dr5xx.cpp
  test/SemaCXX/friend.cpp
  test/SemaCXX/overload-call.cpp

Index: test/SemaCXX/overload-call.cpp
===
--- test/SemaCXX/overload-call.cpp
+++ test/SemaCXX/overload-call.cpp
@@ -574,13 +574,15 @@
   // Ensure that overload resolution attempts to complete argument types when
   // performing ADL.
   template struct S {
-friend int f(const S&);
+friend int f(const S&);  // expected-warning{{friend declaration 'IncompleteArg::f' depends on template parameter but is not a template function}}
+ // expected-note@-1{{if this is intentional, add function declaration to suppress the warning, if not - make sure the function template has already been declared and use '<>'}}
   };
   extern S s;
   int k = f(s);
 
   template struct Op {
-friend bool operator==(const Op &, const Op &);
+friend bool operator==(const Op &, const Op &);  // expected-warning{{friend declaration 'IncompleteArg::operator==' depends on template parameter but is not a template function}}
+ // expected-note@-1{{if this is intentional, add function declaration to suppress the warning, if not - make sure the function template has already been declared and use '<>'}}
   };
   extern Op op;
   bool b = op == op;
Index: test/SemaCXX/friend.cpp
===
--- test/SemaCXX/friend.cpp
+++ test/SemaCXX/friend.cpp
@@ -363,3 +363,102 @@
   f_pr6954(5); // expected-error{{undeclared identifier 'f_pr6954'}}
 }
 
+
+template void pr23342_func(T x);
+template
+struct pr23342_C1 {
+  friend void pr23342_func<>(T x);
+  friend bool func(T x);  // expected-warning{{friend declaration 'func' depends on template parameter but is not a template function}}
+  // expected-note@-1{{if this is intentional, add function declaration to suppress the warning, if not - make sure the function template has already been declared and use '<>'}}
+  friend bool func2(int x);
+  template friend bool func3(T2 x);
+  friend T func4();  // expected-warning{{friend declaration 'func4' depends on template parameter but is not a template function}}
+ // expected-note@-1{{if this is intentional, add function declaration to suppress the warning, if not - make sure the function template has already been declared and use '<>'}}
+};
+
+namespace pr23342 {
+
+template
+struct C1 {
+  friend void pr23342_func<>(T x);
+  friend bool func(T x);  // expected-warning{{friend declaration 'pr23342::func' depends on template parameter but is not a template function}}
+  // expected-note@-1{{if this is intentional, add function declaration to suppress the warning, if not - make sure the function template has already been declared and use '<>'}}
+  friend bool func2(int x);
+  template friend bool func3(T2 x);
+  friend T func4();// expected-warning{{friend declaration 'pr23342::func4' depends on template parameter but is not a template function}}
+   // expected-note@-1{{if this is intentional, add function declaration to suppress the warning, if not - make sure the function template has already been declared and use '<>'}}
+};
+
+template 
+struct Arg {
+  friend bool operator==(const Arg& lhs, T rhs) {
+   return false;
+  }
+  friend bool operator!=(const Arg& lhs, T rhs);  // expected-warning{{friend declaration 'pr23342::operator!=' depends on template parameter but is not a template function}}
+   // expected-note@-1{{to befriend a template specialization, use '<>'}}
+};
+template 
+bool operator!=(const Arg& lhs, T rhs) {
+  return true;
+}
+bool foo() {
+  Arg arg;
+  return (arg == 42) || (arg != 42);
+}
+
+
+template class C0 {
+  friend void func0(C0 &);  // expected-warning{{friend declaration 'pr23342::func0' depends on template parameter but is not a template function}}
+   // expected-note@-1{{if this is intentional, add function declaration to suppress the warning, if not - make sure the function template has already been declared and use '<>'}}
+};
+
+template class C0a {
+  friend void func0a(C0a &);  // expected-warning{{friend declaration 'pr23342::func0a' depends on template parameter but is not 

Re: [PATCH] D16579: Warn if friend function depends on template parameters.

[Serge Pavlov via cfe-commits]

2016-03-18 0:35 GMT+06:00 Arthur O'Dwyer :

> I'm not qualified to comment on the implementation, but I'm a bit
> skeptical that this warning is appropriate in the first place. I've often
> declared friend non-template functions, e.g. swap(). I've never intended to
> declare a friend *template specialization*. Is declaring friend template
> specializations something that people do often? Is it something that Clang
> should be encouraging newbies to do *more* often?
>

It is not about intention but misunderstanding. Users often think that if a
function is declared inside a template class and depends on template
parameters, it is template. That expect the following code to work:

template void foo(T *x) {…}
template class Bar {
  void foo(T *x);
}

There are lots of questions in forums (search by keywords "friend template
function link error"), in which people feel surprised why such code results
in undefined reference. The proposed compiler message is just a hint how to
make the code working as expected by a user.


>
> GCC suppresses the diagnostic when the friend function is declared inline,
> which is good, because it shuts up the diagnostic in this very common case:
>
> template
> struct vector {
> void swap(vector& b) { /* swap members */ }
> friend void swap(vector& a, vector& b) { a.swap(b); }  // friend
> non-template function
> };
>
> IMHO you should add an explicit test confirming that the warning is
> suppressed in this case.
>

There is such check already, file `test/SemaCXX/friend.cpp`:

template 
struct Arg {
  friend bool operator==(const Arg& lhs, T rhs) {
   return false;
  }
…
};
…
bool foo() {
  Arg arg;
  return (arg == 42) || (arg != 42);
}

Operator `==` is defined inline, no warning expected.


>
The example in PR23342 doesn't seem like a good example, because GCC gives
> not only the proposed warning for that code, but also a linker error; i.e.,
> there's no danger of someone accidentally getting wrong runtime behavior
> there. Is there a compelling
>

Clang behaves exactly the same, compilation eventually results in a linker
error.


> example of how someone could write code that has wrong runtime behavior,
> *and* which code would be *fixed* by the suggested addition of template<>?
> I think it's much more likely that the appropriate fix would be to move the
> function definition inline.
>

There seems to be no danger to get wrong behavior here, only undefined
symbols at link stage. If code builds, it works right.


> https://llvm.org/bugs/show_bug.cgi?id=23342
>
> Why did you need to suppress the warning in
> "test/CXX/temp/temp.decls/temp.friend/p1.cpp"? Unless I'm mistaken, GCC
> doesn't give the diagnostic on that file (and nor should it).
>
> my $.02,
>

Good catch, thank you for your $0.02 :) There is no point in warnings for
class members. Patch fixed accordingly.

Thanks,
--Serge
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D16579: Warn if friend function depends on template parameters.

[Serge Pavlov via cfe-commits]

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

Do not emit warning on class methods.


http://reviews.llvm.org/D16579

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDecl.cpp
  test/CXX/drs/dr3xx.cpp
  test/CXX/drs/dr5xx.cpp
  test/SemaCXX/friend.cpp
  test/SemaCXX/overload-call.cpp

Index: test/SemaCXX/overload-call.cpp
===
--- test/SemaCXX/overload-call.cpp
+++ test/SemaCXX/overload-call.cpp
@@ -574,13 +574,15 @@
   // Ensure that overload resolution attempts to complete argument types when
   // performing ADL.
   template struct S {
-friend int f(const S&);
+friend int f(const S&);  // expected-warning{{friend declaration 'IncompleteArg::f' declares a non-template function}}
+ // expected-note@-1{{if this is not what you intended, make sure the function template has already been declared and add <> after the function name here}}
   };
   extern S s;
   int k = f(s);
 
   template struct Op {
-friend bool operator==(const Op &, const Op &);
+friend bool operator==(const Op &, const Op &);  // expected-warning{{friend declaration 'IncompleteArg::operator==' declares a non-template function}}
+ // expected-note@-1{{if this is not what you intended, make sure the function template has already been declared and add <> after the function name here}}
   };
   extern Op op;
   bool b = op == op;
Index: test/SemaCXX/friend.cpp
===
--- test/SemaCXX/friend.cpp
+++ test/SemaCXX/friend.cpp
@@ -363,3 +363,47 @@
   f_pr6954(5); // expected-error{{undeclared identifier 'f_pr6954'}}
 }
 
+
+template void pr23342_func(T x);
+template
+struct pr23342_C1 {
+  friend void pr23342_func<>(T x);
+  friend bool func(T x);  // expected-warning{{friend declaration 'func' declares a non-template function}}
+  // expected-note@-1{{if this is not what you intended, make sure the function template has already been declared and add <> after the function name here}}
+  friend bool func2(int x);
+  template friend bool func3(T2 x);
+  friend T func4();  // expected-warning{{friend declaration 'func4' declares a non-template function}}
+ // expected-note@-1{{if this is not what you intended, make sure the function template has already been declared and add <> after the function name here}}
+};
+
+namespace pr23342 {
+
+template
+struct C1 {
+  friend void pr23342_func<>(T x);
+  friend bool func(T x);  // expected-warning{{friend declaration 'pr23342::func' declares a non-template function}}
+  // expected-note@-1{{if this is not what you intended, make sure the function template has already been declared and add <> after the function name here}}
+  friend bool func2(int x);
+  template friend bool func3(T2 x);
+  friend T func4();// expected-warning{{friend declaration 'pr23342::func4' declares a non-template function}}
+   // expected-note@-1{{if this is not what you intended, make sure the function template has already been declared and add <> after the function name here}}};
+};
+
+template 
+struct Arg {
+  friend bool operator==(const Arg& lhs, T rhs) {
+   return false;
+  }
+  friend bool operator!=(const Arg& lhs, T rhs);  // expected-warning{{friend declaration 'pr23342::operator!=' declares a non-template function}}
+   // expected-note@-1{{if this is not what you intended, make sure the function template has already been declared and add <> after the function name here}}};
+};
+template 
+bool operator!=(const Arg& lhs, T rhs) {
+  return true;
+}
+bool foo() {
+  Arg arg;
+  return (arg == 42) || (arg != 42);
+}
+
+}
Index: test/CXX/drs/dr5xx.cpp
===
--- test/CXX/drs/dr5xx.cpp
+++ test/CXX/drs/dr5xx.cpp
@@ -581,8 +581,10 @@
 
 namespace dr557 { // dr557: yes
   template struct S {
-friend void f(S *);
-friend void g(S *);
+friend void f(S *);  // expected-warning{{friend declaration 'dr557::f' declares a non-template function}}
+// expected-note@-1{{if this is not what you intended, make sure the function template has already been declared and add <> after the function name here}}
+friend void g(S *); // expected-warning{{friend declaration 'dr557::g' declares a non-template function}}
+// expected-note@-1{{if this is not what you intended, make sure the function template has already been declared and add <> after the function name here}}
   };
   void x(S *p, S *q) {
 f(p);
Index: test/CXX/drs/dr3xx.cpp
===
--- test/CXX/drs/dr3xx.cpp
+++ test/CXX/drs/dr3xx.cpp
@@ -291,7 +291,8 @@
   template