r283207 - Do not find friend function definitions inside non-instantiated class.

Serge Pavlov via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 4 03:11:44 PDT 2016


Author: sepavloff
Date: Tue Oct  4 05:11:43 2016
New Revision: 283207

URL: http://llvm.org/viewvc/llvm-project?rev=283207&view=rev
Log:
Do not find friend function definitions inside non-instantiated class.

Previously if a file-level function was defined inside befriending
template class, it always was treated as defined. For instance, the code like:
```
  int func(int x);
  template<typename T> class C1 {
    friend int func(int x) { return x; }
  };
  template<typename T> class C2 {
    friend int func(int x) { return x; }
  };
```
could not be compiled due to function redefinition, although not of the templates
is instantiated. Moreover, the body of friend function can contain use of template
parameters, attempt to get definition of such function outside any instantiation
causes compiler abnormal termination.

Other compilers (gcc, icc) follow viewpoint that the body of the function defined
in friend declaration becomes available when corresponding class is instantiated.
This patch implements this viewpoint in clang.

Definitions introduced by friend declarations in template classes are not added
to the redeclaration chain of corresponding function. Only when the template is
instantiated, instantiation of the function definition is placed to the chain.

The fix was made in collaboration with Richard Smith.

This change fixes PR8035, PR17923, PR22307 and PR25848.

Differential Revision: http://reviews.llvm.org/D16989

Added:
    cfe/trunk/test/SemaCXX/PR25848.cpp
    cfe/trunk/test/SemaCXX/friend2.cpp
    cfe/trunk/test/SemaCXX/function-redecl-2.cpp
Modified:
    cfe/trunk/include/clang/Sema/Sema.h
    cfe/trunk/lib/Sema/SemaDecl.cpp
    cfe/trunk/lib/Sema/SemaDeclCXX.cpp

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=283207&r1=283206&r2=283207&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Tue Oct  4 05:11:43 2016
@@ -1760,6 +1760,7 @@ public:
   bool CheckFunctionDeclaration(Scope *S,
                                 FunctionDecl *NewFD, LookupResult &Previous,
                                 bool IsExplicitSpecialization);
+  bool shouldLinkDependentDeclWithPrevious(Decl *D, Decl *OldDecl);
   void CheckMain(FunctionDecl *FD, const DeclSpec &D);
   void CheckMSVCRTEntryPoint(FunctionDecl *FD);
   Decl *ActOnParamDeclarator(Scope *S, Declarator &D);

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=283207&r1=283206&r2=283207&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Oct  4 05:11:43 2016
@@ -8664,6 +8664,32 @@ Sema::ActOnFunctionDeclarator(Scope *S,
   return NewFD;
 }
 
+/// \brief Checks if the new declaration declared in dependent context must be
+/// put in the same redeclaration chain as the specified declaration.
+///
+/// \param D Declaration that is checked.
+/// \param PrevDecl Previous declaration found with proper lookup method for the
+///                 same declaration name.
+/// \returns True if D must be added to the redeclaration chain which PrevDecl
+///          belongs to.
+///
+bool Sema::shouldLinkDependentDeclWithPrevious(Decl *D, Decl *PrevDecl) {
+  // Any declarations should be put into redeclaration chains except for
+  // friend declaration in a dependent context that names a function in
+  // namespace scope.
+  //
+  // This allows to compile code like:
+  //
+  //       void func();
+  //       template<typename T> class C1 { friend void func() { } };
+  //       template<typename T> class C2 { friend void func() { } };
+  //
+  // This code snippet is a valid code unless both templates are instantiated.
+  return !(D->getLexicalDeclContext()->isDependentContext() &&
+           D->getDeclContext()->isFileContext() &&
+           D->getFriendObjectKind() != Decl::FOK_None);
+}
+
 /// \brief Perform semantic checking of a new function declaration.
 ///
 /// Performs semantic analysis of the new function declaration
