Skip to content

Commit

Permalink
Avoid duplicate calls in relation chains via refs
Browse files Browse the repository at this point in the history
Fixes #50
  • Loading branch information
edemaine committed Oct 31, 2024
1 parent d325fb7 commit e968e70
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 19 deletions.
38 changes: 28 additions & 10 deletions source/parser/op.civet
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type {
} from ./types.civet

import {
assert
makeLeftHandSideExpression
replaceNode
trimFirstSpace
Expand All @@ -15,6 +16,10 @@ import {
processPatternTest
} from ./pattern-matching.civet

import {
maybeRefAssignment
} from ./ref.civet

// Binary operator precedence, from low to high
// See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Operator_precedence#table
// NOTE: Added ^^ between || and &&, just like ^ is between | and &
Expand Down Expand Up @@ -285,7 +290,7 @@ function expandChainedComparisons([first, binops]: [ASTNode, [ASTNode, BinaryOp,
return results

function processChains: void
if chains.length > 0
if chains# > 0
// At least one relational op, so expand any existence operators `x?`
first = expandExistence first
for each index, k of chains
Expand All @@ -297,28 +302,41 @@ function expandChainedComparisons([first, binops]: [ASTNode, [ASTNode, BinaryOp,
exp := binop[3] = expandExistence binop[3]

results.push first
endIndex := chains[k + 1] ?? i + 1
results.push ...binops[start...endIndex].flat()
// TODO: add refs to ensure middle expressions are evaluated only once
// NOTE: This first gets discarded if we're in the last iteration of the chain
first = [exp] ++ binops[index + 1...endIndex]
start = endIndex
// If there's more to the chain, ref the right-hand side of this
// relation (which is the left-hand side of the next op)
if k+1 < chains#
endIndex := chains[k + 1]
rhs :=
index + 1 < endIndex ? [exp] ++ binops[index + 1...endIndex] : exp
{ ref, refAssignment } := maybeRefAssignment rhs
// TODO: do we need to recurse on the binary ops here?
binops[index][3] = makeLeftHandSideExpression refAssignment ?? rhs
results.push ...binops[start...index + 1].flat()
first = ref
start = endIndex
else
results.push ...binops[start...i + 1].flat()
else
// Advance start if there was no chain
results.push first
results.push ...binops[start...i + 1].flat()
start = i + 1
start = i + 1

chains.length = 0

function expandExistence(exp: ASTNode): ASTNode
// Expand existence operator like x?
if existence := isExistence(exp)
{ ref, refAssignment } := maybeRefAssignment existence.expression
if refAssignment?
replaceNode
existence.expression
makeLeftHandSideExpression refAssignment
existence
results.push existence, " ", chainOp, " "
existence.expression
ref
else
exp
; // avoid implicit return of hoisted function

export {
getPrecedence
Expand Down
1 change: 1 addition & 0 deletions source/parser/ref.civet
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ function needsRef(expression: ASTNode, base = "ref"): ASTRef | undefined
case "Ref":
case "Identifier":
case "Literal":
case "Placeholder":
return
}
return makeRef(base)
Expand Down
2 changes: 1 addition & 1 deletion source/parser/types.civet
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,6 @@ export type BinaryOp = (string &
) | (ChainOp &
token?: never
relational?: never
assoc?: never
)

export type NonNullAssertion
Expand All @@ -214,6 +213,7 @@ export type ChainOp
type: "ChainOp"
special: true
prec: number
assoc: string
children?: never
parent?: never

Expand Down
4 changes: 2 additions & 2 deletions source/parser/util.civet
Original file line number Diff line number Diff line change
Expand Up @@ -456,8 +456,8 @@ function deepCopy<T extends ASTNode>(root: T): T
/**
* Replace this node with another, by modifying its parent's children.
*/
function replaceNode(node: ASTNodeObject, newNode: ASTNode, parent?: ASTNodeParent): void
parent ??= node.parent
function replaceNode(node: ASTNode, newNode: ASTNode, parent?: ASTNodeParent): void
parent ??= (node as ASTNodeObject?)?.parent
unless parent?
throw new Error "replaceNode failed: node has no parent"

Expand Down
2 changes: 1 addition & 1 deletion test/binary-op.civet
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ describe "binary operations", ->
---
a+b is in c*d is in e%f+g
---
(c*d).includes(a+b) && (e%f+g).includes(c*d)
let ref;(ref = c*d).includes(a+b) && (e%f+g).includes(ref)
"""

testCase """
Expand Down
12 changes: 7 additions & 5 deletions test/chained-comparisons.civet
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,15 @@ describe "chained comparisons", ->
---
a + b < c + d < e + f
---
a + b < c + d && c + d < e + f
let ref;a + b < (ref = c + d) && ref < e + f
"""

testCase """
higher precedence
more higher precedence
---
a + b + x + y < c + d < e + f
---
a + b + x + y < c + d && c + d < e + f
let ref;a + b + x + y < (ref = c + d) && ref < e + f
"""

testCase """
Expand Down Expand Up @@ -68,9 +68,11 @@ describe "chained comparisons", ->
---
(a < b) < c
(a + b) < (c + d) < (e + f)
(a < b) < c
---
(a < b) < c;
(a + b) < (c + d) && (c + d) < (e + f)
(a < b) < c
let ref;(a + b) < (ref = (c + d)) && ref < (e + f);
(a < b) < c
"""

testCase """
Expand Down

0 comments on commit e968e70

Please sign in to comment.