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