From: Gauthier Harnisch Date: Fri, 21 Jun 2019 08:26:21 +0000 (+0000) Subject: [clang] Small improvments after Adding APValue to ConstantExpr X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=c6c5c094139f877bf67346c71439b915ec7806c2;p=clang [clang] Small improvments after Adding APValue to ConstantExpr Summary: this patch has multiple small improvements related to the APValue in ConstantExpr. changes: - APValue in ConstantExpr are now cleaned up using ASTContext::addDestruction instead of there own system. - ConstantExprBits Stores the ValueKind of the result beaing stored. - VerifyIntegerConstantExpression now stores the evaluated value in ConstantExpr. - the Constant Evaluator uses the stored value of ConstantExpr when available. Reviewers: rsmith Reviewed By: rsmith Subscribers: cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D63376 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@364011 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/AST/ASTContext.h b/include/clang/AST/ASTContext.h index 1485046684..1d1aaf4fb1 100644 --- a/include/clang/AST/ASTContext.h +++ b/include/clang/AST/ASTContext.h @@ -2751,12 +2751,11 @@ public: /// /// \param Data Pointer data that will be provided to the callback function /// when it is called. - void AddDeallocation(void (*Callback)(void*), void *Data); + void AddDeallocation(void (*Callback)(void *), void *Data) const; /// If T isn't trivially destructible, calls AddDeallocation to register it /// for destruction. - template - void addDestruction(T *Ptr) { + template void addDestruction(T *Ptr) const { if (!std::is_trivially_destructible::value) { auto DestroyPtr = [](void *V) { static_cast(V)->~T(); }; AddDeallocation(DestroyPtr, Ptr); @@ -2819,10 +2818,6 @@ public: APValue *getMaterializedTemporaryValue(const MaterializeTemporaryExpr *E, bool MayCreate); - /// Adds an APValue that will be destructed during the destruction of the - /// ASTContext. - void AddAPValueCleanup(APValue *Ptr) const { APValueCleanups.push_back(Ptr); } - /// Return a string representing the human readable name for the specified /// function declaration or file name. Used by SourceLocExpr and /// PredefinedExpr to cache evaluated results. @@ -2989,7 +2984,7 @@ private: // in order to track and run destructors while we're tearing things down. using DeallocationFunctionsAndArguments = llvm::SmallVector, 16>; - DeallocationFunctionsAndArguments Deallocations; + mutable DeallocationFunctionsAndArguments Deallocations; // FIXME: This currently contains the set of StoredDeclMaps used // by DeclContext objects. This probably should not be in ASTContext, diff --git a/include/clang/AST/Expr.h b/include/clang/AST/Expr.h index 508817f8ad..d44a815c86 100644 --- a/include/clang/AST/Expr.h +++ b/include/clang/AST/Expr.h @@ -998,6 +998,9 @@ public: EmptyShell Empty); static ResultStorageKind getStorageKind(const APValue &Value); + static ResultStorageKind getStorageKind(const Type *T, + const ASTContext &Context); + SourceLocation getBeginLoc() const LLVM_READONLY { return SubExpr->getBeginLoc(); } @@ -1009,25 +1012,20 @@ public: return T->getStmtClass() == ConstantExprClass; } - void SetResult(APValue Value) { MoveIntoResult(Value); } - void MoveIntoResult(APValue &Value); + void SetResult(APValue Value, const ASTContext &Context) { + MoveIntoResult(Value, Context); + } + void MoveIntoResult(APValue &Value, const ASTContext &Context); APValue::ValueKind getResultAPValueKind() const { - switch (ConstantExprBits.ResultKind) { - case ConstantExpr::RSK_APValue: - return APValueResult().getKind(); - case ConstantExpr::RSK_Int64: - return APValue::Int; - case ConstantExpr::RSK_None: - return APValue::None; - } - llvm_unreachable("invalid ResultKind"); + return static_cast(ConstantExprBits.APValueKind); } ResultStorageKind getResultStorageKind() const { return static_cast(ConstantExprBits.ResultKind); } APValue getAPValueResult() const; - + const APValue &getResultAsAPValue() const { return APValueResult(); } + llvm::APSInt getResultAsAPSInt() const; // Iterators child_range children() { return child_range(&SubExpr, &SubExpr+1); } const_child_range children() const { diff --git a/include/clang/AST/Stmt.h b/include/clang/AST/Stmt.h index 71a7d966a7..88aa535af2 100644 --- a/include/clang/AST/Stmt.h +++ b/include/clang/AST/Stmt.h @@ -332,6 +332,9 @@ protected: /// The kind of result that is trail-allocated. unsigned ResultKind : 2; + /// Kind of Result as defined by APValue::Kind + unsigned APValueKind : 4; + /// When ResultKind == RSK_Int64. whether the trail-allocated integer is /// signed. unsigned IsUnsigned : 1; @@ -340,6 +343,10 @@ protected: /// integer. 7 bits because it is the minimal number of bit to represent a /// value from 0 to 64 (the size of the trail-allocated number). unsigned BitWidth : 7; + + /// When ResultKind == RSK_APValue. Wether the ASTContext will cleanup the + /// destructor on the trail-allocated APValue. + unsigned HasCleanup : 1; }; class PredefinedExprBitfields { diff --git a/lib/AST/ASTContext.cpp b/lib/AST/ASTContext.cpp index 7f73531a83..d7778a7196 100644 --- a/lib/AST/ASTContext.cpp +++ b/lib/AST/ASTContext.cpp @@ -908,7 +908,7 @@ void ASTContext::setTraversalScope(const std::vector &TopLevelDecls) { Parents.reset(); } -void ASTContext::AddDeallocation(void (*Callback)(void*), void *Data) { +void ASTContext::AddDeallocation(void (*Callback)(void *), void *Data) const { Deallocations.push_back({Callback, Data}); } diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp index cc92689cb9..d342bda962 100644 --- a/lib/AST/Expr.cpp +++ b/lib/AST/Expr.cpp @@ -239,6 +239,7 @@ ConstantExpr::ResultStorageKind ConstantExpr::getStorageKind(const APValue &Value) { switch (Value.getKind()) { case APValue::None: + case APValue::Indeterminate: return ConstantExpr::RSK_None; case APValue::Int: if (!Value.getInt().needsCleanup()) @@ -249,9 +250,18 @@ ConstantExpr::getStorageKind(const APValue &Value) { } } +ConstantExpr::ResultStorageKind +ConstantExpr::getStorageKind(const Type *T, const ASTContext &Context) { + if (T->isIntegralOrEnumerationType() && Context.getTypeInfo(T).Width <= 64) + return ConstantExpr::RSK_Int64; + return ConstantExpr::RSK_APValue; +} + void ConstantExpr::DefaultInit(ResultStorageKind StorageKind) { ConstantExprBits.ResultKind = StorageKind; - if (StorageKind == RSK_APValue) + ConstantExprBits.APValueKind = APValue::None; + ConstantExprBits.HasCleanup = false; + if (StorageKind == ConstantExpr::RSK_APValue) ::new (getTrailingObjects()) APValue(); } @@ -269,8 +279,6 @@ ConstantExpr *ConstantExpr::Create(const ASTContext &Context, Expr *E, StorageKind == ConstantExpr::RSK_Int64); void *Mem = Context.Allocate(Size, alignof(ConstantExpr)); ConstantExpr *Self = new (Mem) ConstantExpr(E, StorageKind); - if (StorageKind == ConstantExpr::RSK_APValue) - Context.AddAPValueCleanup(&Self->APValueResult()); return Self; } @@ -278,7 +286,7 @@ ConstantExpr *ConstantExpr::Create(const ASTContext &Context, Expr *E, const APValue &Result) { ResultStorageKind StorageKind = getStorageKind(Result); ConstantExpr *Self = Create(Context, E, StorageKind); - Self->SetResult(Result); + Self->SetResult(Result, Context); return Self; } @@ -296,14 +304,13 @@ ConstantExpr *ConstantExpr::CreateEmpty(const ASTContext &Context, StorageKind == ConstantExpr::RSK_Int64); void *Mem = Context.Allocate(Size, alignof(ConstantExpr)); ConstantExpr *Self = new (Mem) ConstantExpr(StorageKind, Empty); - if (StorageKind == ConstantExpr::RSK_APValue) - Context.AddAPValueCleanup(&Self->APValueResult()); return Self; } -void ConstantExpr::MoveIntoResult(APValue &Value) { +void ConstantExpr::MoveIntoResult(APValue &Value, const ASTContext &Context) { assert(getStorageKind(Value) == ConstantExprBits.ResultKind && "Invalid storage for this value kind"); + ConstantExprBits.APValueKind = Value.getKind(); switch (ConstantExprBits.ResultKind) { case RSK_None: return; @@ -313,12 +320,28 @@ void ConstantExpr::MoveIntoResult(APValue &Value) { ConstantExprBits.IsUnsigned = Value.getInt().isUnsigned(); return; case RSK_APValue: + if (!ConstantExprBits.HasCleanup && Value.needsCleanup()) { + ConstantExprBits.HasCleanup = true; + Context.addDestruction(&APValueResult()); + } APValueResult() = std::move(Value); return; } llvm_unreachable("Invalid ResultKind Bits"); } +llvm::APSInt ConstantExpr::getResultAsAPSInt() const { + switch (ConstantExprBits.ResultKind) { + case ConstantExpr::RSK_APValue: + return APValueResult().getInt(); + case ConstantExpr::RSK_Int64: + return llvm::APSInt(llvm::APInt(ConstantExprBits.BitWidth, Int64Result()), + ConstantExprBits.IsUnsigned); + default: + llvm_unreachable("invalid Accessor"); + } +} + APValue ConstantExpr::getAPValueResult() const { switch (ConstantExprBits.ResultKind) { case ConstantExpr::RSK_APValue: diff --git a/lib/AST/ExprConstant.cpp b/lib/AST/ExprConstant.cpp index c0c807b11c..3282bc0a0a 100644 --- a/lib/AST/ExprConstant.cpp +++ b/lib/AST/ExprConstant.cpp @@ -8963,6 +8963,8 @@ static bool tryEvaluateBuiltinObjectSize(const Expr *E, unsigned Type, bool IntExprEvaluator::VisitConstantExpr(const ConstantExpr *E) { llvm::SaveAndRestore InConstantContext(Info.InConstantContext, true); + if (E->getResultAPValueKind() != APValue::None) + return Success(E->getAPValueResult(), E); return ExprEvaluatorBaseTy::VisitConstantExpr(E); } diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index 6dda693c6a..0fda8ea515 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/lib/Sema/SemaDeclCXX.cpp @@ -14013,8 +14013,6 @@ Decl *Sema::BuildStaticAssertDeclaration(SourceLocation StaticAssertLoc, ExprResult Converted = PerformContextuallyConvertToBool(AssertExpr); if (Converted.isInvalid()) Failed = true; - else - Converted = ConstantExpr::Create(Context, Converted.get()); llvm::APSInt Cond; if (!Failed && VerifyIntegerConstantExpression(Converted.get(), &Cond, diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index 9b1e362c26..9b09059c15 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -14538,14 +14538,13 @@ Sema::VerifyIntegerConstantExpression(Expr *E, llvm::APSInt *Result, return ExprError(); } - if (!isa(E)) - E = ConstantExpr::Create(Context, E); - // Circumvent ICE checking in C++11 to avoid evaluating the expression twice // in the non-ICE case. if (!getLangOpts().CPlusPlus11 && E->isIntegerConstantExpr(Context)) { if (Result) *Result = E->EvaluateKnownConstIntCheckOverflow(Context); + if (!isa(E)) + E = ConstantExpr::Create(Context, E); return E; } @@ -14555,8 +14554,12 @@ Sema::VerifyIntegerConstantExpression(Expr *E, llvm::APSInt *Result, // Try to evaluate the expression, and produce diagnostics explaining why it's // not a constant expression as a side-effect. - bool Folded = E->EvaluateAsRValue(EvalResult, Context) && - EvalResult.Val.isInt() && !EvalResult.HasSideEffects; + bool Folded = + E->EvaluateAsRValue(EvalResult, Context, /*isConstantContext*/ true) && + EvalResult.Val.isInt() && !EvalResult.HasSideEffects; + + if (!isa(E)) + E = ConstantExpr::Create(Context, E, EvalResult.Val); // In C++11, we can rely on diagnostics being produced for any expression // which is not a constant expression. If no diagnostics were produced, then