`when foo: bar` puts symbols declared in `foo` in scope

[timotheecour]

after upgrading to latest nim, #11985 (new gensym handling) caused the following previously working (and IMO valid) code to fail:

Example1

  template boo(): untyped =
    template fun(a: int): int = a*2
    fun(2)

  # when (block: boo()) is type: # this would work
  when boo() is type:
    type T = boo()
  else:
    let a = boo()
  echo a

Current Output1

 Error: ambiguous call; both t0686.fun(a`gensym406407: int) [declared in /Users/timothee/git_clone/nim/timn/tests/nim/all/t0686.nim(89, 14)] and t0686.fun(a`gensym406601: int) [declared in /Users/timothee/git_clone/nim/timn/tests/nim/all/t0686.nim(89, 14)] match for: (int literal(2))

Expected Output1

should compile

Example2

  when (var a = 10; false): echo (a, "ok1")
  else: echo (a, "ok1")

this gives a codegen error:

/t06886.nim.c:155:15: error: use of undeclared identifier 'a__DsmUiYxdIJpEovXpT9c5NcQ'

Example3

  template boo(): untyped =
    var a {.inject.} = 10
    false
  var a = 3.5
  when boo(): discard
  else: echo (a, "ok1")

gives:

Error: redefinition of 'a';

this is especially odd since the else branch is taken; but symbols injected in condition of when condition: foo1 else: foo2 ; rest should IMO affect neither branch foo1 nor foo2 nor rest

Possible Solution

• when foo: bar should evaluate foo in its own (block) scope, in case foo leaks inject'd declarations
• likewise with if, elif (same issue)

Additional Information

• Your Nim version (output of nim -v).
  f2e8c39

links

• also related to Stange issues for "when (compiles do: import a):" #16287

[timotheecour]

I just hit this again; it's same root cause as above:

when defined case1: # bug
  template fun(expr) =
    when typeof(expr) is int: # BUG: Error: redefinition of 'a';
      discard
    else:
      let temp = expr
  fun (var a = 1.0; a)

when defined case2: # ok
  template fun(expr) =
    type Bar = typeof(expr)
    when Bar is int:
      discard
    else:
      let temp = expr
  fun (var a = 1.0; a)

[Araq]

The manual mentions no scope for the condition of a when statement.

[timotheecour]

so we should fix both the manual and the implementation to give sane semantics instead of above behaviors.
I suggest the following:

when cond: body1
else: body2

should behave like:

const condVal {.gensym.} = block:
  cond
when condVal: body1
else: body2

so that whatever's defined in cond doesn't leak in body2; and (slightly controversial) also doesn't leak in body1

[Araq]

Well keep in mind that the scope for if is if [. cond: action .] so that if (let x = f(); x): echo x works. The idea was that if opens a scope and when doesn't. I'm not sure the typeof example is convincing, probably typeof should create a temporary scope instead.

[Araq]

So ... this needs an RFC, it would be a language change.

[timotheecour]

Well keep in mind that the scope for if is if [. cond: action .] so that if (let x = f(); x): echo x works

yes but the scope doesn't leak to the else branch of the if:

if (let x = true; let y = x; x):
  echo (x, y) # ok
else:
  echo (x, y) # error, as it should

the main problem in this issue is that the scope leaks to the else branch of the when

The idea was that if opens a scope and when doesn't.

that property is fundamental and obviously should remain; but I'm talking about the condition (cond), not the bodies of the when AST.
There are 2 parts:

• P1: cond should not leak to body1 (maybe..., not sure)
• P2: cond should not leak to body2

P2 sounds like a no brainer and can be fixed regardless of P1.. but if an RFC is needed, sure...

[Araq]

Well it's not exactly trivial and unfortunately you haven't addressed my idea: Shouldn't typeof and is open scopes instead? Then the scoping rules for when can remain simple.

[zah]

I think one possible general fix here is to agree that templates used as expressions should not inject symbols into the caller scope. Consider @timotheecour's definitions in the following example:

template boo(): untyped =
  var a {.inject.} = 10
  false

foo(bar(), bar())

Should this compile?

Alternatively, the other possible resolution is to keep the injections open and then require the code to be written like this:

template expressionScope(e: untyped): untyped =
  ## This would be a general helper for solving this problem
  block:
    e

when expressionScope(bar()) is type:
  var x = bar()

[timotheecour]

Well it's not exactly trivial

(just to clarify, no brainer doesn't imply easy to implement; it just means that it should be non-controversial)

you haven't addressed my idea: Shouldn't typeof and is open scopes instead?

yes I agree completely, except for the "instead" part: these are 2 separate issues that happen to intersect in this example

I think one possible general fix here is to agree that templates used as expressions should not inject symbols into the caller scope

likewise, for the same reason, even if your proposal is implemented, underlying problem remains.

Alternatively, the other possible resolution is to keep the injections open

the problem is you'd have to defensively use expressionScope everywhere if you are to be correct, when you don't know the contents of a template (eg when it's passed as a parameter). That's not a good thing.

it's not about template/typeof/is

it doesn't involve typeof/is/nor a template call, and yet is the same problem:

when (const x = "a".cstring; compiles($$x)): 
  discard x # whether or not this compiles is off topic here
else: discard x # BUG: this should not compile
discard x # BUG: this should not compile

IMO this should not compile, in other words, x should not escape to the else branch.

Hopefully the following argument should convince you:
there is no loss of functionality because, unlike if, when doesn't create a scope, so if user intended for definitions to leak (to else branch or to after the when statement), they could simply use:

const ok = (const x = "a".cstring; compiles($$x)) # this leaks x to current scope
when ok: discard x # ok, x defined
else: discard x # ok, x defined
discard x # ok, x defined