r287774 - Remove C++ default arg side table for MS ABI ctor closures
Nico Weber via cfe-commits
cfe-commits at lists.llvm.org
Wed Nov 23 09:06:56 PST 2016
Cool, thanks! Should we also have a test for using a default arg with a
pch? Now that ASTContext doesn't have this table any more, it'll work, but
maybe it's good to have a regression test for the issue in the PR?
On Wed, Nov 23, 2016 at 11:51 AM, Reid Kleckner via cfe-commits <
cfe-commits at lists.llvm.org> wrote:
> 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;
> +}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20161123/fe36e5ce/attachment-0001.html>
More information about the cfe-commits
mailing list