genericsOpenSym not respecting injections across modules

[arnetheduck]

Description

Followup on #23091

This is the same test as

Nim/tests/generics/tmacroinjectedsym.nim

Line 100 in 94c5996

proc g(T: type): string =

but split up over two modules:

testit.nim:

{.experimental: "genericsOpenSym".}

type
  Result*[T, E] = object
    when T is void:
      when E is void:
        oResultPrivate*: bool
      else:
        case oResultPrivate*: bool
        of false:
          eResultPrivate*: E
        of true:
          discard
    else:
      when E is void:
        case oResultPrivate*: bool
        of false:
          discard
        of true:
          vResultPrivate*: T
      else:
        case oResultPrivate*: bool
        of false:
          eResultPrivate*: E
        of true:
          vResultPrivate*: T

template valueOr*[T: not void, E](self: Result[T, E], def: untyped): untyped =
  let s = (self) # TODO avoid copy
  case s.oResultPrivate
  of true:
    s.vResultPrivate
  of false:
    when E isnot void:
      template error: untyped {.used, inject.} = s.eResultPrivate
    def

testit2.nim:

import testit

type Xxx = enum
  error
  value

proc f(): Result[int, cstring] =
  Result[int, cstring](oResultPrivate: false, eResultPrivate: "f")

proc g(T: type): string =
  let x = f().valueOr:
    return $error

  "ok"

doAssert g(int) == "f"

Error: unhandled exception: /tmp/testit2.nim(16, 1) `g(int) == "f"`  [AssertionDefect]

The local symbol in testit2 gets precedence over the injected symbol from testit, even though the injected symbol as a more local scope than the Xxx enum.

Nim Version

Nim Compiler Version 2.1.1 [Linux: amd64]
Compiled at 2024-02-20
Copyright (c) 2006-2024 by Andreas Rumpf

git hash: 773c066

Current Output

No response

Expected Output

No response

Possible Solution

No response

Additional Information

No response

[metagn]

The way this switch works when not enabled globally is admittedly terrible, since we want to store the information of whether a symbol should be opened so we can give a warning, caring about the switch is delayed until the instantiation context, i.e. not even in the context of the generic proc using it, but where the generic proc gets instantiated. (I'm guessing that would be the expected behavior, the switch should only matter at the generic proc declaration, when the symbols get marked as open).

I hoped that making it into its own node kind nkOpenSym as a derivative of nkSym would work so we could more cleanly store the information of what should be done but this is proving very difficult given how ingrained nkSym is for both macros and the compiler at a binary level. Probably for now we can just add another node flag and deal with it.

[arnetheduck]

Right, this I guess is a bit of a broader issue in nim, that it's hard to know what pragmas apply to in general (push vs top-level vs inside-proc etc).

I guess it's not terrible though it does make it more difficult to provide a good out-of-the-box experience for a library like results that heavily benefits from this lookup order - it would be useful to be able to enable it on specific expansions that make use of it / rely on it (like the valueOr template).

#23385 is a lower-hanging fruit perhaps where I can either warn or enforce ({.error.}) it when the template gets used.

[Araq]

Probably for now we can just add another node flag and deal with it.

Nooooo....... :-)