Skip to content

Commit

Permalink
[Constexpr] fix a bug in conditional operator constexpr evaluation
Browse files Browse the repository at this point in the history
  • Loading branch information
isuckatcs committed Jul 2, 2024
1 parent 608b246 commit 88715ee
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 13 deletions.
43 changes: 30 additions & 13 deletions src/constexpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,42 @@
#include <optional>

namespace {
bool toBool(double d) { return d != 0.0; }
std::optional<bool> toBool(std::optional<double> d) {
if (!d)
return std::nullopt;

return d != 0.0;
}
} // namespace

std::optional<double> ConstantExpressionEvaluator::evaluateBinaryOperator(
const ResolvedBinaryOperator &binop) {
std::optional<double> lhs = evaluate(*binop.lhs);
if (!lhs)
return std::nullopt;

// If the LHS of || is true, we don't need to evaluate the RHS.
if (binop.op == TokenKind::PipePipe && toBool(*lhs))
return 1.0;
if (binop.op == TokenKind::PipePipe) {
// If the LHS of || is true, we don't need to evaluate the RHS.
if (toBool(lhs).value_or(false))
return 1.0;

return toBool(evaluate(*binop.rhs));
}

// If the LHS of && is false, we don't need to evaluate the RHS.
if (binop.op == TokenKind::AmpAmp && !toBool(*lhs))
return 0.0;
if (binop.op == TokenKind::AmpAmp) {
// If the LHS of && is false, we don't need to evaluate the RHS.
if (binop.op == TokenKind::AmpAmp && !toBool(lhs).value_or(true))
return 0.0;

// If the LHS is unknown, but the RHS is false, the expression is false.
std::optional<double> rhs = evaluate(*binop.rhs);
if (!lhs && rhs == 0.0)
return 0.0;

// Otherwise LHS is known to be true, so the result depends on the RHS.
return toBool(rhs);
}

if (!lhs)
return std::nullopt;

std::optional<double> rhs = evaluate(*binop.rhs);
if (!rhs)
Expand All @@ -40,9 +60,6 @@ std::optional<double> ConstantExpressionEvaluator::evaluateBinaryOperator(
return *lhs > *rhs;
case TokenKind::EqualEqual:
return *lhs == *rhs;
case TokenKind::AmpAmp:
case TokenKind::PipePipe:
return toBool(*rhs); // The LHS is already handled.
default:
assert(false && "unexpected binary operator");
}
Expand All @@ -55,7 +72,7 @@ std::optional<double> ConstantExpressionEvaluator::evaluateUnaryOperator(
return std::nullopt;

if (op.op == TokenKind::Excl)
return !toBool(*rhs);
return !*toBool(rhs);

assert(false && "unexpected unary operator");
}
Expand Down
10 changes: 10 additions & 0 deletions test/constexpr/special_cases.al
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,14 @@ fn lhsKnownRhsNot(): number {
// CHECK-NEXT: | value: 1
// CHECK-NEXT: ResolvedDeclRefExpr: @({{.*}}) y

fn lhsUnknownRhsFalse(x: number): number {
return x && 0.0;
}
// CHECK: ResolvedReturnStmt
// CHECK-NEXT: ResolvedBinaryOperator: '&&'
// CHECK-NEXT: | value: 0
// CHECK-NEXT: ResolvedDeclRefExpr: @({{.*}}) x
// CHECK-NEXT: NumberLiteral: '0'
// CHECK-NEXT: | value: 0

fn main(): void {}

0 comments on commit 88715ee

Please sign in to comment.