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<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 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<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 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<char> 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<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 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<typename T2> 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<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 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<typename T2> 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 <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 template function}}
+                       // expected-note@-1{{to befriend a template specialization, 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 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<typename T> class C0a {
+  friend void func0a(C0a<T> &);  // expected-warning{{friend declaration 'pr23342::func0a' 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 '<>'}}
+};
+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 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 '<>'}}
+};
+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 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<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 template function}}
+                                // expected-note@-1{{to befriend a template specialization, 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> &);
+};
+
+}
\ No newline at end of file
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<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 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 void g(S<S<T> > *); // expected-warning{{friend declaration 'dr557::g' 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 '<>'}}
   };
   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,8 @@
   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 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 '<>'}}
   }
   N::I<int> i, j;
   bool x = i == j;
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -8431,6 +8431,14 @@
     AddToScope = false;
   }
 
+  if (isFriend && !NewFD->isInvalidDecl()) {
+    if (R->isDependentType() && TemplateParamLists.empty() &&
+        !DC->isRecord() && !isFunctionTemplateSpecialization &&
+        (D.getFunctionDefinitionKind() == FDK_Declaration)) {
+      DependentFriend.insert(NewFD);
+    }
+  }
+
   return NewFD;
 }
 
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -24,6 +24,7 @@
 #include "clang/AST/ExprOpenMP.h"
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
+#include "clang/AST/TypeVisitor.h"
 #include "clang/Analysis/Analyses/FormatString.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/TargetBuiltins.h"
@@ -10398,3 +10399,243 @@
         << ArgumentExpr->getSourceRange()
         << TypeTagExpr->getSourceRange();
 }
