Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Clang] Handle consteval expression in array bounds expressions #66222

Merged
merged 8 commits into from
Oct 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,9 @@ Bug Fixes to C++ Support
- Fix crash caused by a spaceship operator returning a comparision category by
reference. Fixes:
(`#64162 <https://github.com/llvm/llvm-project/issues/64162>`_)
- Fix a crash when calling a consteval function in an expression used as
the size of an array.
(`#65520 <https://github.com/llvm/llvm-project/issues/65520>`_)

- Clang no longer tries to capture non-odr-used variables that appear
in the enclosing expression of a lambda expression with a noexcept specifier.
Expand Down
1 change: 1 addition & 0 deletions clang/include/clang/Parse/Parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -1766,6 +1766,7 @@ class Parser : public CodeCompletionHandler {
ExprResult ParseConstantExpressionInExprEvalContext(
TypeCastState isTypeCast = NotTypeCast);
ExprResult ParseConstantExpression();
ExprResult ParseArrayBoundExpression();
ExprResult ParseCaseExpression(SourceLocation CaseLoc);
ExprResult ParseConstraintExpression();
ExprResult
Expand Down
61 changes: 33 additions & 28 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -1062,20 +1062,6 @@ class Sema final {
}
};

/// Whether the AST is currently being rebuilt to correct immediate
/// invocations. Immediate invocation candidates and references to consteval
/// functions aren't tracked when this is set.
bool RebuildingImmediateInvocation = false;

/// Used to change context to isConstantEvaluated without pushing a heavy
/// ExpressionEvaluationContextRecord object.
bool isConstantEvaluatedOverride;

bool isConstantEvaluated() const {
return ExprEvalContexts.back().isConstantEvaluated() ||
isConstantEvaluatedOverride;
}

