r326419 - Function definition may have uninstantiated body

Serge Pavlov via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 28 23:04:12 PST 2018


Author: sepavloff
Date: Wed Feb 28 23:04:11 2018
New Revision: 326419

URL: http://llvm.org/viewvc/llvm-project?rev=326419&view=rev
Log:
Function definition may have uninstantiated body

Current implementation of `FunctionDecl::isDefined` does not take into
account redeclarations that do not have bodies, but the bodies can be
instantiated from corresponding templated definition. This behavior does
not allow to detect function redefinition in the cases where friend
functions is defined in class templates. For instance, the code:
```
    template<typename T> struct X { friend void f() {} };
    X<int> xi;
    void f() {}
```
compiles successfully but must fail due to redefinition of `f`. The
declaration of the friend `f` is created when the containing template
`X` is instantiated, but it does not have a body as per 14.5.4p4
because `f` is not odr-used.

With this change the function `Sema::CheckForFunctionRedefinition`
considers functions with uninstantiated bodies as definitions.

Differential Revision: https://reviews.llvm.org/D30170

Modified:
    cfe/trunk/include/clang/AST/Decl.h
    cfe/trunk/lib/Sema/SemaDecl.cpp
    cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
    cfe/trunk/test/SemaCXX/friend2.cpp

Modified: cfe/trunk/include/clang/AST/Decl.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=326419&r1=326418&r2=326419&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Wed Feb 28 23:04:11 2018
@@ -1925,11 +1925,25 @@ public:
 
   SourceRange getSourceRange() const override LLVM_READONLY;
 
