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

Fixed DR136 for template classes

Previously clang created declarations of friend functions declared in template 
classes even
if that class was not instantiated. It allowed to make check for default 
arguments required
by DR136. This fix enables the check for the new implementation.

Also extended comment about putting friend function declarations into 
redeclaration chains.

Putting a friend declaration to the appropriate redeclaration chain is not 
necessary, any
errors in the declaration will be detected at instantiation time. However 
proper organization
of the declarations enables early detection of potential problems.

If a friend function is defined inline, it is almost certain that it should not 
be added to
the redeclaration chain. The following code demonstrates the problem:

  void func();
  template<typename T> class C1 {
      friend void func() {}
  };
  template<typename T> class C2 {
      friend void func() {}
  }

Putting definitions of the friend functions into the same chain would require 
additional flag
to distinguish between 'actual' and 'potential' definitions. Operation with the 
chain becomes
more complicated. Putting only 'actual' definitions saves from the need to 
modify the mechanism
of redeclaration chain.

Putting non-defining declaration into the chain seems unnecessary. If there is 
corresponding
declaration at namespace level, as in the code:

  void func();
  template<typename T> class C1 {
      friend bool func() {}    // <- cannot be redeclared with different return 
type
  };

compiler can detect invalid redeclaration because it finds prior declaration in 
the translation
unit in this example. Declaration of the friend function does not need to be 
saved for any check
in this case.

If there is no the function declaration at namespace level, putting 
non-defining declaration into
the chain can cause problems, for instance:

  template<typename T> class C1 {
      friend void func() {}
  };
  template<typename T> class C2 {
      friend bool func() {}    // <- different return type
  }

This code does not violates any requirement of the C++ standard. Only if both 
templates are
instantiated, code becomes ill-formed. But these friend function declarations 
cannot be put into
the same redeclaration chain.

So, friend function declarations defined in dependent context that names 
file-level functions
should not be put into redeclaration chain.


https://reviews.llvm.org/D16989

Files:
  include/clang/Sema/Sema.h
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclCXX.cpp
  test/SemaCXX/PR25848.cpp
  test/SemaCXX/friend2.cpp
  test/SemaCXX/function-redecl-2.cpp

Index: test/SemaCXX/function-redecl-2.cpp
===================================================================
--- /dev/null
+++ test/SemaCXX/function-redecl-2.cpp
@@ -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;
+}
+}
Index: test/SemaCXX/friend2.cpp
===================================================================
--- /dev/null
+++ test/SemaCXX/friend2.cpp
@@ -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@-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>;
+
+}
Index: test/SemaCXX/PR25848.cpp
===================================================================
--- /dev/null
+++ test/SemaCXX/PR25848.cpp
@@ -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}}
+}
Index: lib/Sema/SemaDeclCXX.cpp
===================================================================
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -13767,9 +13767,10 @@
     // and shall be the only declaration of the function or function
     // template in the translation unit.
     if (functionDeclHasDefaultArgument(FD)) {
-      if (FunctionDecl *OldFD = FD->getPreviousDecl()) {
+      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);
     }
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -8640,6 +8640,58 @@
   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) {
+  DeclContext *LexicalDC = D->getLexicalDeclContext();
+  if (!LexicalDC->isDependentContext())
+    return true;
+
+  // If declarations are from the same lexical context, link them even in
+  // dependent context.
+  if (LexicalDC == PrevDecl->getLexicalDeclContext())
+    return true;
+
+  if (!isa<FunctionDecl>(D) && !isa<FunctionTemplateDecl>(D))
+    return true;
+
+  if (isa<FunctionDecl>(LexicalDC))
+    return !D->getFunctionType(false)->isDependentType();
+
+  DeclContext *SemanticDC = D->getDeclContext();
+  if (SemanticDC->isDependentContext())
+    return false;
+  // Get here only for out-of-line declarations: lexical context is dependent,
+  // semantic is not.
+
+  // Case of friend methods.
+  if (SemanticDC->isRecord())
+    return true;
+
+  // Case of friend specialization:
+  //
+  // template<typename T> class C1 {
+  //   friend void func<>(int);
+  // };
+  //
+  if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D))
+    if (FD->isFunctionTemplateSpecialization())
+      return true;
+
+  // A file level function may be defined inline in friend declarations. These
+  // definitions cannot be used until the containing template class is
+  // instantiated. So do not put such declaration into redeclaration chains
+  // while the template is processed.
+  return !LexicalDC->isRecord();
+}
+
 /// \brief Perform semantic checking of a new function declaration.
 ///
 /// Performs semantic analysis of the new function declaration
@@ -8823,11 +8875,14 @@
       }
 
     } 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;
+      }
     }
   }
 
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -1751,6 +1751,7 @@
   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);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to