From 770f8d551372893ed480550062a89e76c60ff4a8 Mon Sep 17 00:00:00 2001 From: metagn Date: Wed, 28 Aug 2024 21:51:13 +0300 Subject: [PATCH] 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. --- changelog.md | 56 ++++-- compiler/ast.nim | 7 +- compiler/lineinfos.nim | 4 +- compiler/lookups.nim | 4 +- compiler/options.nim | 4 +- compiler/semexprs.nim | 62 +++--- compiler/semgnrc.nim | 20 +- compiler/semtempl.nim | 71 +++++-- compiler/semtypes.nim | 12 +- doc/manual_experimental.md | 57 ++++-- lib/pure/pegs.nim | 4 +- lib/pure/sugar.nim | 4 +- tests/compileoption/texperimental.nim | 4 +- tests/compileoption/texperimental.nims | 4 +- tests/config.nims | 2 +- tests/generics/tmacroinjectedsymwarning.nim | 4 +- tests/template/topensym.nim | 209 ++++++++++++++++++++ tests/template/topensymwarning.nim | 60 ++++++ 18 files changed, 468 insertions(+), 120 deletions(-) create mode 100644 tests/template/topensym.nim create mode 100644 tests/template/topensymwarning.nim diff --git a/changelog.md b/changelog.md index d98c34474e..c5f30bbe25 100644 --- a/changelog.md +++ b/changelog.md @@ -82,30 +82,38 @@ is often an easy workaround. let (a, (b, c)): (byte, (float, cstring)) = (1, (2, "abc")) ``` -- 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): @@ -117,14 +125,28 @@ is often an easy workaround. 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 index db6cd97612..eac4bf387f 100644 --- a/compiler/ast.nim +++ b/compiler/ast.nim @@ -323,7 +323,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] @@ -895,7 +895,7 @@ const nfAllFieldsSet* = nfBase2 nkIdentKinds* = {nkIdent, nkSym, nkAccQuoted, nkOpenSymChoice, - nkClosedSymChoice} + nkClosedSymChoice, nkOpenSym} nkPragmaCallKinds* = {nkExprColonExpr, nkCall, nkCallStrLit} nkLiterals* = {nkCharLit..nkTripleStrLit} @@ -1284,6 +1284,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 index b48252acef..5535dc109e 100644 --- a/compiler/lineinfos.nim +++ b/compiler/lineinfos.nim @@ -92,7 +92,7 @@ type warnStmtListLambda = "StmtListLambda", warnBareExcept = "BareExcept", warnImplicitDefaultValue = "ImplicitDefaultValue", - warnGenericsIgnoredInjection = "GenericsIgnoredInjection", + warnIgnoredSymbolInjection = "IgnoredSymbolInjection", warnStdPrefix = "StdPrefix" warnUser = "User", warnGlobalVarConstructorTemporary = "GlobalVarConstructorTemporary", @@ -198,7 +198,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", warnStdPrefix: "$1 needs the 'std' prefix", warnUser: "$1", warnGlobalVarConstructorTemporary: "global variable '$1' initialization requires a temporary variable", diff --git a/compiler/lookups.nim b/compiler/lookups.nim index c6940a4dcb..0e576d765b 100644 --- a/compiler/lookups.nim +++ b/compiler/lookups.nim @@ -50,7 +50,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: @@ -668,6 +668,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 index de412979fe..34268259b6 100644 --- a/compiler/options.nim +++ b/compiler/options.nim @@ -226,7 +226,9 @@ type strictDefs, strictCaseObjects, inferGenericTypes, - genericsOpenSym, # remove nfDisabledOpenSym when this switch is default + openSym, # remove nfDisabledOpenSym when this is default + # separated alternatives to above: + genericsOpenSym, templateOpenSym, vtables LegacyFeature* = enum diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim index f52de67505..0c36913623 100644 --- a/compiler/semexprs.nim +++ b/compiler/semexprs.nim @@ -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 @@ -189,23 +170,24 @@ 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 @@ -213,6 +195,29 @@ proc semOpenSym(c: PContext, n: PNode, flags: TExprFlags, expectedType: PType, # 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: @@ -1780,7 +1785,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 @@ -3230,9 +3235,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 index 8efc8a94ec..e3a8daf994 100644 --- 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, isAmbiguous: bool, @@ -77,8 +74,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 @@ -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 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, 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 @@ -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 genericsOpenSym in c.features: + if {openSym, genericsOpenSym} * 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 genericsOpenSym in c.features: + if {openSym, genericsOpenSym} * 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 genericsOpenSym in c.features: + if {openSym, genericsOpenSym} * c.features != {}: result = newOpenSym(result) else: result.flags.incl nfDisabledOpenSym diff --git a/compiler/semtempl.nim b/compiler/semtempl.nim index f7653a8904..aef0ce9b37 100644 --- a/compiler/semtempl.nim +++ b/compiler/semtempl.nim @@ -218,47 +218,76 @@ proc addLocalDecl(c: var TemplCtx, n: var PNode, k: TSymKind) = if k == skParam and c.inTemplateHeader > 0: local.flags.incl sfTemplateParam -proc semTemplSymbol(c: PContext, n: PNode, s: PSym; isField, isAmbiguous: bool): PNode = +proc semTemplSymbol(c: var TemplCtx, n: PNode, s: PSym; isField, isAmbiguous: bool): PNode = incl(s.flags, sfUsed) # bug #12885; ideally sem'checking is performed again afterwards marking # the symbol as used properly, but the nfSem mechanism currently prevents # that from happening, so we mark the module as used here already: - markOwnerModuleAsUsed(c, s) + markOwnerModuleAsUsed(c.c, s) # we do not call onUse here, as the identifier is not really # resolved here. We will fixup the used identifiers later. case s.kind of skUnknown: # Introduced in this pass! Leave it as an identifier. result = n - of OverloadableSyms-{skTemplate,skMacro}: - result = symChoice(c, n, s, scOpen, isField) - of skTemplate, skMacro: - result = symChoice(c, n, s, scOpen, isField) - if result.kind == nkSym: - # template/macro symbols might need to be semchecked again - # prepareOperand etc don't do this without setting the type to nil - result.typ = nil + 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 result.kind == nkSym: + result = newOpenSym(result) + else: + result.typ = nil + else: + result.flags.incl nfDisabledOpenSym + result.typ = nil of skGenericParam: if isField and sfGenSym in s.flags: result = n - else: result = newSymNodeTypeDesc(s, c.idgen, n.info) + else: + result = newSymNodeTypeDesc(s, c.c.idgen, n.info) + if not isField and s.owner != c.owner: + if {openSym, templateOpenSym} * c.c.features != {}: + result = newOpenSym(result) + else: + result.flags.incl nfDisabledOpenSym + result.typ = nil of skParam: result = n of skType: if isField and sfGenSym in s.flags: result = n - elif isAmbiguous: - # ambiguous types should be symchoices since lookup behaves - # differently for them in regular expressions - result = symChoice(c, n, s, scOpen, isField) - else: result = newSymNodeTypeDesc(s, c.idgen, n.info) + else: + if isAmbiguous: + # ambiguous types should be symchoices since lookup behaves + # differently for them in regular expressions + result = symChoice(c.c, n, s, scOpen, isField) + else: result = newSymNodeTypeDesc(s, c.c.idgen, n.info) + 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 result.kind == nkSym: + result = newOpenSym(result) + else: + result.typ = nil + else: + result.flags.incl nfDisabledOpenSym + result.typ = nil else: if isField and sfGenSym in s.flags: result = n - else: result = newSymNode(s, n.info) + else: + result = newSymNode(s, n.info) + if not isField: + if {openSym, templateOpenSym} * c.c.features != {}: + result = newOpenSym(result) + else: + result.flags.incl nfDisabledOpenSym + result.typ = nil # Issue #12832 when defined(nimsuggest): - suggestSym(c.graph, n.info, s, c.graph.usageSym, false) + suggestSym(c.c.graph, n.info, s, c.c.graph.usageSym, false) # field access (dot expr) will be handled by builtinFieldAccess if not isField: - styleCheckUse(c, n.info, s) + styleCheckUse(c.c, n.info, s) proc semRoutineInTemplName(c: var TemplCtx, n: PNode, explicitInject: bool): PNode = result = n @@ -369,7 +398,7 @@ proc semTemplBody(c: var TemplCtx, n: PNode): PNode = else: if s.kind in {skVar, skLet, skConst}: discard qualifiedLookUp(c.c, n, {checkAmbiguity, checkModule}) - result = semTemplSymbol(c.c, n, s, c.noGenSym > 0, c.c.isAmbiguous) + result = semTemplSymbol(c, n, s, c.noGenSym > 0, c.c.isAmbiguous) of nkBind: result = semTemplBody(c, n[0]) of nkBindStmt: @@ -580,7 +609,7 @@ proc semTemplBody(c: var TemplCtx, n: PNode): PNode = else: if s.kind in {skVar, skLet, skConst}: discard qualifiedLookUp(c.c, n, {checkAmbiguity, checkModule}) - return semTemplSymbol(c.c, n, s, c.noGenSym > 0, c.c.isAmbiguous) + return semTemplSymbol(c, n, s, c.noGenSym > 0, c.c.isAmbiguous) if n.kind == nkDotExpr: result = n result[0] = semTemplBody(c, n[0]) diff --git a/compiler/semtypes.nim b/compiler/semtypes.nim index 292558b5b9..2b6a7b6a71 100644 --- a/compiler/semtypes.nim +++ b/compiler/semtypes.nim @@ -1282,7 +1282,7 @@ proc semParamType(c: PContext, n: PNode, constraint: var PNode): PType = result = semTypeNode(c, n[0], nil) constraint = semNodeKindConstraints(n, c.config, 1) elif n.kind == nkCall and - n[0].kind in {nkIdent, nkSym, nkOpenSymChoice, nkClosedSymChoice} and + n[0].kind in {nkIdent, nkSym, nkOpenSymChoice, nkClosedSymChoice, nkOpenSym} and considerQuotedIdent(c, n[0]).s == "{}": result = semTypeNode(c, n[1], nil) constraint = semNodeKindConstraints(n, c.config, 2) @@ -1965,11 +1965,7 @@ proc semTypeNode(c: PContext, n: PNode, prev: PType): PType = of nkTupleConstr: result = semAnonTuple(c, n, prev) of nkCallKinds: let x = n[0] - let ident = case x.kind - of nkIdent: x.ident - of nkSym: x.sym.name - of nkClosedSymChoice, nkOpenSymChoice: x[0].sym.name - else: nil + let ident = x.getPIdent if ident != nil and ident.s == "[]": let b = newNodeI(nkBracketExpr, n.info) for i in 1..