scope based symbol resolution; fixes generic sandwich problem; fix #13747, #17965, #13970: declarations at non-module scope now work with generics

[timotheecour]

trafficstars

this PR fixes long standing major usability issues with generics

• fix generic sandwich problem (will explain in more details)
• fix #13747 (see tests)
• fix #17965 (was already closed as same root cause; this PR adds a test)
• fix #13970 (ditto)

note

• the fix for https://github.com/nim-lang/Nim/issues/2752 introduced in https://github.com/nim-lang/Nim/commit/35f8cc0bdde8e923eaca8830676f1b2ad6ffe300 (from 2015) had broken visibility of non-module scope declarations from generics; this PR makes those work again, while still ensuring https://github.com/nim-lang/Nim/issues/2752 stays fixed
• tests/bind/tbind.nim came from tests/bind/tmixin.nim and its output was changed in https://github.com/nim-lang/Nim/pull/9318 after it was nested in block scope; the test was never meant to ensure that proc test(f: TFoo2) = echo "2" would be hidden; that was an artifact of moving it at block scope; after this PR, it now works properly and doesn't ignore these even if at block scope
• tests/generics/t13747.nim has comprehensive tests

Example 1

# funs.nim:
proc bar*[T](a: T) =
  # mixin val
  doAssert val == 1, $val
let val* = 1

# main.nim:
import funs
# const val = 2
bar(0)

before PR

(eg: devel 1.5.1: 7f077a76fea389ce4f55e18857c499fadb3958fa) nim r main passes nim r main makes the val == 1 assert fail if you uncomment const val = 2 nim r main works again if you move const val = 2; bar(0) at block or proc scope.

after PR

• all these give CT error Error: undeclared identifier: 'val'
• if you uncomment mixin val, the CT errors go away
• main.val (when const val = 2 is uncommented) is used regardless whether it's at module scope, block scope or proc scope

Example 2

(related to https://github.com/zevv/npeg/pull/33 and https://github.com/mratsim/Arraymancer/pull/508) similar to Example 1 but with:

# stack.nim:
# does not define `NPegException`
proc grow*[T](s: var Stack[T]) =
  if s.top >= s.max:
    raise newException(NPegException, s.name & " stack overflow, depth>" & $s.max)
  s.frames.setLen s.frames.len * 2

# patt.nim:
import stack, common # common contains `NPegException`
grow(Stack[int].default)

before PR

• compiles even though NPegException is not defined in stack.nim

after PR:

• does not compile unless we add for eg mixin NPegException inside stack.grow

Example 3

(related to https://github.com/status-im/nim-stint/pull/107)

# funs.nim:
template impl(T): untyped =
  when T is int: int
  else: float
type Foo*[T] = object
  f0*: impl(T)

# main.nim:
template impl(T): untyped = string # distractor
proc main[T](a: T)=
  var a: Foo[T]
  echo a.f0.type
main(1)
main(1.5)

before PR:

string string

after PR:

int float

Example 4

(related to #13747)

when true:
  proc fn[T](a: T): auto =
    mixin foo
    foo(a)
  proc main1 =
    proc foo(a: float): auto = "v1"
    doAssert fn(1.0) == "v1"
  main1()
  block:
    proc foo(a: int): auto = "v2"
    doAssert fn(1) == "v2"

before PR

CT error: Error: undeclared identifier: 'foo' even though it was mixed in

after PR

works

future work

• address generic caching issue, which is a different topic, I have some ideas how to fix this in future work after this PR

CI failures

all the PR's i sent are forward and backward compatible

• [x] npeg => https://github.com/zevv/npeg/pull/33 (forward and backward compatible)
• [x] ggplotnim => https://github.com/Vindaar/ggplotnim/pull/125
• [x] numericalnim => https://github.com/mratsim/Arraymancer/pull/508 (failed on 1 line in arraymancer fixed by that PR)
• [x] zero_functional => https://github.com/zero-functional/zero-functional/pull/70 (needed because it'd otherwise hit https://github.com/nim-lang/Nim/issues/11167)
• [x] stint => https://github.com/status-im/nim-stint/pull/107
• [x] pixie => https://github.com/treeform/pixie/pull/201 (but needs pkg "pixie", useHead = false => pkg "pixie", useHead = true to take effect)

[timotheecour]

status as of https://github.com/nim-lang/Nim/pull/18050/commits/e425f0c082fce805a8dd67b727b52c06fd753a44: nim CI passes, 3 packages broken (last commit: fixup, containing: # this is needed, see D20210519T200936 and D20210519T201000)

[Araq]

Very interesting solution but I don't really understand what it implies for the spec. Please write an RFC or at least patch the manual to reflect your approach.

[Clyybber]

This does not actually fix https://github.com/nim-lang/Nim/issues/11225 from what I can tell.

[timotheecour]

This does not actually fix #11225 from what I can tell.

#11225 was already fixed (by using bind and making bind work properly), it's a separate topic under the broader name of generic sandwich. I'm not claiming I'm fixing #11225 (and don't even reference it) in this PR.

[timotheecour]

PTAL, all important_packages now pass (each package that needed patching involved a tiny patch, typically adding a mixin, and the patch made sense in each case)

[Araq]

PTAL, all important_packages now pass (each package that needed patching involved a tiny patch, typically adding a mixin, and the patch made sense in each case)

Sorry, I missed this remark. The change must first be opt-in regardless.

[stale[bot]]

This pull request has been automatically marked as stale because it has not had recent activity. If you think it is still a valid PR, please rebase it on the latest devel; otherwise it will be closed. Thank you for your contributions.