Skip to content

Commit cbb80d5

Browse files
committed
Fix VariableResolver
Filter out variables that only appear in their own definition.
1 parent 921136a commit cbb80d5

File tree

3 files changed

+127
-31
lines changed

3 files changed

+127
-31
lines changed

liquidjava-verifier/src/main/java/liquidjava/rj_language/opt/VariableResolver.java

Lines changed: 38 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,25 +12,43 @@
1212
public class VariableResolver {
1313

1414
/**
15-
* Extracts variables with constant values from an expression Returns a map from variable names to their values
15+
* Extracts variables with constant values from an expression
16+
* @param exp
17+
* @returns map from variable names to their values
1618
*/
1719
public static Map<String, Expression> resolve(Expression exp) {
18-
// if the expression is just a single equality (not a conjunction) don't extract it
19-
// this avoids creating tautologies like "1 == 1" after substitution, which are then simplified to "true"
20-
if (exp instanceof BinaryExpression be) {
21-
if ("==".equals(be.getOperator())) {
22-
return new HashMap<>();
23-
}
24-
}
25-
2620
Map<String, Expression> map = new HashMap<>();
2721
resolveRecursive(exp, map);
22+
23+
// filter out variables that only appear in their own definition
24+
map.entrySet().removeIf(entry -> countOccurrences(exp, entry.getKey()) < 2);
25+
2826
return resolveTransitive(map);
2927
}
3028

29+
/**
30+
* Counts occurrences of a variable in an expression
31+
* @param exp
32+
* @param varName
33+
* @return number of occurrences
34+
*/
35+
private static int countOccurrences(Expression exp, String varName) {
36+
if (exp instanceof Var var && var.getName().equals(varName)) {
37+
return 1;
38+
}
39+
if (exp instanceof BinaryExpression be) {
40+
return countOccurrences(be.getFirstOperand(), varName) + countOccurrences(be.getSecondOperand(), varName);
41+
}
42+
if (exp.getChildren() != null) {
43+
return exp.getChildren().stream().mapToInt(child -> countOccurrences(child, varName)).sum();
44+
}
45+
return 0;
46+
}
47+
3148
/**
3249
* Recursively extracts variable equalities from an expression (e.g. ... && x == 1 && y == 2 => map: x -> 1, y -> 2)
33-
* Modifies the given map in place
50+
* @param exp
51+
* @param map
3452
*/
3553
private static void resolveRecursive(Expression exp, Map<String, Expression> map) {
3654
if (!(exp instanceof BinaryExpression be))
@@ -43,16 +61,18 @@ private static void resolveRecursive(Expression exp, Map<String, Expression> map
4361
} else if ("==".equals(op)) {
4462
Expression left = be.getFirstOperand();
4563
Expression right = be.getSecondOperand();
46-
if (left instanceof Var && (right.isLiteral() || right instanceof Var)) {
47-
map.put(((Var) left).getName(), right.clone());
48-
} else if (right instanceof Var && left.isLiteral()) {
49-
map.put(((Var) right).getName(), left.clone());
64+
if (left instanceof Var var && right.isLiteral()) {
65+
map.put(var.getName(), right.clone());
66+
} else if (right instanceof Var var && left.isLiteral()) {
67+
map.put(var.getName(), left.clone());
5068
}
5169
}
5270
}
5371

5472
/**
5573
* Handles transitive variable equalities in the map (e.g. map: x -> y, y -> 1 => map: x -> 1, y -> 1)
74+
* @param map
75+
* @return new map with resolved values
5676
*/
5777
private static Map<String, Expression> resolveTransitive(Map<String, Expression> map) {
5878
Map<String, Expression> result = new HashMap<>();
@@ -65,6 +85,10 @@ private static Map<String, Expression> resolveTransitive(Map<String, Expression>
6585
/**
6686
* Returns the value of a variable by looking up in the map recursively Uses the seen set to avoid circular
6787
* references (e.g. x -> y, y -> x) which would cause infinite recursion
88+
* @param exp
89+
* @param map
90+
* @param seen
91+
* @return resolved expression
6892
*/
6993
private static Expression lookup(Expression exp, Map<String, Expression> map, Set<String> seen) {
7094
if (!(exp instanceof Var))

liquidjava-verifier/src/test/java/liquidjava/rj_language/opt/ExpressionSimplifierTest.java

Lines changed: 75 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,35 @@ void testComplexArithmeticWithMultipleOperations() {
304304
assertDerivationEquals(expected, result, "");
305305
}
306306

307+
@Test
308+
void testFixedPointSimplification() {
309+
// Given: x == -y && y == a / b && a == 6 && b == 3
310+
// Expected: x == -2
311+
Expression varX = new Var("x");
312+
Expression varY = new Var("y");
313+
Expression varA = new Var("a");
314+
Expression varB = new Var("b");
315+
316+
Expression aDivB = new BinaryExpression(varA, "/", varB);
317+
Expression yEqualsADivB = new BinaryExpression(varY, "==", aDivB);
318+
Expression negY = new UnaryExpression("-", varY);
319+
Expression xEqualsNegY = new BinaryExpression(varX, "==", negY);
320+
Expression six = new LiteralInt(6);
321+
Expression aEquals6 = new BinaryExpression(varA, "==", six);
322+
Expression three = new LiteralInt(3);
323+
Expression bEquals3 = new BinaryExpression(varB, "==", three);
324+
Expression firstAnd = new BinaryExpression(xEqualsNegY, "&&", yEqualsADivB);
325+
Expression secondAnd = new BinaryExpression(aEquals6, "&&", bEquals3);
326+
Expression fullExpression = new BinaryExpression(firstAnd, "&&", secondAnd);
327+
328+
// When
329+
ValDerivationNode result = ExpressionSimplifier.simplify(fullExpression);
330+
331+
// Then
332+
assertNotNull(result, "Result should not be null");
333+
assertEquals("x == -2", result.getValue().toString(), "Expected result to be x == -2");
334+
}
335+
307336
@Test
308337
void testSingleEqualityShouldNotSimplify() {
309338
// Given: x == 1
@@ -330,32 +359,61 @@ void testSingleEqualityShouldNotSimplify() {
330359
}
331360

332361
@Test
333-
void testFixedPointSimplification() {
334-
// Given: x == -y && y == a / b && a == 6 && b == 3
335-
// Expected: x == -2
362+
void testTwoEqualitiesShouldNotSimplify() {
363+
// Given: x == 1 && y == 2
364+
// Expected: x == 1 && y == 2 (should not be simplified to "true")
365+
336366
Expression varX = new Var("x");
367+
Expression one = new LiteralInt(1);
368+
Expression xEquals1 = new BinaryExpression(varX, "==", one);
369+
337370
Expression varY = new Var("y");
338-
Expression varA = new Var("a");
339-
Expression varB = new Var("b");
371+
Expression two = new LiteralInt(2);
372+
Expression yEquals2 = new BinaryExpression(varY, "==", two);
340373

341-
Expression aDivB = new BinaryExpression(varA, "/", varB);
342-
Expression yEqualsADivB = new BinaryExpression(varY, "==", aDivB);
343-
Expression negY = new UnaryExpression("-", varY);
344-
Expression xEqualsNegY = new BinaryExpression(varX, "==", negY);
345-
Expression six = new LiteralInt(6);
346-
Expression aEquals6 = new BinaryExpression(varA, "==", six);
347-
Expression three = new LiteralInt(3);
348-
Expression bEquals3 = new BinaryExpression(varB, "==", three);
349-
Expression firstAnd = new BinaryExpression(xEqualsNegY, "&&", yEqualsADivB);
350-
Expression secondAnd = new BinaryExpression(aEquals6, "&&", bEquals3);
351-
Expression fullExpression = new BinaryExpression(firstAnd, "&&", secondAnd);
374+
Expression fullExpression = new BinaryExpression(xEquals1, "&&", yEquals2);
352375

353376
// When
354377
ValDerivationNode result = ExpressionSimplifier.simplify(fullExpression);
355378

356379
// Then
357380
assertNotNull(result, "Result should not be null");
358-
assertEquals("x == -2", result.getValue().toString(), "Expected result to be x == -2");
381+
assertEquals("x == 1 && y == 2", result.getValue().toString(),
382+
"Two equalities should not be simplified to a boolean literal");
383+
384+
// The result should be the original expression unchanged
385+
assertTrue(result.getValue() instanceof BinaryExpression, "Result should still be a binary expression");
386+
BinaryExpression resultExpr = (BinaryExpression) result.getValue();
387+
assertEquals("&&", resultExpr.getOperator(), "Operator should still be &&");
388+
assertEquals("x == 1", resultExpr.getFirstOperand().toString(), "Left operand should be x == 1");
389+
assertEquals("y == 2", resultExpr.getSecondOperand().toString(), "Right operand should be y == 2");
390+
}
391+
392+
@Test
393+
void testCircularDependencyShouldNotSimplify() {
394+
// Given: x == y && y == x
395+
// Expected: x == y && y == x (should not be simplified to "true")
396+
397+
Expression varX = new Var("x");
398+
Expression varY = new Var("y");
399+
Expression xEqualsY = new BinaryExpression(varX, "==", varY);
400+
Expression yEqualsX = new BinaryExpression(varY, "==", varX);
401+
Expression fullExpression = new BinaryExpression(xEqualsY, "&&", yEqualsX);
402+
403+
// When
404+
ValDerivationNode result = ExpressionSimplifier.simplify(fullExpression);
405+
406+
// Then
407+
assertNotNull(result, "Result should not be null");
408+
assertEquals("x == y && y == x", result.getValue().toString(),
409+
"Circular dependency should not be simplified to a boolean literal");
410+
411+
// The result should be the original expression unchanged
412+
assertTrue(result.getValue() instanceof BinaryExpression, "Result should still be a binary expression");
413+
BinaryExpression resultExpr = (BinaryExpression) result.getValue();
414+
assertEquals("&&", resultExpr.getOperator(), "Operator should still be &&");
415+
assertEquals("x == y", resultExpr.getFirstOperand().toString(), "Left operand should be x == y");
416+
assertEquals("y == x", resultExpr.getSecondOperand().toString(), "Right operand should be y == x");
359417
}
360418

361419
/**

liquidjava-verifier/src/test/java/liquidjava/rj_language/opt/VariableResolverTest.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,4 +107,18 @@ void testGroupedEquality() {
107107
Map<String, Expression> result = VariableResolver.resolve(grouped);
108108
assertTrue(result.isEmpty(), "Grouped single equality should not extract variable mapping");
109109
}
110+
111+
@Test
112+
void testCircularDependency() {
113+
// x == y && y == x should not extract anything due to circular dependency
114+
Expression varX = new Var("x");
115+
Expression varY = new Var("y");
116+
117+
Expression xEqualsY = new BinaryExpression(varX, "==", varY);
118+
Expression yEqualsX = new BinaryExpression(varY, "==", varX);
119+
Expression conjunction = new BinaryExpression(xEqualsY, "&&", yEqualsX);
120+
121+
Map<String, Expression> result = VariableResolver.resolve(conjunction);
122+
assertTrue(result.isEmpty(), "Circular dependency should not extract variable mappings");
123+
}
110124
}

0 commit comments

Comments
 (0)