Re: r263740 - Revert "For MS ABI, emit dllexport friend functions defined inline in class"

[Stephan Bergmann via cfe-commits]

On 03/22/2016 06:38 PM, Reid Kleckner wrote:

Ugh, templates and friend function definitions strike again!

I think this is where "semantic" vs. "lexical" DeclContexts come into
play. Try doing D->getLexicalDeclContext()->isDependentContext().


Great, thanks, that's what I'd been searching for.

I've put an updated version up for review now at 
.



On Tue, Mar 22, 2016 at 5:23 AM, Stephan Bergmann mailto:sberg...@redhat.com>> wrote:

On 03/17/2016 09:06 PM, Reid Kleckner via cfe-commits wrote:

Author: rnk
Date: Thu Mar 17 15:06:58 2016
New Revision: 263740

URL: http://llvm.org/viewvc/llvm-project?rev=263740&view=rev
Log:
Revert "For MS ABI, emit dllexport friend functions defined
inline in class"

This reverts commit r263738.

This appears to cause a failure in
CXX/temp/temp.decls/temp.friend/p1.cpp


Ah, indeed, if you stick "-triple %ms_abi_triple" in
test/CXX/temp/temp.decls/temp.friend/p1.cpp, it would consistently
have failed on all platforms.

The problem is with friend functions defined inline within class
templates (instead of classes), as in

   template struct S { friend void f() {} };

But which MSVC apparently wouldn't emit when parsing the class
template, anyway, so we shouldn't either.

So we should filter out friend functions that are defined within
class templates:

(a) either in Parser::ParseLexedMethodDef
(lib/Parse/ParseCXXInlineMethods.cpp) before calling
ActOnFinishInlineFunctionDef,

(b) or (probably preferably) later in the "Handle friend functions"
block in HandleInlineFunctionDefinition (lib/CodeGen/ModuleBuilder.cpp).

However, I'm having trouble how to determine that condition, in
either case.  For (a), I thought that maybe

   CurTemplateDepthTracker.getDepth() != 0

could work, but apparently doesn't.  And for (b),

   !D->getDeclContext()->isDependentContext()

doesn't work, either (as the context of the declaration presumably
isn't the class template, but rather the surrounding namespace).

Any hints?


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

Re: r263740 - Revert "For MS ABI, emit dllexport friend functions defined inline in class"

[Reid Kleckner via cfe-commits]

Ugh, templates and friend function definitions strike again!

I think this is where "semantic" vs. "lexical" DeclContexts come into play.
Try doing D->getLexicalDeclContext()->isDependentContext().

On Tue, Mar 22, 2016 at 5:23 AM, Stephan Bergmann 
wrote:

