feat(normalization): Phase 253 - mutable_accumulator_analyzer detector pattern
**Core Change**: Refactor from "validator" to "detector" pattern **Before** (Fail-Fast): - Err for non-accumulator assignments (i = i - 1, i = s.length(), etc.) - JoinIR pipeline aborted on detection **After** (Detector): - Ok(None) for non-accumulator patterns (other lowering paths can handle) - Only Add operator supported (i = i + 1) - Other operators/patterns return None gracefully **Modified Behavior** (6 locations): 1. L112-116: operator != Add → Ok(None) 2. L149-152: RHS MethodCall/Call → Ok(None) 3. L153-156: RHS complex expression → Ok(None) 4. L158-161: Left operand reversed → Ok(None) 5. L162-165: Left operand not Variable → Ok(None) 6. L166-170: Value not BinaryOp → Ok(None) **Tests** (11/11 PASS): - 2 existing tests updated (Err → Ok(None)) - 3 new tests added: - test_decrement_not_accumulator (i = i - 1) - test_complex_rhs_not_accumulator (x = x + (i + 1)) - test_non_binop_assignment_not_accumulator (i = s.length()) **Documentation**: - Updated docstring to reflect detector strategy - Detection Strategy section added with all Ok(None) cases **Status**: - ✅ mutable-acc-spec errors eliminated - ⚠️ quick profile still FAIL: StringUtils.index_of/2 not JoinIR-supported (Phase 254 scope) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
@ -64,16 +64,21 @@ impl MutableAccumulatorAnalyzer {
|
||||
///
|
||||
/// # Returns
|
||||
///
|
||||
/// * `Ok(Some(spec))` - Accumulator pattern detected
|
||||
/// * `Ok(None)` - No accumulator pattern found
|
||||
/// * `Err(String)` - Pattern violation (Fail-Fast)
|
||||
/// * `Ok(Some(spec))` - Accumulator pattern detected (target = target + x)
|
||||
/// * `Ok(None)` - No accumulator pattern found (other patterns may apply)
|
||||
/// * `Err(String)` - Internal consistency error (should be rare)
|
||||
///
|
||||
/// # Fail-Fast Cases
|
||||
/// # Detection Strategy (Phase 253)
|
||||
///
|
||||
/// Returns `Ok(None)` for non-accumulator patterns:
|
||||
/// - Multiple assignments to the same variable
|
||||
/// - Assignment form not accumulator pattern (e.g., reversed operands)
|
||||
/// - RHS contains function call (not supported)
|
||||
/// - Operator not Add (other ops not supported yet)
|
||||
/// - Operator not Add (e.g., i = i - 1)
|
||||
/// - Left operand not matching target (e.g., x = y + x)
|
||||
/// - RHS contains function call (e.g., x = x + f())
|
||||
/// - RHS is complex expression (e.g., x = x + (i + 1))
|
||||
/// - Value is not BinaryOp (e.g., i = s.length())
|
||||
///
|
||||
/// This allows other loop lowering patterns to handle non-accumulator cases.
|
||||
pub fn analyze(loop_body: &[ASTNode]) -> Result<Option<MutableAccumulatorSpec>, String> {
|
||||
// Collect all assignments in loop body
|
||||
let assignments = collect_assignments(loop_body);
|
||||
@ -109,12 +114,10 @@ impl MutableAccumulatorAnalyzer {
|
||||
..
|
||||
} = value_node
|
||||
{
|
||||
// Check operator is Add
|
||||
// Phase 253: Check operator is Add (return Ok(None) if not)
|
||||
// Only Add is supported as accumulator pattern
|
||||
if operator != &BinaryOperator::Add {
|
||||
return Err(format!(
|
||||
"[joinir/mutable-acc-spec] Operator '{}' not supported (only '+' allowed)",
|
||||
operator
|
||||
));
|
||||
return Ok(None); // Not our pattern (e.g., i = i - 1)
|
||||
}
|
||||
|
||||
// Check left operand is Variable with same name as target
|
||||
@ -149,39 +152,26 @@ impl MutableAccumulatorAnalyzer {
|
||||
}));
|
||||
}
|
||||
ASTNode::MethodCall { .. } | ASTNode::Call { .. } => {
|
||||
return Err(
|
||||
"[joinir/mutable-acc-spec] RHS contains function call (not supported)"
|
||||
.to_string(),
|
||||
);
|
||||
// Phase 253: RHS with function call is not accumulator pattern
|
||||
return Ok(None); // Not our pattern (e.g., x = x + f())
|
||||
}
|
||||
_ => {
|
||||
return Err(format!(
|
||||
"[joinir/mutable-acc-spec] RHS expression type not supported: {:?}",
|
||||
right
|
||||
));
|
||||
// Phase 253: Complex RHS expression is not accumulator pattern
|
||||
return Ok(None); // Not our pattern (e.g., x = x + (i + 1))
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// Left operand is different variable (reversed operands)
|
||||
return Err(
|
||||
"[joinir/mutable-acc-spec] Assignment form not accumulator pattern (required: target = target + x)"
|
||||
.to_string(),
|
||||
);
|
||||
// Phase 253: Left operand is different variable (reversed operands)
|
||||
return Ok(None); // Not our pattern (e.g., x = y + x)
|
||||
}
|
||||
} else {
|
||||
// Left operand is not Variable
|
||||
return Err(
|
||||
"[joinir/mutable-acc-spec] Assignment form not accumulator pattern (required: target = target + x)"
|
||||
.to_string(),
|
||||
);
|
||||
// Phase 253: Left operand is not Variable
|
||||
return Ok(None); // Not our pattern
|
||||
}
|
||||
} else {
|
||||
// Value is not BinaryOp, check if it's a potential mutation
|
||||
// If the assignment is to a candidate variable, it's a non-accumulator mutation
|
||||
return Err(format!(
|
||||
"[joinir/mutable-acc-spec] Assignment to '{}' is not accumulator form (required: target = target + x)",
|
||||
target_name
|
||||
));
|
||||
// Phase 253: Value is not BinaryOp - not accumulator pattern
|
||||
// e.g., i = s.length() - 1 (MethodCall result, not x = x + y)
|
||||
return Ok(None); // Not our pattern
|
||||
}
|
||||
}
|
||||
|
||||
@ -278,7 +268,8 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn test_mutable_accumulator_spec_ng_reversed() {
|
||||
// Build AST for: out = ch + out (reversed operands)
|
||||
// Phase 253: Build AST for: out = ch + out (reversed operands)
|
||||
// Expected: Ok(None) - not accumulator pattern
|
||||
let loop_body = vec![ASTNode::Assignment {
|
||||
target: Box::new(ASTNode::Variable {
|
||||
name: "out".to_string(),
|
||||
@ -300,10 +291,8 @@ mod tests {
|
||||
}];
|
||||
|
||||
let result = MutableAccumulatorAnalyzer::analyze(&loop_body);
|
||||
assert!(result.is_err());
|
||||
assert!(result
|
||||
.unwrap_err()
|
||||
.contains("Assignment form not accumulator pattern"));
|
||||
assert!(result.is_ok());
|
||||
assert!(result.unwrap().is_none(), "Reversed operands should return None");
|
||||
}
|
||||
|
||||
#[test]
|
||||
@ -358,7 +347,8 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn test_mutable_accumulator_spec_ng_method_call() {
|
||||
// Build AST for: out = out + f()
|
||||
// Phase 253: Build AST for: out = out + f()
|
||||
// Expected: Ok(None) - RHS contains function call
|
||||
let loop_body = vec![ASTNode::Assignment {
|
||||
target: Box::new(ASTNode::Variable {
|
||||
name: "out".to_string(),
|
||||
@ -385,10 +375,8 @@ mod tests {
|
||||
}];
|
||||
|
||||
let result = MutableAccumulatorAnalyzer::analyze(&loop_body);
|
||||
assert!(result.is_err());
|
||||
assert!(result
|
||||
.unwrap_err()
|
||||
.contains("RHS contains function call (not supported)"));
|
||||
assert!(result.is_ok());
|
||||
assert!(result.unwrap().is_none(), "RHS with function call should return None");
|
||||
}
|
||||
|
||||
#[test]
|
||||
@ -512,4 +500,96 @@ mod tests {
|
||||
assert_eq!(spec.op, BinaryOperator::Add);
|
||||
assert_eq!(spec.kind, AccumulatorKind::Int);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_decrement_not_accumulator() {
|
||||
// Phase 253: Build AST for: i = i - 1
|
||||
// Expected: Ok(None) - decrement is not accumulator pattern (only Add supported)
|
||||
let loop_body = vec![ASTNode::Assignment {
|
||||
target: Box::new(ASTNode::Variable {
|
||||
name: "i".to_string(),
|
||||
span: Span::unknown(),
|
||||
}),
|
||||
value: Box::new(ASTNode::BinaryOp {
|
||||
operator: BinaryOperator::Subtract,
|
||||
left: Box::new(ASTNode::Variable {
|
||||
name: "i".to_string(),
|
||||
span: Span::unknown(),
|
||||
}),
|
||||
right: Box::new(ASTNode::Literal {
|
||||
value: LiteralValue::Integer(1),
|
||||
span: Span::unknown(),
|
||||
}),
|
||||
span: Span::unknown(),
|
||||
}),
|
||||
span: Span::unknown(),
|
||||
}];
|
||||
|
||||
let result = MutableAccumulatorAnalyzer::analyze(&loop_body);
|
||||
assert!(result.is_ok());
|
||||
assert!(result.unwrap().is_none(), "Decrement (i = i - 1) should return None");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_complex_rhs_not_accumulator() {
|
||||
// Phase 253: Build AST for: x = x + (i + 1)
|
||||
// Expected: Ok(None) - complex RHS expression
|
||||
let loop_body = vec![ASTNode::Assignment {
|
||||
target: Box::new(ASTNode::Variable {
|
||||
name: "x".to_string(),
|
||||
span: Span::unknown(),
|
||||
}),
|
||||
value: Box::new(ASTNode::BinaryOp {
|
||||
operator: BinaryOperator::Add,
|
||||
left: Box::new(ASTNode::Variable {
|
||||
name: "x".to_string(),
|
||||
span: Span::unknown(),
|
||||
}),
|
||||
right: Box::new(ASTNode::BinaryOp {
|
||||
operator: BinaryOperator::Add,
|
||||
left: Box::new(ASTNode::Variable {
|
||||
name: "i".to_string(),
|
||||
span: Span::unknown(),
|
||||
}),
|
||||
right: Box::new(ASTNode::Literal {
|
||||
value: LiteralValue::Integer(1),
|
||||
span: Span::unknown(),
|
||||
}),
|
||||
span: Span::unknown(),
|
||||
}),
|
||||
span: Span::unknown(),
|
||||
}),
|
||||
span: Span::unknown(),
|
||||
}];
|
||||
|
||||
let result = MutableAccumulatorAnalyzer::analyze(&loop_body);
|
||||
assert!(result.is_ok());
|
||||
assert!(result.unwrap().is_none(), "Complex RHS (x = x + (i + 1)) should return None");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_non_binop_assignment_not_accumulator() {
|
||||
// Phase 253: Build AST for: i = s.length()
|
||||
// Expected: Ok(None) - not BinaryOp (MethodCall result)
|
||||
let loop_body = vec![ASTNode::Assignment {
|
||||
target: Box::new(ASTNode::Variable {
|
||||
name: "i".to_string(),
|
||||
span: Span::unknown(),
|
||||
}),
|
||||
value: Box::new(ASTNode::MethodCall {
|
||||
object: Box::new(ASTNode::Variable {
|
||||
name: "s".to_string(),
|
||||
span: Span::unknown(),
|
||||
}),
|
||||
method: "length".to_string(),
|
||||
arguments: vec![],
|
||||
span: Span::unknown(),
|
||||
}),
|
||||
span: Span::unknown(),
|
||||
}];
|
||||
|
||||
let result = MutableAccumulatorAnalyzer::analyze(&loop_body);
|
||||
assert!(result.is_ok());
|
||||
assert!(result.unwrap().is_none(), "Non-BinaryOp assignment (i = s.length()) should return None");
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user