make genericsOpenSym work at instantiation time, new behavior in openSym (#24111)

alternative to #24101

#23892 changed the opensym experimental switch so that it has to be
enabled in the context of the generic/template declarations capturing
the symbols, not the context of the instantiation of the
generics/templates. This was to be in line with where the compiler gives
the warnings and changes behavior in a potentially breaking way.

However `results` [depends on the old
behavior](71d404b314/results.nim (L1428)),
so that the callers of the macros provided by results always take
advantage of the opensym behavior. To accomodate this, we change the
behavior of the old experimental option that `results` uses,
`genericsOpenSym`, so that ignores the information of whether or not
symbols are intentionally opened and always gives the opensym behavior
as long as it's enabled at instantiation time. This should keep
`results` working as is. However this differs from the normal opensym
switch in that it doesn't generate `nnkOpenSym`.

Before it was just a generics-only version of `openSym` along with
`templateOpenSym` which was only for templates. So `templateOpenSym` is
removed along with this change, but no one appears to have used it.

changelog.md

@@ -128,8 +128,7 @@ is often an easy workaround.
   context changes.
 
   Since this change may affect runtime behavior, the experimental switch
-  `openSym`, or `genericsOpenSym` and `templateOpenSym` for only the respective
-  routines, needs to be enabled; and a warning is given in the case where an
+  `openSym` 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.
 
@@ -150,7 +149,7 @@ is often an easy workaround.
         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
+  {.experimental: "openSym".}
 
   proc bar[T](): string =
     foo(123):
@@ -163,8 +162,6 @@ is often an easy workaround.
       return value
   assert baz[int]() == "captured"
 
-  # {.experimental: "templateOpenSym".} would be needed here if genericsOpenSym was used
-
   template barTempl(): string =
     block:
       foo(123):
@@ -185,6 +182,34 @@ is often an easy workaround.
   experimental feature should still handle `nnkOpenSym`, as the node kind would
   simply not be generated as opposed to being removed.
 
+  Another experimental switch `genericsOpenSym` exists that enables this behavior
+  at instantiation time, meaning templates etc can enable it specifically when
+  they are being called. However this does not generate `nnkOpenSym` nodes
+  (unless the other switch is enabled) and so doesn't reflect the regular
+  behavior of the switch.
+
+  ```nim
+  const value = "captured"
+  template foo(x: int, body: untyped): untyped =
+    let value {.inject.} = "injected"
+    {.push experimental: "genericsOpenSym".}
+    body
+    {.pop.}
+
+  proc bar[T](): string =
+    foo(123):
+      return value
+  echo bar[int]() # "injected"
+
+  template barTempl(): string =
+    block:
+      var res: string
+      foo(123):
+        res = value
+      res
+  assert barTempl() == "injected"
+  ```
+
 ## Compiler changes
 
 - `--nimcache` using a relative path as the argument in a config file is now relative to the config file instead of the current directory.

compiler/condsyms.nim

@@ -167,4 +167,5 @@ proc initDefines*(symbols: StringTableRef) =
 
   defineSymbol("nimHasVtables")
   defineSymbol("nimHasGenericsOpenSym2")
+  defineSymbol("nimHasGenericsOpenSym3")
   defineSymbol("nimHasJsNoLambdaLifting")

compiler/options.nim

@@ -226,8 +226,8 @@ type
     strictCaseObjects,
     inferGenericTypes,
     openSym, # remove nfDisabledOpenSym when this is default
-    # separated alternatives to above:
-    genericsOpenSym, templateOpenSym,
+    # alternative to above:
+    genericsOpenSym
     vtables
 
   LegacyFeature* = enum

compiler/semexprs.nim

@@ -187,6 +187,7 @@ proc semOpenSym(c: PContext, n: PNode, flags: TExprFlags, expectedType: PType,
           break
       o = o.owner
   # nothing found
+  n.flags.excl nfDisabledOpenSym
   if not warnDisabled and isSym:
     result = semExpr(c, n, flags, expectedType)
   else:
@@ -197,7 +198,9 @@ proc semOpenSym(c: PContext, n: PNode, flags: TExprFlags, expectedType: PType,
 
 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)
+    result = semOpenSym(c, n, flags, expectedType,
+      warnDisabled = nfDisabledOpenSym in n.flags and
+        genericsOpenSym notin c.features)
     if result != nil:
       return
   result = n
@@ -3293,8 +3296,12 @@ proc semExpr(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType: PType
   of nkSym:
     let s = n.sym
     if nfDisabledOpenSym in n.flags:
-      let res = semOpenSym(c, n, flags, expectedType, warnDisabled = true)
-      assert res == nil
+      let override = genericsOpenSym in c.features
+      let res = semOpenSym(c, n, flags, expectedType,
+        warnDisabled = not override)
+      if res != nil:
+        assert override
+        return res
     # 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

@@ -74,7 +74,7 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym,
     else:
       result = symChoice(c, n, s, scOpen)
       if canOpenSym(s):
-        if {openSym, genericsOpenSym} * c.features != {}:
+        if openSym in c.features:
           if result.kind == nkSym:
             result = newOpenSym(result)
           else:
@@ -112,7 +112,7 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym,
         # we are in a generic context and `prepareNode` will be called
         result = newSymNodeTypeDesc(s, c.idgen, n.info)
         if canOpenSym(result.sym):
-          if {openSym, genericsOpenSym} * c.features != {}:
+          if openSym in c.features:
             result = newOpenSym(result)
           else:
             result.flags.incl nfDisabledOpenSym
@@ -122,7 +122,7 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym,
     else:
       result = newSymNodeTypeDesc(s, c.idgen, n.info)
       if canOpenSym(result.sym):
-        if {openSym, genericsOpenSym} * c.features != {}:
+        if openSym in c.features:
           result = newOpenSym(result)
         else:
           result.flags.incl nfDisabledOpenSym
@@ -141,7 +141,7 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym,
         return
       result = newSymNodeTypeDesc(s, c.idgen, n.info)
       if canOpenSym(result.sym):
-        if {openSym, genericsOpenSym} * c.features != {}:
+        if openSym in c.features:
           result = newOpenSym(result)
         else:
           result.flags.incl nfDisabledOpenSym
@@ -153,7 +153,7 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym,
       # we are in a generic context and `prepareNode` will be called
       result = newSymNodeTypeDesc(s, c.idgen, n.info)
       if canOpenSym(result.sym):
-        if {openSym, genericsOpenSym} * c.features != {}:
+        if openSym in c.features:
           result = newOpenSym(result)
         else:
           result.flags.incl nfDisabledOpenSym
@@ -164,7 +164,7 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym,
   else:
     result = newSymNode(s, n.info)
     if canOpenSym(result.sym):
-      if {openSym, genericsOpenSym} * c.features != {}:
+      if openSym in c.features:
         result = newOpenSym(result)
       else:
         result.flags.incl nfDisabledOpenSym

compiler/semtempl.nim

@@ -233,7 +233,7 @@ proc semTemplSymbol(c: var TemplCtx, n: PNode, s: PSym; isField, isAmbiguous: bo
   of OverloadableSyms:
     result = symChoice(c.c, n, s, scOpen, isField)
     if not isField and result.kind in {nkSym, nkOpenSymChoice}:
-      if {openSym, templateOpenSym} * c.c.features != {}:
+      if openSym in c.c.features:
         if result.kind == nkSym:
           result = newOpenSym(result)
         else:
@@ -246,7 +246,7 @@ proc semTemplSymbol(c: var TemplCtx, n: PNode, s: PSym; isField, isAmbiguous: bo
     else:
       result = newSymNodeTypeDesc(s, c.c.idgen, n.info)
       if not isField and s.owner != c.owner:
-        if {openSym, templateOpenSym} * c.c.features != {}:
+        if openSym in c.c.features:
           result = newOpenSym(result)
         else:
           result.flags.incl nfDisabledOpenSym
@@ -264,7 +264,7 @@ proc semTemplSymbol(c: var TemplCtx, n: PNode, s: PSym; isField, isAmbiguous: bo
       if not isField and not (s.owner == c.owner and
           s.typ != nil and s.typ.kind == tyGenericParam) and
           result.kind in {nkSym, nkOpenSymChoice}:
-        if {openSym, templateOpenSym} * c.c.features != {}:
+        if openSym in c.c.features:
           if result.kind == nkSym:
             result = newOpenSym(result)
           else:
@@ -277,7 +277,7 @@ proc semTemplSymbol(c: var TemplCtx, n: PNode, s: PSym; isField, isAmbiguous: bo
     else:
       result = newSymNode(s, n.info)
       if not isField:
-        if {openSym, templateOpenSym} * c.c.features != {}:
+        if openSym in c.c.features:
           result = newOpenSym(result)
         else:
           result.flags.incl nfDisabledOpenSym

doc/manual_experimental.md

@@ -2533,8 +2533,7 @@ 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
-`openSym`, or `genericsOpenSym` and `templateOpenSym` for only the respective
-routines, needs to be enabled; and a warning is given in the case where an
+`openSym` 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.
 
@@ -2555,7 +2554,7 @@ template oldTempl(): string =
       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
+{.experimental: "openSym".}
 
 proc bar[T](): string =
   foo(123):
@@ -2568,8 +2567,6 @@ proc baz[T](): string =
     return value
 assert baz[int]() == "captured"
 
-# {.experimental: "templateOpenSym".} would be needed here if genericsOpenSym was used
-
 template barTempl(): string =
   block:
     foo(123):
@@ -2590,6 +2587,34 @@ 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.
 
+Another experimental switch `genericsOpenSym` exists that enables this behavior
+at instantiation time, meaning templates etc can enable it specifically when
+they are being called. However this does not generate `nnkOpenSym` nodes
+(unless the other switch is enabled) and so doesn't reflect the regular
+behavior of the switch.
+
+```nim
+const value = "captured"
+template foo(x: int, body: untyped): untyped =
+  let value {.inject.} = "injected"
+  {.push experimental: "genericsOpenSym".}
+  body
+  {.pop.}
+
+proc bar[T](): string =
+  foo(123):
+    return value
+echo bar[int]() # "injected"
+
+template barTempl(): string =
+  block:
+    var res: string
+    foo(123):
+      res = value
+    res
+assert barTempl() == "injected"
+```
+
 
 VTable for methods
 ==================

tests/generics/mopensymimport2.nim

@@ -1,4 +1,4 @@
-{.experimental: "genericsOpenSym".}
+{.experimental: "openSym".}
 
 import mopensymimport1
 

tests/generics/tmacroinjectedsym.nim

@@ -1,4 +1,4 @@
-{.experimental: "genericsOpenSym".}
+{.experimental: "openSym".}
 
 block: # issue #22605, normal call syntax
   const error = "bad"

tests/template/topensym.nim

@@ -1,4 +1,4 @@
-{.experimental: "templateOpenSym".}
+{.experimental: "openSym".}
 
 block: # issue #24002
   type Result[T, E] = object

tests/template/topensymoverride.nim

@@ -0,0 +1,39 @@
+discard """
+  matrix: "--skipParentCfg --filenames:legacyRelProj"
+"""
+
+const value = "captured"
+template fooOld(x: int, body: untyped): untyped =
+  let value {.inject.} = "injected"
+  body
+template foo(x: int, body: untyped): untyped =
+  let value {.inject.} = "injected"
+  {.push experimental: "genericsOpenSym".}
+  body
+  {.pop.}
+
+proc old[T](): string =
+  fooOld(123):
+    return value
+doAssert old[int]() == "captured"
+
+template oldTempl(): string =
+  block:
+    var res: string
+    fooOld(123):
+      res = value
+    res
+doAssert oldTempl() == "captured"
+
+proc bar[T](): string =
+  foo(123):
+    return value
+doAssert bar[int]() == "injected"
+
+template barTempl(): string =
+  block:
+    var res: string
+    foo(123):
+      res = value
+    res
+doAssert barTempl() == "injected"