opensym as node kind + fixed experimental switch (#23892)

refs #23873 (comment),
fixes #23386, fixes #23385, supersedes #23572

Turns the `nfOpenSym` node flag implemented in #23091 and extended in
containing either `nkSym` or `nkOpenSymChoice`. Since this affects
macros working on generic proc AST, the node kind is now only generated
when the experimental switch `genericsOpenSym` is enabled, and a new
node flag `nfDisabledOpenSym` is set to the `nkSym` or `nkOpenSymChoice`
when the switch is not enabled so that we can give a warning.

Now that the experimental switch has more reasonable semantics, we
define `nimHasGenericsOpenSym2`.

(cherry picked from commit 0c890ff)

changelog.md

@@ -33,7 +33,9 @@ slots when enlarging a sequence.
 - An experimental option `genericsOpenSym` has been added to allow captured
   symbols in generic routine bodies to be replaced by symbols injected locally
   by templates/macros at instantiation time. `bind` may be used to keep the
-  captured symbols over the injected ones regardless of enabling the option.
+  captured symbols over the injected ones regardless of enabling the option,
+  but other methods like renaming the captured symbols should be used instead
+  so that the code is not affected by context changes.
 
   Since this change may affect runtime behavior, the experimental switch
   `genericsOpenSym` needs to be enabled, and a warning is given in the case
@@ -65,6 +67,13 @@ slots when enlarging a sequence.
   assert baz[int]() == "captured"
   ```
 
+  This option also generates a new node kind `nnkOpenSym` which contains
+  exactly 1 of either an `nnkSym` or an `nnkOpenSymChoice` node. In the future
+  this might be merged with a slightly modified `nnkOpenSymChoice` node but
+  macros that want to support the experimental feature should still handle
+  `nnkOpenSym`, as the node kind would simply not be generated as opposed to
+  being removed.
+
 ## Compiler changes
 
 

compiler/ast.nim

@@ -227,6 +227,7 @@ type
     nkModuleRef           # for .rod file support: A (moduleId, itemId) pair
     nkReplayAction        # for .rod file support: A replay action
     nkNilRodNode          # for .rod file support: a 'nil' PNode
+    nkOpenSym             # container for captured sym that can be overriden by local symbols
 
   TNodeKinds* = set[TNodeKind]
 
@@ -517,9 +518,9 @@ type
     nfFirstWrite # this node is a first write
     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
+    nfDisabledOpenSym # temporary: node should be nkOpenSym but cannot
+                      # because genericsOpenSym experimental switch is disabled
+                      # gives warning instead
 
   TNodeFlags* = set[TNodeFlag]
   TTypeFlag* = enum   # keep below 32 for efficiency reasons (now: 47)
@@ -1093,7 +1094,7 @@ const
                                       nfFromTemplate, nfDefaultRefsParam,
                                       nfExecuteOnReload, nfLastRead,
                                       nfFirstWrite, nfSkipFieldChecking,
-                                      nfOpenSym}
+                                      nfDisabledOpenSym}
   namePos* = 0
   patternPos* = 1    # empty except for term rewriting macros
   genericParamsPos* = 2
@@ -1137,6 +1138,7 @@ proc getPIdent*(a: PNode): PIdent {.inline.} =
   of nkSym: a.sym.name
   of nkIdent: a.ident
   of nkOpenSymChoice, nkClosedSymChoice: a.sons[0].sym.name
+  of nkOpenSym: getPIdent(a.sons[0])
   else: nil
 
 const

compiler/condsyms.nim

@@ -160,3 +160,4 @@ proc initDefines*(symbols: StringTableRef) =
   defineSymbol("nimHasNoReturnError")
 
   defineSymbol("nimHasCastExtendedVm")
+  defineSymbol("nimHasGenericsOpenSym2")

compiler/docgen.nim

@@ -821,7 +821,7 @@ proc getName(n: PNode): string =
     result = "`"
     for i in 0..<n.len: result.add(getName(n[i]))
     result = "`"
-  of nkOpenSymChoice, nkClosedSymChoice:
+  of nkOpenSymChoice, nkClosedSymChoice, nkOpenSym:
     result = getName(n[0])
   else:
     result = ""
@@ -839,7 +839,7 @@ proc getNameIdent(cache: IdentCache; n: PNode): PIdent =
     var r = ""
     for i in 0..<n.len: r.add(getNameIdent(cache, n[i]).s)
     result = getIdent(cache, r)
-  of nkOpenSymChoice, nkClosedSymChoice:
+  of nkOpenSymChoice, nkClosedSymChoice, nkOpenSym:
     result = getNameIdent(cache, n[0])
   else:
     result = nil
@@ -853,7 +853,7 @@ proc getRstName(n: PNode): PRstNode =
   of nkAccQuoted:
     result = getRstName(n[0])
     for i in 1..<n.len: result.text.add(getRstName(n[i]).text)
-  of nkOpenSymChoice, nkClosedSymChoice:
+  of nkOpenSymChoice, nkClosedSymChoice, nkOpenSym:
     result = getRstName(n[0])
   else:
     result = nil

compiler/ic/ic.nim

@@ -768,6 +768,8 @@ proc loadNodes*(c: var PackedDecoder; g: var PackedModuleGraph; thisModule: int;
     result.ident = getIdent(c.cache, g[thisModule].fromDisk.strings[n.litId])
   of nkSym:
     result.sym = loadSym(c, g, thisModule, PackedItemId(module: LitId(0), item: tree.nodes[n.int].operand))
+    if result.typ == nil:
+      result.typ = result.sym.typ
   of directIntLit:
     result.intVal = tree.nodes[n.int].operand
   of externIntLit:
@@ -782,6 +784,8 @@ proc loadNodes*(c: var PackedDecoder; g: var PackedModuleGraph; thisModule: int;
     assert n2.kind == nkInt32Lit
     transitionNoneToSym(result)
     result.sym = loadSym(c, g, thisModule, PackedItemId(module: n1.litId, item: tree.nodes[n2.int].operand))
+    if result.typ == nil:
+      result.typ = result.sym.typ
   else:
     for n0 in sonsReadonly(tree, n):
       result.addAllowNil loadNodes(c, g, thisModule, tree, n0)

compiler/lookups.nim

@@ -61,6 +61,8 @@ proc considerQuotedIdent*(c: PContext; n: PNode, origin: PNode = nil): PIdent =
       result = n[0].sym.name
     else:
       handleError(n, origin)
+  of nkOpenSym:
+    result = considerQuotedIdent(c, n[0], origin)
   else:
     handleError(n, origin)
 
@@ -676,6 +678,10 @@ proc qualifiedLookUp*(c: PContext, n: PNode, flags: set[TLookupFlag]): PSym =
     if result != nil and result.kind == skStub: loadStub(result)
 
 proc initOverloadIter*(o: var TOverloadIter, c: PContext, n: PNode): PSym =
+  if n.kind == nkOpenSym:
+    # maybe the logic in semexprs should be mirrored here instead
+    # for now it only seems this is called for `pickSym` in `getTypeIdent` 
+    return initOverloadIter(o, c, n[0])
   o.importIdx = -1
   o.marked = initIntSet()
   case n.kind

compiler/options.nim

@@ -221,7 +221,7 @@ type
     flexibleOptionalParams,
     strictDefs,
     strictCaseObjects,
-    genericsOpenSym
+    genericsOpenSym # remove nfDisabledOpenSym when this switch is default
 
   LegacyFeature* = enum
     allowSemcheckedAstModification,

compiler/renderer.nim

@@ -515,6 +515,7 @@ proc lsub(g: TSrcGen; n: PNode): int =
     result = if n.len > 0: lcomma(g, n) + 2 else: len("{:}")
   of nkClosedSymChoice, nkOpenSymChoice:
     if n.len > 0: result += lsub(g, n[0])
+  of nkOpenSym: result = lsub(g, n[0])
   of nkTupleTy: result = lcomma(g, n) + len("tuple[]")
   of nkTupleClassTy: result = len("tuple")
   of nkDotExpr: result = lsons(g, n) + 1
@@ -1019,7 +1020,7 @@ proc bracketKind*(g: TSrcGen, n: PNode): BracketKind =
 proc skipHiddenNodes(n: PNode): PNode =
   result = n
   while result != nil:
-    if result.kind in {nkHiddenStdConv, nkHiddenSubConv, nkHiddenCallConv} and result.len > 1:
+    if result.kind in {nkHiddenStdConv, nkHiddenSubConv, nkHiddenCallConv, nkOpenSym} and result.len > 1:
       result = result[1]
     elif result.kind in {nkCheckedFieldExpr, nkHiddenAddr, nkHiddenDeref, nkStringToCString, nkCStringToString} and
         result.len > 0:
@@ -1279,6 +1280,7 @@ proc gsub(g: var TSrcGen, n: PNode, c: TContext, fromStmtList = false) =
       put(g, tkParRi, if n.kind == nkOpenSymChoice: "|...)" else: ")")
     else:
       gsub(g, n, 0)
+  of nkOpenSym: gsub(g, n, 0)
   of nkPar, nkClosure:
     put(g, tkParLe, "(")
     gcomma(g, n, c)

compiler/semexprs.nim

@@ -148,13 +148,14 @@ 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
+proc semOpenSym(c: PContext, n: PNode, flags: TExprFlags, expectedType: PType,
+                warnDisabled = false): PNode =
+  ## sem the child of an `nkOpenSym` node, 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 isSym = n.kind == nkSym
   let ident = n.getPIdent
   assert ident != nil
   let id = newIdentNode(ident, n.info)
@@ -167,36 +168,41 @@ proc semOpenSym(c: PContext, n: PNode, s: PSym, flags: TExprFlags, expectedType:
   # but of the overloadable sym kinds, semExpr does not handle skModule, skMacro, skTemplate
   # as overloaded in the case where `nkIdent` finds them first
   if s2 != nil and not c.isAmbiguous and
-      ((s == nil and s2.kind notin OverloadableSyms-{skModule, skMacro, skTemplate}) or
-        (s != nil and s2 != s)):
+      ((isSym and s2 != n.sym) or
+        (not isSym and s2.kind notin OverloadableSyms-{skModule, skMacro, skTemplate})):
     # only consider symbols defined under current proc:
     var o = s2.owner
     while o != nil:
       if o == c.p.owner:
-        if genericsOpenSym in c.features:
+        if not warnDisabled:
           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:
+          if isSym:
             msg.add(
-              "overloads of " & ident.s & " will be used instead; " &
+              getSymRepr(c.config, n.sym) & " captured at " &
+              "the proc declaration will be used instead; " &
               "either enable --experimental:genericsOpenSym to use the " &
-              "injected symbol or `bind` this symbol explicitly")
+              "injected symbol or `bind` this captured symbol explicitly")
           else:
             msg.add(
-              getSymRepr(c.config, s) & " captured at " &
-              "the proc declaration will be used instead; " &
+              "overloads of " & ident.s & " will be used instead; " &
               "either enable --experimental:genericsOpenSym to use the " &
-              "injected symbol or `bind` this captured symbol explicitly")
+              "injected symbol or `bind` this symbol explicitly")
           message(c.config, n.info, warnGenericsIgnoredInjection, msg)
           break
       o = o.owner
-  if s == nil:
-    # set symchoice node type back to None
-    n.typ = newTypeS(tyNone, c)
+  # nothing found
+  if not warnDisabled:
+    result = semExpr(c, n, flags, expectedType)
+  else:
+    result = nil
+    if not isSym:
+      # set symchoice node type back to None
+      n.typ = newTypeS(tyNone, c)
 
 proc inlineConst(c: PContext, n: PNode, s: PSym): PNode {.inline.} =
   result = copyTree(s.astdef)
@@ -2177,6 +2183,8 @@ proc lookUpForDeclared(c: PContext, n: PNode, onlyCurrentScope: bool): PSym =
     result = n.sym
   of nkOpenSymChoice, nkClosedSymChoice:
     result = n[0].sym
+  of nkOpenSym:
+    result = lookUpForDeclared(c, n[0], onlyCurrentScope)
   else:
     localError(c.config, n.info, "identifier expected, but got: " & renderTree(n))
     result = nil
@@ -2632,7 +2640,9 @@ proc semWhen(c: PContext, n: PNode, semCheck = true): PNode =
   var typ = commonTypeBegin
   if n.len in 1..2 and n[0].kind == nkElifBranch and (
       n.len == 1 or n[1].kind == nkElse):
-    let exprNode = n[0][0]
+    var exprNode = n[0][0]
+    if exprNode.kind == nkOpenSym:
+      exprNode = exprNode[0]
     if exprNode.kind == nkIdent:
       whenNimvm = lookUp(c, exprNode).magic == mNimvm
     elif exprNode.kind == nkSym:
@@ -3139,20 +3149,22 @@ proc semExpr(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType: PType
     if isSymChoice(result):
       result = semSymChoice(c, result, flags, expectedType)
   of nkClosedSymChoice, nkOpenSymChoice:
-    if n.kind == nkOpenSymChoice and nfOpenSym in n.flags:
-      result = semOpenSym(c, n, nil, flags, expectedType)
-      if result != nil:
-        return
+    if nfDisabledOpenSym in n.flags:
+      let res = semOpenSym(c, n, flags, expectedType, warnDisabled = true)
+      assert res == nil
     result = semSymChoice(c, n, flags, expectedType)
   of nkSym:
     let s = n.sym
-    if nfOpenSym in n.flags:
-      result = semOpenSym(c, n, s, flags, expectedType)
-      if result != nil:
-        return
+    if nfDisabledOpenSym in n.flags:
+      let res = semOpenSym(c, n, flags, expectedType, warnDisabled = true)
+      assert res == nil
     # 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)
+  of nkOpenSym:
+    assert n.len == 1
+    let inner = n[0]
+    result = semOpenSym(c, inner, flags, expectedType)
   of nkEmpty, nkNone, nkCommentStmt, nkType:
     discard
   of nkNilLit:

compiler/semgnrc.nim

@@ -59,6 +59,9 @@ template isMixedIn(sym): bool =
 template canOpenSym(s): bool =
   {withinMixin, withinConcept} * flags == {withinMixin} and s.id notin ctx.toBind
 
+proc newOpenSym*(n: PNode): PNode {.inline.} =
+  result = newTreeI(nkOpenSym, n.info, n)
+
 proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym,
                           ctx: var GenericCtx; flags: TSemGenericFlags,
                           fromDotExpr=false): PNode =
@@ -72,8 +75,11 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym,
     else:
       result = symChoice(c, n, s, scOpen)
       if canOpenSym(s):
-        result.flags.incl nfOpenSym
-        result.typ = nil
+        if genericsOpenSym in c.features:
+          result = newOpenSym(result)
+        else:
+          result.flags.incl nfDisabledOpenSym
+          result.typ = nil
   case s.kind
   of skUnknown:
     # Introduced in this pass! Leave it as an identifier.
@@ -102,8 +108,11 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym,
     else:
       result = newSymNodeTypeDesc(s, c.idgen, n.info)
       if canOpenSym(result.sym):
-        result.flags.incl nfOpenSym
-        result.typ = nil
+        if genericsOpenSym in c.features:
+          result = newOpenSym(result)
+        else:
+          result.flags.incl nfDisabledOpenSym
+          result.typ = nil
     onUse(n.info, s)
   of skParam:
     result = n
@@ -113,16 +122,22 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym,
        (s.typ.flags * {tfGenericTypeParam, tfImplicitTypeParam} == {}):
       result = newSymNodeTypeDesc(s, c.idgen, n.info)
       if canOpenSym(result.sym):
-        result.flags.incl nfOpenSym
-        result.typ = nil
+        if genericsOpenSym in c.features:
+          result = newOpenSym(result)
+        else:
+          result.flags.incl nfDisabledOpenSym
+          result.typ = nil
     else:
       result = n
     onUse(n.info, s)
   else:
     result = newSymNode(s, n.info)
     if canOpenSym(result.sym):
-      result.flags.incl nfOpenSym
-      result.typ = nil
+      if genericsOpenSym in c.features:
+        result = newOpenSym(result)
+      else:
+        result.flags.incl nfDisabledOpenSym
+        result.typ = nil
     onUse(n.info, s)
 
 proc lookup(c: PContext, n: PNode, flags: TSemGenericFlags,

compiler/semtypes.nim

@@ -2176,6 +2176,7 @@ proc semTypeNode(c: PContext, n: PNode, prev: PType): PType =
   of nkType: result = n.typ
   of nkStmtListType: result = semStmtListType(c, n, prev)
   of nkBlockType: result = semBlockType(c, n, prev)
+  of nkOpenSym: result = semTypeNode(c, n[0], prev)
   else:
     result = semTypeExpr(c, n, prev)
     when false:

lib/core/macros.nim

@@ -93,6 +93,8 @@ type
     nnkFuncDef,
     nnkTupleConstr,
     nnkError,  ## erroneous AST node
+    nnkModuleRef, nnkReplayAction, nnkNilRodNode ## internal IC nodes
+    nnkOpenSym
 
   NimNodeKinds* = set[NimNodeKind]
   NimTypeKind* = enum  # some types are no longer used, see ast.nim
@@ -1407,7 +1409,7 @@ proc `$`*(node: NimNode): string =
     result = node.basename.strVal & "*"
   of nnkStrLit..nnkTripleStrLit, nnkCommentStmt, nnkSym, nnkIdent:
     result = node.strVal
-  of nnkOpenSymChoice, nnkClosedSymChoice:
+  of nnkOpenSymChoice, nnkClosedSymChoice, nnkOpenSym:
     result = $node[0]
   of nnkAccQuoted:
     result = ""

lib/pure/sugar.nim

@@ -345,7 +345,7 @@ proc collectImpl(init, body: NimNode): NimNode {.since: (1, 1).} =
   let res = genSym(nskVar, "collectResult")
   var bracketExpr: NimNode
   if init != nil:
-    expectKind init, {nnkCall, nnkIdent, nnkSym}
+    expectKind init, {nnkCall, nnkIdent, nnkSym, nnkOpenSym}
     bracketExpr = newTree(nnkBracketExpr,
       if init.kind == nnkCall: freshIdentNodes(init[0]) else: freshIdentNodes(init))
   else: