For MS ABI, emit dllexport friend functions defined inline in class

[sberg]

Summary

...as that is apparently what MSVC does

[rnk]

include/clang/AST/ASTConsumer.h

58–64

I'm pretty sure we can relax HandleInlineMethodDefinition to take a FunctionDecl and then we don't need the extra AST consumer callback.

lib/Parse/ParseCXXInlineMethods.cpp

568–569

I'd check for the friend specification here rather than asserting later. There probably are or will eventually be ways to sneak a non-friend, non-method FunctionDecl into a class context.

[sberg]

include/clang/AST/ASTConsumer.h

58–64

...and then also rename it from HandleInlineMethodDefinition to, say,HandleInlineFunctionDefinition? Or better stick with the (then no longer accurate) name?

[rnk]

include/clang/AST/ASTConsumer.h

58–64

Renaming would be good if you're up for it. I have a feeling that nobody outside of Clang is overriding this ASTConsumer callback. It's purpose is very specific to dllexport.

[sberg]

updated as discussed in the comments

[sberg]

friendly ping

[rnk]

lgtm

[sberg]

Can you please push this, I do not have commit access. Thanks

include/clang/AST/ASTConsumer.h

public:
   virtual void Initialize(ASTContext &Context) {}
 
   /// HandleTopLevelDecl - Handle the specified top-level declaration.  This is
   /// called by the parser to process every top-level Decl*.
   ///
   /// \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) {}
 
+  /// \brief This callback is invoked each time an inline friend function
+  /// definition is completed.
+  virtual void HandleInlineFriendFunctionDefinition(FunctionDecl *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) {}

include/clang/Frontend/MultiplexConsumer.h

public:
   MultiplexConsumer(std::vector<std::unique_ptr<ASTConsumer>> C);
   ~MultiplexConsumer() override;
 
   // ASTConsumer
   void Initialize(ASTContext &Context) override;
   void HandleCXXStaticMemberVarInstantiation(VarDecl *VD) override;
   bool HandleTopLevelDecl(DeclGroupRef D) override;
   void HandleInlineMethodDefinition(CXXMethodDecl *D) override;
+  void HandleInlineFriendFunctionDefinition(FunctionDecl *D) override;
   void HandleInterestingDecl(DeclGroupRef D) override;
   void HandleTranslationUnit(ASTContext &Ctx) override;
   void HandleTagDeclDefinition(TagDecl *D) override;
   void HandleTagDeclRequiredDefinition(const TagDecl *D) override;
   void HandleCXXImplicitFunctionInstantiation(FunctionDecl *D) override;
   void HandleTopLevelDeclInObjCContainer(DeclGroupRef D) override;
   void HandleImplicitImportDecl(ImportDecl *D) override;
   void HandleLinkerOptionPragma(llvm::StringRef Opts) override;

include/clang/Sema/Sema.h

public:
   /// \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);
