Skip to content

Commit 10b067f

Browse files
rcosta358rcosta358
authored
Fix Usage Detection for Repeated Equalities (#226)
1 parent 1d0a755 commit 10b067f

4 files changed

Lines changed: 147 additions & 38 deletions

File tree

‎CONTRIBUTING.md‎

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,27 +7,40 @@ Thanks for your interest in contributing! This guide covers the essentials.
77
- Java 20+
88
- Maven 3.6+
99

10-
## Build & test
10+
## Build and Run
11+
12+
To build the project, run:
1113

1214
```bash
13-
mvn clean install # build everything
14-
mvn test # run the test suite
15+
mvn clean install
1516
```
1617

17-
Run a single test:
18+
Code formatting runs automatically via `formatter-maven-plugin` during the `validate` phase.
19+
20+
To verify a single file from the CLI, run:
1821

1922
```bash
20-
mvn -pl liquidjava-verifier -Dtest=TestExamples#testMultiplePaths test
23+
./liquidjava path/to/File.java
2124
```
2225

23-
Verify a single file from the CLI:
26+
This script recompiles `liquidjava-api` and `liquidjava-verifier` when local sources or Maven files have changed.
27+
28+
## Testing
29+
30+
To run all tests, run:
2431

2532
```bash
26-
./liquidjava path/to/File.java
33+
mvn test
2734
```
2835

29-
The launcher recompiles `liquidjava-api` and `liquidjava-verifier` only when local sources or Maven files have changed.
30-
Code formatting runs automatically via `formatter-maven-plugin` during the `validate` phase.
36+
To run specific tests, run:
37+
38+
```bash
39+
mvn -pl liquidjava-verifier -Dtest=ExpressionFormatterTest test
40+
mvn -pl liquidjava-verifier -Dtest=ExpressionSimplifierTest test
41+
mvn -pl liquidjava-verifier -Dtest=RefinementsParserTest test
42+
mvn -pl liquidjava-verifier -Dtest=VariableResolverTest test
43+
```
3144

3245
## Release
3346

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

Lines changed: 21 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public static Map<String, Expression> resolve(Expression exp) {
2727
resolveRecursive(exp, map);
2828

2929
// remove variables that were not used in the expression
30-
map.entrySet().removeIf(entry -> !hasUsage(exp, entry.getKey(), entry.getValue()));
30+
map.entrySet().removeIf(entry -> !hasUsage(exp, entry.getKey(), entry.getValue(), true));
3131

3232
// transitively resolve variables
3333
return resolveTransitive(map);
@@ -136,43 +136,35 @@ private static Expression lookup(Expression exp, Map<String, Expression> map, Se
136136
*
137137
* @param exp
138138
* @param name
139+
* @param value
140+
* @param topLevel
139141
*
140142
* @return true if used, false otherwise
141143
*/
142-
private static boolean hasUsage(Expression exp, String name, Expression value) {
143-
// exclude own definitions
144-
if (exp instanceof BinaryExpression binary && "==".equals(binary.getOperator())) {
144+
private static boolean hasUsage(Expression exp, String name, Expression value, boolean topLevel) {
145+
if (topLevel && exp instanceof BinaryExpression binary && "&&".equals(binary.getOperator())) {
146+
return hasUsage(binary.getFirstOperand(), name, value, true)
147+
|| hasUsage(binary.getSecondOperand(), name, value, true);
148+
}
149+
150+
if (topLevel && exp instanceof BinaryExpression binary && "==".equals(binary.getOperator())) {
145151
Expression left = binary.getFirstOperand();
146152
Expression right = binary.getSecondOperand();
147-
if (left instanceof Var v && v.getName().equals(name) && right.equals(value)
148-
&& (isConstant(right) || (!(right instanceof Var) && canSubstitute(v, right))))
149-
return false;
150-
if (left instanceof FunctionInvocation && left.toString().equals(name) && right.equals(value)
151-
&& (isConstant(right) || (!(right instanceof Var) && !containsExpression(right, left))))
152-
return false;
153-
if (right instanceof Var v && v.getName().equals(name) && left.equals(value) && isConstant(left))
154-
return false;
155-
if (right instanceof FunctionInvocation && right.toString().equals(name) && left.equals(value)
156-
&& isConstant(left))
153+
boolean leftDefinition = name.equals(substitutionKey(left)) && right.equals(value)
154+
&& (isConstant(right)
155+
|| (left instanceof Var v && !(right instanceof Var) && canSubstitute(v, right))
156+
|| (left instanceof FunctionInvocation && !(right instanceof Var)
157+
&& !containsExpression(right, left)));
158+
boolean rightDefinition = name.equals(substitutionKey(right)) && left.equals(value) && isConstant(left);
159+
if (leftDefinition || rightDefinition)
157160
return false;
158161
}
159162

160-
// usage found
161-
if (exp instanceof Var var && var.getName().equals(name)) {
162-
return true;
163-
}
164-
if (exp instanceof FunctionInvocation && exp.toString().equals(name)) {
163+
if (name.equals(substitutionKey(exp)))
165164
return true;
166-
}
167-
168-
// recurse children
169-
if (exp.hasChildren()) {
170-
for (Expression child : exp.getChildren())
171-
if (hasUsage(child, name, value))
172-
return true;
173-
}
174-
175-
// usage not found
165+
for (Expression child : exp.getChildren())
166+
if (hasUsage(child, name, value, false))
167+
return true;
176168
return false;
177169
}
178170

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

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -658,4 +658,53 @@ void testEnumConstantsPropagateIntoTernaryCondition() {
658658

659659
assertEquals("start(param)", result.getValue().toString());
660660
}
661+
662+
@Test
663+
void testRepeatedEqualDefinitionPropagatesIntoTernaryCondition() {
664+
Expression expression = parse("mode == 2 && (mode == 2 ? explicit(param) : start(param))");
665+
ValDerivationNode result = ExpressionSimplifier.simplify(expression);
666+
667+
assertEquals("explicit(param)", result.getValue().toString());
668+
}
669+
670+
@Test
671+
void testRepeatedEqualDefinitionsPropagateIntoCompoundTernaryCondition() {
672+
Expression expression = parse(
673+
"mode == 2 && other == 5 && ((mode == 2 && other == 5) ? explicit(param) : start(param))");
674+
ValDerivationNode result = ExpressionSimplifier.simplify(expression);
675+
676+
assertEquals("explicit(param)", result.getValue().toString());
677+
}
678+
679+
@Test
680+
void testRepeatedEqualDefinitionPropagatesWhenUsageComesFirst() {
681+
Expression expression = parse("(mode == 2 ? explicit(param) : start(param)) && mode == 2");
682+
ValDerivationNode result = ExpressionSimplifier.simplify(expression);
683+
684+
assertEquals("explicit(param)", result.getValue().toString());
685+
}
686+
687+
@Test
688+
void testRepeatedFunctionInvocationDefinitionPropagatesIntoTernaryCondition() {
689+
Expression expression = parse("modeOf(param) == 2 && (modeOf(param) == 2 ? explicit(param) : start(param))");
690+
ValDerivationNode result = ExpressionSimplifier.simplify(expression);
691+
692+
assertEquals("explicit(param)", result.getValue().toString());
693+
}
694+
695+
@Test
696+
void testRepeatedEqualDefinitionsPropagateIntoNestedTernaryConditions() {
697+
Expression expression = parse("mode == 2 && other == 3 && (mode == 2 ? (other == 3 ? a(p) : b(p)) : c(p))");
698+
ValDerivationNode result = ExpressionSimplifier.simplify(expression);
699+
700+
assertEquals("a(p)", result.getValue().toString());
701+
}
702+
703+
@Test
704+
void testRepeatedEqualDefinitionsPropagateIntoNestedTernaryElseBranch() {
705+
Expression expression = parse("mode == 2 && other == 4 && (mode == 2 ? (other == 3 ? a(p) : b(p)) : c(p))");
706+
ValDerivationNode result = ExpressionSimplifier.simplify(expression);
707+
708+
assertEquals("b(p)", result.getValue().toString());
709+
}
661710
}

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

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,61 @@ void testDifferentEqualityInIteConditionCountsAsUsage() {
9999
assertEquals("1", result.get("mode").toString());
100100
}
101101

102+
@Test
103+
void testRepeatedEqualDefinitionCountsAsUsage() {
104+
Expression expression = parse("mode == 2 && (mode == 2 ? explicit(param) : start(param))");
105+
Map<String, Expression> result = VariableResolver.resolve(expression);
106+
107+
assertEquals(1, result.size(), "Repeated equalities should keep one definition and treat the other as usage");
108+
assertEquals("2", result.get("mode").toString());
109+
}
110+
111+
@Test
112+
void testRepeatedEqualDefinitionsInCompoundIteConditionCountAsUsage() {
113+
Expression expression = parse(
114+
"mode == 2 && other == 5 && ((mode == 2 && other == 5) ? explicit(param) : start(param))");
115+
Map<String, Expression> result = VariableResolver.resolve(expression);
116+
117+
assertEquals(2, result.size(), "Compound ternary condition should count as a use of both variables");
118+
assertEquals("2", result.get("mode").toString());
119+
assertEquals("5", result.get("other").toString());
120+
}
121+
122+
@Test
123+
void testRepeatedEqualDefinitionCountsAsUsageBeforeDefinitionConjunct() {
124+
Expression expression = parse("(mode == 2 ? explicit(param) : start(param)) && mode == 2");
125+
Map<String, Expression> result = VariableResolver.resolve(expression);
126+
127+
assertEquals(1, result.size(), "Conjunct order should not affect repeated equality usage detection");
128+
assertEquals("2", result.get("mode").toString());
129+
}
130+
131+
@Test
132+
void testRepeatedFunctionInvocationDefinitionCountsAsUsage() {
133+
Expression expression = parse("modeOf(param) == 2 && (modeOf(param) == 2 ? explicit(param) : start(param))");
134+
Map<String, Expression> result = VariableResolver.resolve(expression);
135+
136+
assertEquals(1, result.size(), "Function invocation definition should count repeated nested equality as usage");
137+
assertEquals("2", result.get("modeOf(param)").toString());
138+
}
139+
140+
@Test
141+
void testRepeatedExpressionDefinitionCountsAsUsage() {
142+
Expression expression = parse("limit == max - 1 && (limit == max - 1 ? a(p) : b(p))");
143+
Map<String, Expression> result = VariableResolver.resolve(expression);
144+
145+
assertEquals(1, result.size(), "Expression definition should count repeated nested equality as usage");
146+
assertEquals("max - 1", result.get("limit").toString());
147+
}
148+
149+
@Test
150+
void testRepeatedTopLevelDefinitionsOnlyDoNotCountAsUsage() {
151+
Expression expression = parse("mode == 2 && mode == 2");
152+
Map<String, Expression> result = VariableResolver.resolve(expression);
153+
154+
assertTrue(result.isEmpty(), "Repeated top-level definitions alone should not count as usage");
155+
}
156+
102157
@Test
103158
void testReturnVariableIsNotSubstituted() {
104159
Expression expression = parse("x > 0 && #ret_1 == x");

0 commit comments

Comments
 (0)