Skip to content

Commit

Permalink
opensym for templates + move behavior of opensymchoice to itself (#24007
Browse files Browse the repository at this point in the history
)

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)
  • Loading branch information
narimiran committed Aug 29, 2024
1 parent c3fa332 commit 6c38984
Show file tree
Hide file tree
Showing 14 changed files with 144 additions and 94 deletions.
56 changes: 39 additions & 17 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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

Expand Down
7 changes: 5 additions & 2 deletions compiler/ast.nim
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions compiler/lineinfos.nim
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ type
warnStmtListLambda = "StmtListLambda",
warnBareExcept = "BareExcept",
warnImplicitDefaultValue = "ImplicitDefaultValue",
warnGenericsIgnoredInjection = "GenericsIgnoredInjection",
warnIgnoredSymbolInjection = "IgnoredSymbolInjection",
warnUser = "User",
# hints
hintSuccess = "Success", hintSuccessX = "SuccessX",
Expand Down Expand Up @@ -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`
Expand Down
4 changes: 3 additions & 1 deletion compiler/lookups.nim
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -652,6 +652,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})
Expand Down
4 changes: 3 additions & 1 deletion compiler/options.nim
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
62 changes: 32 additions & 30 deletions compiler/semexprs.nim
Original file line number Diff line number Diff line change
Expand Up @@ -138,25 +138,6 @@ proc resolveSymChoice(c: PContext, n: var PNode, flags: TExprFlags = {}, expecte
# to mirror behavior before overloadable enums
n = n[0]

proc semSymChoice(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType: PType = nil): PNode =
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 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
Expand Down Expand Up @@ -189,30 +170,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:
Expand Down Expand Up @@ -1756,7 +1761,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
Expand Down Expand Up @@ -3192,9 +3197,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
Expand Down
16 changes: 8 additions & 8 deletions compiler/semgnrc.nim
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
Loading

0 comments on commit 6c38984

Please sign in to comment.