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<typename T> 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<int> s;
   int k = f(s);
 
   template<typename T> 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<char> 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<typename T> void pr23342_func(T x);
+template<typename T>
+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<typename T2> 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<typename T>
+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<typename T2> 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 <typename T>
+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 <typename T>
+bool operator!=(const Arg<T>& lhs, T rhs) {
+  return true;
+}
+bool foo() {
+  Arg<int> arg;
+  return (arg == 42) || (arg != 42);
+}
+
+
+template<typename T> class C0 {
+  friend void func0(C0<T> &);  // 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 suppress this warning}}
+                               // expected-note@-2{{to befriend a template specialization, make sure the function template has already been declared and use '<>'}}
+};
+
+template<typename T> class C0a {
+  friend void func0a(C0a<T> &);  // expected-warning{{friend declaration 'pr23342::func0a' 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 '<>'}}
+};
+void func0a(C0a<int> x);
+void func0a(C0a<int> *x);
+void func0a(const C0a<int> &x);
+int func0a(C0a<int> &x);
+
+template<typename T> class C0b {
+  friend void func0b(C0b<T> &x);
+};
+void func0b(C0b<int> &x);
+
+
+template<typename T> class C1a {
+  friend void func1a(T x, C1a<T> &y); // expected-warning{{friend declaration 'pr23342::func1a' 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 '<>'}}
+};
+void func1a(char x, C1a<int> &y);
+
+template<typename T> class C1b {
+  friend void func1b(T x, C1b<T> &y);
+};
+void func1b(int x, C1b<int> &y);
+
+
+template<typename T> class C2a {
+  friend void func2a(C2a<T> &); // expected-warning{{friend declaration 'pr23342::func2a' 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<typename T>
+void func2a(const C2a<T> &x);
+
+
+template<typename T> class C2b {
+  friend void func2b(C2b<T> &); // expected-warning{{friend declaration 'pr23342::func2b' 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<typename T>
+void func2b(C2b<T> &x);
+
+template<typename T> class C2c;
+template<typename T>
+void func2c(C2c<T> &x);
+template<typename T> class C2c {
+  friend void func2c<>(C2c<T> &);
+};
+
+template<typename T> class C3 {
+  friend void func3(T &);
+};
+void func3(int &);
+template<typename T> void func3(T &);
+
+}
Index: test/CXX/temp/temp.res/p3.cpp
===================================================================
--- test/CXX/temp/temp.res/p3.cpp
+++ test/CXX/temp/temp.res/p3.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -verify %s -std=c++11
+// RUN: %clang_cc1 -verify %s -std=c++11 -Wno-non-template-friend
 
 template<typename T> struct A {
   template<typename U> struct B;
Index: test/CXX/drs/dr5xx.cpp
===================================================================
--- test/CXX/drs/dr5xx.cpp
+++ test/CXX/drs/dr5xx.cpp
@@ -587,8 +587,12 @@
 
 namespace dr557 { // dr557: yes
   template<typename T> struct S {
-    friend void f(S<T> *);
-    friend void g(S<S<T> > *);
+    friend void f(S<T> *);  // expected-warning{{friend declaration 'dr557::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 '<>'}}
+    friend void g(S<S<T> > *); // expected-warning{{friend declaration 'dr557::g' 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 '<>'}}
   };
   void x(S<int> *p, S<S<int> > *q) {
     f(p);
Index: test/CXX/drs/dr3xx.cpp
===================================================================
--- test/CXX/drs/dr3xx.cpp
+++ test/CXX/drs/dr3xx.cpp
@@ -291,7 +291,9 @@
   template void g(N::A<0>::B<0>);
 
   namespace N {
-    template<typename> struct I { friend bool operator==(const I&, const I&); };
+    template<typename> struct I { friend bool operator==(const I&, const I&); };  // expected-warning{{friend declaration 'dr321::N::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 '<>'}}
   }
   N::I<int> i, j;
   bool x = i == j;
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -9155,6 +9155,25 @@
     AddToScope = false;
   }
 
+  // If this is a friend function declaration inside a template class and the
+  // function depends on the template parameter as in:
+  //
+  //     template<typename T> class C {
+  //         friend void func(T &x);
+  //     };
+  //
+  // store it in the Sema. Later on such declaration will be checked if it is
+  // a misprint of a function template.
+  if (isFriend && !NewFD->isInvalidDecl()) {
+    if (TemplateParamLists.empty() &&
+        !DC->isRecord() &&
+        !isFunctionTemplateSpecialization &&
+        NewFD->getType()->isDependentType() &&
+        !NewFD->isThisDeclarationADefinition()) {
+      FriendsOfTemplates.push_back(NewFD);
+    }
+  }
+
   return NewFD;
 }
 
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -12191,3 +12191,203 @@
                      _2, _3, _4));
 }
 
+
+namespace {
+/// Helper class used to check if a friend declaration may refer to another
+/// function declaration.
+///
+/// The class is used to compare two function declarations, one is a friend
+/// function declared in template class, the other is a function with the same
+/// name declared at file level.
+///
+/// The class is used to produce appropriate warning if friend function declared
+/// in a template class depends on that class template parameters.
+///
+class FunctionMatcher {
+  ASTContext &Ctx;
+  std::map<const TemplateTypeParmType *, QualType> TemplParamMapping;
+
+  bool match(QualType TFriend, QualType TFunc) {
+    if (TFriend == TFunc)
+      return true;
+    if (TFriend.getQualifiers() != TFunc.getQualifiers())
+      return false;
+    if (auto FrT = TFriend->getAs<TemplateTypeParmType>()) {
+      if (auto FT = TFunc->getAs<TemplateTypeParmType>())
+        return FrT->getDepth() == FT->getDepth() &&
+        FrT->getIndex() == FT->getIndex();
+
+      const TemplateTypeParmType *CanonFrTParm = cast<TemplateTypeParmType>(
+        FrT->getCanonicalTypeUnqualified().getTypePtr());
+      CanQualType CanonFT = TFunc->getCanonicalTypeUnqualified();
+      auto Ptr = TemplParamMapping.find(CanonFrTParm);
+      if (Ptr != TemplParamMapping.end())
+        return Ptr->second == CanonFT;
+      TemplParamMapping[CanonFrTParm] = CanonFT;
+      return true;
+    }
+    if (auto FrPT = TFriend->getAs<PointerType>()) {
+      if (auto PT = TFunc->getAs<PointerType>())
+        return match(FrPT->getPointeeType(), PT->getPointeeType());
+      return false;
+    }
+    if (auto FrLR = TFriend->getAs<LValueReferenceType>()) {
+      if (auto LR = TFunc->getAs<LValueReferenceType>())
+        return match(FrLR->getPointeeType(), LR->getPointeeType());
+      return false;
+    }
+    if (auto FrRR = TFriend->getAs<RValueReferenceType>()) {
+      if (auto RR = TFunc->getAs<RValueReferenceType>())
+        return match(FrRR->getPointeeType(), RR->getPointeeType());
+      return false;
+    }
+    if (auto FrT = TFriend->getAs<InjectedClassNameType>()) {
+      QualType FrT2 = Ctx.getQualifiedType(FrT->getInjectedSpecializationType(),
+                                           TFriend.getQualifiers());
+      return match(FrT2, TFunc);
+    }
+    if (auto FrT = TFriend->getAs<FunctionProtoType>()) {
+      if (auto FT = TFunc->getAs<FunctionProtoType>()) {
+        if (FrT->getNumParams() != FT->getNumParams())
+          return false;
+        for (unsigned I = 0, E = FT->getNumParams(); I < E; ++I) {
+          if (!match(FrT->getParamType(I), FT->getParamType(I)))
+            return false;
+        }
+        return match(FrT->getReturnType(), FT->getReturnType());
+      }
+      return false;
+    }
+    if (auto *FrT = TFriend->getAs<TemplateSpecializationType>()) {
+      if (auto FT = TFunc->getAs<TemplateSpecializationType>()) {
+        TemplateName FrTN = FrT->getTemplateName();
+        TemplateName FTN = FT->getTemplateName();
+        if (FrTN.getKind() != TemplateName::Template ||
+            FrTN.getKind() != FTN.getKind())
+          return false;
+        if (FrTN.isInstantiationDependent() || FTN.isInstantiationDependent())
+          return false;
+        const Decl *FrD = FrTN.getAsTemplateDecl()->getCanonicalDecl();
+        const Decl *FD = FTN.getAsTemplateDecl()->getCanonicalDecl();
+        if (FrD != FD)
+          return false;
+
+        auto IFrT = FrT->begin(), EFrT = FrT->end();
+        auto IFT = FT->begin();
+        for (; IFrT != EFrT; ++IFrT, ++IFT) {
+          assert(IFT != FT->end());
+          const TemplateArgument &FrTA = *IFrT;
+          const TemplateArgument &FTA = *IFT;
+          if (FrTA.getKind() != FTA.getKind())
+            return false;
+          if (FrTA.getKind() == TemplateArgument::Type) {
+            QualType FrTy = FrTA.getAsType();
+            QualType FTy = FTA.getAsType();
+            if (!match(FrTy, FTy))
+              return false;
+          } else if (FrTA.getKind() == TemplateArgument::Integral) {
+            if (FrTA.getIntegralType() != FTA.getIntegralType())
+              return false;
+            llvm::APSInt FrV = FrTA.getAsIntegral();
+            llvm::APSInt FV = FTA.getAsIntegral();
+            if (FrV != FV)
+              return false;
+          } else
+            return false;
+        }
+        return true;
+      }
+      return false;
+    }
+
+    return false;
+  }
+
+public:
+  FunctionMatcher (ASTContext &Ctx) : Ctx(Ctx) {}
+
+  bool isPossibleMatch(FunctionDecl *FrD, FunctionDecl *F) {
+    TemplParamMapping.clear();
+    return match(FrD->getType(), F->getType());
+  }
+
+  bool isPossibleMatch(FunctionDecl *FrD, FunctionTemplateDecl *F) {
+    TemplParamMapping.clear();
+    FunctionDecl *Pattern = F->getTemplatedDecl();
+    return match(FrD->getType(), Pattern->getType());
+  }
+};
+}
+
+/// Given a friend function declaration checks if it might be misused.
+static void checkDependentFriend(Sema &S, FunctionDecl *FriendD) {
+  if (FriendD->isInvalidDecl())
+    return;
+
+  // Scan all function and function template declarations with the same name as
+  // the specified friend function to see if some of them could be referenced in
+  // the friend declaration.
+  LookupResult FRes(S, FriendD->getNameInfo(), Sema::LookupOrdinaryName);
+  if (S.LookupQualifiedName(FRes, FriendD->getDeclContext())) {
+
+    // First check for suitable functions that uses particular specialization of
+    // parameter type, for example:
+    //
+    //     template<typename T> class C {
+    //         friend void func(T &x);
+    //     };
+    //     void func(int &x);
+    //
+    // Presence of such function can be considered as an evidence that the
+    // friend function is dependent intentionally. No warning in this case.
+    for (NamedDecl *D : FRes) {
+      if (D->isInvalidDecl())
+        continue;
+      if (auto *FD = dyn_cast<FunctionDecl>(D)) {
+        if (FunctionMatcher(S.getASTContext()).isPossibleMatch(FriendD, FD))
+          return;
+      }
+    }
+
+    // Next check if there is suitable function template, like:
+    //
+    //     template<typename T> class C {
+    //         friend void func(T &x);
+    //     };
+    //     template<typename T> void func(T &x);
+    //
+    // If such template exists, issue warning and propose the template as a
+    // friend.
+    for (NamedDecl *D : FRes) {
+      if (D->isInvalidDecl())
+        continue;
+      if (auto *FTD = dyn_cast<FunctionTemplateDecl>(D))
+        if (FunctionMatcher(S.getASTContext()).isPossibleMatch(FriendD, FTD)) {
+          // Appropriate function template is found.
+          S.Diag(FriendD->getLocation(), diag::warn_non_template_friend)
+              << FriendD;
+          SourceLocation NameLoc = FriendD->getNameInfo().getLocEnd();
+          SourceLocation PastName = S.getLocForEndOfToken(NameLoc);
+          S.Diag(PastName, diag::note_befriend_template)
+            << FixItHint::CreateInsertion(PastName, "<>");
+          return;
+        }
+    }
+  }
+
+  // No appropriate function or function template was found, so issue generic
+  // warning.
+  S.Diag(FriendD->getLocation(), diag::warn_non_template_friend) << FriendD;
+  S.Diag(FriendD->getLocation(), diag::note_add_template_friend_decl);
+  S.Diag(FriendD->getLocation(), diag::note_befriend_template);
+}
+
+
+void Sema::checkDependentFriends() {
+  for (FunctionDecl *FriendD : FriendsOfTemplates) {
+    if (FriendD->getType()->isDependentType() &&
+        !FriendD->isThisDeclarationADefinition())
+      checkDependentFriend(*this, FriendD);
+  }
+  FriendsOfTemplates.clear();
+}
Index: lib/Sema/Sema.cpp
===================================================================
--- lib/Sema/Sema.cpp
+++ lib/Sema/Sema.cpp
@@ -764,6 +764,7 @@
       LateTemplateParserCleanup(OpaqueParser);
 
     CheckDelayedMemberExceptionSpecs();
