implement genericsOpenSym for symchoices (#23873)

fixes #23865

The node flag `nfOpenSym` implemented in #23091 for sym nodes is now
also implemented for open symchoices. This means the intended behavior
is still achieved when multiple overloads are in scope to be captured,
so the issue is fixed. The code for the flag is documented and moved
into a helper proc and the experimental switch is now enabled for the
compiler test suite.

6 files changed, 122 insertions, 25 deletions

diff --git a/compiler/ast.nim b/compiler/ast.nim
index c74861bd8..624bc32f9 100644
--- a/compiler/ast.nim
+++ b/compiler/ast.nim
@@ -326,6 +326,8 @@ type
     nfHasComment # node has a comment
     nfSkipFieldChecking # node skips field visable checking
     nfOpenSym # node is a captured sym but can be overriden by local symbols
+              # ideally a unary node containing nkSym/nkOpenSymChoice or an
+              # extension over nkOpenSymChoice
 
   TNodeFlags* = set[TNodeFlag]
   TTypeFlag* = enum   # keep below 32 for efficiency reasons (now: 47)
diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim
index f727fcbfd..54a844035 100644
--- a/compiler/semexprs.nim
+++ b/compiler/semexprs.nim
@@ -157,6 +157,51 @@ proc semSymChoice(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType: P
   if result.kind == nkSym:
     result = semSym(c, result, result.sym, flags)
 
+proc semOpenSym(c: PContext, n: PNode, s: PSym, flags: TExprFlags, expectedType: PType): PNode =
+  ## sem a node marked `nfOpenSym`, that is, captured symbols that can be
+  ## replaced by newly injected symbols in generics. `s` must be the captured
+  ## symbol if the original node is an `nkSym` node; and `nil` if it is an
+  ## `nkOpenSymChoice`, in which case only non-overloadable injected symbols
+  ## will be considered.
+  result = nil
+  let ident = n.getPIdent
+  assert ident != nil
+  let id = newIdentNode(ident, n.info)
+  c.isAmbiguous = false
+  let s2 = qualifiedLookUp(c, id, {})
+  # for `nkSym`, the first found symbol being different and unambiguous is
+  # enough to replace the original
+  # for `nkOpenSymChoice`, the first found symbol must be non-overloadable,
+  # since otherwise we have to use regular `nkOpenSymChoice` functionality
+  if s2 != nil and not c.isAmbiguous and
+      ((s == nil and s2.kind notin OverloadableSyms) or
+        (s != nil and s2 != s)):
+    # only consider symbols defined under current proc:
+    var o = s2.owner
+    while o != nil:
+      if o == c.p.owner:
+        if genericsOpenSym in c.features:
+          result = semExpr(c, id, flags, expectedType)
+          return
+        else:
+          var msg =
+            "a new symbol '" & ident.s & "' has been injected during " &
+            "instantiation of " & c.p.owner.name.s & ", however "
+          if s == nil:
+            msg.add(
+              "overloads of " & ident.s & " will be used instead; " &
+              "either enable --experimental:genericsOpenSym to use the " &
+              "injected symbol or `bind` this symbol explicitly")
+          else:
+            msg.add(
+              getSymRepr(c.config, s) & " captured at " &
+              "the proc declaration will be used instead; " &
+              "either enable --experimental:genericsOpenSym to use the " &
+              "injected symbol or `bind` this captured symbol explicitly")
+          message(c.config, n.info, warnGenericsIgnoredInjection, msg)
+          break
+      o = o.owner
+
 proc inlineConst(c: PContext, n: PNode, s: PSym): PNode {.inline.} =
   result = copyTree(s.astdef)
   if result.isNil:
@@ -3141,31 +3186,17 @@ proc semExpr(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType: PType
     if isSymChoice(result):
       result = semSymChoice(c, result, flags, expectedType)
   of nkClosedSymChoice, nkOpenSymChoice:
-    result = semSymChoice(c, result, flags, expectedType)
+    if n.kind == nkOpenSymChoice and nfOpenSym in n.flags:
+      result = semOpenSym(c, n, nil, flags, expectedType)
+      if result != nil:
+        return
+    result = semSymChoice(c, n, flags, expectedType)
   of nkSym:
     let s = n.sym
     if nfOpenSym in n.flags:
-      let id = newIdentNode(s.name, n.info)
-      c.isAmbiguous = false
-      let s2 = qualifiedLookUp(c, id, {})
-      if s2 != nil and s2 != s and not c.isAmbiguous:
-        # only consider symbols defined under current proc:
-        var o = s2.owner
-        while o != nil:
-          if o == c.p.owner:
-            if genericsOpenSym in c.features:
-              result = semExpr(c, id, flags, expectedType)
-              return
-            else:
-              message(c.config, n.info, warnGenericsIgnoredInjection,
-                "a new symbol '" & s.name.s & "' has been injected during " &
-                "instantiation of " & c.p.owner.name.s & ", " &
-                "however " & getSymRepr(c.config, s) & " captured at " &
-                "the proc declaration will be used instead; " &
-                "either enable --experimental:genericsOpenSym to use the " &
-                "injected symbol or `bind` this captured symbol explicitly")
-              break
-          o = o.owner
+      result = semOpenSym(c, n, s, flags, expectedType)
+      if result != nil:
+        return
     # because of the changed symbol binding, this does not mean that we
     # don't have to check the symbol for semantics here again!
     result = semSym(c, n, s, flags)
diff --git a/compiler/semgnrc.nim b/compiler/semgnrc.nim
index 638b4311b..e914cf00d 100644
--- a/compiler/semgnrc.nim
+++ b/compiler/semgnrc.nim
@@ -72,9 +72,9 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym,
         result.transitionSonsKind(nkClosedSymChoice)
     else:
       result = symChoice(c, n, s, scOpen)
-      if result.kind == nkSym and canOpenSym(result.sym):
+      if canOpenSym(s):
         result.flags.incl nfOpenSym
-        result.typ = nil
+        if result.kind == nkSym: result.typ = nil
   case s.kind
   of skUnknown:
     # Introduced in this pass! Leave it as an identifier.
diff --git a/tests/config.nims b/tests/config.nims
index 690123c4a..91934cc74 100644
--- a/tests/config.nims
+++ b/tests/config.nims
@@ -45,3 +45,4 @@ switch("define", "nimPreviewNonVarDestructor")
 switch("warningAserror", "UnnamedBreak")
 switch("legacy", "verboseTypeMismatch")
 switch("experimental", "vtables")
+switch("experimental", "genericsOpenSym")
diff --git a/tests/generics/tmacroinjectedsym.nim b/tests/generics/tmacroinjectedsym.nim
index d36d34cdd..5271a667f 100644
--- a/tests/generics/tmacroinjectedsym.nim
+++ b/tests/generics/tmacroinjectedsym.nim
@@ -113,3 +113,62 @@ block: # issue #22605, original complex example
     "ok"
 
   doAssert g2(int) == "error"
+
+block: # issue #23865
+  type Xxx = enum
+    error
+    value
+
+  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
+
+  func error[T, E](self: Result[T, E]): E =
+    ## Fetch error of result if set, or raise Defect
+    case self.oResultPrivate
+    of true:
+      when T isnot void:
+        raiseResultDefect("Trying to access error when value is set", self.vResultPrivate)
+      else:
+        raiseResultDefect("Trying to access error when value is set")
+    of false:
+      when E isnot void:
+        self.eResultPrivate
+
+  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
+  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) == "error"
diff --git a/tests/generics/tmacroinjectedsymwarning.nim b/tests/generics/tmacroinjectedsymwarning.nim
index 7adb759e8..43d11aed7 100644
--- a/tests/generics/tmacroinjectedsymwarning.nim
+++ b/tests/generics/tmacroinjectedsymwarning.nim
@@ -1,3 +1,7 @@
+discard """
+  matrix: "--skipParentCfg --filenames:legacyRelProj"
+"""
+
 type Xxx = enum
   error
   value
@@ -43,7 +47,7 @@ proc f(): Result[int, cstring] =
 proc g(T: type): string =
   let x = f().valueOr:
     return $error #[tt.Warning
-            ^ a new symbol 'error' has been injected during instantiation of g, however 'error' [enumField declared in tmacroinjectedsymwarning.nim(2, 3)] captured at the proc declaration will be used instead; either enable --experimental:genericsOpenSym to use the injected symbol or `bind` this captured symbol explicitly [GenericsIgnoredInjection]]#
+            ^ a new symbol 'error' has been injected during instantiation of g, however 'error' [enumField declared in tmacroinjectedsymwarning.nim(6, 3)] captured at the proc declaration will be used instead; either enable --experimental:genericsOpenSym to use the injected symbol or `bind` this captured symbol explicitly [GenericsIgnoredInjection]]#
 
   "ok"