sepavloff created this revision.
sepavloff added a reviewer: rsmith.
sepavloff added a subscriber: cfe-commits.

Previously if a file-level function was defined inside befriending
template class, it was treated as defined in the code like:
```
  void func(int);
  template<typename T> class C1 {
    friend void func(int) {}
  };
  void m() {
    func(1);
  }
```

The Standard requests ([temp.friend], p5) that `When a function is defined in
a friend function declaration in a class template, the function is instantiated
when the function is odr-used`. This statement however does not specify which
template is used for the instantiation, if the function is defined in several,
and what parameters are used for the instantiation. The latter is important if
the function implementation uses template parameter, such usage is not 
prohibited
by the Standard.

Other compilers (gcc, icc) follow viewpoint that 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 added to
the redeclaration chain of corresponding function, but they do not make the
function defined. Only if such definition corresponds to the template
instantiation, the function becomes defined. Presence of function body is not
enough for function definition anymore.

This change fixes PR17923, PR22307 and PR25848.

http://reviews.llvm.org/D16989

Files:
  include/clang/AST/Decl.h
  lib/AST/Decl.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  test/SemaCXX/PR25848.cpp
  test/SemaCXX/friend2.cpp

Index: test/SemaCXX/friend2.cpp
===================================================================
--- /dev/null
+++ test/SemaCXX/friend2.cpp
@@ -0,0 +1,135 @@
+// 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 must 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}}
+};
+
+
+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();
+}
+
+}
Index: test/SemaCXX/PR25848.cpp
===================================================================
--- /dev/null
+++ test/SemaCXX/PR25848.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct A;
+typedef int A::* P;
+
+inline P g();  // expected-warning{{inline function 'g' is not defined}}
+
+template<P M>
+struct R {
+  friend P g() {
+    return M;
+  }
+};
+
+void m() {
+  g();  // expected-note{{used here}}
+}
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===================================================================
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -1630,12 +1630,10 @@
         if (R->getFriendObjectKind()) {
           if (const FunctionDecl *RPattern =
                   R->getTemplateInstantiationPattern()) {
-            if (RPattern->isDefined(RPattern)) {
-              SemaRef.Diag(Function->getLocation(), diag::err_redefinition)
-                << Function->getDeclName();
-              SemaRef.Diag(R->getLocation(), diag::note_previous_definition);
-              break;
-            }
+            SemaRef.Diag(Function->getLocation(), diag::err_redefinition)
+              << Function->getDeclName();
+            SemaRef.Diag(R->getLocation(), diag::note_previous_definition);
+            break;
           }
         }
       }
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -10848,6 +10848,13 @@
 Sema::CheckForFunctionRedefinition(FunctionDecl *FD,
                                    const FunctionDecl *EffectiveDefinition,
                                    SkipBodyInfo *SkipBody) {
+  // Don't complain if the given declaration corresponds to the friend function
+  // declared in a template class. Such declaration defines the function only if
+  // the template is instantiated, in the latter case the definition must be
+  // found in corresponding class instantiation.
+  if (FD->isPotentialDefinition())
+    return;
+
   // Don't complain if we're in GNU89 mode and the previous definition
   // was an extern inline function.
   const FunctionDecl *Definition = EffectiveDefinition;
Index: lib/Sema/Sema.cpp
===================================================================
--- lib/Sema/Sema.cpp
+++ lib/Sema/Sema.cpp
@@ -492,9 +492,11 @@
     if (FunctionDecl *FD = dyn_cast<FunctionDecl>(ND)) {
       if (FD->isDefined())
         continue;
-      if (FD->isExternallyVisible() &&
-          !FD->getMostRecentDecl()->isInlined())
-        continue;
+      if (FD->isExternallyVisible()) {
+        FunctionDecl *AFD = FD->getMostRecentDecl()->getActiveRedeclaration();
+        if (AFD && !AFD->isInlined())
+          continue;
+      }
     } else {
       if (cast<VarDecl>(ND)->hasDefinition() != VarDecl::DeclarationOnly)
         continue;
Index: lib/CodeGen/CodeGenModule.cpp
===================================================================
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -1897,7 +1897,8 @@
       for (const auto *FD = cast<FunctionDecl>(D)->getMostRecentDecl(); FD;
            FD = FD->getPreviousDecl()) {
         if (isa<CXXRecordDecl>(FD->getLexicalDeclContext())) {
-          if (FD->doesThisDeclarationHaveABody()) {
+          if (FD->doesThisDeclarationHaveABody() &&
+              !FD->isPotentialDefinition()) {
             addDeferredDeclToEmit(F, GD.getWithDecl(FD));
             break;
           }
Index: lib/AST/Decl.cpp
===================================================================
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -2486,6 +2486,10 @@
   for (auto I : redecls()) {
     if (I->IsDeleted || I->IsDefaulted || I->Body || I->IsLateTemplateParsed ||
         I->hasAttr<AliasAttr>()) {
+      // Friend function defined in template class is not considered a
+      // definition until that template is instantiated.
+      if (I->isPotentialDefinition())
+        continue;
       Definition = I->IsDeleted ? I->getCanonicalDecl() : I;
       return true;
     }
@@ -2664,6 +2668,21 @@
          getType()->getAs<FunctionType>()->getNoReturnAttr();
 }
 
+bool FunctionDecl::isPotentialDefinition() const {
+  return isOutOfLine() && getLexicalDeclContext()->isRecord() &&
+         getLexicalDeclContext()->isDependentContext();
+}
+
+FunctionDecl *FunctionDecl::getActiveRedeclaration() {
+  FunctionDecl *I = this;
+  while (I->isPotentialDefinition()) {
+    if (I->isFirstDecl())
+      return nullptr;
+    I = I->getNextRedeclaration();
+  }
+  return I;
+}
+
 void
 FunctionDecl::setPreviousDeclaration(FunctionDecl *PrevDecl) {
   redeclarable_base::setPreviousDecl(PrevDecl);
Index: include/clang/AST/Decl.h
===================================================================
--- include/clang/AST/Decl.h
+++ include/clang/AST/Decl.h
@@ -1692,9 +1692,9 @@
   /// not require any specific codegen.
   bool hasTrivialBody() const;
 
-  /// isDefined - Returns true if the function is defined at all, including
-  /// a deleted definition. Except for the behavior when the function is
-  /// deleted, behaves like hasBody.
+  /// \brief Returns true if the function is defined at all, including a deleted
+  /// definition. It does not takes into account functions defined in friend
+  /// declaration in class template.
   bool isDefined(const FunctionDecl *&Definition) const;
 
   virtual bool isDefined() const {
@@ -1887,6 +1887,18 @@
   bool hasSkippedBody() const { return HasSkippedBody; }
   void setHasSkippedBody(bool Skipped = true) { HasSkippedBody = Skipped; }
 
+  /// \brief Determines if this declaration should be skipped when function
+  /// definition is searched for. A declaration is skipped if it is a file level
+  /// function defined as friend inside befriending class template. Although it
+  /// has body, it is not considered as definition until the class template is
+  /// instantiated.
+  bool isPotentialDefinition() const;
+
+  /// \brief Returns closest redeclaration, either this or preceding, that is
+  /// not skipped when definition is searched for. If no such declaration found
+  /// before the first redeclaration is met, returns nullptr.
+  FunctionDecl *getActiveRedeclaration();
+
   void setPreviousDeclaration(FunctionDecl * PrevDecl);
 
   FunctionDecl *getCanonicalDecl() override;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to