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

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@287774 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/AST/ASTContext.h b/include/clang/AST/ASTContext.h
index 64f3870a68c25189fa2e0c11b9069db02f71e243..cfd744a475a498d02f342825f24b2cb555ba24c8 100644 (file)
--- a/include/clang/AST/ASTContext.h
+++ b/include/clang/AST/ASTContext.h
@@ -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);

diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index b81b424e8a6dbb4887794458b43997df59f07450..397c0347739a0980e9061766c5927de2fbbe479c 100644 (file)
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -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,

diff --git a/lib/AST/ASTContext.cpp b/lib/AST/ASTContext.cpp
index 622898dde27a630d2eb4661bd314fe9224ce88b6..7c9fda65327f7724be82581211262a92bd42953e 100644 (file)
--- a/lib/AST/ASTContext.cpp
+++ b/lib/AST/ASTContext.cpp
@@ -9172,18 +9172,6 @@ void ASTContext::addCopyConstructorForExceptionObject(CXXRecordDecl *RD,
       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);

diff --git a/lib/AST/CXXABI.h b/lib/AST/CXXABI.h
index e1cf934ee0b3a2868be5d4d0b9be6d39aa4d0400..924ef00e8147aa156e039c5b96df0a0dcf7555c8 100644 (file)
--- a/lib/AST/CXXABI.h
+++ b/lib/AST/CXXABI.h
@@ -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;
 

diff --git a/lib/AST/ItaniumCXXABI.cpp b/lib/AST/ItaniumCXXABI.cpp
index 091d5f031f8d0cc1351489cc157cfddc4e210aac..692a455eafc0832992f92287649f77dc6b99ed18 100644 (file)
--- a/lib/AST/ItaniumCXXABI.cpp
+++ b/lib/AST/ItaniumCXXABI.cpp
@@ -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 {}
 

diff --git a/lib/AST/MicrosoftCXXABI.cpp b/lib/AST/MicrosoftCXXABI.cpp
index bb59fba5eec2a6f08b4fcfccfad4f2c8ee072685..73324e40f3b1076f81396b8fe5d261da24fe64d0 100644 (file)
--- a/lib/AST/MicrosoftCXXABI.cpp
+++ b/lib/AST/MicrosoftCXXABI.cpp
@@ -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];

diff --git a/lib/CodeGen/MicrosoftCXXABI.cpp b/lib/CodeGen/MicrosoftCXXABI.cpp
index 60940fa8fdeb4271efa0106f4e2ce82981f8a425..c87ac40972cf42df181eb7b909a7fc7309fe2680 100644 (file)
--- a/lib/CodeGen/MicrosoftCXXABI.cpp
+++ b/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -3869,11 +3869,11 @@ MicrosoftCXXABI::getAddrOfCXXCtorClosure(const CXXConstructorDecl *CD,
     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);

diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index b036e0c0dc2b8051a04eaf1fb4d078a06d208019..a702d6aaeec667ef689150d2803161e29ef00c2f 100644 (file)
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -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 getDefaultArgExprsForConstructors(Sema &S, CXXRecordDecl *Class) {
     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 getDefaultArgExprsForConstructors(Sema &S, CXXRecordDecl *Class) {
     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(Decl *D) {
   // 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();
 }

diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 9d0d55e68768a601770a2ca0d9fe308af2c29dc3..d2bfb37ebb519802199210389735ed3622123963 100644 (file)
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -4513,16 +4513,15 @@ Sema::CreateBuiltinArraySubscriptExpr(Expr *Base, SourceLocation LLoc,
       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(SourceLocation CallLoc,
     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(SourceLocation CallLoc,
                                 /*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(SourceLocation CallLoc,
     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(SourceLocation CallLoc,
   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(SourceLocation CallLoc,
   // 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,

diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp
index 3392d1021844c16ebe7dbb8b7e84bec413900059..91cdb67f30379a369d1b618539691b221570d805 100644 (file)
--- a/lib/Sema/SemaExprCXX.cpp
+++ b/lib/Sema/SemaExprCXX.cpp
@@ -863,13 +863,8 @@ bool Sema::CheckCXXThrowOperand(SourceLocation ThrowLoc,
       // 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;
       }
     }
   }

diff --git a/test/SemaCXX/default-arg-closures.cpp b/test/SemaCXX/default-arg-closures.cpp
new file mode 100644 (file)
index 0000000..e076cc0
--- /dev/null
+++ b/test/SemaCXX/default-arg-closures.cpp
@@ -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@+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@+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;
+}