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.

(cherry picked from commit 469a604)

compiler/ast.nim

@@ -518,6 +518,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)

compiler/semexprs.nim

@@ -148,6 +148,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:
@@ -3089,31 +3134,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)

compiler/semgnrc.nim

@@ -71,9 +71,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.

tests/config.nims

@@ -44,4 +44,4 @@ switch("define", "nimPreviewNonVarDestructor")
 
 switch("warningAserror", "UnnamedBreak")
 switch("legacy", "verboseTypeMismatch")
-
+switch("experimental", "genericsOpenSym")

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"

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"