elves / elvish

Powerful scripting language & versatile interactive shell
https://elv.sh/
BSD 2-Clause "Simplified" License
5.64k stars 299 forks source link

Consider letting () / if / while introduce new scopes #1496

Open hanche opened 2 years ago

hanche commented 2 years ago

The docs for parenthesised output capture should mention that this construct does not introduce a new scope. It might also mention that one can always introduce a new scope, if desired:

⬥ put ({ var x = XX; put $x':'$x })
⮕ XX:XX
⬥ put $x
compilation error: 4-6 in [tty]: variable $x not found
compilation error: variable $x not found

At the same time, since parenthesised expressions are often used for the condition of if and while, the docs for those commands should also note this, perhaps just by a brief reference to the docs for parenthesised output capture.

See also #905, where this was initially discussed.

krader1961 commented 2 years ago

Note that using the ({ }) construct to keep $x from leaking into the outer scope breaks the example in #905:

> put $x
compilation error: 4-6 in [tty]: variable $x not found
compilation error: variable $x not found
[tty 1], line 1: put $x
> if ({ var x = $true; put $x }) { echo $x }
compilation error: 38-40 in [tty]: variable $x not found
compilation error: variable $x not found
[tty 2], line 1: if ({ var x = $true; put $x }) { echo $x }

It still seems to me that if, including its conditional, should introduce a new scope. That is, as if the user had typed this:

> { if (var x = $true; put $x) { echo $x } }
$true
> put $x
compilation error: 4-6 in [tty]: variable $x not found
compilation error: variable $x not found
[tty 2], line 1: put $x
hanche commented 2 years ago

Regarding the use of ({…}): Yes, but that is no problem, as it is easily fixed by using { if … } instead, as you show.

I really don't have a very strong opinion on whether if should introduce a scope or not. But for consistency's sake, at least if, while and for ought to be treated the same. And indeed, I think the case for for introducing its own scope is a bit stronger, since it usually creates a new variable. I say usually, because a simple experiment shows it will reuse an existing variable, rather than shadowing it:

⬥ var x = first
⬥ fn x { put $x }
⬥ for x [a] { put $x }
⮕ a
⬥ x
⮕ a

If the for construct were to introduce its own scope, I think it would be less surprising if for x … always behaved like { var x; for x … }

Final thought: Provided if and friends introduce a new scope, we need to consider whether and and or (and even not?) should do so, too. I think not – but at least we should make a conscious decision about it.

krader1961 commented 2 years ago

But for consistency's sake, at least if, while and for ought to be treated the same.

I agree that if, while, and for should behave the same in this regard. Each of them should introduce a new scope with the usual caveats regarding capture of vars the in the enclosing scope. In other words, as if each code block were explicitly enclosed in a { ... } code block.

P.S., Whether this discussion I started really belongs in this issue is debatable. The original problem statement was that the documentation regarding output captures would benefit from added verbiage about its lexical scope and this is only tangentially related to that observation. I started this discussion because issue #905, which is the proximate reason this issue was opened, was closed without resolving the issue of variable scope, not just for output capture, but also if, for and while.

hanche commented 2 years ago

Each of them should introduce a new scope with the usual caveats regarding capture of vars the in the enclosing scope.

A bit of caution in the case of for perhaps? With reference to the syntax for ⟨var⟩ ⟨container⟩ …, I think it might be surprising to the user if the ⟨container⟩, which could really be a parenthesised expression, was hidden inside an implicit scope.

The treatment of ⟨var⟩ is similar: Currently, if the named variable already exists, it is used, whereas if it does not, one is created. Would you retain that behaviour, suitably modified? I.e., if the named variable exists, it is captured and used, whereas if it does not, one is created, and then lost/invisible in the outer scope?

Which makes me wonder if the syntax ought to be expanded into for [var|set] ⟨var⟩ ⟨container⟩ …? The idea being that, if the var keyword is present, a new variable is always created in the implicit scope, whereas if the set keyword is present, a variable from the calling scope is used (and an error resulting if the variable does not exist). I can think of situations where either behaviour is desirable. And if you want to use either var or set as the name of the loop variable, you'd be forced to add the keyword in front, to avoid grammatical ambiguity.

(And indeed, this issue might not be an appropriate place for this discussion, but for lack of a better alternative, I guess we just roll with it.)

xiaq commented 2 years ago

The current behavior is now documented.

I'm keeping this open for now, since whether (), if, while should actually introduce new scopes might be worth revisiting later.

The behavior of for has been changed to always introduce a new variable, but maybe the variable it introduces should only be valid in the body. Filed #1553 for that.