r287774 - Remove C++ default arg side table for MS ABI ctor closures

Reid Kleckner via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 23 08:51:30 PST 2016


Author: rnk
Date: Wed Nov 23 10:51:30 2016
New Revision: 287774

URL: http://llvm.org/viewvc/llvm-project?rev=287774&view=rev
Log:
Remove C++ default arg side table for MS ABI ctor closures

Summary:
We don't need a side table in ASTContext to hold CXXDefaultArgExprs. The
important part of building the CXXDefaultArgExprs was to ODR use the
default argument expressions, not to make AST nodes. Refactor the code
to only check the default argument, and remove the side table in
ASTContext which wasn't being serialized.

Fixes PR31121

Reviewers: thakis, rsmith, majnemer

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D27007

Added:
    cfe/trunk/test/SemaCXX/default-arg-closures.cpp
Modified:
    cfe/trunk/include/clang/AST/ASTContext.h
    cfe/trunk/include/clang/Sema/Sema.h
    cfe/trunk/lib/AST/ASTContext.cpp
    cfe/trunk/lib/AST/CXXABI.h
    cfe/trunk/lib/AST/ItaniumCXXABI.cpp
    cfe/trunk/lib/AST/MicrosoftCXXABI.cpp
    cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
    cfe/trunk/lib/Sema/SemaDeclCXX.cpp
    cfe/trunk/lib/Sema/SemaExpr.cpp
    cfe/trunk/lib/Sema/SemaExprCXX.cpp

Modified: cfe/trunk/include/clang/AST/ASTContext.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=287774&r1=287773&r2=287774&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/ASTContext.h (original)
+++ cfe/trunk/include/clang/AST/ASTContext.h Wed Nov 23 10:51:30 2016
@@ -2452,12 +2452,6 @@ public:
   void addCopyConstructorForExceptionObject(CXXRecordDecl *RD,
                                             CXXConstructorDecl *CD);
 
-  void addDefaultArgExprForConstructor(const CXXConstructorDecl *CD,
-                                       unsigned ParmIdx, Expr *DAE);
-
-  Expr *getDefaultArgExprForConstructor(const CXXConstructorDecl *CD,
-                                        unsigned ParmIdx);
-
   void addTypedefNameForUnnamedTagDecl(TagDecl *TD, TypedefNameDecl *TND);
 
   TypedefNameDecl *getTypedefNameForUnnamedTagDecl(const TagDecl *TD);

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=287774&r1=287773&r2=287774&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Wed Nov 23 10:51:30 2016
@@ -4408,6 +4408,12 @@ public:
 
   ExprResult BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field);
 
+
+  /// Instantiate or parse a C++ default argument expression as necessary.
+  /// Return true on error.
+  bool CheckCXXDefaultArgExpr(SourceLocation CallLoc, FunctionDecl *FD,
+                              ParmVarDecl *Param);
+
   /// BuildCXXDefaultArgExpr - Creates a CXXDefaultArgExpr, instantiating
   /// the default expr if needed.
   ExprResult BuildCXXDefaultArgExpr(SourceLocation CallLoc,

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=287774&r1=287773&r2=287774&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Wed Nov 23 10:51:30 2016
@@ -9172,18 +9172,6 @@ void ASTContext::addCopyConstructorForEx
       cast<CXXConstructorDecl>(CD->getFirstDecl()));
 }
 