+  void ActOnFinishInlineFriendFunctionDef(FunctionDecl *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,

lib/CodeGen/CodeGenAction.cpp

void HandleInlineMethodDefinition(CXXMethodDecl *D) override {
         LLVMIRGeneration.startTimer();
 
       Gen->HandleInlineMethodDefinition(D);
 
       if (llvm::TimePassesIsEnabled)
         LLVMIRGeneration.stopTimer();
     }
 
+    void HandleInlineFriendFunctionDefinition(FunctionDecl *D) override {
+      PrettyStackTraceDecl CrashInfo(D, SourceLocation(),
+                                     Context->getSourceManager(),
+                                     "LLVM IR generation of inline friend function");
+      if (llvm::TimePassesIsEnabled)
+        LLVMIRGeneration.startTimer();
+
+      Gen->HandleInlineFriendFunctionDefinition(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);
 

lib/CodeGen/ModuleBuilder.cpp

void HandleInlineMethodDefinition(CXXMethodDecl *D) override {
 
       // Provide some coverage mapping even for methods that aren't emitted.
       // Don't do this for templated classes though, as they may not be
       // instantiable.
       if (!D->getParent()->getDescribedClassTemplate())
         Builder->AddDeferredUnusedCoverageMapping(D);
     }
 
+    void HandleInlineFriendFunctionDefinition(FunctionDecl *D) override {
+      if (Diags.hasErrorOccurred())
+        return;
+
+      assert(D->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend) &&
+             "Must be a friend decl");
+      assert(D->doesThisDeclarationHaveABody());
+
+      if (Ctx->getTargetInfo().getCXXABI().isMicrosoft())
+        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;
 

lib/Frontend/MultiplexConsumer.cpp

bool MultiplexConsumer::HandleTopLevelDecl(DeclGroupRef D) {
   return Continue;
 }
 
 void MultiplexConsumer::HandleInlineMethodDefinition(CXXMethodDecl *D) {
   for (auto &Consumer : Consumers)
     Consumer->HandleInlineMethodDefinition(D);
 }
 
+void MultiplexConsumer::HandleInlineFriendFunctionDefinition(FunctionDecl *D) {
+  for (auto &Consumer : Consumers)
+    Consumer->HandleInlineFriendFunctionDefinition(D);
+}
+
 void MultiplexConsumer::HandleCXXStaticMemberVarInstantiation(VarDecl *VD) {
   for (auto &Consumer : Consumers)
     Consumer->HandleCXXStaticMemberVarInstantiation(VD);
 }
 
 void MultiplexConsumer::HandleInterestingDecl(DeclGroupRef D) {
   for (auto &Consumer : Consumers)
     Consumer->HandleInterestingDecl(D);

lib/Parse/ParseCXXInlineMethods.cpp

void Parser::ParseLexedMethodDef(LexedMethod &LM) {
   while (Tok.isNot(tok::eof))
     ConsumeAnyToken();
 
   if (Tok.is(tok::eof) && Tok.getEofData() == LM.D)
     ConsumeAnyToken();
 
   if (CXXMethodDecl *MD = dyn_cast_or_null<CXXMethodDecl>(LM.D))
     Actions.ActOnFinishInlineMethodDef(MD);
+  else if (auto *FD = dyn_cast_or_null<FunctionDecl>(LM.D))
+    Actions.ActOnFinishInlineFriendFunctionDef(FD);
 }
 
 /// 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,

lib/Sema/SemaDecl.cpp

Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, Declarator &D,
   Decl *DP = HandleDeclarator(ParentScope, D, TemplateParameterLists);
   return ActOnStartOfFunctionDef(FnBodyScope, DP, SkipBody);
 }
 
 void Sema::ActOnFinishInlineMethodDef(CXXMethodDecl *D) {
   Consumer.HandleInlineMethodDefinition(D);
 }
 
+void Sema::ActOnFinishInlineFriendFunctionDef(FunctionDecl *D) {
+  Consumer.HandleInlineFriendFunctionDefinition(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())

test/CodeGenCXX/dllexport.cpp

 __declspec(dllexport) void redecl2();
                       void redecl2() {}
 
 // Friend functions
 // MSC-DAG: define dllexport void @"\01?friend1@@YAXXZ"()
 // GNU-DAG: define dllexport void @_Z7friend1v()
 // MSC-DAG: define dllexport void @"\01?friend2@@YAXXZ"()
 // GNU-DAG: define dllexport void @_Z7friend2v()
+// MSC-DAG: define weak_odr dllexport void @"\01?friend3@@YAXXZ"()
 struct FuncFriend {
   friend __declspec(dllexport) void friend1();
   friend __declspec(dllexport) void friend2();
+  friend __declspec(dllexport) void friend3() {}
 };
 __declspec(dllexport) void friend1() {}
                       void friend2() {}
 
 // Implicit declarations can be redeclared with dllexport.
 // MSC-DAG: define dllexport noalias i8* @"\01??2@{{YAPAXI|YAPEAX_K}}@Z"(
 // GNU-DAG: define dllexport noalias i8* @_Znw{{[yj]}}(
 void* alloc(__SIZE_TYPE__ n);