opensym for templates + move behavior of opensymchoice to itself (#24007

) fixes #15314, fixes #24002 The OpenSym behavior first added to generics in #23091 now also applies to templates, since templates can also capture symbols that are meant to be replaced by local symbols if the context imports symbols with the same name, as in the issue #24002. The experimental switch `templateOpenSym` is added to enable this behavior for templates only, and the experimental switch `openSym` is added to enable it for both templates and generics, and the documentation now mainly mentions this switch. Additionally the logic for `nkOpenSymChoice` nodes that were previously wrapped in `nkOpenSym` now apply to all `nkOpenSymChoice` nodes, and so these nodes aren't wrapped in `nkOpenSym` anymore. This means `nkOpenSym` can only have children of kind `nkSym` again, so it is more in line with the structure of symchoice nodes. As for why they aren't merged with `nkOpenSymChoice` nodes yet, we need some way to signal that the node shouldn't become ambiguous if other options exist at instantiation time, we already captured a symbol at the beginning and another symbol can only replace it if it's closer in scope and unambiguous. (cherry picked from commit 770f8d5)
nim-lang · Aug 29, 2024 · d1202f8 · d1202f8
1 parent 352c82a
commit d1202f8
Show file tree

Hide file tree

Showing 15 changed files with 408 additions and 101 deletions.
diff --git a/changelog.md b/changelog.md
@@ -30,30 +30,38 @@ 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,
-  but other methods like renaming the captured symbols should be used instead
-  so that the code is not affected by context changes.
+- The experimental option `--experimental:openSym` has been added to allow
+  captured symbols in generic routine and template bodies respectively 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, 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
-  where an injected symbol would replace a captured symbol not bound by `bind`
-  and the experimental switch isn't enabled.
+  `openSym`, or `genericsOpenSym` and `templateOpenSym` for only the respective
+  routines, needs to be enabled; and a warning is given in the case where an
+  injected symbol would replace a captured symbol not bound by `bind` and
+  the experimental switch isn't enabled.
 
   ```nim
   const value = "captured"
-  template foo(x: int, body: untyped) =
+  template foo(x: int, body: untyped): untyped =
     let value {.inject.} = "injected"
     body
 
   proc old[T](): string =
     foo(123):
-      return value # warning: a new `value` has been injected, use `bind` or turn on `experimental:genericsOpenSym`
+      return value # warning: a new `value` has been injected, use `bind` or turn on `experimental:openSym`
   echo old[int]() # "captured"
 
-  {.experimental: "genericsOpenSym".}
+  template oldTempl(): string =
+    block:
+      foo(123):
+        value # warning: a new `value` has been injected, use `bind` or turn on `experimental:openSym`
+  echo oldTempl() # "captured"
+
+  {.experimental: "openSym".} # or {.experimental: "genericsOpenSym".} for just generic procs
 
   proc bar[T](): string =
     foo(123):
@@ -65,14 +73,28 @@ slots when enlarging a sequence.
     foo(123):
       return value
   assert baz[int]() == "captured"
+
+  # {.experimental: "templateOpenSym".} would be needed here if genericsOpenSym was used
+
+  template barTempl(): string =
+    block:
+      foo(123):
+        value
+  assert barTempl() == "injected" # previously it would be "captured"
+
+  template bazTempl(): string =
+    bind value
+    block:
+      foo(123):
+        value
+  assert bazTempl() == "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.
+  exactly 1 `nnkSym` 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
 

diff --git a/compiler/ast.nim b/compiler/ast.nim
@@ -519,7 +519,7 @@ type
     nfHasComment # node has a comment
     nfSkipFieldChecking # node skips field visable checking
     nfDisabledOpenSym # temporary: node should be nkOpenSym but cannot
-                      # because genericsOpenSym experimental switch is disabled
+                      # because openSym experimental switch is disabled
                       # gives warning instead
 
   TNodeFlags* = set[TNodeFlag]
@@ -1110,7 +1110,7 @@ const
   nkCallKinds* = {nkCall, nkInfix, nkPrefix, nkPostfix,
                   nkCommand, nkCallStrLit, nkHiddenCallConv}
   nkIdentKinds* = {nkIdent, nkSym, nkAccQuoted, nkOpenSymChoice,
-                   nkClosedSymChoice}
+                   nkClosedSymChoice, nkOpenSym}
 
   nkPragmaCallKinds* = {nkExprColonExpr, nkCall, nkCallStrLit}
   nkLiterals* = {nkCharLit..nkTripleStrLit}
@@ -1457,6 +1457,9 @@ proc newSymNode*(sym: PSym, info: TLineInfo): PNode =
   result.typ = sym.typ
   result.info = info
 
+proc newOpenSym*(n: PNode): PNode {.inline.} =
+  result = newTreeI(nkOpenSym, n.info, n)
+
 proc newIntNode*(kind: TNodeKind, intVal: BiggestInt): PNode =
   result = newNode(kind)
   result.intVal = intVal

diff --git a/compiler/lineinfos.nim b/compiler/lineinfos.nim
@@ -91,7 +91,7 @@ type
     warnStmtListLambda = "StmtListLambda",
     warnBareExcept = "BareExcept",
     warnImplicitDefaultValue = "ImplicitDefaultValue",
-    warnGenericsIgnoredInjection = "GenericsIgnoredInjection",
+    warnIgnoredSymbolInjection = "IgnoredSymbolInjection",
     warnUser = "User",
     # hints
     hintSuccess = "Success", hintSuccessX = "SuccessX",
@@ -195,7 +195,7 @@ const
     warnStmtListLambda: "statement list expression assumed to be anonymous proc; this is deprecated, use `do (): ...` or `proc () = ...` instead",
     warnBareExcept: "$1",
     warnImplicitDefaultValue: "$1",
-    warnGenericsIgnoredInjection: "$1",
+    warnIgnoredSymbolInjection: "$1",
     warnUser: "$1",
     hintSuccess: "operation successful: $#",
     # keep in sync with `testament.isSuccess`

diff --git a/compiler/lookups.nim b/compiler/lookups.nim
@@ -48,7 +48,7 @@ proc considerQuotedIdent*(c: PContext; n: PNode, origin: PNode = nil): PIdent =
         case x.kind
         of nkIdent: id.add(x.ident.s)
         of nkSym: id.add(x.sym.name.s)
-        of nkSymChoices:
+        of nkSymChoices, nkOpenSym:
           if x[0].kind == nkSym:
             id.add(x[0].sym.name.s)
           else:
@@ -642,6 +642,8 @@ proc qualifiedLookUp*(c: PContext, n: PNode, flags: set[TLookupFlag]): PSym =
     c.isAmbiguous = amb
   of nkSym:
     result = n.sym
+  of nkOpenSym:
+    result = qualifiedLookUp(c, n[0], flags)
   of nkDotExpr:
     result = nil
     var m = qualifiedLookUp(c, n[0], (flags * {checkUndeclared}) + {checkModule})

diff --git a/compiler/options.nim b/compiler/options.nim
@@ -221,7 +221,9 @@ type
     flexibleOptionalParams,
     strictDefs,
     strictCaseObjects,
-    genericsOpenSym # remove nfDisabledOpenSym when this switch is default
+    openSym, # remove nfDisabledOpenSym when this is default
+    # separated alternatives to above:
+    genericsOpenSym, templateOpenSym
 
   LegacyFeature* = enum
     allowSemcheckedAstModification,

diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim
@@ -120,34 +120,6 @@ proc semSym(c: PContext, n: PNode, sym: PSym, flags: TExprFlags): PNode
 proc isSymChoice(n: PNode): bool {.inline.} =
   result = n.kind in nkSymChoices
 
-proc semSymChoice(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType: PType = nil): PNode =
-  result = n
-  if expectedType != nil:
-    result = fitNode(c, expectedType, result, n.info)
-  if isSymChoice(result) and efAllowSymChoice notin flags:
-    # some contexts might want sym choices preserved for later disambiguation
-    # in general though they are ambiguous
-    let first = n[0].sym
-    var foundSym: PSym = nil
-    if first.kind == skEnumField and
-        not isAmbiguous(c, first.name, {skEnumField}, foundSym) and
-        foundSym == first:
-      # choose the first resolved enum field, i.e. the latest in scope
-      # to mirror behavior before overloadable enums
-      result = n[0]
-    else:
-      var err = "ambiguous identifier '" & first.name.s &
-        "' -- use one of the following:\n"
-      for child in n:
-        let candidate = child.sym
-        err.add "  " & candidate.owner.name.s & "." & candidate.name.s
-        err.add ": " & typeToString(candidate.typ) & "\n"
-      localError(c.config, n.info, err)
-      n.typ = errorType(c)
-      result = n
-  if result.kind == nkSym:
-    result = semSym(c, result, result.sym, flags)
-
 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
@@ -180,30 +152,54 @@ proc semOpenSym(c: PContext, n: PNode, flags: TExprFlags, expectedType: PType,
         else:
           var msg =
             "a new symbol '" & ident.s & "' has been injected during " &
-            "instantiation of " & c.p.owner.name.s & ", however "
+            # msgContext should show what is being instantiated:
+            "template or generic instantiation, however "
           if isSym:
             msg.add(
               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 captured symbol explicitly")
+              "either enable --experimental:openSym to use the injected symbol, " &
+              "or `bind` this captured symbol explicitly")
           else:
             msg.add(
               "overloads of " & ident.s & " will be used instead; " &
-              "either enable --experimental:genericsOpenSym to use the " &
-              "injected symbol or `bind` this symbol explicitly")
-          message(c.config, n.info, warnGenericsIgnoredInjection, msg)
+              "either enable --experimental:openSym to use the injected symbol, " &
+              "or `bind` this symbol explicitly")
+          message(c.config, n.info, warnIgnoredSymbolInjection, msg)
           break
       o = o.owner
   # nothing found
-  if not warnDisabled:
+  if not warnDisabled and isSym:
     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 semSymChoice(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType: PType = nil): PNode =
+  if n.kind == nkOpenSymChoice:
+    result = semOpenSym(c, n, flags, expectedType, warnDisabled = nfDisabledOpenSym in n.flags)
+    if result != nil:
+      return
+  result = n
+  resolveSymChoice(c, result, flags, expectedType)
+  if isSymChoice(result) and result.len == 1:
+    # resolveSymChoice can leave 1 sym
+    result = result[0]
+  if isSymChoice(result) and efAllowSymChoice notin flags:
+    var err = "ambiguous identifier: '" & result[0].sym.name.s &
+      "' -- use one of the following:\n"
+    for child in n:
+      let candidate = child.sym
+      err.add "  " & candidate.owner.name.s & "." & candidate.name.s
+      err.add ": " & typeToString(candidate.typ) & "\n"
+    localError(c.config, n.info, err)
+    n.typ = errorType(c)
+    result = n
+  if result.kind == nkSym:
+    result = semSym(c, result, result.sym, flags)
+
 proc inlineConst(c: PContext, n: PNode, s: PSym): PNode {.inline.} =
   result = copyTree(s.astdef)
   if result.isNil:
@@ -1747,7 +1743,7 @@ proc semSubscript(c: PContext, n: PNode, flags: TExprFlags): PNode =
       result = nil
   else:
     let s = if n[0].kind == nkSym: n[0].sym
-            elif n[0].kind in nkSymChoices: n[0][0].sym
+            elif n[0].kind in nkSymChoices + {nkOpenSym}: n[0][0].sym
             else: nil
     if s != nil:
       case s.kind
@@ -3149,9 +3145,6 @@ proc semExpr(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType: PType
     if isSymChoice(result):
       result = semSymChoice(c, result, flags, expectedType)
   of nkClosedSymChoice, nkOpenSymChoice:
-    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

diff --git a/compiler/semgnrc.nim b/compiler/semgnrc.nim
@@ -59,9 +59,6 @@ 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 =
@@ -75,8 +72,11 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym,
     else:
       result = symChoice(c, n, s, scOpen)
       if canOpenSym(s):
-        if genericsOpenSym in c.features:
-          result = newOpenSym(result)
+        if {openSym, genericsOpenSym} * c.features != {}:
+          if result.kind == nkSym:
+            result = newOpenSym(result)
+          else:
+            result.typ = nil
         else:
           result.flags.incl nfDisabledOpenSym
           result.typ = nil
@@ -108,7 +108,7 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym,
     else:
       result = newSymNodeTypeDesc(s, c.idgen, n.info)
       if canOpenSym(result.sym):
-        if genericsOpenSym in c.features:
+        if {openSym, genericsOpenSym} * c.features != {}:
           result = newOpenSym(result)
         else:
           result.flags.incl nfDisabledOpenSym
@@ -122,7 +122,7 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym,
        (s.typ.flags * {tfGenericTypeParam, tfImplicitTypeParam} == {}):
       result = newSymNodeTypeDesc(s, c.idgen, n.info)
       if canOpenSym(result.sym):
-        if genericsOpenSym in c.features:
+        if {openSym, genericsOpenSym} * c.features != {}:
           result = newOpenSym(result)
         else:
           result.flags.incl nfDisabledOpenSym
@@ -133,7 +133,7 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym,
   else:
     result = newSymNode(s, n.info)
     if canOpenSym(result.sym):
-      if genericsOpenSym in c.features:
+      if {openSym, genericsOpenSym} * c.features != {}:
         result = newOpenSym(result)
       else:
         result.flags.incl nfDisabledOpenSym