+
+namespace {
+
+/// \brief 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 declared at
+/// file level. Both functions must have the same name, this class checks only
+/// function types.
+class FunctionMatcher : public TypeVisitor<FunctionMatcher, bool> {
+  const Type *InstType;
+  std::map<const TemplateTypeParmType *, const Type *> ParamMapping;
+public:
+  FunctionMatcher() : InstType(nullptr) {}
+
+  bool match(const Type *ProtoT, const Type *InstT) {
+    InstType = InstT;
+    return Visit(ProtoT);
+  }
+  bool match(QualType ProtoT, QualType InstT) {
+    if (ProtoT.getCVRQualifiers() != InstT.getCVRQualifiers())
+      return false;
+    InstType = InstT.getCanonicalType().getTypePtr();
+    return Visit(ProtoT.getCanonicalType().getTypePtr());
+  }
+
+  bool VisitType(const Type *T) {
+    return T == InstType;
+  }
+
+  bool VisitComplexType(const ComplexType *T) {
+    if (T == InstType)
+      return true;
+    if (const auto *IT = dyn_cast<ComplexType>(InstType))
+      return match(T->getElementType(), IT->getElementType());
+    return false;
+  }
+
+  bool VisitPointerType(const PointerType *T) {
+    if (T == InstType)
+      return true;
+    if (const auto *IT = dyn_cast<PointerType>(InstType))
+      return match(T->getPointeeType(), IT->getPointeeType());
+    return false;
+  }
+
+  bool VisitBlockPointerType(const BlockPointerType *T) {
+    if (T == InstType)
+      return true;
+    if (const auto *IT = dyn_cast<BlockPointerType>(InstType))
+      return match(T->getPointeeType(), IT->getPointeeType());
+    return false;
+  }
+
+  bool VisitReferenceType(const ReferenceType *T) {
+    if (T == InstType)
+      return true;
+    if (T->getTypeClass() != InstType->getTypeClass())
+      return false;
+    if (const auto *IT = dyn_cast<ReferenceType>(InstType))
+      return match(T->getPointeeType(), IT->getPointeeType());
+    return false;
+  }
+
+  bool VisitMemberPointerType(const MemberPointerType *T) {
+    if (T == InstType)
+      return true;
+    if (const auto *IT = dyn_cast<MemberPointerType>(InstType))
+      return match(T->getPointeeType(), IT->getPointeeType()) &&
+             match(T->getClass(), IT->getClass());
+    return false;
+  }
+
+  bool VisitArrayType(const ArrayType *T) {
+    if (T == InstType)
+      return true;
+    if (const auto *IT = dyn_cast<ArrayType>(InstType))
+      return match(T->getElementType(), IT->getElementType());
+    return false;
+  }
+
+  bool VisitConstantArrayType(const ConstantArrayType *T) {
+    if (T == InstType)
+      return true;
+    if (const auto *IT = dyn_cast<ConstantArrayType>(InstType))
+      return match(T->getElementType(), IT->getElementType()) &&
+             T->getSize() == IT->getSize();
+    return false;
+  }
+
+  bool VisitVectorType(const VectorType *T) {
+    if (T == InstType)
+      return true;
+    if (const auto *IT = dyn_cast<VectorType>(InstType))
+      return match(T->getElementType(), IT->getElementType());
+    return false;
+  }
+
+  bool VisitDependentSizedExtVectorType(const DependentSizedExtVectorType *T) {
+    if (T == InstType)
+      return true;
+    if (const auto *IT = dyn_cast<DependentSizedExtVectorType>(InstType))
+      return match(T->getElementType(), IT->getElementType());
+    return false;
+  }
+
+   bool VisitFunctionProtoType(const FunctionProtoType *T) {
+    if (T == InstType)
+      return true;
+    if (const auto *IT = dyn_cast<FunctionProtoType>(InstType)) {
+      if (T->getNumParams() != IT->getNumParams())
+        return false;
+      InstType = IT->getReturnType().getTypePtr();
+      if (!Visit(T->getReturnType().getTypePtr()))
+        return false;
+      for (unsigned I = 0; I < T->getNumParams(); I++) {
+        QualType AT = T->getParamType(I);
+        QualType IAT = IT->getParamType(I);
+        InstType = IAT.getTypePtr();
+        if (!Visit(AT.getTypePtr()))
+          return false;
+      }
+      return true;
+    }
+    return false;
+  }
+
+  bool VisitTemplateTypeParmType(const TemplateTypeParmType *T) {
+    auto CanT = cast<TemplateTypeParmType>(
+                                    T->getCanonicalTypeInternal().getTypePtr());
+    auto Ptr = ParamMapping.find(CanT);
+    if (Ptr != ParamMapping.end())
+      return Ptr->second == InstType;
+    ParamMapping[CanT] = InstType;
+    return true;
+  }
+
+  bool VisitInjectedClassNameType(const InjectedClassNameType *T) {
+    return match(T->getInjectedSpecializationType().getTypePtr(), InstType);
+  }
+
+  bool VisitTemplateSpecializationType(const TemplateSpecializationType *T) {
+    if (T == InstType)
+      return true;
+    TemplateName TN = T->getTemplateName();
+    if (TN.getKind() != TemplateName::Template)
+      return false;
+    TemplateDecl *TemplD = TN.getAsTemplateDecl();
+    auto *ClassTD = dyn_cast<ClassTemplateDecl>(TemplD);
+    if (!ClassTD)
+      return false;
+    auto Params = ClassTD->getTemplateParameters();
+
+    if (CXXRecordDecl *IClassD = InstType->getAsCXXRecordDecl()) {
+      auto *IClassSD = dyn_cast<ClassTemplateSpecializationDecl>(IClassD);
+      if (!IClassSD)
+        return false;
+      ClassTemplateDecl *IClassTD = IClassSD->getSpecializedTemplate();
+      if (ClassTD->getCanonicalDecl() != IClassTD->getCanonicalDecl())
+        return false;
+      const TemplateArgumentList &IArgs = IClassSD->getTemplateArgs();
+      if (Params->size() != IArgs.size())
+        return false;
+      for (unsigned I = 0; I < Params->size(); ++I) {
+        TypeDecl *Param = cast<TypeDecl>(Params->getParam(I));
+        const TemplateArgument &IArg = IArgs.get(I);
+        QualType IArgT = IArg.getAsType();
+        if (!IArgT.isNull()) {
+          if (!match(Param->getTypeForDecl(), IArgT.getTypePtr()))
+            return false;
+        }
+      }
+      return true;
+    }
+
+    if (auto *ITST = dyn_cast<TemplateSpecializationType>(InstType)) {
+      TemplateName ITN = ITST->getTemplateName();
+      if (ITN.getKind() != TemplateName::Template)
+        return false;
+      TemplateDecl *ITemplD = ITN.getAsTemplateDecl();
+      auto *IClassTD = dyn_cast<ClassTemplateDecl>(ITemplD);
+      if (!IClassTD)
+        return false;
+      return IClassTD->getCanonicalDecl() == ClassTD->getCanonicalDecl();
+    }
+   return false;
+  }
+};
+
+/// \brief Given a friend function declaration checks if it might be misused.
+static void CheckDependentFriend(Sema &S, FunctionDecl *FriendD) {
+  if (FriendD->isInvalidDecl())
+    return;
+  LookupResult FRes(S, FriendD->getNameInfo(), Sema::LookupOrdinaryName);
+  if (S.LookupQualifiedName(FRes, FriendD->getDeclContext())) {
+    QualType FriendT = FriendD->getType().getCanonicalType();
+    // First check if there is suitable function template, this is more probable
+    // misuse case.
+    for (NamedDecl *D : FRes) {
+      if (D->isInvalidDecl())
+        continue;
+      if (auto *FTD = dyn_cast<FunctionTemplateDecl>(D)) {
+        FunctionDecl *FD = FTD->getTemplatedDecl();
+        QualType FDT = FD->getType().getCanonicalType();
+        if (FunctionMatcher().match(FriendT, FDT)) {
+          // 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;
+        }
+      }
+    }
+    // Then check for suitable functions that uses particular specialization of
+    // parameter type.
+    for (NamedDecl *D : FRes) {
+      if (D->isInvalidDecl())
+        continue;
+      if (auto *FD = dyn_cast<FunctionDecl>(D)) {
+        QualType FT = FD->getType().getCanonicalType();
+        if (FunctionMatcher().match(FriendT, FT))
+          // This is suitable file-level function, do not issue warnings.
+          return;
+      }
+    }
+  }
+  S.Diag(FriendD->getLocation(), diag::warn_non_template_friend) << FriendD;
+  S.Diag(FriendD->getLocation(), diag::note_non_template_friend);
+}
+
+}
+
+void Sema::CheckDependentFriends() {
+  for (FunctionDecl *FriendD : DependentFriend)
+    CheckDependentFriend(*this, FriendD);
+}
Index: lib/Sema/Sema.cpp
===================================================================
--- lib/Sema/Sema.cpp
+++ lib/Sema/Sema.cpp
@@ -687,6 +687,7 @@
       LateTemplateParserCleanup(OpaqueParser);
 
     CheckDelayedMemberExceptionSpecs();
+    CheckDependentFriends();
   }
 
   // All delayed member exception specs should be checked or we end up accepting
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -9254,6 +9254,15 @@
   /// attempts to add itself into the container
   void CheckObjCCircularContainer(ObjCMessageExpr *Message);
 
+  /// \brief Set of friend function declared in template classes and dependent
+  /// on their type parameters. These declarations must be checked for possible
+  /// misuse as templates.
+  llvm::SmallPtrSet<FunctionDecl *, 4> DependentFriend;
+
+  /// \brief Check dependent friends functions for misinterpretation as template
+  /// ones.
+  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
@@ -1139,6 +1139,14 @@
   "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 template function">,
+   InGroup<NonTemplateFriend>;
+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, "
+  "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
@@ -273,6 +273,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