-  /// \brief Returns true if the function has a body (definition). The
-  /// function body might be in any of the (re-)declarations of this
-  /// function. The variant that accepts a FunctionDecl pointer will
-  /// set that function declaration to the actual declaration
-  /// containing the body (if there is one).
+  // Function definitions.
+  //
+  // A function declaration may be:
+  // - a non defining declaration,
+  // - a definition. A function may be defined because:
+  //   - it has a body, or will have it in the case of late parsing.
+  //   - it has an uninstantiated body. The body does not exist because the
+  //     function is not used yet, but the declaration is considered a
+  //     definition and does not allow other definition of this function.
+  //   - it does not have a user specified body, but it does not allow
+  //     redefinition, because it is deleted/defaulted or is defined through
+  //     some other mechanism (alias, ifunc).
+
+  /// Returns true if the function has a body.
+  ///
+  /// The function body might be in any of the (re-)declarations of this
+  /// function. The variant that accepts a FunctionDecl pointer will set that
+  /// function declaration to the actual declaration containing the body (if
+  /// there is one).
   bool hasBody(const FunctionDecl *&Definition) const;
 
   bool hasBody() const override {
@@ -1941,9 +1955,11 @@ public:
   /// specific codegen.
   bool hasTrivialBody() const;
 
-  /// 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.
+  /// Returns true if the function has a definition that does not need to be
+  /// instantiated.
+  ///
+  /// The variant that accepts a FunctionDecl pointer will set that function
+  /// declaration to the declaration that is a definition (if there is one).
   bool isDefined(const FunctionDecl *&Definition) const;
 
   virtual bool isDefined() const {
@@ -1985,8 +2001,7 @@ public:
            IsLateTemplateParsed || WillHaveBody || hasDefiningAttr();
   }
 
-  /// Returns whether this specific declaration of the function has a body -
-  /// that is, if it is a non-deleted definition.
+  /// Returns whether this specific declaration of the function has a body.
   bool doesThisDeclarationHaveABody() const {
     return Body || IsLateTemplateParsed;
   }

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=326419&r1=326418&r2=326419&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed Feb 28 23:04:11 2018
@@ -12283,9 +12283,36 @@ Sema::CheckForFunctionRedefinition(Funct
                                    const FunctionDecl *EffectiveDefinition,
                                    SkipBodyInfo *SkipBody) {
   const FunctionDecl *Definition = EffectiveDefinition;
+  if (!Definition && !FD->isDefined(Definition) && !FD->isCXXClassMember()) {
+    // If this is a friend function defined in a class template, it does not
+    // have a body until it is used, nevertheless it is a definition, see
+    // [temp.inst]p2:
+    //
+    // ... for the purpose of determining whether an instantiated redeclaration
+    // is valid according to [basic.def.odr] and [class.mem], a declaration that
+    // corresponds to a definition in the template is considered to be a
+    // definition.
+    //
+    // The following code must produce redefinition error:
+    //
+    //     template<typename T> struct C20 { friend void func_20() {} };
+    //     C20<int> c20i;
+    //     void func_20() {}
+    //
+    for (auto I : FD->redecls()) {
+      if (I != FD && !I->isInvalidDecl() &&
+          I->getFriendObjectKind() != Decl::FOK_None) {
+        if (FunctionDecl *Original = I->getInstantiatedFromMemberFunction()) {
+          if (Original->isThisDeclarationADefinition()) {
+            Definition = I;
+            break;
+          }
+        }
+      }
+    }
+  }
   if (!Definition)
-    if (!FD->isDefined(Definition))
-      return;
+    return;
 
   if (canRedefineFunction(Definition, getLangOpts()))
     return;

Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=326419&r1=326418&r2=326419&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Wed Feb 28 23:04:11 2018
@@ -1806,45 +1806,24 @@ Decl *TemplateDeclInstantiator::VisitFun
     //   apply to non-template function declarations and definitions also apply
     //   to these implicit definitions.
     if (D->isThisDeclarationADefinition()) {
-      // Check for a function body.
-      const FunctionDecl *Definition = nullptr;
-      if (Function->isDefined(Definition) &&
-          Definition->getTemplateSpecializationKind() == TSK_Undeclared) {
-        SemaRef.Diag(Function->getLocation(), diag::err_redefinition)
-            << Function->getDeclName();
-        SemaRef.Diag(Definition->getLocation(), diag::note_previous_definition);
-      }
-      // Check for redefinitions due to other instantiations of this or
-      // a similar friend function.
-      else for (auto R : Function->redecls()) {
-        if (R == Function)
-          continue;
+      SemaRef.CheckForFunctionRedefinition(Function);
+      if (!Function->isInvalidDecl()) {
+        for (auto R : Function->redecls()) {
+          if (R == Function)
+            continue;
 
-        // If some prior declaration of this function has been used, we need
-        // to instantiate its definition.
-        if (!QueuedInstantiation && R->isUsed(false)) {
-          if (MemberSpecializationInfo *MSInfo =
-                  Function->getMemberSpecializationInfo()) {
-            if (MSInfo->getPointOfInstantiation().isInvalid()) {
-              SourceLocation Loc = R->getLocation(); // FIXME
-              MSInfo->setPointOfInstantiation(Loc);
-              SemaRef.PendingLocalImplicitInstantiations.push_back(
-                                               std::make_pair(Function, Loc));
-              QueuedInstantiation = true;
-            }
-          }
-        }
-
-        // If some prior declaration of this function was a friend with an
-        // uninstantiated definition, reject it.
-        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;
+          // If some prior declaration of this function has been used, we need
+          // to instantiate its definition.
+          if (!QueuedInstantiation && R->isUsed(false)) {
+            if (MemberSpecializationInfo *MSInfo =
+                Function->getMemberSpecializationInfo()) {
+              if (MSInfo->getPointOfInstantiation().isInvalid()) {
+                SourceLocation Loc = R->getLocation(); // FIXME
+                MSInfo->setPointOfInstantiation(Loc);
+                SemaRef.PendingLocalImplicitInstantiations.push_back(
+                    std::make_pair(Function, Loc));
+                QueuedInstantiation = true;
+              }
             }
           }
         }

Modified: cfe/trunk/test/SemaCXX/friend2.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/friend2.cpp?rev=326419&r1=326418&r2=326419&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/friend2.cpp (original)
+++ cfe/trunk/test/SemaCXX/friend2.cpp Wed Feb 28 23:04:11 2018
@@ -101,6 +101,34 @@ 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}}
 };
 
+// Friend function with uninstantiated body is still a definition.
+
+template<typename T> struct C20 {
+  friend void func_20() {} // expected-note{{previous definition is here}}
+};
+C20<int> c20i;
+void func_20() {} // expected-error{{redefinition of 'func_20'}}
+
+template<typename T> struct C21a {
+  friend void func_21() {} // expected-note{{previous definition is here}}
+};
+template<typename T> struct C21b {
+  friend void func_21() {} // expected-error{{redefinition of 'func_21'}}
+};
+C21a<int> c21ai;
+C21b<int> c21bi; // expected-note{{in instantiation of template class 'C21b<int>' requested here}}
+
+template<typename T> struct C22a {
+  friend void func_22() {} // expected-note{{previous definition is here}}
+};
+template<typename T> struct C22b {
+  friend void func_22();
+};
+C22a<int> c22ai;
+C22b<int> c22bi;
+void func_22() {} // expected-error{{redefinition of 'func_22'}}
+
+
 
 namespace pr22307 {
 




More information about the cfe-commits mailing list