r263740 - Revert "For MS ABI, emit dllexport friend functions defined inline in class"
Reid Kleckner via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 22 10:38:19 PDT 2016
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 <sbergman at 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<typename> 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
>>
>> ==============================================================================
>> --- 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.cpp Thu Mar 17 15:06:58 2016
>> @@ -143,22 +143,12 @@ namespace {
>> DeferredInlineMethodDefinitions.clear();
>> }
>>
>> - void HandleInlineFunctionDefinition(FunctionDecl *D) override {
>> + void HandleInlineMethodDefinition(CXXMethodDecl *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<CXXMethodDecl>(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,
>> @@ -167,13 +157,13 @@ namespace {
>> // void bar();
>> // void foo() { bar(); }
>> // } A;
>> - DeferredInlineMethodDefinitions.push_back(MD);
>> + DeferredInlineMethodDefinitions.push_back(D);
>>
>> // 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 (!MD->getParent()->getDescribedClassTemplate())
>> - Builder->AddDeferredUnusedCoverageMapping(MD);
>> + if (!D->getParent()->getDescribedClassTemplate())
>> + Builder->AddDeferredUnusedCoverageMapping(D);
>> }
>>
>> /// HandleTagDeclDefinition - This callback is invoked each time a
>> TagDecl
>>
>> Modified: cfe/trunk/lib/Frontend/MultiplexConsumer.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/MultiplexConsumer.cpp?rev=263740&r1=263739&r2=263740&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Frontend/MultiplexConsumer.cpp (original)
>> +++ cfe/trunk/lib/Frontend/MultiplexConsumer.cpp Thu Mar 17 15:06:58 2016
>> @@ -272,9 +272,9 @@ bool MultiplexConsumer::HandleTopLevelDe
>> return Continue;
>> }
>>
>> -void MultiplexConsumer::HandleInlineFunctionDefinition(FunctionDecl *D) {
>> +void MultiplexConsumer::HandleInlineMethodDefinition(CXXMethodDecl *D) {
>> for (auto &Consumer : Consumers)
>> - Consumer->HandleInlineFunctionDefinition(D);
>> + Consumer->HandleInlineMethodDefinition(D);
>> }
>>
>> void MultiplexConsumer::HandleCXXStaticMemberVarInstantiation(VarDecl
>> *VD) {
>>
>> Modified: cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp?rev=263740&r1=263739&r2=263740&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp (original)
>> +++ cfe/trunk/lib/Parse/ParseCXXInlineMethods.cpp Thu Mar 17 15:06:58 2016
>> @@ -564,10 +564,8 @@ void Parser::ParseLexedMethodDef(LexedMe
>> if (Tok.is(tok::eof) && Tok.getEofData() == LM.D)
>> ConsumeAnyToken();
>>
>> - if (auto *FD = dyn_cast_or_null<FunctionDecl>(LM.D))
>> - if (isa<CXXMethodDecl>(FD) ||
>> - FD->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend))
>> - Actions.ActOnFinishInlineFunctionDef(FD);
>> + if (CXXMethodDecl *MD = dyn_cast_or_null<CXXMethodDecl>(LM.D))
>> + Actions.ActOnFinishInlineMethodDef(MD);
>> }
>>
>> /// ParseLexedMemberInitializers - We finished parsing the member
>> specification
>>
>> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=263740&r1=263739&r2=263740&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Thu Mar 17 15:06:58 2016
>> @@ -10872,8 +10872,8 @@ Sema::ActOnStartOfFunctionDef(Scope *FnB
>> return ActOnStartOfFunctionDef(FnBodyScope, DP, SkipBody);
>> }
>>
>> -void Sema::ActOnFinishInlineFunctionDef(FunctionDecl *D) {
>> - Consumer.HandleInlineFunctionDefinition(D);
>> +void Sema::ActOnFinishInlineMethodDef(CXXMethodDecl *D) {
>> + Consumer.HandleInlineMethodDefinition(D);
>> }
>>
>> static bool ShouldWarnAboutMissingPrototype(const FunctionDecl *FD,
>>
>> Modified: cfe/trunk/test/CodeGenCXX/dllexport.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/dllexport.cpp?rev=263740&r1=263739&r2=263740&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/CodeGenCXX/dllexport.cpp (original)
>> +++ cfe/trunk/test/CodeGenCXX/dllexport.cpp Thu Mar 17 15:06:58 2016
>> @@ -255,11 +255,9 @@ __declspec(dllexport) void redecl2();
>> // 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() {}
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160322/8d721050/attachment-0001.html>
More information about the cfe-commits
mailing list