@@ -8847,11 +8873,14 @@ bool Sema::CheckFunctionDeclaration(Scop
       }
 
     } else {
-      // This needs to happen first so that 'inline' propagates.
-      NewFD->setPreviousDeclaration(cast<FunctionDecl>(OldDecl));
-
-      if (isa<CXXMethodDecl>(NewFD))
-        NewFD->setAccess(OldDecl->getAccess());
+      if (shouldLinkDependentDeclWithPrevious(NewFD, OldDecl)) {
+        // This needs to happen first so that 'inline' propagates.
+        NewFD->setPreviousDeclaration(cast<FunctionDecl>(OldDecl));
+        if (isa<CXXMethodDecl>(NewFD))
+          NewFD->setAccess(OldDecl->getAccess());
+      } else {
+        Redeclaration = false;
+      }
     }
   }
 

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=283207&r1=283206&r2=283207&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Tue Oct  4 05:11:43 2016
@@ -13786,9 +13786,14 @@ NamedDecl *Sema::ActOnFriendFunctionDecl
     // and shall be the only declaration of the function or function
     // template in the translation unit.
     if (functionDeclHasDefaultArgument(FD)) {
-      if (FunctionDecl *OldFD = FD->getPreviousDecl()) {
+      // We can't look at FD->getPreviousDecl() because it may not have been set
+      // if we're in a dependent context. If we get this far with a non-empty
+      // Previous set, we must have a valid previous declaration of this
+      // function.
+      if (!Previous.empty()) {
         Diag(FD->getLocation(), diag::err_friend_decl_with_def_arg_redeclared);
-        Diag(OldFD->getLocation(), diag::note_previous_declaration);
+        Diag(Previous.getRepresentativeDecl()->getLocation(),
+             diag::note_previous_declaration);
       } else if (!D.isFunctionDefinition())
         Diag(FD->getLocation(), diag::err_friend_decl_with_def_arg_must_be_def);
     }

Added: cfe/trunk/test/SemaCXX/PR25848.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/PR25848.cpp?rev=283207&view=auto
==============================================================================
--- cfe/trunk/test/SemaCXX/PR25848.cpp (added)
+++ cfe/trunk/test/SemaCXX/PR25848.cpp Tue Oct  4 05:11:43 2016
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct A;
+
+inline int g();  // expected-warning{{inline function 'g' is not defined}}
+
+template<int M>
+struct R {
+  friend int g() {
+    return M;
+  }
+};
+
+void m() {
+  g();  // expected-note{{used here}}
+}

Added: cfe/trunk/test/SemaCXX/friend2.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/friend2.cpp?rev=283207&view=auto
==============================================================================
--- cfe/trunk/test/SemaCXX/friend2.cpp (added)
+++ cfe/trunk/test/SemaCXX/friend2.cpp Tue Oct  4 05:11:43 2016
@@ -0,0 +1,172 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11
+
+// If a friend function is defined in several non-template classes,
+// it is an error.
+
+void func1(int);
+struct C1a {
+  friend void func1(int) {}  // expected-note{{previous definition is here}}
+};
+struct C1b {
+  friend void func1(int) {}  // expected-error{{redefinition of 'func1'}}
+};
+
+
+// If a friend function is defined in both non-template and template
+// classes it is an error only if the template is instantiated.
+
+void func2(int);
+struct C2a {
+  friend void func2(int) {}
+};
+template<typename T> struct C2b {
+  friend void func2(int) {}
+};
+
+void func3(int);
+struct C3a {
+  friend void func3(int) {}  // expected-note{{previous definition is here}}
+};
+template<typename T> struct C3b {
+  friend void func3(int) {}  // expected-error{{redefinition of 'func3'}}
+};
+C3b<long> c3;  // expected-note{{in instantiation of template class 'C3b<long>' requested here}}
+
+
+// If a friend function is defined in several template classes it is an error
+// only if several templates are instantiated.
+
+void func4(int);
+template<typename T> struct C4a {
+  friend void func4(int) {}
+};
+template<typename T> struct C4b {
+  friend void func4(int) {}
+};
+
+
+void func5(int);
+template<typename T> struct C5a {
+  friend void func5(int) {}
+};
+template<typename T> struct C5b {
+  friend void func5(int) {}
+};
+C5a<long> c5a;
+
+void func6(int);
+template<typename T> struct C6a {
+  friend void func6(int) {}  // expected-note{{previous definition is here}}
+};
+template<typename T> struct C6b {
+  friend void func6(int) {}  // expected-error{{redefinition of 'func6'}}
+};
+C6a<long> c6a;
+C6b<int*> c6b;  // expected-note{{in instantiation of template class 'C6b<int *>' requested here}}
+
+void func7(int);
+template<typename T> struct C7 {
+  friend void func7(int) {}  // expected-error{{redefinition of 'func7'}}
+                             // expected-note at -1{{previous definition is here}}
+};
+C7<long> c7a;
+C7<int*> c7b;  // expected-note{{in instantiation of template class 'C7<int *>' requested here}}
+
+
+// Even if clases are not instantiated and hence friend functions defined in them are not
+// available, their declarations can be checked.
+
+void func8(int);  // expected-note{{previous declaration is here}}
+template<typename T> struct C8a {
+  friend long func8(int);  // expected-error{{functions that differ only in their return type cannot be overloaded}}
+};
+
+void func9(int);  // expected-note{{previous declaration is here}}
+template<typename T> struct C9a {
+  friend int func9(int);  // expected-error{{functions that differ only in their return type cannot be overloaded}}
+};
+
+void func10(int);  // expected-note{{previous declaration is here}}
+template<typename T> struct C10a {
+  friend int func10(int);  // expected-error{{functions that differ only in their return type cannot be overloaded}}
+};
+
+void func_11();  // expected-note{{previous declaration is here}}
+template<typename T> class C11 {
+  friend int func_11();  // expected-error{{functions that differ only in their return type cannot be overloaded}}
+};
+
+void func_12(int x);  // expected-note{{previous declaration is here}}
+template<typename T> class C12 {
+  friend void func_12(int x = 0);  // expected-error{{friend declaration specifying a default argument must be the only declaration}}
+};
+
+
+namespace pr22307 {
+
+struct t {
+  friend int leak(t);
+};
+
+template<typename v>
+struct m {
+  friend int leak(t) { return sizeof(v); }  // expected-error{{redefinition of 'leak'}} expected-note{{previous definition is here}}
+};
+
+template struct m<char>;
+template struct m<short>;  // expected-note{{in instantiation of template class 'pr22307::m<short>' requested here}}
+
+int main() {
+  leak(t());
+}
+
+}
+
+namespace pr17923 {
+
+void f(unsigned long long);
+
+template<typename T> struct X {
+  friend void f(unsigned long long) {
+     T t;
+  }
+};
+
+int main() { f(1234); }
+
+}
+
+namespace pr17923a {
+
+int get();
+
+template< int value >
+class set {
+  friend int get()
+    { return value; } // return 0; is OK
+};
+
+template class set< 5 >;
+
+int main() {
+  get();
+}
+
+}
+
+namespace pr8035 {
+
+void Function();
+
+int main(int argc, char* argv[]) {
+  Function();
+}
+
+template <typename T>
+struct Test {
+  friend void Function() { }
+};
+
+template class Test<int>;
+
+}

Added: cfe/trunk/test/SemaCXX/function-redecl-2.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/function-redecl-2.cpp?rev=283207&view=auto
==============================================================================
--- cfe/trunk/test/SemaCXX/function-redecl-2.cpp (added)
+++ cfe/trunk/test/SemaCXX/function-redecl-2.cpp Tue Oct  4 05:11:43 2016
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+
+namespace redecl_in_templ {
+template<typename T> void redecl_in_templ() {
+  extern void func_1();  // expected-note  {{previous declaration is here}}
+  extern int  func_1();  // expected-error {{functions that differ only in their return type cannot be overloaded}}
+}
+
+void g();
+constexpr void (*p)() = g;
+
+template<bool> struct X {};
+template<> struct X<true> { typedef int type; };
+
+template<typename T> void f() {
+  extern void g();
+  X<&g == p>::type n;
+}
+}




More information about the cfe-commits mailing list