+    checkDependentFriends();
   }
 
   DiagnoseUnterminatedPragmaAttribute();
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -10318,6 +10318,27 @@
   /// attempts to add itself into the container
   void CheckObjCCircularContainer(ObjCMessageExpr *Message);
 
+  /// Set of file level friend function declared in template classes and
+  /// dependent on the template parameter.
+  ///
+  /// It is kept to check, if friend function declaration in the code like:
+  /// \code
+  ///     template<typename T> class C {
+  ///       friend void func(T &x);
+  ///     };
+  /// \endcode
+  ///
+  /// is actually intended to be:
+  ///
+  /// \code
+  ///       template<typename T2> friend void func(T2 &x);
+  /// \endcode
+  SmallVector<FunctionDecl *, 16> FriendsOfTemplates;
+
+  /// Check dependent friend functions for misinterpretation as function
+  /// templates.
+  void checkDependentFriends();
+
   void AnalyzeDeleteExprMismatch(const CXXDeleteExpr *DE);
   void AnalyzeDeleteExprMismatch(FieldDecl *Field, SourceLocation DeleteLoc,
                                  bool DeleteWasArrayForm);
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -1271,6 +1271,13 @@
   "enclosing namespace is a Microsoft extension; add a nested name specifier">,
   InGroup<MicrosoftUnqualifiedFriend>;
 def err_pure_friend : Error<"friend declaration cannot have a pure-specifier">;
+def warn_non_template_friend : Warning<"friend declaration %q0 depends on "
+  "template parameter but is not a function template">,
+   InGroup<NonTemplateFriend>;
+def note_add_template_friend_decl : Note<"declare function outside class "
+  "template to suppress this warning">;
+def note_befriend_template : Note<"to befriend a template specialization, "
+  "make sure the function template has already been declared and use '<>'">;
 
 def err_invalid_member_in_interface : Error<
   "%select{data member |non-public member function |static member function |"
Index: include/clang/Basic/DiagnosticGroups.td
===================================================================
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -306,6 +306,7 @@
 def InitializerOverrides : DiagGroup<"initializer-overrides">;
 def NonNull : DiagGroup<"nonnull">;
 def NonPODVarargs : DiagGroup<"non-pod-varargs">;
+def NonTemplateFriend : DiagGroup<"non-template-friend">;
 def ClassVarargs : DiagGroup<"class-varargs", [NonPODVarargs]>;
 def : DiagGroup<"nonportable-cfstrings">;
 def NonVirtualDtor : DiagGroup<"non-virtual-dtor">;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to