Skip to content

Conversation

@Galoretka
Copy link

Describe your changes

The issue was in the MIR constant propagation pass where exponentiation folding handled the base-equals-zero case before checking for a zero exponent, causing 0^0 to fold to 0. This contradicted both the AST constant folding, which uses checked_pow and yields 1 for 0^0, and the MIR evaluator, which evaluates x^0 as 1 in the field. The fix changes the folding order in visit_exp_bis so that we fold x^0 to 1 first and only fold 0^k to 0 when k is a known, nonzero constant, delegating to generic constant folding otherwise. A MIR-level unit test was added to assert that 0^0 folds to a constant 1, guarding against regressions even if AST behavior changes.

Copy link
Collaborator

@Leo-Besancon Leo-Besancon left a comment

Choose a reason for hiding this comment

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

Thank you for the PR, good catch! I've left a few comments inline.

use crate::ir::{Enf, Exp, Link, Mir, MirValue, Op, SpannedMirValue, Value, ConstantValue};

#[test]
fn constant_propagation_folds_zero_pow_zero_to_one() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you remove the test (it does not build as is, is not included when running cargo test and it does not seem needed to me)

} else if let Some(0) = get_inner_const(&lhs) {
// 0^k = 0, but only when k is known and non-zero
if let Some(k) = get_inner_const(&rhs)
&& k != 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

The case "k == 0" is already handled in the check "rhs = 0" above, you do not need it here.

span: exp_ref.span,
})));
}
try_fold_const_binary_op(lhs, rhs, exp.clone(), exp_ref.span())
Copy link
Collaborator

Choose a reason for hiding this comment

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

If k is not constant, this function call will always return Ok(None), so we should return it directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants