Re: [PATCH] D15267: For MS ABI, emit dllexport friend functions defined inline in class

2016-03-03 Thread Stephan Bergmann via cfe-commits 
sberg added a comment.

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


http://reviews.llvm.org/D15267



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15267: For MS ABI, emit dllexport friend functions defined inline in class

2016-03-02 Thread Reid Kleckner via cfe-commits 
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm


http://reviews.llvm.org/D15267



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15267: For MS ABI, emit dllexport friend functions defined inline in class

2016-03-02 Thread Stephan Bergmann via cfe-commits 
sberg added a comment.

friendly ping


http://reviews.llvm.org/D15267



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15267: For MS ABI, emit dllexport friend functions defined inline in class

2016-01-27 Thread Reid Kleckner via cfe-commits 
rnk added inline comments.


Comment at: include/clang/AST/ASTConsumer.h:58-64
@@ -57,5 +57,9 @@
 
   /// \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) {}
+

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


Comment at: lib/Parse/ParseCXXInlineMethods.cpp:568-569
@@ -567,2 +567,4 @@
 Actions.ActOnFinishInlineMethodDef(MD);
+  else if (auto *FD = dyn_cast_or_null(LM.D))
+Actions.ActOnFinishInlineFriendFunctionDef(FD);
 }

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.


http://reviews.llvm.org/D15267



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15267: For MS ABI, emit dllexport friend functions defined inline in class

2016-01-27 Thread Stephan Bergmann via cfe-commits 
sberg added inline comments.


Comment at: include/clang/AST/ASTConsumer.h:58-64
@@ -57,5 +57,9 @@
 
   /// \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) {}
+

rnk wrote:
> I'm pretty sure we can relax HandleInlineMethodDefinition to take a 
> FunctionDecl and then we don't need the extra AST consumer callback.
...and then also rename it from HandleInlineMethodDefinition to, 
say,HandleInlineFunctionDefinition?  Or better stick with the (then no longer 
accurate) name?


http://reviews.llvm.org/D15267



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15267: For MS ABI, emit dllexport friend functions defined inline in class

2016-01-27 Thread Stephan Bergmann via cfe-commits 
sberg updated this revision to Diff 46227.
sberg added a comment.

updated as discussed in the comments


http://reviews.llvm.org/D15267

Files:
  include/clang/AST/ASTConsumer.h
  include/clang/Frontend/MultiplexConsumer.h
  include/clang/Sema/Sema.h
  lib/CodeGen/CodeGenAction.cpp
  lib/CodeGen/ModuleBuilder.cpp
  lib/Frontend/MultiplexConsumer.cpp
  lib/Parse/ParseCXXInlineMethods.cpp
  lib/Sema/SemaDecl.cpp
  test/CodeGenCXX/dllexport.cpp

Index: test/CodeGenCXX/dllexport.cpp
===
--- test/CodeGenCXX/dllexport.cpp
+++ test/CodeGenCXX/dllexport.cpp
@@ -255,9 +255,11 @@
 // 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() {}
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -10701,8 +10701,8 @@
   return ActOnStartOfFunctionDef(FnBodyScope, DP, SkipBody);
 }
 
-void Sema::ActOnFinishInlineMethodDef(CXXMethodDecl *D) {
-  Consumer.HandleInlineMethodDefinition(D);
+void Sema::ActOnFinishInlineFunctionDef(FunctionDecl *D) {
+  Consumer.HandleInlineFunctionDefinition(D);
 }
 
 static bool ShouldWarnAboutMissingPrototype(const FunctionDecl *FD, 
Index: lib/Parse/ParseCXXInlineMethods.cpp
===
--- lib/Parse/ParseCXXInlineMethods.cpp
+++ lib/Parse/ParseCXXInlineMethods.cpp
@@ -563,8 +563,10 @@
   if (Tok.is(tok::eof) && Tok.getEofData() == LM.D)
 ConsumeAnyToken();
 
-  if (CXXMethodDecl *MD = dyn_cast_or_null(LM.D))
-Actions.ActOnFinishInlineMethodDef(MD);
+  if (auto *FD = dyn_cast_or_null(LM.D))
+if (isa(FD) ||
+FD->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend))
+  Actions.ActOnFinishInlineFunctionDef(FD);
 }
 
 /// ParseLexedMemberInitializers - We finished parsing the member specification
Index: lib/Frontend/MultiplexConsumer.cpp
===
--- lib/Frontend/MultiplexConsumer.cpp
+++ lib/Frontend/MultiplexConsumer.cpp
@@ -272,9 +272,9 @@
   return Continue;
 }
 
-void MultiplexConsumer::HandleInlineMethodDefinition(CXXMethodDecl *D) {
+void MultiplexConsumer::HandleInlineFunctionDefinition(FunctionDecl *D) {
   for (auto  : Consumers)
-Consumer->HandleInlineMethodDefinition(D);
+Consumer->HandleInlineFunctionDefinition(D);
 }
 
 void MultiplexConsumer::HandleCXXStaticMemberVarInstantiation(VarDecl *VD) {
Index: lib/CodeGen/ModuleBuilder.cpp
===
--- lib/CodeGen/ModuleBuilder.cpp
+++ lib/CodeGen/ModuleBuilder.cpp
@@ -142,27 +142,37 @@
   DeferredInlineMethodDefinitions.clear();
 }
 
-void HandleInlineMethodDefinition(CXXMethodDecl *D) override {
+void HandleInlineFunctionDefinition(FunctionDecl *D) override {
   if (Diags.hasErrorOccurred())
 return;
 
   assert(D->doesThisDeclarationHaveABody());
 
+  // Handle friend functions.
+  if (D->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend)) {
+if (Ctx->getTargetInfo().getCXXABI().isMicrosoft())
+  Builder->EmitTopLevelDecl(D);
+return;
+  }
+
+  // Otherwise, must be a method.
+  auto MD = cast(D);
+
   // We may want to emit this definition. However, that decision might be
   // based on computing the linkage, and we have to defer that in case we
   // are inside of something that will change the method's final linkage,
   // e.g.
   //   typedef struct {
   // void bar();
   // void foo() { bar(); }
   //   } A;
-  DeferredInlineMethodDefinitions.push_back(D);
+  DeferredInlineMethodDefinitions.push_back(MD);
 
   // 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);
+  if (!MD->getParent()->getDescribedClassTemplate())
+Builder->AddDeferredUnusedCoverageMapping(MD);
 }
 
 /// HandleTagDeclDefinition - This callback is invoked each time a TagDecl
Index: lib/CodeGen/CodeGenAction.cpp
===
--- lib/CodeGen/CodeGenAction.cpp
+++ lib/CodeGen/CodeGenAction.cpp
@@ -123,14 +123,14 @@
   return true;
 }
 
-void HandleInlineMethodDefinition(CXXMethodDecl *D) override {
+void HandleInlineFunctionDefinition(FunctionDecl *D)

Re: [PATCH] D15267: For MS ABI, emit dllexport friend functions defined inline in class

2016-01-27 Thread Reid Kleckner via cfe-commits 
rnk added inline comments.


Comment at: include/clang/AST/ASTConsumer.h:58-64
@@ -57,5 +57,9 @@
 
   /// \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) {}
+

sberg wrote:
> rnk wrote:
> > I'm pretty sure we can relax HandleInlineMethodDefinition to take a 
> > FunctionDecl and then we don't need the extra AST consumer callback.
> ...and then also rename it from HandleInlineMethodDefinition to, 
> say,HandleInlineFunctionDefinition?  Or better stick with the (then no longer 
> accurate) name?
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.


http://reviews.llvm.org/D15267



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits