<div dir="ltr">Ugh, templates and friend function definitions strike again!<div><br></div><div>I think this is where "semantic" vs. "lexical" DeclContexts come into play. Try doing D->getLexicalDeclContext()->isDependentContext().</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Mar 22, 2016 at 5:23 AM, Stephan Bergmann <span dir="ltr"><<a href="mailto:sbergman@redhat.com" target="_blank">sbergman@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 03/17/2016 09:06 PM, Reid Kleckner via cfe-commits wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Author: rnk<br>
Date: Thu Mar 17 15:06:58 2016<br>
New Revision: 263740<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=263740&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=263740&view=rev</a><br>
Log:<br>
Revert "For MS ABI, emit dllexport friend functions defined inline in class"<br>
<br>
This reverts commit r263738.<br>
<br>
This appears to cause a failure in<br>
CXX/temp/temp.decls/temp.friend/p1.cpp<br>
</blockquote>
<br></span>
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.<br>
<br>
The problem is with friend functions defined inline within class templates (instead of classes), as in<br>
<br>
template<typename> struct S { friend void f() {} };<br>
<br>
But which MSVC apparently wouldn't emit when parsing the class template, anyway, so we shouldn't either.<br>
<br>
So we should filter out friend functions that are defined within class templates:<br>
<br>
(a) either in Parser::ParseLexedMethodDef (lib/Parse/ParseCXXInlineMethods.cpp) before calling ActOnFinishInlineFunctionDef,<br>
<br>
(b) or (probably preferably) later in the "Handle friend functions" block in HandleInlineFunctionDefinition (lib/CodeGen/ModuleBuilder.cpp).<br>
<br>
However, I'm having trouble how to determine that condition, in either case. For (a), I thought that maybe<br>
<br>
CurTemplateDepthTracker.getDepth() != 0<br>
<br>
could work, but apparently doesn't. And for (b),<br>
<br>
!D->getDeclContext()->isDependentContext()<br>
<br>
doesn't work, either (as the context of the declaration presumably isn't the class template, but rather the surrounding namespace).<br>
<br>
Any hints?<div class="HOEnZb"><div class="h5"><br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Modified:<br>
cfe/trunk/include/clang/AST/ASTConsumer.h<br>
cfe/trunk/include/clang/Frontend/MultiplexConsumer.h<br>
cfe/trunk/include/clang/Sema/Sema.h<br>
cfe/trunk/lib/CodeGen/CodeGenAction.cpp<br>
cfe/trunk/lib/CodeGen/ModuleBuilder.cpp<br>
cfe/trunk/lib/Frontend/MultiplexConsumer.cpp<br>
cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp<br>
cfe/trunk/lib/Sema/SemaDecl.cpp<br>
cfe/trunk/test/CodeGenCXX/dllexport.cpp<br>
<br>
Modified: cfe/trunk/include/clang/AST/ASTConsumer.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTConsumer.h?rev=263740&r1=263739&r2=263740&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTConsumer.h?rev=263740&r1=263739&r2=263740&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/AST/ASTConsumer.h (original)<br>
+++ cfe/trunk/include/clang/AST/ASTConsumer.h Thu Mar 17 15:06:58 2016<br>
@@ -55,9 +55,9 @@ public:<br>
/// \returns true to continue parsing, or false to abort parsing.<br>
virtual bool HandleTopLevelDecl(DeclGroupRef D);<br>
<br>
- /// \brief This callback is invoked each time an inline (method or friend)<br>
- /// function definition in a class is completed.<br>
- virtual void HandleInlineFunctionDefinition(FunctionDecl *D) {}<br>
+ /// \brief This callback is invoked each time an inline method definition is<br>
+ /// completed.<br>
+ virtual void HandleInlineMethodDefinition(CXXMethodDecl *D) {}<br>
<br>
/// HandleInterestingDecl - Handle the specified interesting declaration. This<br>
/// is called by the AST reader when deserializing things that might interest<br>
<br>
Modified: cfe/trunk/include/clang/Frontend/MultiplexConsumer.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/MultiplexConsumer.h?rev=263740&r1=263739&r2=263740&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/MultiplexConsumer.h?rev=263740&r1=263739&r2=263740&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Frontend/MultiplexConsumer.h (original)<br>
+++ cfe/trunk/include/clang/Frontend/MultiplexConsumer.h Thu Mar 17 15:06:58 2016<br>
@@ -36,7 +36,7 @@ public:<br>
void Initialize(ASTContext &Context) override;<br>
void HandleCXXStaticMemberVarInstantiation(VarDecl *VD) override;<br>
bool HandleTopLevelDecl(DeclGroupRef D) override;<br>
- void HandleInlineFunctionDefinition(FunctionDecl *D) override;<br>
+ void HandleInlineMethodDefinition(CXXMethodDecl *D) override;<br>
void HandleInterestingDecl(DeclGroupRef D) override;<br>
void HandleTranslationUnit(ASTContext &Ctx) override;<br>
void HandleTagDeclDefinition(TagDecl *D) override;<br>
<br>
Modified: cfe/trunk/include/clang/Sema/Sema.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=263740&r1=263739&r2=263740&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=263740&r1=263739&r2=263740&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Sema/Sema.h (original)<br>
+++ cfe/trunk/include/clang/Sema/Sema.h Thu Mar 17 15:06:58 2016<br>
@@ -1773,7 +1773,7 @@ public:<br>
Decl *ActOnFinishFunctionBody(Decl *Decl, Stmt *Body);<br>
Decl *ActOnFinishFunctionBody(Decl *Decl, Stmt *Body, bool IsInstantiation);<br>
Decl *ActOnSkippedFunctionBody(Decl *Decl);<br>
- void ActOnFinishInlineFunctionDef(FunctionDecl *D);<br>
+ void ActOnFinishInlineMethodDef(CXXMethodDecl *D);<br>
<br>
/// ActOnFinishDelayedAttribute - Invoked when we have finished parsing an<br>
/// attribute for which parsing is delayed.<br>
<br>
Modified: cfe/trunk/lib/CodeGen/CodeGenAction.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenAction.cpp?rev=263740&r1=263739&r2=263740&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenAction.cpp?rev=263740&r1=263739&r2=263740&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/CodeGen/CodeGenAction.cpp (original)<br>
+++ cfe/trunk/lib/CodeGen/CodeGenAction.cpp Thu Mar 17 15:06:58 2016<br>
@@ -123,14 +123,14 @@ namespace clang {<br>
return true;<br>
}<br>
<br>
- void HandleInlineFunctionDefinition(FunctionDecl *D) override {<br>
+ void HandleInlineMethodDefinition(CXXMethodDecl *D) override {<br>
PrettyStackTraceDecl CrashInfo(D, SourceLocation(),<br>
Context->getSourceManager(),<br>
- "LLVM IR generation of inline function");<br>
+ "LLVM IR generation of inline method");<br>
if (llvm::TimePassesIsEnabled)<br>
LLVMIRGeneration.startTimer();<br>
<br>
- Gen->HandleInlineFunctionDefinition(D);<br>
+ Gen->HandleInlineMethodDefinition(D);<br>
<br>
if (llvm::TimePassesIsEnabled)<br>
LLVMIRGeneration.stopTimer();<br>
<br>
Modified: cfe/trunk/lib/CodeGen/ModuleBuilder.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ModuleBuilder.cpp?rev=263740&r1=263739&r2=263740&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ModuleBuilder.cpp?rev=263740&r1=263739&r2=263740&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/CodeGen/ModuleBuilder.cpp (original)<br>
+++ cfe/trunk/lib/CodeGen/ModuleBuilder.cpp Thu Mar 17 15:06:58 2016<br>
@@ -143,22 +143,12 @@ namespace {<br>
DeferredInlineMethodDefinitions.clear();<br>
}<br>
<br>
- void HandleInlineFunctionDefinition(FunctionDecl *D) override {<br>
+ void HandleInlineMethodDefinition(CXXMethodDecl *D) override {<br>
if (Diags.hasErrorOccurred())<br>
return;<br>
<br>
assert(D->doesThisDeclarationHaveABody());<br>
<br>
- // Handle friend functions.<br>
- if (D->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend)) {<br>
- if (Ctx->getTargetInfo().getCXXABI().isMicrosoft())<br>
- Builder->EmitTopLevelDecl(D);<br>
- return;<br>
- }<br>
-<br>
- // Otherwise, must be a method.<br>
- auto MD = cast<CXXMethodDecl>(D);<br>
-<br>
// We may want to emit this definition. However, that decision might be<br>
// based on computing the linkage, and we have to defer that in case we<br>
// are inside of something that will change the method's final linkage,<br>
@@ -167,13 +157,13 @@ namespace {<br>
// void bar();<br>
// void foo() { bar(); }<br>
// } A;<br>
- DeferredInlineMethodDefinitions.push_back(MD);<br>
+ DeferredInlineMethodDefinitions.push_back(D);<br>
<br>
// Provide some coverage mapping even for methods that aren't emitted.<br>
// Don't do this for templated classes though, as they may not be<br>
// instantiable.<br>
- if (!MD->getParent()->getDescribedClassTemplate())<br>
- Builder->AddDeferredUnusedCoverageMapping(MD);<br>
+ if (!D->getParent()->getDescribedClassTemplate())<br>
+ Builder->AddDeferredUnusedCoverageMapping(D);<br>
}<br>
<br>
/// HandleTagDeclDefinition - This callback is invoked each time a TagDecl<br>
<br>
Modified: cfe/trunk/lib/Frontend/MultiplexConsumer.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/MultiplexConsumer.cpp?rev=263740&r1=263739&r2=263740&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/MultiplexConsumer.cpp?rev=263740&r1=263739&r2=263740&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Frontend/MultiplexConsumer.cpp (original)<br>
+++ cfe/trunk/lib/Frontend/MultiplexConsumer.cpp Thu Mar 17 15:06:58 2016<br>
@@ -272,9 +272,9 @@ bool MultiplexConsumer::HandleTopLevelDe<br>
return Continue;<br>
}<br>
<br>
-void MultiplexConsumer::HandleInlineFunctionDefinition(FunctionDecl *D) {<br>
+void MultiplexConsumer::HandleInlineMethodDefinition(CXXMethodDecl *D) {<br>
for (auto &Consumer : Consumers)<br>
- Consumer->HandleInlineFunctionDefinition(D);<br>
+ Consumer->HandleInlineMethodDefinition(D);<br>
}<br>
<br>
void MultiplexConsumer::HandleCXXStaticMemberVarInstantiation(VarDecl *VD) {<br>
<br>
Modified: cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp?rev=263740&r1=263739&r2=263740&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp?rev=263740&r1=263739&r2=263740&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp (original)<br>
+++ cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp Thu Mar 17 15:06:58 2016<br>
@@ -564,10 +564,8 @@ void Parser::ParseLexedMethodDef(LexedMe<br>
if (Tok.is(tok::eof) && Tok.getEofData() == LM.D)<br>
ConsumeAnyToken();<br>
<br>
- if (auto *FD = dyn_cast_or_null<FunctionDecl>(LM.D))<br>
- if (isa<CXXMethodDecl>(FD) ||<br>
- FD->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend))<br>
- Actions.ActOnFinishInlineFunctionDef(FD);<br>
+ if (CXXMethodDecl *MD = dyn_cast_or_null<CXXMethodDecl>(LM.D))<br>
+ Actions.ActOnFinishInlineMethodDef(MD);<br>
}<br>
<br>
/// ParseLexedMemberInitializers - We finished parsing the member specification<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaDecl.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=263740&r1=263739&r2=263740&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=263740&r1=263739&r2=263740&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Thu Mar 17 15:06:58 2016<br>
@@ -10872,8 +10872,8 @@ Sema::ActOnStartOfFunctionDef(Scope *FnB<br>
return ActOnStartOfFunctionDef(FnBodyScope, DP, SkipBody);<br>
}<br>
<br>
-void Sema::ActOnFinishInlineFunctionDef(FunctionDecl *D) {<br>
- Consumer.HandleInlineFunctionDefinition(D);<br>
+void Sema::ActOnFinishInlineMethodDef(CXXMethodDecl *D) {<br>
+ Consumer.HandleInlineMethodDefinition(D);<br>
}<br>
<br>
static bool ShouldWarnAboutMissingPrototype(const FunctionDecl *FD,<br>
<br>
Modified: cfe/trunk/test/CodeGenCXX/dllexport.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/dllexport.cpp?rev=263740&r1=263739&r2=263740&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/dllexport.cpp?rev=263740&r1=263739&r2=263740&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/CodeGenCXX/dllexport.cpp (original)<br>
+++ cfe/trunk/test/CodeGenCXX/dllexport.cpp Thu Mar 17 15:06:58 2016<br>
@@ -255,11 +255,9 @@ __declspec(dllexport) void redecl2();<br>
// GNU-DAG: define dllexport void @_Z7friend1v()<br>
// MSC-DAG: define dllexport void @"\01?friend2@@YAXXZ"()<br>
// GNU-DAG: define dllexport void @_Z7friend2v()<br>
-// MSC-DAG: define weak_odr dllexport void @"\01?friend3@@YAXXZ"()<br>
struct FuncFriend {<br>
friend __declspec(dllexport) void friend1();<br>
friend __declspec(dllexport) void friend2();<br>
- friend __declspec(dllexport) void friend3() {}<br>
};<br>
__declspec(dllexport) void friend1() {}<br>
void friend2() {}<br>
</blockquote>
</div></div></blockquote></div><br></div>