r263740 - Revert "For MS ABI, emit dllexport friend functions defined inline in class"
Stephan Bergmann via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 22 05:23:32 PDT 2016
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() {}
More information about the cfe-commits
mailing list