Timm =?utf-8?q?Bäder?= <[email protected]>, Timm =?utf-8?q?Bäder?= <[email protected]> Message-ID: In-Reply-To: <llvm.org/llvm/llvm-project/pull/[email protected]>
https://github.com/tbaederr updated https://github.com/llvm/llvm-project/pull/179230 >From 11dd99c10743f55e86b6d7cd0c3a5ed63003ccfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= <[email protected]> Date: Mon, 2 Feb 2026 14:29:05 +0100 Subject: [PATCH 1/3] [clang] Return std::optional from all Expr::tryEvaluate* API tryEvaluateString was returning an std::optional, but the other try* API was not. Update tryEvaluateObjectSize and tryEvaluateStrLen to return an std::optional<uint64_t>. --- clang/include/clang/AST/Expr.h | 6 ++--- clang/lib/AST/ExprConstant.cpp | 35 +++++++++++++++++-------- clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp | 9 ++++--- clang/lib/CodeGen/CGBuiltin.cpp | 9 ++++--- clang/lib/Sema/SemaChecking.cpp | 24 +++++++++-------- 5 files changed, 50 insertions(+), 33 deletions(-) diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h index 697dc73c46c2d..873ddb38e832a 100644 --- a/clang/include/clang/AST/Expr.h +++ b/clang/include/clang/AST/Expr.h @@ -777,14 +777,14 @@ class Expr : public ValueStmt { /// /// \param Type - How to evaluate the size of the Expr, as defined by the /// "type" parameter of __builtin_object_size - bool tryEvaluateObjectSize(uint64_t &Result, ASTContext &Ctx, - unsigned Type) const; + std::optional<uint64_t> tryEvaluateObjectSize(const ASTContext &Ctx, + unsigned Type) const; /// If the current Expr is a pointer, this will try to statically /// determine the strlen of the string pointed to. /// Returns true if all of the above holds and we were able to figure out the /// strlen, false otherwise. - bool tryEvaluateStrLen(uint64_t &Result, ASTContext &Ctx) const; + std::optional<uint64_t> tryEvaluateStrLen(const ASTContext &Ctx) const; bool EvaluateCharRangeAsString(std::string &Result, const Expr *SizeExpression, diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index d4068368f5b9d..dfcd40626c128 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -21661,18 +21661,25 @@ bool Expr::isPotentialConstantExprUnevaluated(Expr *E, return Diags.empty(); } -bool Expr::tryEvaluateObjectSize(uint64_t &Result, ASTContext &Ctx, - unsigned Type) const { +std::optional<uint64_t> Expr::tryEvaluateObjectSize(const ASTContext &Ctx, + unsigned Type) const { if (!getType()->isPointerType()) return false; Expr::EvalStatus Status; EvalInfo Info(Ctx, Status, EvaluationMode::ConstantFold); - if (Info.EnableNewConstInterp) { - return Info.Ctx.getInterpContext().tryEvaluateObjectSize(Info, this, Type, - Result); - } - return tryEvaluateBuiltinObjectSize(this, Type, Info, Result); + uint64_t Result = ~0u; + bool Success; + if (Info.EnableNewConstInterp) + Success = Info.Ctx.getInterpContext().tryEvaluateObjectSize(Info, this, + Type, Result); + else + Success = tryEvaluateBuiltinObjectSize(this, Type, Info, Result); + + if (!Success) + return std::nullopt; + + return Result; } static bool EvaluateBuiltinStrLen(const Expr *E, uint64_t &Result, @@ -21816,14 +21823,20 @@ bool Expr::EvaluateCharRangeAsString(APValue &Result, PtrExpression, Ctx, Status); } -bool Expr::tryEvaluateStrLen(uint64_t &Result, ASTContext &Ctx) const { +std::optional<uint64_t> Expr::tryEvaluateStrLen(const ASTContext &Ctx) const { Expr::EvalStatus Status; EvalInfo Info(Ctx, Status, EvaluationMode::ConstantFold); - if (Info.EnableNewConstInterp) - return Info.Ctx.getInterpContext().evaluateStrlen(Info, this, Result); + uint64_t Result = ~0u; + if (Info.EnableNewConstInterp) { + if (!Info.Ctx.getInterpContext().evaluateStrlen(Info, this, Result)) + return std::nullopt; + } else { + if (!EvaluateBuiltinStrLen(this, Result, Info)) + return std::nullopt; + } - return EvaluateBuiltinStrLen(this, Result, Info); + return Result; } namespace { diff --git a/clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp b/clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp index ca2b8a5b192a6..5e6c9e8e2490e 100644 --- a/clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp @@ -2037,8 +2037,9 @@ mlir::Value CIRGenFunction::emitBuiltinObjectSize(const Expr *e, unsigned type, mlir::Value CIRGenFunction::evaluateOrEmitBuiltinObjectSize( const Expr *e, unsigned type, cir::IntType resType, mlir::Value emittedE, bool isDynamic) { - uint64_t objectSize; - if (!e->tryEvaluateObjectSize(objectSize, getContext(), type)) - return emitBuiltinObjectSize(e, type, resType, emittedE, isDynamic); - return builder.getConstInt(getLoc(e->getSourceRange()), resType, objectSize); + if (std::optional<uint64_t> objectSize = + e->tryEvaluateObjectSize(getContext(), type)) + return builder.getConstInt(getLoc(e->getSourceRange()), resType, + *objectSize); + return emitBuiltinObjectSize(e, type, resType, emittedE, isDynamic); } diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp index cf686581240a5..8820fa81e2ae5 100644 --- a/clang/lib/CodeGen/CGBuiltin.cpp +++ b/clang/lib/CodeGen/CGBuiltin.cpp @@ -926,10 +926,11 @@ CodeGenFunction::evaluateOrEmitBuiltinObjectSize(const Expr *E, unsigned Type, llvm::IntegerType *ResType, llvm::Value *EmittedE, bool IsDynamic) { - uint64_t ObjectSize; - if (!E->tryEvaluateObjectSize(ObjectSize, getContext(), Type)) - return emitBuiltinObjectSize(E, Type, ResType, EmittedE, IsDynamic); - return ConstantInt::get(ResType, ObjectSize, /*isSigned=*/true); + + if (std::optional<uint64_t> ObjectSize = + E->tryEvaluateObjectSize(getContext(), Type)) + return ConstantInt::get(ResType, *ObjectSize, /*isSigned=*/true); + return emitBuiltinObjectSize(E, Type, ResType, EmittedE, IsDynamic); } namespace { diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index d1c46a875208a..1057bafa1f93b 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -1217,12 +1217,12 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, return std::nullopt; const Expr *ObjArg = TheCall->getArg(NewIndex); - uint64_t Result; - if (!ObjArg->tryEvaluateObjectSize(Result, getASTContext(), BOSType)) - return std::nullopt; - - // Get the object size in the target's size_t width. - return llvm::APSInt::getUnsigned(Result).extOrTrunc(SizeTypeWidth); + if (std::optional<uint64_t> ObjSize = + ObjArg->tryEvaluateObjectSize(getASTContext(), BOSType)) { + // Get the object size in the target's size_t width. + return llvm::APSInt::getUnsigned(*ObjSize).extOrTrunc(SizeTypeWidth); + } + return std::nullopt; }; auto ComputeStrLenArgument = @@ -1233,11 +1233,13 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD, unsigned NewIndex = *IndexOptional; const Expr *ObjArg = TheCall->getArg(NewIndex); - uint64_t Result; - if (!ObjArg->tryEvaluateStrLen(Result, getASTContext())) - return std::nullopt; - // Add 1 for null byte. - return llvm::APSInt::getUnsigned(Result + 1).extOrTrunc(SizeTypeWidth); + + if (std::optional<uint64_t> Result = + ObjArg->tryEvaluateStrLen(getASTContext())) { + // Add 1 for null byte. + return llvm::APSInt::getUnsigned(*Result + 1).extOrTrunc(SizeTypeWidth); + } + return std::nullopt; }; std::optional<llvm::APSInt> SourceSize; >From 64d220d79a8cdcfaa7ca36470bcdb87fce3af907 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= <[email protected]> Date: Tue, 3 Feb 2026 05:32:38 +0100 Subject: [PATCH 2/3] Review comments --- clang/lib/AST/ExprConstant.cpp | 2 +- clang/lib/CodeGen/CGBuiltin.cpp | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index dfcd40626c128..6be0ab88946a8 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -21664,7 +21664,7 @@ bool Expr::isPotentialConstantExprUnevaluated(Expr *E, std::optional<uint64_t> Expr::tryEvaluateObjectSize(const ASTContext &Ctx, unsigned Type) const { if (!getType()->isPointerType()) - return false; + return std::nullopt; Expr::EvalStatus Status; EvalInfo Info(Ctx, Status, EvaluationMode::ConstantFold); diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp index 8820fa81e2ae5..3db880f63a4fe 100644 --- a/clang/lib/CodeGen/CGBuiltin.cpp +++ b/clang/lib/CodeGen/CGBuiltin.cpp @@ -926,7 +926,6 @@ CodeGenFunction::evaluateOrEmitBuiltinObjectSize(const Expr *E, unsigned Type, llvm::IntegerType *ResType, llvm::Value *EmittedE, bool IsDynamic) { - if (std::optional<uint64_t> ObjectSize = E->tryEvaluateObjectSize(getContext(), Type)) return ConstantInt::get(ResType, *ObjectSize, /*isSigned=*/true); >From 41cb487fe1a944bac33bd5a06afb5b050fdc310d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= <[email protected]> Date: Mon, 9 Feb 2026 07:09:35 +0100 Subject: [PATCH 3/3] more std::optional --- clang/lib/AST/ByteCode/Context.cpp | 19 +++--- clang/lib/AST/ByteCode/Context.h | 6 +- clang/lib/AST/ExprConstant.cpp | 92 ++++++++++++------------------ 3 files changed, 49 insertions(+), 68 deletions(-) diff --git a/clang/lib/AST/ByteCode/Context.cpp b/clang/lib/AST/ByteCode/Context.cpp index 3dc36ce1a5204..b4b8939f3fe00 100644 --- a/clang/lib/AST/ByteCode/Context.cpp +++ b/clang/lib/AST/ByteCode/Context.cpp @@ -285,10 +285,11 @@ bool Context::evaluateString(State &Parent, const Expr *E, return true; } -bool Context::evaluateStrlen(State &Parent, const Expr *E, uint64_t &Result) { +std::optional<uint64_t> Context::evaluateStrlen(State &Parent, const Expr *E) { assert(Stk.empty()); Compiler<EvalEmitter> C(*this, *P, Parent, Stk); + std::optional<uint64_t> Result; auto PtrRes = C.interpretAsPointer(E, [&](const Pointer &Ptr) { const Descriptor *FieldDesc = Ptr.getFieldDesc(); if (!FieldDesc->isPrimitiveArray()) @@ -312,7 +313,7 @@ bool Context::evaluateStrlen(State &Parent, const Expr *E, uint64_t &Result) { auto Elem = Ptr.elem<T>(I); if (Elem.isZero()) return true; - ++Result; + ++(*Result); }); } // We didn't find a 0 byte. @@ -322,16 +323,18 @@ bool Context::evaluateStrlen(State &Parent, const Expr *E, uint64_t &Result) { if (PtrRes.isInvalid()) { C.cleanup(); Stk.clear(); - return false; + return std::nullopt; } - return true; + return Result; } -bool Context::tryEvaluateObjectSize(State &Parent, const Expr *E, unsigned Kind, - uint64_t &Result) { +std::optional<uint64_t> +Context::tryEvaluateObjectSize(State &Parent, const Expr *E, unsigned Kind) { assert(Stk.empty()); Compiler<EvalEmitter> C(*this, *P, Parent, Stk); + std::optional<uint64_t> Result; + auto PtrRes = C.interpretAsPointer(E, [&](const Pointer &Ptr) { const Descriptor *DeclDesc = Ptr.getDeclDesc(); if (!DeclDesc) @@ -353,9 +356,9 @@ bool Context::tryEvaluateObjectSize(State &Parent, const Expr *E, unsigned Kind, if (PtrRes.isInvalid()) { C.cleanup(); Stk.clear(); - return false; + return std::nullopt; } - return true; + return Result; } const LangOptions &Context::getLangOpts() const { return Ctx.getLangOpts(); } diff --git a/clang/lib/AST/ByteCode/Context.h b/clang/lib/AST/ByteCode/Context.h index 313c040f84743..53afafdb49c0d 100644 --- a/clang/lib/AST/ByteCode/Context.h +++ b/clang/lib/AST/ByteCode/Context.h @@ -73,7 +73,7 @@ class Context final { /// Evalute \param E and if it can be evaluated to a string literal, /// run strlen() on it. - bool evaluateStrlen(State &Parent, const Expr *E, uint64_t &Result); + std::optional<uint64_t> evaluateStrlen(State &Parent, const Expr *E); /// If \param E evaluates to a pointer the number of accessible bytes /// past the pointer is estimated in \param Result as if evaluated by @@ -85,8 +85,8 @@ class Context final { /// as the one referred to by E are considered, when Kind & 1 == 0 /// bytes belonging to the same storage (stack, heap allocation, /// global variable) are considered. - bool tryEvaluateObjectSize(State &Parent, const Expr *E, unsigned Kind, - uint64_t &Result); + std::optional<uint64_t> tryEvaluateObjectSize(State &Parent, const Expr *E, + unsigned Kind); /// Returns the AST context. ASTContext &getASTContext() const { return Ctx; } diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index 6be0ab88946a8..44629b8bae194 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -1775,9 +1775,9 @@ static bool EvaluateComplex(const Expr *E, ComplexValue &Res, EvalInfo &Info); static bool EvaluateAtomic(const Expr *E, const LValue *This, APValue &Result, EvalInfo &Info); static bool EvaluateAsRValue(EvalInfo &Info, const Expr *E, APValue &Result); -static bool EvaluateBuiltinStrLen(const Expr *E, uint64_t &Result, - EvalInfo &Info, - std::string *StringResult = nullptr); +static std::optional<uint64_t> +EvaluateBuiltinStrLen(const Expr *E, EvalInfo &Info, + std::string *StringResult = nullptr); /// Evaluate an integer or fixed point expression into an APResult. static bool EvaluateFixedPointOrInteger(const Expr *E, APFixedPoint &Result, @@ -15781,8 +15781,8 @@ static bool determineEndOffset(EvalInfo &Info, SourceLocation ExprLoc, /// /// If @p WasError is non-null, this will report whether the failure to evaluate /// is to be treated as an Error in IntExprEvaluator. -static bool tryEvaluateBuiltinObjectSize(const Expr *E, unsigned Type, - EvalInfo &Info, uint64_t &Size) { +static std::optional<uint64_t> +tryEvaluateBuiltinObjectSize(const Expr *E, unsigned Type, EvalInfo &Info) { // Determine the denoted object. LValue LVal; { @@ -15797,31 +15797,27 @@ static bool tryEvaluateBuiltinObjectSize(const Expr *E, unsigned Type, // Expr::tryEvaluateObjectSize. APValue RVal; if (!EvaluateAsRValue(Info, E, RVal)) - return false; + return std::nullopt; LVal.setFrom(Info.Ctx, RVal); } else if (!EvaluatePointer(ignorePointerCastsAndParens(E), LVal, Info, /*InvalidBaseOK=*/true)) - return false; + return std::nullopt; } // If we point to before the start of the object, there are no accessible // bytes. - if (LVal.getLValueOffset().isNegative()) { - Size = 0; - return true; - } + if (LVal.getLValueOffset().isNegative()) + return 0; CharUnits EndOffset; if (!determineEndOffset(Info, E->getExprLoc(), Type, LVal, EndOffset)) - return false; + return std::nullopt; // If we've fallen outside of the end offset, just pretend there's nothing to // write to/read from. if (EndOffset <= LVal.getLValueOffset()) - Size = 0; - else - Size = (EndOffset - LVal.getLValueOffset()).getQuantity(); - return true; + return 0; + return (EndOffset - LVal.getLValueOffset()).getQuantity(); } bool IntExprEvaluator::VisitCallExpr(const CallExpr *E) { @@ -15952,9 +15948,9 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E, E->getArg(1)->EvaluateKnownConstInt(Info.Ctx).getZExtValue(); assert(Type <= 3 && "unexpected type"); - uint64_t Size; - if (tryEvaluateBuiltinObjectSize(E->getArg(0), Type, Info, Size)) - return Success(Size, E); + if (std::optional<uint64_t> Size = + tryEvaluateBuiltinObjectSize(E->getArg(0), Type, Info)) + return Success(*Size, E); if (E->getArg(0)->HasSideEffects(Info.Ctx)) return Success((Type & 2) ? 0 : -1, E); @@ -16552,9 +16548,9 @@ bool IntExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E, case Builtin::BI__builtin_wcslen: { // As an extension, we support __builtin_strlen() as a constant expression, // and support folding strlen() to a constant. - uint64_t StrLen; - if (EvaluateBuiltinStrLen(E->getArg(0), StrLen, Info)) - return Success(StrLen, E); + if (std::optional<uint64_t> StrLen = + EvaluateBuiltinStrLen(E->getArg(0), Info)) + return Success(*StrLen, E); return false; } @@ -21668,29 +21664,21 @@ std::optional<uint64_t> Expr::tryEvaluateObjectSize(const ASTContext &Ctx, Expr::EvalStatus Status; EvalInfo Info(Ctx, Status, EvaluationMode::ConstantFold); - uint64_t Result = ~0u; - bool Success; if (Info.EnableNewConstInterp) - Success = Info.Ctx.getInterpContext().tryEvaluateObjectSize(Info, this, - Type, Result); - else - Success = tryEvaluateBuiltinObjectSize(this, Type, Info, Result); - - if (!Success) - return std::nullopt; - - return Result; + return Info.Ctx.getInterpContext().tryEvaluateObjectSize(Info, this, Type); + return tryEvaluateBuiltinObjectSize(this, Type, Info); } -static bool EvaluateBuiltinStrLen(const Expr *E, uint64_t &Result, - EvalInfo &Info, std::string *StringResult) { +static std::optional<uint64_t> +EvaluateBuiltinStrLen(const Expr *E, EvalInfo &Info, + std::string *StringResult) { if (!E->getType()->hasPointerRepresentation() || !E->isPRValue()) - return false; + return std::nullopt; LValue String; if (!EvaluatePointer(E, String, Info)) - return false; + return std::nullopt; QualType CharTy = E->getType()->getPointeeType(); @@ -21709,10 +21697,9 @@ static bool EvaluateBuiltinStrLen(const Expr *E, uint64_t &Result, if (Pos != StringRef::npos) Str = Str.substr(0, Pos); - Result = Str.size(); if (StringResult) *StringResult = Str; - return true; + return Str.size(); } // Fall through to slow path. @@ -21723,21 +21710,19 @@ static bool EvaluateBuiltinStrLen(const Expr *E, uint64_t &Result, APValue Char; if (!handleLValueToRValueConversion(Info, E, CharTy, String, Char) || !Char.isInt()) - return false; - if (!Char.getInt()) { - Result = Strlen; - return true; - } else if (StringResult) + return std::nullopt; + if (!Char.getInt()) + return Strlen; + else if (StringResult) StringResult->push_back(Char.getInt().getExtValue()); if (!HandleLValueArrayAdjustment(Info, E, String, CharTy, 1)) - return false; + return std::nullopt; } } std::optional<std::string> Expr::tryEvaluateString(ASTContext &Ctx) const { Expr::EvalStatus Status; EvalInfo Info(Ctx, Status, EvaluationMode::ConstantFold); - uint64_t Result; std::string StringResult; if (Info.EnableNewConstInterp) { @@ -21746,7 +21731,7 @@ std::optional<std::string> Expr::tryEvaluateString(ASTContext &Ctx) const { return StringResult; } - if (EvaluateBuiltinStrLen(this, Result, Info, &StringResult)) + if (EvaluateBuiltinStrLen(this, Info, &StringResult)) return StringResult; return std::nullopt; } @@ -21827,16 +21812,9 @@ std::optional<uint64_t> Expr::tryEvaluateStrLen(const ASTContext &Ctx) const { Expr::EvalStatus Status; EvalInfo Info(Ctx, Status, EvaluationMode::ConstantFold); - uint64_t Result = ~0u; - if (Info.EnableNewConstInterp) { - if (!Info.Ctx.getInterpContext().evaluateStrlen(Info, this, Result)) - return std::nullopt; - } else { - if (!EvaluateBuiltinStrLen(this, Result, Info)) - return std::nullopt; - } - - return Result; + if (Info.EnableNewConstInterp) + return Info.Ctx.getInterpContext().evaluateStrlen(Info, this); + return EvaluateBuiltinStrLen(this, Info); } namespace { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
