fix #13502 a and b now short-circuits semcheck in VM

[timotheecour]

• fix when a() and b() should short-circuit semcheck #13502
• when declared(Foo) and T is Foo: discard now works, and more generally, in a static context, a and b now short-circuits semcheck in a static section (eg const a = false and nonexistant), meaning that b doesn't have to type-check if a is false
• see added tests

• TODO: also do short-circuit for or

[Araq]

Too hacky. I understand why you need to do it this way but I wonder if there is a better way...

[Araq]

Tried to fix this line instead:

  # semfold.nim
  of mBitandI, mAnd: result = newIntNodeT(bitand(a.getInt, b.getInt), n, g)

Or to add logic to semmagic.magicsAfterOverloadResolution? Sem'checking both operands for and and or does make sense.

[timotheecour]

and or

good point, or should also short-circuit, I will update PR after the design for and is accepted (should be straightforward to adapt)

But for the remaining points:

How could that work? if you semcheck the 2nd operand for and , const a = false and nonexistant would not compile; nor would when declared(Foo) and T is Foo: discard

Error: undeclared identifier would be issued before code reaches semmagic.magicsAfterOverloadResolution or semfold.evalOp.semcheck, which is why I'm doing it instead in semMagic

If you nim c -r main with:

static:echo "ok1"
const a = false and nonexistant

and you add

# semfold.nim
  of mBitandI, mAnd: 
    echo ("semfold", g.config$a.info,)
    ...

and

# semmagic
proc magicsAfterOverloadResolution(c: PContext, n: PNode, flags: TExprFlags): PNode =
  echo ("magicsAfterOverloadResolution", c.config$n.info,)
  ...

you'll see:

ok1
main.nim: Error: undeclared identifier: 'nonexistant'

[Araq]

How could that work? if you semcheck the 2nd operand for and , const a = false and nonexistant would not compile; nor would when declared(Foo) and T is Foo: discard
Error: undeclared identifier would be issued before code reaches

Exactly. As it should be. I'm sorry, I seem to have misunderstood this point previously. Short cut evaluation refers to evaluation, not to the semantic checking of and and or.

[timotheecour]

you seemed to agree here #13502 (comment) it should be legal

This is a bug, not a feature request. In fact, it's a duplicate of some other bug report.

feature or bug is a matter of perspective, here's a motivating example from https://github.com/nim-lang/Nim/pull/13498/files#diff-7eb4d1654e7ba69a150250a34148c9c9

template ctAnd(a, b): bool =
  # pending https://github.com/nim-lang/Nim/issues/13502
  when a:
    when b: true
    else: false
  else: false

template initImpl(result: typed, size: int) =
  when ctAnd(declared(SharedTable), type(result) is SharedTable):
    init(result, size)
  else: ...

or, without ctAnd you'd need to duplicate the else branch (worst option) or write boilerplate:

template initImpl(result: typed, size: int) =
  template fallback() = ...
  when declared(SharedTable):
    when type(result) is SharedTable:
      init(result, size)
    else:
      fallback()
  else:
    fallback()

with this PR you'd simply write:

template initImpl(result: typed, size: int) =
  when declared(SharedTable) and type(result) is SharedTable:
    init(result, size)
  else: ...

which is arguably simpler. Are you implying #13502 should be closed as wontfix? I don't see downsides and only upsides to making when declared(Foo) and T is Foo work. If it can't be accepted, then we should add ctAnd, ctOr to stdlib

[Araq]

which is arguably simpler.

Well it's simpler to use but a weird special case.

[timotheecour]

note that I ensured it only affects static/when/const blocks, not general VM code, so that:

template fun() = # ditto with generics, procs etc
  let a = b and c # uses full semcheck despite running inside VM
  const a2 = b and c # uses short-circuit (ditto with when b and c)
static: fun()

seems reasonable feature, if feature is accepted I can make sure doc is clear

[Araq]

Sorry, but I'm really happier without this special case, regardless of "only const/when/static blocks" are affected or not.

[timotheecour]

:-( would you accept a modified PR that adds insteadsugar.lazyAnd, sugar.lazyOr ?

template lazyAnd*(lhs: bool, rhs: untyped): bool =
  when lhs: rhs
  else: false

and analog for lazyOr

[Araq]

As I said, do not circumvent the RFC process. lazyAnd is a good name, but consider that your very own implementation is a single when statement, why not write it as this when the need arises:

when (when declared(SharedTable): (typeof(result) is SharedTable) else: false):
  code here

[Araq]

This new way of "fixing" things by adding implementation details to system.nim is not acceptable.

[Araq]

Rejected for now, nesting when works and doesn't introduce more weird special casing in the language.

[timotheecour]

this could be revisited using nim-lang/RFCs#390

when declared(Foo) 'vmand (T is Foo): discard