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() {}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to