From: Reid Kleckner Date: Wed, 23 Nov 2016 16:51:30 +0000 (+0000) Subject: Remove C++ default arg side table for MS ABI ctor closures X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=c480c8c15397e2859863d4a4dbb72994ddf35a33;p=clang 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 64f3870a68..cfd744a475 100644 --- 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 b81b424e8a..397c034773 100644 --- 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 622898dde2..7c9fda6532 100644 --- a/lib/AST/ASTContext.cpp +++ b/lib/AST/ASTContext.cpp @@ -9172,18 +9172,6 @@ void ASTContext::addCopyConstructorForExceptionObject(CXXRecordDecl *RD, cast(CD->getFirstDecl())); } -void ASTContext::addDefaultArgExprForConstructor(const CXXConstructorDecl *CD, - unsigned ParmIdx, Expr *DAE) { - ABI->addDefaultArgExprForConstructor( - cast(CD->getFirstDecl()), ParmIdx, DAE); -} - -Expr *ASTContext::getDefaultArgExprForConstructor(const CXXConstructorDecl *CD, - unsigned ParmIdx) { - return ABI->getDefaultArgExprForConstructor( - cast(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 e1cf934ee0..924ef00e81 100644 --- 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 091d5f031f..692a455eaf 100644 --- 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 bb59fba5ee..73324e40f3 100644 --- a/lib/AST/MicrosoftCXXABI.cpp +++ b/lib/AST/MicrosoftCXXABI.cpp @@ -67,8 +67,6 @@ public: class MicrosoftCXXABI : public CXXABI { ASTContext &Context; llvm::SmallDenseMap RecordToCopyCtor; - llvm::SmallDenseMap, Expr *> - CtorToDefaultArgExpr; llvm::SmallDenseMap 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 60940fa8fd..c87ac40972 100644 --- 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 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 ArgVec; + ArrayRef 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 b036e0c0dc..a702d6aaee 100644 --- 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(Member)) - getDefaultArgExprsForConstructors(S, NestedRD); + checkDefaultArgExprsForConstructors(S, NestedRD); continue; } else if (!CD->isDefaultConstructor() || !CD->hasAttr()) { 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 9d0d55e687..d2bfb37ebb 100644 --- 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(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(), Param->getOuterLocStart()); if (Result.isInvalid()) - return ExprError(); + return true; // Remember the instantiated default argument. Param->setDefaultArg(Result.getAs()); @@ -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 3392d10218..91cdb67f30 100644 --- 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 index 0000000000..e076cc05cd --- /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 +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 // expected-note {{in instantiation of template class}} +{}; + +template +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(); } + +void ErrorInDependentArg() { + throw DependentDefaultCopyArg(); // expected-note {{required here}} +} + +struct HasCleanup { + ~HasCleanup(); +}; + +struct Default { + Default(const Default &o, int d = (HasCleanup(), 42)); +}; + +void f(const Default &d) { + throw d; +}