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

[hans]

Summary

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;

}

[rnk]

lgtm

This seems exactly in line with Richard's suggested direction.

[hans]

Closed by commit rL209549 (authored by @hans).

cfe/trunk/include/clang/AST/ASTConsumer.h

 
 #ifndef LLVM_CLANG_AST_ASTCONSUMER_H
 #define LLVM_CLANG_AST_ASTCONSUMER_H
 
 #include "llvm/ADT/StringRef.h"
 
 namespace clang {
   class ASTContext;
+  class CXXMethodDecl;
   class CXXRecordDecl;
   class Decl;
   class DeclGroupRef;
   class ASTMutationListener;
   class ASTDeserializationListener; // layering violation because void* is ugly
   class SemaConsumer; // layering violation required for safe SemaConsumer
   class TagDecl;
   class VarDecl;
public:
   /// HandleTopLevelDecl - Handle the specified top-level declaration.  This is
   /// called by the parser to process every top-level Decl*. Note that D can be
   /// the head of a chain of Decls (e.g. for `int a, b` the chain will have two
   /// elements). Use Decl::getNextDeclarator() to walk the chain.
   ///
   /// \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.
   virtual void HandleInterestingDecl(DeclGroupRef D);
 
   /// HandleTranslationUnit - This method is called when the ASTs for entire
   /// translation unit have been parsed.
   virtual void HandleTranslationUnit(ASTContext &Ctx) {}

cfe/trunk/include/clang/Frontend/MultiplexConsumer.h

public:
   // Takes ownership of the pointers in C.
   MultiplexConsumer(ArrayRef<ASTConsumer*> C);
   ~MultiplexConsumer();
 
   // ASTConsumer
   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;
   void HandleCXXImplicitFunctionInstantiation(FunctionDecl *D) override;
   void HandleTopLevelDeclInObjCContainer(DeclGroupRef D) override;
   void CompleteTentativeDefinition(VarDecl *D) override;
   void HandleVTable(CXXRecordDecl *RD, bool DefinitionRequired) override;
   ASTMutationListener *GetASTMutationListener() override;

cfe/trunk/include/clang/Sema/Sema.h

public:
   /// order to parse the rest of the program (for instance, if it is
   /// \c constexpr in C++11 or has an 'auto' return type in C++14).
   bool canSkipFunctionBody(Decl *D);
 
   void computeNRVO(Stmt *Body, sema::FunctionScopeInfo *Scope);
   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.
   void ActOnFinishDelayedAttribute(Scope *S, Decl *D, ParsedAttributes &Attrs);
 
   /// \brief Diagnose any unused parameters in the given sequence of
   /// ParmVarDecl pointers.
   void DiagnoseUnusedParameters(ParmVarDecl * const *Begin,

cfe/trunk/lib/CodeGen/CodeGenAction.cpp

 //===--- CodeGenAction.cpp - LLVM Code Generation Frontend Action ---------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/CodeGen/CodeGenAction.h"
 #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"
 #include "clang/CodeGen/BackendUtil.h"
 #include "clang/CodeGen/ModuleBuilder.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
 #include "llvm/ADT/SmallString.h"
bool HandleTopLevelDecl(DeclGroupRef D) override {
       Gen->HandleTopLevelDecl(D);
 
       if (llvm::TimePassesIsEnabled)
         LLVMIRGeneration.stopTimer();
 
       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");
         if (llvm::TimePassesIsEnabled)
           LLVMIRGeneration.startTimer();
 
         Gen->HandleTranslationUnit(C);
 

cfe/trunk/lib/CodeGen/ModuleBuilder.cpp

bool HandleTopLevelDecl(DeclGroupRef DG) override {
         return true;
 
       // Make sure to emit all elements of a Decl.
       for (DeclGroupRef::iterator I = DG.begin(), E = DG.end(); I != E; ++I)
         Builder->EmitTopLevelDecl(*I);
       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
     /// (because these can be defined in declspecs).
     void HandleTagDeclDefinition(TagDecl *D) override {
       if (Diags.hasErrorOccurred())
         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 {
       if (Diags.hasErrorOccurred())
         return;
 
       if (CodeGen::CGDebugInfo *DI = Builder->getModuleDebugInfo())
         if (const RecordDecl *RD = dyn_cast<RecordDecl>(D))

cfe/trunk/lib/Frontend/MultiplexConsumer.cpp

 
 bool MultiplexConsumer::HandleTopLevelDecl(DeclGroupRef D) {
   bool Continue = true;
   for (size_t i = 0, e = Consumers.size(); i != e; ++i)
     Continue = Continue && Consumers[i]->HandleTopLevelDecl(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);
 }
 
 void MultiplexConsumer::HandleInterestingDecl(DeclGroupRef D) {
   for (size_t i = 0, e = Consumers.size(); i != e; ++i)
     Consumers[i]->HandleInterestingDecl(D);

cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp

if (Tok.getLocation() != origLoc) {
     // leftover tokens.
     // Since this is an uncommon situation that should be avoided, use the
     // expensive isBeforeInTranslationUnit call.
     if (PP.getSourceManager().isBeforeInTranslationUnit(Tok.getLocation(),
                                                         origLoc))
       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
 /// of a top (non-nested) C++ class. Now go over the stack of lexed data member
 /// initializers that were collected during its parsing and parse them all.
 void Parser::ParseLexedMemberInitializers(ParsingClass &Class) {
   bool HasTemplateScope = !Class.TopLevelClass && Class.TemplateScope;
   ParseScope ClassTemplateScope(this, Scope::TemplateParamScope,

cfe/trunk/lib/Sema/SemaDecl.cpp

Decl *Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, Declarator &D) {
   assert(D.isFunctionDeclarator() && "Not a function declarator!");
   Scope *ParentScope = FnBodyScope->getParent();
 
   D.setFunctionDefinitionKind(FDK_Definition);
   Decl *DP = HandleDeclarator(ParentScope, D, MultiTemplateParamsArg());
   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.
   if (FD->isInvalidDecl())
     return false;
 
   // Or declarations that aren't global.
   if (!FD->isGlobal())

cfe/trunk/test/CodeGenCXX/attr-used.cpp

 // RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -o - %s | FileCheck %s
 
 // <rdar://problem/8684363>: clang++ not respecting __attribute__((used)) on destructors
 struct X0 {
   // CHECK: define linkonce_odr {{.*}} @_ZN2X0C1Ev
   __attribute__((used)) 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() {}
+  };
+};

cfe/trunk/test/CodeGenCXX/dllexport.cpp

 
 template <typename T> void c() {}
 template void DLLEXPORT c<int>();
 // CHECK-DAG: define weak_odr dllexport void @"\01??$c@H@@YAXXZ"()
 
 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;
 }