-void ASTContext::addDefaultArgExprForConstructor(const CXXConstructorDecl *CD,
-                                                 unsigned ParmIdx, Expr *DAE) {
-  ABI->addDefaultArgExprForConstructor(
-      cast<CXXConstructorDecl>(CD->getFirstDecl()), ParmIdx, DAE);
-}
-
-Expr *ASTContext::getDefaultArgExprForConstructor(const CXXConstructorDecl *CD,
-                                                  unsigned ParmIdx) {
-  return ABI->getDefaultArgExprForConstructor(
-      cast<CXXConstructorDecl>(CD->getFirstDecl()), ParmIdx);
-}
-
 void ASTContext::addTypedefNameForUnnamedTagDecl(TagDecl *TD,
                                                  TypedefNameDecl *DD) {
   return ABI->addTypedefNameForUnnamedTagDecl(TD, DD);

Modified: cfe/trunk/lib/AST/CXXABI.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/CXXABI.h?rev=287774&r1=287773&r2=287774&view=diff
==============================================================================
--- cfe/trunk/lib/AST/CXXABI.h (original)
+++ cfe/trunk/lib/AST/CXXABI.h Wed Nov 23 10:51:30 2016
@@ -54,12 +54,6 @@ public:
   virtual const CXXConstructorDecl *
   getCopyConstructorForExceptionObject(CXXRecordDecl *) = 0;
 
-  virtual void addDefaultArgExprForConstructor(const CXXConstructorDecl *CD,
-                                               unsigned ParmIdx, Expr *DAE) = 0;
-
-  virtual Expr *getDefaultArgExprForConstructor(const CXXConstructorDecl *CD,
-                                                unsigned ParmIdx) = 0;
-
   virtual void addTypedefNameForUnnamedTagDecl(TagDecl *TD,
                                                TypedefNameDecl *DD) = 0;
 

Modified: cfe/trunk/lib/AST/ItaniumCXXABI.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ItaniumCXXABI.cpp?rev=287774&r1=287773&r2=287774&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ItaniumCXXABI.cpp (original)
+++ cfe/trunk/lib/AST/ItaniumCXXABI.cpp Wed Nov 23 10:51:30 2016
@@ -142,14 +142,6 @@ public:
   void addCopyConstructorForExceptionObject(CXXRecordDecl *RD,
                                             CXXConstructorDecl *CD) override {}
 
-  void addDefaultArgExprForConstructor(const CXXConstructorDecl *CD,
-                                       unsigned ParmIdx, Expr *DAE) override {}
-
-  Expr *getDefaultArgExprForConstructor(const CXXConstructorDecl *CD,
-                                        unsigned ParmIdx) override {
-    return nullptr;
-  }
-
   void addTypedefNameForUnnamedTagDecl(TagDecl *TD,
                                        TypedefNameDecl *DD) override {}
 

Modified: cfe/trunk/lib/AST/MicrosoftCXXABI.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/MicrosoftCXXABI.cpp?rev=287774&r1=287773&r2=287774&view=diff
==============================================================================
--- cfe/trunk/lib/AST/MicrosoftCXXABI.cpp (original)
+++ cfe/trunk/lib/AST/MicrosoftCXXABI.cpp Wed Nov 23 10:51:30 2016
@@ -67,8 +67,6 @@ public:
 class MicrosoftCXXABI : public CXXABI {
   ASTContext &Context;
   llvm::SmallDenseMap<CXXRecordDecl *, CXXConstructorDecl *> RecordToCopyCtor;
-  llvm::SmallDenseMap<std::pair<const CXXConstructorDecl *, unsigned>, Expr *>
-      CtorToDefaultArgExpr;
 
   llvm::SmallDenseMap<TagDecl *, DeclaratorDecl *>
       UnnamedTagDeclToDeclaratorDecl;
@@ -92,16 +90,6 @@ public:
     llvm_unreachable("unapplicable to the MS ABI");
   }
 
-  void addDefaultArgExprForConstructor(const CXXConstructorDecl *CD,
-                                       unsigned ParmIdx, Expr *DAE) override {
-    CtorToDefaultArgExpr[std::make_pair(CD, ParmIdx)] = DAE;
-  }
-
-  Expr *getDefaultArgExprForConstructor(const CXXConstructorDecl *CD,
-                                        unsigned ParmIdx) override {
-    return CtorToDefaultArgExpr[std::make_pair(CD, ParmIdx)];
-  }
-
   const CXXConstructorDecl *
   getCopyConstructorForExceptionObject(CXXRecordDecl *RD) override {
     return RecordToCopyCtor[RD];

Modified: cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp?rev=287774&r1=287773&r2=287774&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp (original)
+++ cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp Wed Nov 23 10:51:30 2016
@@ -3869,11 +3869,11 @@ MicrosoftCXXABI::getAddrOfCXXCtorClosure
     Args.add(RValue::get(SrcVal), SrcParam.getType());
 
   // Add the rest of the default arguments.
-  std::vector<Stmt *> ArgVec;
-  for (unsigned I = IsCopy ? 1 : 0, E = CD->getNumParams(); I != E; ++I) {
-    Stmt *DefaultArg = getContext().getDefaultArgExprForConstructor(CD, I);
-    assert(DefaultArg && "sema forgot to instantiate default args");
-    ArgVec.push_back(DefaultArg);
+  SmallVector<const Stmt *, 4> ArgVec;
+  ArrayRef<ParmVarDecl *> params = CD->parameters().drop_front(IsCopy ? 1 : 0);
+  for (const ParmVarDecl *PD : params) {
+    assert(PD->hasDefaultArg() && "ctor closure lacks default args");
+    ArgVec.push_back(PD->getDefaultArg());
   }
 
   CodeGenFunction::RunCleanupsScope Cleanups(CGF);

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=287774&r1=287773&r2=287774&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Wed Nov 23 10:51:30 2016
@@ -10365,7 +10365,7 @@ void Sema::ActOnFinishCXXMemberDecls() {
   }
 }
 
-static void getDefaultArgExprsForConstructors(Sema &S, CXXRecordDecl *Class) {
+static void checkDefaultArgExprsForConstructors(Sema &S, CXXRecordDecl *Class) {
   // Don't do anything for template patterns.
   if (Class->getDescribedClassTemplate())
     return;
@@ -10379,7 +10379,7 @@ static void getDefaultArgExprsForConstru
     if (!CD) {
       // Recurse on nested classes.
       if (auto *NestedRD = dyn_cast<CXXRecordDecl>(Member))
-        getDefaultArgExprsForConstructors(S, NestedRD);
+        checkDefaultArgExprsForConstructors(S, NestedRD);
       continue;
     } else if (!CD->isDefaultConstructor() || !CD->hasAttr<DLLExportAttr>()) {
       continue;
@@ -10404,14 +10404,9 @@ static void getDefaultArgExprsForConstru
     LastExportedDefaultCtor = CD;
 
     for (unsigned I = 0; I != NumParams; ++I) {
-      // Skip any default arguments that we've already instantiated.
-      if (S.Context.getDefaultArgExprForConstructor(CD, I))
-        continue;
-
-      Expr *DefaultArg = S.BuildCXXDefaultArgExpr(Class->getLocation(), CD,
-                                                  CD->getParamDecl(I)).get();
+      (void)S.CheckCXXDefaultArgExpr(Class->getLocation(), CD,
+                                     CD->getParamDecl(I));
       S.DiscardCleanupsInEvaluationContext();
-      S.Context.addDefaultArgExprForConstructor(CD, I, DefaultArg);
     }
   }
 }
@@ -10423,7 +10418,7 @@ void Sema::ActOnFinishCXXNonNestedClass(
   // have default arguments or don't use the standard calling convention are
   // wrapped with a thunk called the default constructor closure.
   if (RD && Context.getTargetInfo().getCXXABI().isMicrosoft())
-    getDefaultArgExprsForConstructors(*this, RD);
+    checkDefaultArgExprsForConstructors(*this, RD);
 
   referenceDLLExportedClassMethods();
 }

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=287774&r1=287773&r2=287774&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Wed Nov 23 10:51:30 2016
@@ -4513,16 +4513,15 @@ Sema::CreateBuiltinArraySubscriptExpr(Ex
       ArraySubscriptExpr(LHSExp, RHSExp, ResultType, VK, OK, RLoc);
 }
 
-ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,
-                                        FunctionDecl *FD,
-                                        ParmVarDecl *Param) {
+bool Sema::CheckCXXDefaultArgExpr(SourceLocation CallLoc, FunctionDecl *FD,
+                                  ParmVarDecl *Param) {
   if (Param->hasUnparsedDefaultArg()) {
     Diag(CallLoc,
          diag::err_use_of_default_argument_to_function_declared_later) <<
       FD << cast<CXXRecordDecl>(FD->getDeclContext())->getDeclName();
     Diag(UnparsedDefaultArgLocs[Param],
          diag::note_default_argument_declared_here);
-    return ExprError();
+    return true;
   }
   
   if (Param->hasUninstantiatedDefaultArg()) {
@@ -4538,11 +4537,11 @@ ExprResult Sema::BuildCXXDefaultArgExpr(
     InstantiatingTemplate Inst(*this, CallLoc, Param,
                                MutiLevelArgList.getInnermost());
     if (Inst.isInvalid())
-      return ExprError();
+      return true;
     if (Inst.isAlreadyInstantiating()) {
       Diag(Param->getLocStart(), diag::err_recursive_default_argument) << FD;
       Param->setInvalidDecl();
-      return ExprError();
+      return true;
     }
 
     ExprResult Result;
@@ -4557,7 +4556,7 @@ ExprResult Sema::BuildCXXDefaultArgExpr(
                                 /*DirectInit*/false);
     }
     if (Result.isInvalid())
-      return ExprError();
+      return true;
 
     // Check the expression as an initializer for the parameter.
     InitializedEntity Entity
@@ -4570,12 +4569,12 @@ ExprResult Sema::BuildCXXDefaultArgExpr(
     InitializationSequence InitSeq(*this, Entity, Kind, ResultE);
     Result = InitSeq.Perform(*this, Entity, Kind, ResultE);
     if (Result.isInvalid())
-      return ExprError();
+      return true;
 
     Result = ActOnFinishFullExpr(Result.getAs<Expr>(),
                                  Param->getOuterLocStart());
     if (Result.isInvalid())
-      return ExprError();
+      return true;
 
     // Remember the instantiated default argument.
     Param->setDefaultArg(Result.getAs<Expr>());
@@ -4588,7 +4587,7 @@ ExprResult Sema::BuildCXXDefaultArgExpr(
   if (!Param->hasInit()) {
     Diag(Param->getLocStart(), diag::err_recursive_default_argument) << FD;
     Param->setInvalidDecl();
-    return ExprError();
+    return true;
   }
 
   // If the default expression creates temporaries, we need to
@@ -4615,9 +4614,15 @@ ExprResult Sema::BuildCXXDefaultArgExpr(
   // as being "referenced".
   MarkDeclarationsReferencedInExpr(Param->getDefaultArg(),
                                    /*SkipLocalVariables=*/true);
-  return CXXDefaultArgExpr::Create(Context, CallLoc, Param);
+  return false;
 }
 
+ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,
+                                        FunctionDecl *FD, ParmVarDecl *Param) {
+  if (CheckCXXDefaultArgExpr(CallLoc, FD, Param))
+    return ExprError();
+  return CXXDefaultArgExpr::Create(Context, CallLoc, Param);
+}
 
 Sema::VariadicCallType
 Sema::getVariadicCallType(FunctionDecl *FDecl, const FunctionProtoType *Proto,

Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=287774&r1=287773&r2=287774&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Wed Nov 23 10:51:30 2016
@@ -863,13 +863,8 @@ bool Sema::CheckCXXThrowOperand(SourceLo
       // We don't keep the instantiated default argument expressions around so
       // we must rebuild them here.
       for (unsigned I = 1, E = CD->getNumParams(); I != E; ++I) {
-        // Skip any default arguments that we've already instantiated.
-        if (Context.getDefaultArgExprForConstructor(CD, I))
-          continue;
-
-        Expr *DefaultArg =
-            BuildCXXDefaultArgExpr(ThrowLoc, CD, CD->getParamDecl(I)).get();
-        Context.addDefaultArgExprForConstructor(CD, I, DefaultArg);
+        if (CheckCXXDefaultArgExpr(ThrowLoc, CD, CD->getParamDecl(I)))
+          return true;
       }
     }
   }

Added: cfe/trunk/test/SemaCXX/default-arg-closures.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/default-arg-closures.cpp?rev=287774&view=auto
==============================================================================
--- cfe/trunk/test/SemaCXX/default-arg-closures.cpp (added)
+++ cfe/trunk/test/SemaCXX/default-arg-closures.cpp Wed Nov 23 10:51:30 2016
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -fexceptions -fcxx-exceptions -fms-extensions -verify %s -std=c++11
+
+// The MS ABI has a few ways to generate constructor closures, which require
+// instantiating and checking the semantics of default arguments. Make sure we
+// do that right.
+
+// FIXME: Don't diagnose this issue twice.
+template <typename T>
+struct DependentDefaultCtorArg { // expected-note {{in instantiation of default function argument}}
+  // expected-error at +1 2 {{type 'int' cannot be used prior to '::' because it has no members}}
+  DependentDefaultCtorArg(int n = T::error);
+};
+struct
+__declspec(dllexport) // expected-note {{due to 'ExportDefaultCtorClosure' being dllexported}}
+ExportDefaultCtorClosure // expected-note {{implicit default constructor for 'ExportDefaultCtorClosure' first required here}}
+: DependentDefaultCtorArg<int> // expected-note {{in instantiation of template class}}
+{};
+
+template <typename T>
+struct DependentDefaultCopyArg {
+  DependentDefaultCopyArg() {}
+  // expected-error at +1 {{type 'int' cannot be used prior to '::' because it has no members}}
+  DependentDefaultCopyArg(const DependentDefaultCopyArg &o, int n = T::member) {}
+};
+
+struct HasMember {
+  enum { member = 0 };
+};
+void UseDependentArg() { throw DependentDefaultCopyArg<HasMember>(); }
+
+void ErrorInDependentArg() {
+  throw DependentDefaultCopyArg<int>(); // expected-note {{required here}}
+}
+
+struct HasCleanup {
+  ~HasCleanup();
+};
+
+struct Default {
+  Default(const Default &o, int d = (HasCleanup(), 42));
+};
+
+void f(const Default &d) {
+  throw d;
+}




More information about the cfe-commits mailing list