/// RAII object to handle the state changes required to synthesize
/// a function body.
class SynthesizedFunctionScope {
Expand Down Expand Up @@ -1361,6 +1347,11 @@ class Sema final {

bool IsCurrentlyCheckingDefaultArgumentOrInitializer = false;

// We are in a constant context, but we also allow
// non constant expressions, for example for array bounds (which may be
// VLAs).
bool InConditionallyConstantEvaluateContext = false;

// When evaluating immediate functions in the initializer of a default
// argument or default member initializer, this is the declaration whose
// default initializer is being evaluated and the location of the call
Expand Down Expand Up @@ -9878,30 +9869,44 @@ class Sema final {
/// diagnostics that will be suppressed.
std::optional<sema::TemplateDeductionInfo *> isSFINAEContext() const;

/// Determines whether we are currently in a context that
/// is not evaluated as per C++ [expr] p5.
bool isUnevaluatedContext() const {
/// Whether the AST is currently being rebuilt to correct immediate
/// invocations. Immediate invocation candidates and references to consteval
/// functions aren't tracked when this is set.
bool RebuildingImmediateInvocation = false;

/// Used to change context to isConstantEvaluated without pushing a heavy
/// ExpressionEvaluationContextRecord object.
bool isConstantEvaluatedOverride = false;

const ExpressionEvaluationContextRecord &currentEvaluationContext() const {
assert(!ExprEvalContexts.empty() &&
"Must be in an expression evaluation context");
return ExprEvalContexts.back().isUnevaluated();
}
return ExprEvalContexts.back();
};

bool isConstantEvaluatedContext() const {
assert(!ExprEvalContexts.empty() &&
"Must be in an expression evaluation context");
return ExprEvalContexts.back().isConstantEvaluated();
return currentEvaluationContext().isConstantEvaluated() ||
isConstantEvaluatedOverride;
}

bool isAlwaysConstantEvaluatedContext() const {
const ExpressionEvaluationContextRecord &Ctx = currentEvaluationContext();
return (Ctx.isConstantEvaluated() || isConstantEvaluatedOverride) &&
!Ctx.InConditionallyConstantEvaluateContext;
}

/// Determines whether we are currently in a context that
/// is not evaluated as per C++ [expr] p5.
bool isUnevaluatedContext() const {
return currentEvaluationContext().isUnevaluated();
}

bool isImmediateFunctionContext() const {
assert(!ExprEvalContexts.empty() &&
"Must be in an expression evaluation context");
return ExprEvalContexts.back().isImmediateFunctionContext();
return currentEvaluationContext().isImmediateFunctionContext();
}

bool isCheckingDefaultArgumentOrInitializer() const {
assert(!ExprEvalContexts.empty() &&
"Must be in an expression evaluation context");
const ExpressionEvaluationContextRecord &Ctx = ExprEvalContexts.back();
const ExpressionEvaluationContextRecord &Ctx = currentEvaluationContext();
return (Ctx.Context ==
ExpressionEvaluationContext::PotentiallyEvaluatedIfUsed) ||
Ctx.IsCurrentlyCheckingDefaultArgumentOrInitializer;
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Parse/ParseDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7703,7 +7703,7 @@ void Parser::ParseBracketDeclarator(Declarator &D) {
// Parse the constant-expression or assignment-expression now (depending
// on dialect).
if (getLangOpts().CPlusPlus) {
NumElements = ParseConstantExpression();
NumElements = ParseArrayBoundExpression();
} else {
EnterExpressionEvaluationContext Unevaluated(
Actions, Sema::ExpressionEvaluationContext::ConstantEvaluated);
Expand Down
9 changes: 9 additions & 0 deletions clang/lib/Parse/ParseExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,15 @@ ExprResult Parser::ParseConstantExpression() {
return ParseConstantExpressionInExprEvalContext(NotTypeCast);
}

ExprResult Parser::ParseArrayBoundExpression() {
EnterExpressionEvaluationContext ConstantEvaluated(
Actions, Sema::ExpressionEvaluationContext::ConstantEvaluated);
// If we parse the bound of a VLA... we parse a non-constant
// constant-expression!
Actions.ExprEvalContexts.back().InConditionallyConstantEvaluateContext = true;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd rather this be an argument to the above constructor here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@erichkeane this constructor already has a bunch of defaulted boolean / pointer parameters, it would be a mess. We'd need to add a tag which seems overkill for 1 use

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In reality, I'd probably prefer something like;

auto Expr = ExpressionEvaluationContext::CreateForVLA

kind of thing (or perhaps a builder-pattern like below), but that is probably a sizable refactor for everything. I think I can hold my nose here and be OK for now.

ExprEvalContext Eva(ThingsEverythingNeeds).ForVLA()

return ParseConstantExpressionInExprEvalContext(NotTypeCast);
}

ExprResult Parser::ParseCaseExpression(SourceLocation CaseLoc) {
EnterExpressionEvaluationContext ConstantEvaluated(
Actions, Sema::ExpressionEvaluationContext::ConstantEvaluated);
Expand Down
1 change: 0 additions & 1 deletion clang/lib/Sema/Sema.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,6 @@ Sema::Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer,
CurScope(nullptr), Ident_super(nullptr) {
assert(pp.TUKind == TUKind);
TUScope = nullptr;
isConstantEvaluatedOverride = false;

LoadedExternalKnownNamespaces = false;
for (unsigned I = 0; I != NSAPI::NumNSNumberLiteralMethods; ++I)
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Sema/SemaCUDA.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -812,7 +812,7 @@ bool Sema::CheckCUDACall(SourceLocation Loc, FunctionDecl *Callee) {
assert(getLangOpts().CUDA && "Should only be called during CUDA compilation");
assert(Callee && "Callee may not be null.");

auto &ExprEvalCtx = ExprEvalContexts.back();
const auto &ExprEvalCtx = currentEvaluationContext();
if (ExprEvalCtx.isUnevaluated() || ExprEvalCtx.isConstantEvaluated())
return true;

Expand Down
57 changes: 31 additions & 26 deletions clang/lib/Sema/SemaChecking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1076,7 +1076,7 @@ static bool ProcessFormatStringLiteral(const Expr *FormatExpr,
void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
CallExpr *TheCall) {
if (TheCall->isValueDependent() || TheCall->isTypeDependent() ||
isConstantEvaluated())
isConstantEvaluatedContext())
return;

bool UseDABAttr = false;
Expand Down Expand Up @@ -3192,7 +3192,7 @@ bool Sema::CheckCDEBuiltinFunctionCall(const TargetInfo &TI, unsigned BuiltinID,

bool Sema::CheckARMCoprocessorImmediate(const TargetInfo &TI,
const Expr *CoprocArg, bool WantCDE) {
if (isConstantEvaluated())
if (isConstantEvaluatedContext())
return false;

// We can't check the value of a dependent argument.
Expand Down Expand Up @@ -6616,7 +6616,7 @@ static void CheckNonNullArguments(Sema &S,
assert((FDecl || Proto) && "Need a function declaration or prototype");

// Already checked by constant evaluator.
if (S.isConstantEvaluated())
if (S.isConstantEvaluatedContext())
return;
// Check the attributes attached to the method/function itself.
llvm::SmallBitVector NonNullArgs;
Expand Down Expand Up @@ -8980,7 +8980,7 @@ bool Sema::SemaBuiltinConstantArg(CallExpr *TheCall, int ArgNum,
/// TheCall is a constant expression in the range [Low, High].
bool Sema::SemaBuiltinConstantArgRange(CallExpr *TheCall, int ArgNum,
int Low, int High, bool RangeIsError) {
if (isConstantEvaluated())
if (isConstantEvaluatedContext())
return false;
llvm::APSInt Result;

Expand Down Expand Up @@ -9694,7 +9694,7 @@ checkFormatStringExpr(Sema &S, const Expr *E, ArrayRef<const Expr *> Args,
llvm::SmallBitVector &CheckedVarArgs,
UncoveredArgHandler &UncoveredArg, llvm::APSInt Offset,
bool IgnoreStringsWithoutSpecifiers = false) {
if (S.isConstantEvaluated())
if (S.isConstantEvaluatedContext())
return SLCT_NotALiteral;
tryAgain:
assert(Offset.isSigned() && "invalid offset");
Expand Down Expand Up @@ -9734,8 +9734,8 @@ checkFormatStringExpr(Sema &S, const Expr *E, ArrayRef<const Expr *> Args,
bool CheckLeft = true, CheckRight = true;

bool Cond;
if (C->getCond()->EvaluateAsBooleanCondition(Cond, S.getASTContext(),
S.isConstantEvaluated())) {
if (C->getCond()->EvaluateAsBooleanCondition(
Cond, S.getASTContext(), S.isConstantEvaluatedContext())) {
if (Cond)
CheckRight = false;
else
Expand Down Expand Up @@ -9999,9 +9999,11 @@ checkFormatStringExpr(Sema &S, const Expr *E, ArrayRef<const Expr *> Args,
Expr::EvalResult LResult, RResult;

bool LIsInt = BinOp->getLHS()->EvaluateAsInt(
LResult, S.Context, Expr::SE_NoSideEffects, S.isConstantEvaluated());
LResult, S.Context, Expr::SE_NoSideEffects,
S.isConstantEvaluatedContext());
bool RIsInt = BinOp->getRHS()->EvaluateAsInt(
RResult, S.Context, Expr::SE_NoSideEffects, S.isConstantEvaluated());
RResult, S.Context, Expr::SE_NoSideEffects,
S.isConstantEvaluatedContext());

if (LIsInt != RIsInt) {
BinaryOperatorKind BinOpKind = BinOp->getOpcode();
Expand Down Expand Up @@ -10029,7 +10031,7 @@ checkFormatStringExpr(Sema &S, const Expr *E, ArrayRef<const Expr *> Args,
Expr::EvalResult IndexResult;
if (ASE->getRHS()->EvaluateAsInt(IndexResult, S.Context,
Expr::SE_NoSideEffects,
S.isConstantEvaluated())) {
S.isConstantEvaluatedContext())) {
sumOffsets(Offset, IndexResult.Val.getInt(), BO_Add,
/*RHS is int*/ true);
E = ASE->getBase();
Expand Down Expand Up @@ -13959,7 +13961,7 @@ static bool CheckTautologicalComparison(Sema &S, BinaryOperator *E,
return false;

IntRange OtherValueRange = GetExprRange(
S.Context, Other, S.isConstantEvaluated(), /*Approximate*/ false);
S.Context, Other, S.isConstantEvaluatedContext(), /*Approximate=*/false);

QualType OtherT = Other->getType();
if (const auto *AT = OtherT->getAs<AtomicType>())
Expand Down Expand Up @@ -14174,8 +14176,9 @@ static void AnalyzeComparison(Sema &S, BinaryOperator *E) {
}

// Otherwise, calculate the effective range of the signed operand.
IntRange signedRange = GetExprRange(
S.Context, signedOperand, S.isConstantEvaluated(), /*Approximate*/ true);
IntRange signedRange =
GetExprRange(S.Context, signedOperand, S.isConstantEvaluatedContext(),
/*Approximate=*/true);

// Go ahead and analyze implicit conversions in the operands. Note
// that we skip the implicit conversions on both sides.
Expand All @@ -14193,8 +14196,8 @@ static void AnalyzeComparison(Sema &S, BinaryOperator *E) {
if (E->isEqualityOp()) {
unsigned comparisonWidth = S.Context.getIntWidth(T);
IntRange unsignedRange =
GetExprRange(S.Context, unsignedOperand, S.isConstantEvaluated(),
/*Approximate*/ true);
GetExprRange(S.Context, unsignedOperand, S.isConstantEvaluatedContext(),
/*Approximate=*/true);

// We should never be unable to prove that the unsigned operand is
// non-negative.
Expand Down Expand Up @@ -15057,7 +15060,7 @@ static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
if (Target->isUnsaturatedFixedPointType()) {
Expr::EvalResult Result;
if (E->EvaluateAsFixedPoint(Result, S.Context, Expr::SE_AllowSideEffects,
S.isConstantEvaluated())) {
S.isConstantEvaluatedContext())) {
llvm::APFixedPoint Value = Result.Val.getFixedPoint();
llvm::APFixedPoint MaxVal = S.Context.getFixedPointMax(T);
llvm::APFixedPoint MinVal = S.Context.getFixedPointMin(T);
Expand All @@ -15072,7 +15075,7 @@ static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
}
} else if (Target->isIntegerType()) {
Expr::EvalResult Result;
if (!S.isConstantEvaluated() &&
if (!S.isConstantEvaluatedContext() &&
E->EvaluateAsFixedPoint(Result, S.Context,
Expr::SE_AllowSideEffects)) {
llvm::APFixedPoint FXResult = Result.Val.getFixedPoint();
Expand All @@ -15095,7 +15098,7 @@ static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
} else if (Target->isUnsaturatedFixedPointType()) {
if (Source->isIntegerType()) {
Expr::EvalResult Result;
if (!S.isConstantEvaluated() &&
if (!S.isConstantEvaluatedContext() &&
E->EvaluateAsInt(Result, S.Context, Expr::SE_AllowSideEffects)) {
llvm::APSInt Value = Result.Val.getInt();

Expand All @@ -15121,8 +15124,9 @@ static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
if (SourceBT && TargetBT && SourceBT->isIntegerType() &&
TargetBT->isFloatingType() && !IsListInit) {
// Determine the number of precision bits in the source integer type.
IntRange SourceRange = GetExprRange(S.Context, E, S.isConstantEvaluated(),
/*Approximate*/ true);
IntRange SourceRange =
GetExprRange(S.Context, E, S.isConstantEvaluatedContext(),
/*Approximate=*/true);
unsigned int SourcePrecision = SourceRange.Width;

// Determine the number of precision bits in the
Expand Down Expand Up @@ -15190,16 +15194,16 @@ static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,

IntRange SourceTypeRange =
IntRange::forTargetOfCanonicalType(S.Context, Source);
IntRange LikelySourceRange =
GetExprRange(S.Context, E, S.isConstantEvaluated(), /*Approximate*/ true);
IntRange LikelySourceRange = GetExprRange(
S.Context, E, S.isConstantEvaluatedContext(), /*Approximate=*/true);
IntRange TargetRange = IntRange::forTargetOfCanonicalType(S.Context, Target);

if (LikelySourceRange.Width > TargetRange.Width) {
// If the source is a constant, use a default-on diagnostic.
// TODO: this should happen for bitfield stores, too.
Expr::EvalResult Result;
if (E->EvaluateAsInt(Result, S.Context, Expr::SE_AllowSideEffects,
S.isConstantEvaluated())) {
S.isConstantEvaluatedContext())) {
llvm::APSInt Value(32);
Value = Result.Val.getInt();

Expand Down Expand Up @@ -16063,7 +16067,8 @@ class SequenceChecker : public ConstEvaluatedExprVisitor<SequenceChecker> {
if (!EvalOK || E->isValueDependent())
return false;
EvalOK = E->EvaluateAsBooleanCondition(
Result, Self.SemaRef.Context, Self.SemaRef.isConstantEvaluated());
Result, Self.SemaRef.Context,
Self.SemaRef.isConstantEvaluatedContext());
return EvalOK;
}

Expand Down Expand Up @@ -17190,7 +17195,7 @@ void Sema::CheckArrayAccess(const Expr *BaseExpr, const Expr *IndexExpr,
const ArraySubscriptExpr *ASE,
bool AllowOnePastEnd, bool IndexNegated) {
// Already diagnosed by the constant evaluator.
if (isConstantEvaluated())
if (isConstantEvaluatedContext())
return;

IndexExpr = IndexExpr->IgnoreParenImpCasts();
Expand Down Expand Up @@ -18579,7 +18584,7 @@ void Sema::CheckArgumentWithTypeTag(const ArgumentWithTypeTagAttr *Attr,
TypeTagData TypeInfo;
if (!GetMatchingCType(ArgumentKind, TypeTagExpr, Context,
TypeTagForDatatypeMagicValues.get(), FoundWrongKind,
TypeInfo, isConstantEvaluated())) {
TypeInfo, isConstantEvaluatedContext())) {
if (FoundWrongKind)
Diag(TypeTagExpr->getExprLoc(),
diag::warn_type_tag_for_datatype_wrong_kind)
Expand Down
4 changes: 2 additions & 2 deletions clang/lib/Sema/SemaExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18312,7 +18312,7 @@ void Sema::MarkExpressionAsImmediateEscalating(Expr *E) {

ExprResult Sema::CheckForImmediateInvocation(ExprResult E, FunctionDecl *Decl) {
if (isUnevaluatedContext() || !E.isUsable() || !Decl ||
!Decl->isImmediateFunction() || isConstantEvaluated() ||
!Decl->isImmediateFunction() || isAlwaysConstantEvaluatedContext() ||
isCheckingDefaultArgumentOrInitializer() ||
RebuildingImmediateInvocation || isImmediateFunctionContext())
return E;
Expand Down Expand Up @@ -20736,7 +20736,7 @@ void Sema::MarkDeclRefReferenced(DeclRefExpr *E, const Expr *Base) {
OdrUse = false;

if (auto *FD = dyn_cast<FunctionDecl>(E->getDecl())) {
if (!isUnevaluatedContext() && !isConstantEvaluated() &&
if (!isUnevaluatedContext() && !isConstantEvaluatedContext() &&
!isImmediateFunctionContext() &&
!isCheckingDefaultArgumentOrInitializer() &&
FD->isImmediateFunction() && !RebuildingImmediateInvocation &&
Expand Down
3 changes: 3 additions & 0 deletions clang/lib/Sema/TreeTransform.h
Original file line number Diff line number Diff line change
Expand Up @@ -5490,6 +5490,9 @@ TreeTransform<Derived>::TransformDependentSizedArrayType(TypeLocBuilder &TLB,
EnterExpressionEvaluationContext Unevaluated(
SemaRef, Sema::ExpressionEvaluationContext::ConstantEvaluated);

// If we have a VLA then it won't be a constant.
SemaRef.ExprEvalContexts.back().InConditionallyConstantEvaluateContext = true;

// Prefer the expression from the TypeLoc; the other may have been uniqued.
Expr *origSize = TL.getSizeExpr();
if (!origSize) origSize = T->getSizeExpr();
Expand Down
Loading