> On 03/17/2016 09:06 PM, Reid Kleckner via cfe-commits wrote:
>
>> Author: rnk
>> Date: Thu Mar 17 15:06:58 2016
>> New Revision: 263740
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=263740&view=rev
>> Log:
>> Revert "For MS ABI, emit dllexport friend functions defined inline in
>> class"
>>
>> This reverts commit r263738.
>>
>> This appears to cause a failure in
>> CXX/temp/temp.decls/temp.friend/p1.cpp
>>
>
> Ah, indeed, if you stick "-triple %ms_abi_triple" in
> test/CXX/temp/temp.decls/temp.friend/p1.cpp, it would consistently have
> failed on all platforms.
>
> The problem is with friend functions defined inline within class templates
> (instead of classes), as in
>
>   template struct S { friend void f() {} };
>
> But which MSVC apparently wouldn't emit when parsing the class template,
> anyway, so we shouldn't either.
>
> So we should filter out friend functions that are defined within class
> templates:
>
> (a) either in Parser::ParseLexedMethodDef
> (lib/Parse/ParseCXXInlineMethods.cpp) before calling
> ActOnFinishInlineFunctionDef,
>
> (b) or (probably preferably) later in the "Handle friend functions" block
> in HandleInlineFunctionDefinition (lib/CodeGen/ModuleBuilder.cpp).
>
> However, I'm having trouble how to determine that condition, in either
> case.  For (a), I thought that maybe
>
>   CurTemplateDepthTracker.getDepth() != 0
>
> could work, but apparently doesn't.  And for (b),
>
>   !D->getDeclContext()->isDependentContext()
>
> doesn't work, either (as the context of the declaration presumably isn't
> the class template, but rather the surrounding namespace).
>
> Any hints?
>
>
>
> Modified:
>>  cfe/trunk/include/clang/AST/ASTConsumer.h
>>  cfe/trunk/include/clang/Frontend/MultiplexConsumer.h
>>  cfe/trunk/include/clang/Sema/Sema.h
>>  cfe/trunk/lib/CodeGen/CodeGenAction.cpp
>>  cfe/trunk/lib/CodeGen/ModuleBuilder.cpp
>>  cfe/trunk/lib/Frontend/MultiplexConsumer.cpp
>>  cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp
>>  cfe/trunk/lib/Sema/SemaDecl.cpp
>>  cfe/trunk/test/CodeGenCXX/dllexport.cpp
>>
>> Modified: cfe/trunk/include/clang/AST/ASTConsumer.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTConsumer.h?rev=263740&r1=263739&r2=263740&view=diff
>>
>> ==
>> --- cfe/trunk/include/clang/AST/ASTConsumer.h (original)
>> +++ cfe/trunk/include/clang/AST/ASTConsumer.h Thu Mar 17 15:06:58 2016
>> @@ -55,9 +55,9 @@ 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 or
>> friend)
>> -  /// function definition in a class is completed.
>> -  virtual void HandleInlineFunctionDefinition(FunctionDecl *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
>>
>> Modified: cfe/trunk/include/clang/Frontend/MultiplexConsumer.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/MultiplexConsumer.h?rev=263740&r1=263739&r2=263740&view=diff
>>
>> ==
>> --- cfe/trunk/include/clang/Frontend/MultiplexConsumer.h (original)
>> +++ cfe/trunk/include/clang/Frontend/MultiplexConsumer.h Thu Mar 17
>> 15:06:58 2016
>> @@ -36,7 +36,7 @@ public:
>> void Initialize(ASTContext &Context) override;
>> void HandleCXXStaticMemberVarInstantiation(VarDecl *VD) override;
>> bool HandleTopLevelDecl(DeclGroupRef D) override;
>> -  void HandleInlineFunctionDefinition(FunctionDecl *D) override;
>> +  void HandleInlineMethodDefinition(CXXMethodDecl *D) override;
>> void HandleInterestingDecl(DeclGroupRef D) override;
>> void HandleTranslationUnit(ASTContext &Ctx) override;
>> void HandleTagDeclDefinition(TagDecl *D) override;
>>
>> Modified: cfe/trunk/include/clang/Sema/Sema.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=263740&r1=263739&r2=263740&view=diff
>>
>> ==
>> --- cfe/trunk/include/clang/Sema/Sema.h (original)
>> +++ cfe/trunk/include/clang/Sema/Sema.h Thu Mar 17 15:06:58 2016
>> @@ -1773,7 +1773,7 @@ public:
>> Decl *ActOnFinishFunctionBody(Decl *Decl, Stmt *Body);
>> Decl *ActOnFinishFunctionBody(Decl *Decl, Stmt *B

Re: r263740 - Revert "For MS ABI, emit dllexport friend functions defined inline in class"

[Stephan Bergmann via cfe-commits]

On 03/17/2016 09:06 PM, Reid Kleckner via cfe-commits wrote:

Author: rnk
Date: Thu Mar 17 15:06:58 2016
New Revision: 263740

URL: http://llvm.org/viewvc/llvm-project?rev=263740&view=rev
Log:
Revert "For MS ABI, emit dllexport friend functions defined inline in class"

This reverts commit r263738.

This appears to cause a failure in
CXX/temp/temp.decls/temp.friend/p1.cpp


Ah, indeed, if you stick "-triple %ms_abi_triple" in 
test/CXX/temp/temp.decls/temp.friend/p1.cpp, it would consistently have 
failed on all platforms.


The problem is with friend functions defined inline within class 
templates (instead of classes), as in


  template struct S { friend void f() {} };

But which MSVC apparently wouldn't emit when parsing the class template, 
anyway, so we shouldn't either.


So we should filter out friend functions that are defined within class 
templates:


(a) either in Parser::ParseLexedMethodDef 
(lib/Parse/ParseCXXInlineMethods.cpp) before calling 
ActOnFinishInlineFunctionDef,


(b) or (probably preferably) later in the "Handle friend functions" 
block in HandleInlineFunctionDefinition (lib/CodeGen/ModuleBuilder.cpp).


However, I'm having trouble how to determine that condition, in either 
case.  For (a), I thought that maybe


  CurTemplateDepthTracker.getDepth() != 0

could work, but apparently doesn't.  And for (b),

  !D->getDeclContext()->isDependentContext()

doesn't work, either (as the context of the declaration presumably isn't 
the class template, but rather the surrounding namespace).


Any hints?



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

Modified: cfe/trunk/include/clang/AST/ASTConsumer.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTConsumer.h?rev=263740&r1=263739&r2=263740&view=diff
==
--- cfe/trunk/include/clang/AST/ASTConsumer.h (original)
+++ cfe/trunk/include/clang/AST/ASTConsumer.h Thu Mar 17 15:06:58 2016
@@ -55,9 +55,9 @@ 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 or friend)
-  /// function definition in a class is completed.
-  virtual void HandleInlineFunctionDefinition(FunctionDecl *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

Modified: cfe/trunk/include/clang/Frontend/MultiplexConsumer.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/MultiplexConsumer.h?rev=263740&r1=263739&r2=263740&view=diff
==
--- cfe/trunk/include/clang/Frontend/MultiplexConsumer.h (original)
+++ cfe/trunk/include/clang/Frontend/MultiplexConsumer.h Thu Mar 17 15:06:58 
2016
@@ -36,7 +36,7 @@ public:
void Initialize(ASTContext &Context) override;
void HandleCXXStaticMemberVarInstantiation(VarDecl *VD) override;
bool HandleTopLevelDecl(DeclGroupRef D) override;
-  void HandleInlineFunctionDefinition(FunctionDecl *D) override;
+  void HandleInlineMethodDefinition(CXXMethodDecl *D) override;
void HandleInterestingDecl(DeclGroupRef D) override;
void HandleTranslationUnit(ASTContext &Ctx) override;
void HandleTagDeclDefinition(TagDecl *D) override;

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=263740&r1=263739&r2=263740&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Thu Mar 17 15:06:58 2016
@@ -1773,7 +1773,7 @@ public:
Decl *ActOnFinishFunctionBody(Decl *Decl, Stmt *Body);
Decl *ActOnFinishFunctionBody(Decl *Decl, Stmt *Body, bool IsInstantiation);
Decl *ActOnSkippedFunctionBody(Decl *Decl);
-  void ActOnFinishInlineFunctionDef(FunctionDecl *D);
+  void ActOnFinishInlineMethodDef(CXXMethodDecl *D);

/// ActOnFinishDelayedAttribute - Invoked when we have finished parsing an
/// attribute for which parsing is delayed.

Modified: cfe/trunk/lib/CodeGen/CodeGenAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenAction.cpp?rev=263740&r1=263739&r2=263740&view=diff
===

r263740 - Revert "For MS ABI, emit dllexport friend functions defined inline in class"

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Thu Mar 17 15:06:58 2016
New Revision: 263740

URL: http://llvm.org/viewvc/llvm-project?rev=263740&view=rev
Log:
Revert "For MS ABI, emit dllexport friend functions defined inline in class"

This reverts commit r263738.

This appears to cause a failure in
CXX/temp/temp.decls/temp.friend/p1.cpp

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

Modified: cfe/trunk/include/clang/AST/ASTConsumer.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTConsumer.h?rev=263740&r1=263739&r2=263740&view=diff
==
--- cfe/trunk/include/clang/AST/ASTConsumer.h (original)
+++ cfe/trunk/include/clang/AST/ASTConsumer.h Thu Mar 17 15:06:58 2016
@@ -55,9 +55,9 @@ 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 or friend)
-  /// function definition in a class is completed.
-  virtual void HandleInlineFunctionDefinition(FunctionDecl *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

Modified: cfe/trunk/include/clang/Frontend/MultiplexConsumer.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/MultiplexConsumer.h?rev=263740&r1=263739&r2=263740&view=diff
==
--- cfe/trunk/include/clang/Frontend/MultiplexConsumer.h (original)
+++ cfe/trunk/include/clang/Frontend/MultiplexConsumer.h Thu Mar 17 15:06:58 
2016
@@ -36,7 +36,7 @@ public:
   void Initialize(ASTContext &Context) override;
   void HandleCXXStaticMemberVarInstantiation(VarDecl *VD) override;
   bool HandleTopLevelDecl(DeclGroupRef D) override;
-  void HandleInlineFunctionDefinition(FunctionDecl *D) override;
+  void HandleInlineMethodDefinition(CXXMethodDecl *D) override;
   void HandleInterestingDecl(DeclGroupRef D) override;
   void HandleTranslationUnit(ASTContext &Ctx) override;
   void HandleTagDeclDefinition(TagDecl *D) override;

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=263740&r1=263739&r2=263740&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Thu Mar 17 15:06:58 2016
@@ -1773,7 +1773,7 @@ public:
   Decl *ActOnFinishFunctionBody(Decl *Decl, Stmt *Body);
   Decl *ActOnFinishFunctionBody(Decl *Decl, Stmt *Body, bool IsInstantiation);
   Decl *ActOnSkippedFunctionBody(Decl *Decl);
-  void ActOnFinishInlineFunctionDef(FunctionDecl *D);
+  void ActOnFinishInlineMethodDef(CXXMethodDecl *D);
 
   /// ActOnFinishDelayedAttribute - Invoked when we have finished parsing an
   /// attribute for which parsing is delayed.

Modified: cfe/trunk/lib/CodeGen/CodeGenAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenAction.cpp?rev=263740&r1=263739&r2=263740&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenAction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenAction.cpp Thu Mar 17 15:06:58 2016
@@ -123,14 +123,14 @@ namespace clang {
   return true;
 }
 
-void HandleInlineFunctionDefinition(FunctionDecl *D) override {
+void HandleInlineMethodDefinition(CXXMethodDecl *D) override {
   PrettyStackTraceDecl CrashInfo(D, SourceLocation(),
  Context->getSourceManager(),
- "LLVM IR generation of inline function");
+ "LLVM IR generation of inline method");
   if (llvm::TimePassesIsEnabled)
 LLVMIRGeneration.startTimer();
 
-  Gen->HandleInlineFunctionDefinition(D);
+  Gen->HandleInlineMethodDefinition(D);
 
   if (llvm::TimePassesIsEnabled)
 LLVMIRGeneration.stopTimer();

Modified: cfe/trunk/lib/CodeGen/ModuleBuilder.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ModuleBuilder.cpp?rev=263740&r1=263739&r2=263740&view=diff
==
--- cfe/trunk/lib/CodeGen/ModuleBuilder.cpp (original)
+++ cfe/trunk/lib/CodeGen/ModuleBuilder.