Skip to content

Commit 471002d

Browse files
authored
Merge pull request #44 from elm-tooling/fix-42-precedence-crash
Attempt to fix crash with precedence
2 parents a827467 + 8aea2f8 commit 471002d

File tree

1 file changed

+35
-17
lines changed

1 file changed

+35
-17
lines changed

src/main/kotlin/org/elm/lang/core/types/OperatorPrecedence.kt

Lines changed: 35 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,37 @@ sealed class BinaryExprTree<T : Any> {
1111
data class Binary<T : Any>(val left: BinaryExprTree<T>, val operator: T, val right: BinaryExprTree<T>) : BinaryExprTree<T>()
1212

1313
companion object {
14+
// Treat "unknown op" as strictly lower than anything else so the loop won't enter.
1415
private const val DEFAULT_PRECEDENCE = -1
16+
1517
/**
1618
* Parse a list of operands and operators into a binary tree structured in evaluation
1719
* order based on precedence and associativity.
1820
*
19-
* Note that it is the caller's responsibility to ensure that all functions in the
20-
* [expression] have an associativity of [LEFT] or [RIGHT] (not [NON]) if there is more than
21-
* one operator, and that [expression] and [operatorPrecedences] are in the correct format.
22-
*
23-
* @param expression A list of [Ty]s representing an expression. The list must have odd length,
21+
* @param expression A list of [T]s representing an expression. The list must have odd length,
2422
* and all odd-indexed values must be an operator. All even-indexed values must be operands.
2523
* @param operatorPrecedences operator precedence information for the operators in
26-
* [expression]. Every function in [expression] must have an entry.
24+
* [expression]. Every *operator* in [expression] should have an entry; if one is missing,
25+
* we will treat it as unknown (lowest precedence) and not crash.
2726
*/
2827
fun <T : Any> parse(expression: List<T>, operatorPrecedences: Map<out T, OperatorPrecedence>): BinaryExprTree<T> {
28+
// Fast sanity checks to avoid hard-to-debug states.
29+
require(expression.isNotEmpty()) { "expression must not be empty" }
30+
require(expression.size % 2 == 1) {
31+
"expression must have odd length: operand op operand [op operand]… (size=${expression.size})"
32+
}
33+
// Optional: verify that odd indices are present in the precedence table.
34+
// Don't require it strictly—parser can still proceed safely.
35+
// If you want strict mode, turn the 'check' into 'require'.
36+
for (i in 1 until expression.size step 2) {
37+
if (operatorPrecedences[expression[i]] == null) {
38+
// Leave as a soft check so we don't throw; helpful during tests/dev.
39+
// You can route this to your logger if available.
40+
// e.g., LOG.debug("Unknown operator in expression at index $i: ${expression[i]}")
41+
break
42+
}
43+
}
44+
2945
return parseExpression(expression, operatorPrecedences, DEFAULT_PRECEDENCE, 0).first
3046
}
3147

@@ -41,27 +57,30 @@ sealed class BinaryExprTree<T : Any> {
4157
var left: BinaryExprTree<T> = Operand(expression[idx])
4258

4359
if (idx >= expression.lastIndex) {
44-
return left to idx + 1
60+
return left to (idx + 1)
4561
}
4662

4763
var i = idx + 1
48-
fun nextPrecedence(): Int = when {
49-
i >= expression.lastIndex -> DEFAULT_PRECEDENCE
50-
else -> {
51-
// Spread this over three lines in order to find the operation that caused an NPE.
52-
// May be folded back into one line when the bug is fixed.
53-
val index = expression[i]
54-
val operatorPrecedence = operatorPrecedences[index]
55-
operatorPrecedence!!.precedence
64+
65+
fun nextPrecedence(): Int {
66+
return if (i >= expression.lastIndex) {
67+
DEFAULT_PRECEDENCE
68+
} else {
69+
// SAFE: if the operator isn't in the table, treat it as the lowest precedence.
70+
val op = expression[i]
71+
operatorPrecedences[op]?.precedence ?: DEFAULT_PRECEDENCE
5672
}
5773
}
74+
75+
// Only enter the loop if the *next* operator binds tighter than the current precedence.
5876
while (precedence < nextPrecedence()) {
5977
val operator = expression[i]
60-
val funcPrecedence = operatorPrecedences[operator]!!
78+
val funcPrecedence = operatorPrecedences[operator] ?: break
6179
val rightPrecedence = when (funcPrecedence.associativity) {
6280
LEFT, NON -> funcPrecedence.precedence
6381
RIGHT -> funcPrecedence.precedence - 1
6482
}
83+
6584
val result = parseExpression(expression, operatorPrecedences, rightPrecedence, i + 1)
6685
left = Binary(left, operator, result.first)
6786
i = result.second
@@ -70,4 +89,3 @@ sealed class BinaryExprTree<T : Any> {
7089
}
7190
}
7291
}
73-

0 commit comments

Comments
 (0)