function redefined in block scope gives: invalid type: 'None' for let depending on how it's used

[timotheecour]

Example

when true: # D20210902T094347
  proc fn1*(): int = 1
  block:
    proc fn1(): int = 2
    echo fn1() # ok: would print 2
    let z1 = fn1 # Error: invalid type: 'None' for let
    # const z2 = fn1 # Error: cannot generate VM code for fn1
    # var z3: type(fn1) # Error: invalid type: 'None' for var

Current Output

Error: invalid type: 'None' for let

Expected Output

works

Additional Information

1.5.1 fa7c1aa

[Araq]

"Expected: works" is just your opinion, in reality the 'fn1' is an overloaded symbol and it cannot be disambiguated via the scoping rules. The manual could be clearer about it, as usual, but it works as "expected" for me... We don't do "I think this should work" development anymore, there are specs for everybody to read that we try to adhere to.

[bluenote10]

there are specs for everybody to read that we try to adhere to.

As a general remark: It would be great to approach language design from the principles laid out in e.g. Don Norman's The Design of Everyday Things (in my opinion an important read as a software engineer). Expectation should be defined from the perspective of users. It is similar to weirdly operating door handles: To the creator it is always obvious how they work, but that's irrelevant for someone failing to open the door. Good design means user expectations match how things work, and having to read specs/manuals is unnecessary.

In this case: fn1 looks like a normal function and let z1 = fn1 normally works. It is clearly a head-scratcher, and I'm having a hard time understanding the "why" from my knowledge of the manual. A better error message could really help here though.

[Araq]

Error messages could always be better yes, but I don't think "Design for Everyday Things" helps. It seems to be the old "principle of least surprise" that has proven to be an utter disaster for getting a stable compiler/language.

[disruptek]

My takeaway from Design of Everyday Things is really more about providing a mental model to the consumer which is at least self-consistent and ideally consistent with the reality; this is concidentally the same point behind my prior case expr: comment. To dismiss this as "principle of least surprise" reflects either a very lazy reading of the book or an intentionally lame characterization.

Give the user an interface that they can reason about and you won't need to explain as much in the documentation. Alternatively, document the surprisingly-hard-to-reason-about features of your interface instead of throwing your hands in air and claiming that users would be less surprised if they would simply learn to expect your compiler to surprise them. Not helpful.

[konsumlamm]

@Araq why close this? The error message is still incorrect/confusing, regardless of what the expected behavior is.

[Araq]

is really more about providing a mental model to the consumer which is at least self-consistent and ideally consistent with the reality

"Proc names are not shadowed, they overload" is an easy mental model, self-consistent and consistent with reality.

[timotheecour]

"Proc names are not shadowed, they overload" is an easy mental model, self-consistent and consistent with reality.

but the inconsistency is that echo fn1() works instead of giving overload duplicate error, while let z1 = fn1 doesn't

[Araq]

Overload resolution takes the scope into account but it only kicks in for routine calls, not for routine usages. And yeah, I don't like that it takes the scope into account but it was introduced in order to allow quirky redefinitions of templates. IIrc the overrides that you can unittest.nim can only work with this feature...

[bluenote10]

"Proc names are not shadowed, they overload" is an easy mental model, self-consistent and consistent with reality.

But why does this work:

import sequtils

proc process(): int =
  discard

proc f() =

  proc process(x: int): int =
    x * x

  echo @[1, 2, 3].map(process)

f()

But not this

import sequtils

proc process(): int =
  discard

proc f() =

  proc process(x: int): int =
    x * x

  let p = process

f()

Or this:

import sequtils

proc process(x: int): int =
  discard

proc f() =

  proc process(x: int): int =
    x * x

  echo @[1, 2, 3].map(process)

f()

I'm mainly wondering about that latter example: can an import introduce an overload of process and thus break f?

[disruptek]

Maybe it's wise to decide whether a nominally deprecated unit test library should be allowed to hamper proper language design and implementation for every other program that remains to be written. 🤔

[Araq]

But why does this work: ... But not this ...

I've aleady explained it. Overload resolution kicks in for function calls, not for function usages.

[bluenote10]

I've aleady explained it. Overload resolution kicks in for function calls, not for function usages.

But @[1, 2, 3].map(process) is a usage, not a call.

EDIT: I guess it comes down to the extra context that the compiler has in this case due to the signature of map. In fact, this works as well:

proc fn1(x: int): int = 1
block:
  proc fn1(): int = 2
  let z1: proc(): int = fn1

But it breaks if one either removes the type annotation from the assignment, or the global fn1 matches the signature exactly. In any case, it is more subtle than "proc names are not shadowed, they overload".

[metagn]

Gives a "proper" but unhelpful error now, ideally it should work

Error: ambiguous identifier: 'fn1' -- use one of the following:
  a.fn1: proc (): int{.noSideEffect, gcsafe.}
  a.fn1: proc (): int{.noSideEffect, gcsafe.}