[cfe-commits] r121833 - in /cfe/trunk: include/clang/AST/Decl.h lib/CodeGen/CodeGenModule.cpp lib/Sema/SemaDecl.cpp test/CodeGenCXX/inline-functions.cpp

[John McCall]

Author: rjmccall
Date: Tue Dec 14 22:00:32 2010
New Revision: 121833

URL: http://llvm.org/viewvc/llvm-project?rev=121833&view=rev
Log:
Set the "implicitly inline" bit on a method as soon as we see a definition
within the class.  Teach IR gen to look for function definitions in record
lexical contexts when deciding whether to emit a function whose address    
was taken.  Fixes PR8789.


Modified:
    cfe/trunk/include/clang/AST/Decl.h
    cfe/trunk/lib/CodeGen/CodeGenModule.cpp
    cfe/trunk/lib/Sema/SemaDecl.cpp
    cfe/trunk/test/CodeGenCXX/inline-functions.cpp

Modified: cfe/trunk/include/clang/AST/Decl.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=121833&r1=121832&r2=121833&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Tue Dec 14 22:00:32 2010
@@ -1503,11 +1503,16 @@
     IsInline = I;
   }
 
+  /// Flag that this function is implicitly inline.
+  void setImplicitlyInline() {
+    IsInline = true;
+  }
+
   /// \brief Determine whether this function should be inlined, because it is
   /// either marked "inline" or is a member function of a C++ class that
   /// was defined in the class body.
   bool isInlined() const;
-                       
+
   bool isInlineDefinitionExternallyVisible() const;
                        
   /// isOverloadedOperator - Whether this function declaration

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=121833&r1=121832&r2=121833&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Tue Dec 14 22:00:32 2010
@@ -829,30 +829,31 @@
     // list, and remove it from DeferredDecls (since we don't need it anymore).
     DeferredDeclsToEmit.push_back(DDI->second);
     DeferredDecls.erase(DDI);
-  } else if (const FunctionDecl *FD = cast_or_null<FunctionDecl>(D.getDecl())) {
-    // If this the first reference to a C++ inline function in a class, queue up
-    // the deferred function body for emission.  These are not seen as
-    // top-level declarations.
-    if (FD->isThisDeclarationADefinition() && MayDeferGeneration(FD)) {
-      DeferredDeclsToEmit.push_back(D);
-    // A called constructor which has no definition or declaration need be
-    // synthesized.
-    } else if (const CXXConstructorDecl *CD = dyn_cast<CXXConstructorDecl>(FD)) {
-      if (CD->isImplicit()) {
-        assert(CD->isUsed() && "Sema doesn't consider constructor as used.");
-        DeferredDeclsToEmit.push_back(D);
-      }
-    } else if (const CXXDestructorDecl *DD = dyn_cast<CXXDestructorDecl>(FD)) {
-      if (DD->isImplicit()) {
-        assert(DD->isUsed() && "Sema doesn't consider destructor as used.");
-        DeferredDeclsToEmit.push_back(D);
-      }
-    } else if (const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(FD)) {
-      if (MD->isImplicit() && MD->isCopyAssignmentOperator()) {
-        assert(MD->isUsed() && "Sema doesn't consider CopyAssignment as used.");
-        DeferredDeclsToEmit.push_back(D);
+
+  // Otherwise, there are cases we have to worry about where we're
+  // using a declaration for which we must emit a definition but where
+  // we might not find a top-level definition:
+  //   - member functions defined inline in their classes
+  //   - friend functions defined inline in some class
+  //   - special member functions with implicit definitions
+  // If we ever change our AST traversal to walk into class methods,
+  // this will be unnecessary.
+  } else if (getLangOptions().CPlusPlus && D.getDecl()) {
+    // Look for a declaration that's lexically in a record.
+    const FunctionDecl *FD = cast<FunctionDecl>(D.getDecl());
+    do {
+      if (isa<CXXRecordDecl>(FD->getLexicalDeclContext())) {
+        if (FD->isImplicit()) {
+          assert(FD->isUsed() && "Sema didn't mark implicit function as used!");
+          DeferredDeclsToEmit.push_back(D);
+          break;
+        } else if (FD->isThisDeclarationADefinition()) {
+          DeferredDeclsToEmit.push_back(D);
+          break;
+        }
       }
-    }
+      FD = FD->getPreviousDeclaration();
+    } while (FD);
   }
 
   // Make sure the result is of the requested type.

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=121833&r1=121832&r2=121833&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Dec 14 22:00:32 2010
@@ -3735,6 +3735,12 @@
       NewFD->setAccess(AS_public);
     }
 
+    if (isa<CXXMethodDecl>(NewFD) && DC == CurContext && IsFunctionDefinition) {
+      // A method is implicitly inline if it's defined in its class
+      // definition.
+      NewFD->setImplicitlyInline();
+    }
+
     if (SC == SC_Static && isa<CXXMethodDecl>(NewFD) &&
         !CurContext->isRecord()) {
       // C++ [class.static]p1:

Modified: cfe/trunk/test/CodeGenCXX/inline-functions.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/inline-functions.cpp?rev=121833&r1=121832&r2=121833&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/inline-functions.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/inline-functions.cpp Tue Dec 14 22:00:32 2010
@@ -29,3 +29,27 @@
 void f1(int) { }
 
 void test_f1() { f1(17); }
+
+// PR8789
+namespace test1 {
+  template <typename T> class ClassTemplate {
+  private:
+    friend void T::func();
+    void g() {}
+  };
+
+  // CHECK: define linkonce_odr void @_ZN5test11C4funcEv(
+
+  class C {
+  public:
+    void func() {
+      ClassTemplate<C> ct;
+      ct.g();
+    }
+  };
+
+  void f() {
+    C c;
+    c.func();
+  }
+}