Emit used/dllexport inline method definitions in nested classes (PR19743, PR11170)

The previous code that was supposed to handle this didn't work
since parsing of inline method definitions is delayed to the end
of the outer class definition. Thus, when HandleTagDeclDefinition()
got called for the inner class, the inline functions in that class
had not been parsed yet.

Richard suggested that the way to do this is by handling inline
method definitions through a new ASTConsumer callback.

I really wanted to call ASTContext::DeclMustBeEmitted() instead of
checking for attributes, but doing that causes us to compute linkage,
and then we fail with "error: unsupported: typedef changes linkage
of anonymous type, but linkage was already computed" on tests like
this: (from SemaCXX/undefined-internal.cpp) :-/

  namespace test7 {
    typedef struct {
      void bar();
      void foo() { bar(); }
    } A;
  }

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

llvm-svn: 209549

10 files changed, 61 insertions, 13 deletions

diff --git a/clang/include/clang/AST/ASTConsumer.h b/clang/include/clang/AST/ASTConsumer.h
index fde114711bf..6fa7cd86114 100644
--- a/clang/include/clang/AST/ASTConsumer.h
+++ b/clang/include/clang/AST/ASTConsumer.h
@@ -18,6 +18,7 @@
 
 namespace clang {
   class ASTContext;
+  class CXXMethodDecl;
   class CXXRecordDecl;
   class Decl;
   class DeclGroupRef;
@@ -56,6 +57,10 @@ public:
   /// \returns true to continue parsing, or false to abort parsing.
   virtual bool HandleTopLevelDecl(DeclGroupRef D);
 
+  /// \brief This callback is invoked each time an inline method definition is
+  /// completed.
+  virtual void HandleInlineMethodDefinition(CXXMethodDecl *D) {}
+
   /// HandleInterestingDecl - Handle the specified interesting declaration. This
   /// is called by the AST reader when deserializing things that might interest
   /// the consumer. The default implementation forwards to HandleTopLevelDecl.
diff --git a/clang/include/clang/Frontend/MultiplexConsumer.h b/clang/include/clang/Frontend/MultiplexConsumer.h
index 6ea1d73d282..a7e0444885f 100644
--- a/clang/include/clang/Frontend/MultiplexConsumer.h
+++ b/clang/include/clang/Frontend/MultiplexConsumer.h
@@ -36,6 +36,7 @@ public:
   void Initialize(ASTContext &Context) override;
   void HandleCXXStaticMemberVarInstantiation(VarDecl *VD) override;
   bool HandleTopLevelDecl(DeclGroupRef D) override;
+  void HandleInlineMethodDefinition(CXXMethodDecl *D) override;
   void HandleInterestingDecl(DeclGroupRef D) override;
   void HandleTranslationUnit(ASTContext &Ctx) override;
   void HandleTagDeclDefinition(TagDecl *D) override;
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index f1354e5b8ff..578cc1fe180 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -1670,6 +1670,7 @@ public:
   Decl *ActOnFinishFunctionBody(Decl *Decl, Stmt *Body);
   Decl *ActOnFinishFunctionBody(Decl *Decl, Stmt *Body, bool IsInstantiation);
   Decl *ActOnSkippedFunctionBody(Decl *Decl);
+  void ActOnFinishInlineMethodDef(CXXMethodDecl *D);
 
   /// ActOnFinishDelayedAttribute - Invoked when we have finished parsing an
   /// attribute for which parsing is delayed.
diff --git a/clang/lib/CodeGen/CodeGenAction.cpp b/clang/lib/CodeGen/CodeGenAction.cpp
index 5c0b6a9fd8e..2fe984226e1 100644
--- a/clang/lib/CodeGen/CodeGenAction.cpp
+++ b/clang/lib/CodeGen/CodeGenAction.cpp
@@ -11,6 +11,7 @@
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclGroup.h"
+#include "clang/AST/DeclCXX.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
@@ -104,6 +105,19 @@ namespace clang {
       return true;
     }
 
+    void HandleInlineMethodDefinition(CXXMethodDecl *D) override {
+      PrettyStackTraceDecl CrashInfo(D, SourceLocation(),
+                                     Context->getSourceManager(),
+                                     "LLVM IR generation of inline method");
+      if (llvm::TimePassesIsEnabled)
+        LLVMIRGeneration.startTimer();
+
+      Gen->HandleInlineMethodDefinition(D);
+
+      if (llvm::TimePassesIsEnabled)
+        LLVMIRGeneration.stopTimer();
+    }
+
     void HandleTranslationUnit(ASTContext &C) override {
       {
         PrettyStackTraceString CrashInfo("Per-file LLVM IR generation");
diff --git a/clang/lib/CodeGen/ModuleBuilder.cpp b/clang/lib/CodeGen/ModuleBuilder.cpp
index 7873f44f1de..78cb82dc558 100644
--- a/clang/lib/CodeGen/ModuleBuilder.cpp
+++ b/clang/lib/CodeGen/ModuleBuilder.cpp
@@ -81,6 +81,20 @@ namespace {
       return true;
     }
 
+    void HandleInlineMethodDefinition(CXXMethodDecl *D) override {
+      if (Diags.hasErrorOccurred())
+        return;
+
+      assert(D->doesThisDeclarationHaveABody());
+
+      // We may have member functions that need to be emitted at this point.
+      if (!D->isDependentContext() &&
+          (D->hasAttr<UsedAttr>() || D->hasAttr<ConstructorAttr>() ||
+           D->hasAttr<DLLExportAttr>())) {
+        Builder->EmitTopLevelDecl(D);
+      }
+    }
+
     /// HandleTagDeclDefinition - This callback is invoked each time a TagDecl
     /// to (e.g. struct, union, enum, class) is completed. This allows the
     /// client hack on the type, which can occur at any point in the file
@@ -90,17 +104,6 @@ namespace {
         return;
 
       Builder->UpdateCompletedType(D);
-      
-      // In C++, we may have member functions that need to be emitted at this 
-      // point.
-      if (Ctx->getLangOpts().CPlusPlus && !D->isDependentContext()) {
-        for (auto *M : D->decls())
-          if (auto *Method = dyn_cast<CXXMethodDecl>(M))
-            if (Method->doesThisDeclarationHaveABody() &&
-                (Method->hasAttr<UsedAttr>() || 
-                 Method->hasAttr<ConstructorAttr>()))
-              Builder->EmitTopLevelDecl(Method);
-      }
     }
 
     void HandleTagDeclRequiredDefinition(const TagDecl *D) override {
diff --git a/clang/lib/Frontend/MultiplexConsumer.cpp b/clang/lib/Frontend/MultiplexConsumer.cpp
index 4b4804f0350..058cee8244b 100644
--- a/clang/lib/Frontend/MultiplexConsumer.cpp
+++ b/clang/lib/Frontend/MultiplexConsumer.cpp
@@ -226,6 +226,11 @@ bool MultiplexConsumer::HandleTopLevelDecl(DeclGroupRef D) {
   return Continue;
 }
 
+void MultiplexConsumer::HandleInlineMethodDefinition(CXXMethodDecl *D) {
+  for (size_t i = 0, e = Consumers.size(); i != e; ++i)
+    Consumers[i]->HandleInlineMethodDefinition(D);
+}
+
 void  MultiplexConsumer::HandleCXXStaticMemberVarInstantiation(VarDecl *VD) {
   for (size_t i = 0, e = Consumers.size(); i != e; ++i)
     Consumers[i]->HandleCXXStaticMemberVarInstantiation(VD);
diff --git a/clang/lib/Parse/ParseCXXInlineMethods.cpp b/clang/lib/Parse/ParseCXXInlineMethods.cpp
index 8b368cc4054..19aa664031d 100644
--- a/clang/lib/Parse/ParseCXXInlineMethods.cpp
+++ b/clang/lib/Parse/ParseCXXInlineMethods.cpp
@@ -467,6 +467,9 @@ void Parser::ParseLexedMethodDef(LexedMethod &LM) {
       while (Tok.getLocation() != origLoc && Tok.isNot(tok::eof))
         ConsumeAnyToken();
   }
+
+  if (CXXMethodDecl *MD = dyn_cast_or_null<CXXMethodDecl>(LM.D))
+    Actions.ActOnFinishInlineMethodDef(MD);
 }
 
 /// ParseLexedMemberInitializers - We finished parsing the member specification
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index a522ef4784b..d7b5ba427d6 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -9515,6 +9515,10 @@ Decl *Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, Declarator &D) {
   return ActOnStartOfFunctionDef(FnBodyScope, DP);
 }
 
+void Sema::ActOnFinishInlineMethodDef(CXXMethodDecl *D) {
+  Consumer.HandleInlineMethodDefinition(D);
+}
+
 static bool ShouldWarnAboutMissingPrototype(const FunctionDecl *FD, 
                              const FunctionDecl*& PossibleZeroParamPrototype) {
   // Don't warn about invalid declarations.
diff --git a/clang/test/CodeGenCXX/attr-used.cpp b/clang/test/CodeGenCXX/attr-used.cpp
index 86dd6b959bf..26c1597b589 100644
--- a/clang/test/CodeGenCXX/attr-used.cpp
+++ b/clang/test/CodeGenCXX/attr-used.cpp
@@ -7,3 +7,11 @@ struct X0 {
   // CHECK: define linkonce_odr {{.*}} @_ZN2X0D1Ev
   __attribute__((used)) ~X0() {}
 };
+
+// PR19743: not emitting __attribute__((used)) inline methods in nested classes.
+struct X1 {
+  struct Nested {
+    // CHECK: define linkonce_odr {{.*}} @_ZN2X16Nested1fEv
+    void __attribute__((used)) f() {}
+  };
+};
diff --git a/clang/test/CodeGenCXX/dllexport.cpp b/clang/test/CodeGenCXX/dllexport.cpp
index 7f11d455e3f..04413574dcc 100644
--- a/clang/test/CodeGenCXX/dllexport.cpp
+++ b/clang/test/CodeGenCXX/dllexport.cpp
@@ -15,10 +15,14 @@ template void DLLEXPORT c<int>();
 struct S {
   void DLLEXPORT a() {}
   // CHECK-DAG: define weak_odr dllexport x86_thiscallcc void @"\01?a@S@@QAEXXZ"
+
+  struct T {
+    void DLLEXPORT a() {}
+    // CHECK-DAG: define weak_odr dllexport x86_thiscallcc void @"\01?a@T@S@@QAEXXZ"
+  };
 };
 
+
 void user() {
   a();
-  // FIXME: dllexported methods must be emitted even if they're not referenced in this TU.
-  &S::a;
 }