Variable references in function definitions don't use lexical scoping in the same scope
smking opened this issue · 4 comments
Function definitions are supposed to be closures: "When a lambda function is defined, the evaluation engine takes a snapshot of the environment and stores it with the function body definition." (https://bit.ly/3YmoKHC). However, the evaluator stores a reference instead of a snapshot of the environment, so references inside a function definition to a variable defined later in the same scope will refer to updated variable values.
For example, This returns an undefined value as expected (https://try.jsonata.org/umn7J2aFi):
(
$f := function() { $x };
$f()
)
And this returns 42 as expected because the value of $x
is defined before the function definition (https://try.jsonata.org/xvUAkj0sJ):
(
$x := 42;
$f := function() { $x };
$f()
)
However, if $x
is assigned after the function definition but before the function call, the function call will use the new value of $x
, which is unexpected behaviour (https://try.jsonata.org/ev2AQwZIW):
(
$x := 42;
$f := function() { $x };
$x := 10;
$f()
)
A possible solution is to store a copy instead of a reference to the environment here: https://github.com/jsonata-js/jsonata/blob/master/src/jsonata.js#L1568
For me current behaviour is an expected one.
This may just be a documentation issue. For example, the big example at the top of https://docs.jsonata.org/programming requires the current behavior to define $sin before $cos.
Thanks for raising this, it's an interesting point.
My initial reaction was that this is a bug for the reasons you've given, and that your proposed solution would fix it (notwithstanding that this would be a breaking change, as highlighted by the last comment).
Unfortunately, implementing this fix would preclude the ability to write recursive functions since the variable $f
that's bound to the function won't be in scope at the time that the lambda is evaluated. Even if I was to 'special case' this somehow, it would still preclude writing mutually recursive functions.
So I think we are going to have to stick with the status quo and clarify the documentation.
Thanks Andrew. This makes sense.