From 50ac4f67f3d89d02d3a6f2d9ed4aefebeb144d65 Mon Sep 17 00:00:00 2001 From: metagn Date: Thu, 25 Jul 2024 14:52:35 -0600 Subject: [PATCH 01/11] opensym as node kind --- compiler/ast.nim | 7 ++----- compiler/docgen.nim | 6 +++--- compiler/ic/ic.nim | 4 ++-- compiler/lookups.nim | 4 ++++ compiler/nodekinds.nim | 1 + compiler/renderer.nim | 4 +++- compiler/semexprs.nim | 39 +++++++++++++++++---------------------- compiler/semgnrc.nim | 15 +++++++-------- compiler/semtypes.nim | 1 + 9 files changed, 40 insertions(+), 41 deletions(-) diff --git a/compiler/ast.nim b/compiler/ast.nim index 624bc32f9cca2..8ea86972c7625 100644 --- a/compiler/ast.nim +++ b/compiler/ast.nim @@ -325,9 +325,6 @@ 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 TNodeFlags* = set[TNodeFlag] TTypeFlag* = enum # keep below 32 for efficiency reasons (now: 47) @@ -881,8 +878,7 @@ const nfIsRef, nfIsPtr, nfPreventCg, nfLL, nfFromTemplate, nfDefaultRefsParam, nfExecuteOnReload, nfLastRead, - nfFirstWrite, nfSkipFieldChecking, - nfOpenSym} + nfFirstWrite, nfSkipFieldChecking} namePos* = 0 patternPos* = 1 # empty except for term rewriting macros genericParamsPos* = 2 @@ -924,6 +920,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 diff --git a/compiler/docgen.nim b/compiler/docgen.nim index a6795be44752d..8e5f5e4e79b65 100644 --- a/compiler/docgen.nim +++ b/compiler/docgen.nim @@ -831,7 +831,7 @@ proc getName(n: PNode): string = result = "`" for i in 0.. 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 @@ -1013,7 +1014,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: @@ -1275,6 +1276,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) diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim index 14d154ef4ff9c..f82f81e8ce14a 100644 --- a/compiler/semexprs.nim +++ b/compiler/semexprs.nim @@ -157,13 +157,13 @@ 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): 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) @@ -176,8 +176,8 @@ 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: @@ -189,23 +189,22 @@ proc semOpenSym(c: PContext, n: PNode, s: PSym, flags: TExprFlags, expectedType: 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 + result = semExpr(c, n, flags, expectedType) proc inlineConst(c: PContext, n: PNode, s: PSym): PNode {.inline.} = result = copyTree(s.astdef) @@ -3192,20 +3191,16 @@ 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 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 # 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: diff --git a/compiler/semgnrc.nim b/compiler/semgnrc.nim index b5faac97907a4..b8fcfda6d7526 100644 --- a/compiler/semgnrc.nim +++ b/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 = @@ -73,8 +76,7 @@ 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 + result = newOpenSym(result) case s.kind of skUnknown: # Introduced in this pass! Leave it as an identifier. @@ -103,8 +105,7 @@ 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 + result = newOpenSym(result) onUse(n.info, s) of skParam: result = n @@ -114,16 +115,14 @@ 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 + result = newOpenSym(result) else: result = n onUse(n.info, s) else: result = newSymNode(s, n.info) if canOpenSym(result.sym): - result.flags.incl nfOpenSym - result.typ = nil + result = newOpenSym(result) onUse(n.info, s) proc lookup(c: PContext, n: PNode, flags: TSemGenericFlags, diff --git a/compiler/semtypes.nim b/compiler/semtypes.nim index 8e9e43de8b92e..299c4a1b61956 100644 --- a/compiler/semtypes.nim +++ b/compiler/semtypes.nim @@ -2207,6 +2207,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: From 5cd2b4232521829a7cdfbc7803bc6e5be8e72688 Mon Sep 17 00:00:00 2001 From: metagn Date: Thu, 25 Jul 2024 15:00:15 -0600 Subject: [PATCH 02/11] fix nimvm --- compiler/semexprs.nim | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim index f82f81e8ce14a..bafeeff580de3 100644 --- a/compiler/semexprs.nim +++ b/compiler/semexprs.nim @@ -2642,7 +2642,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: From e85c74d19c28495bec0768a749f3a20d14e05cdb Mon Sep 17 00:00:00 2001 From: metagn Date: Thu, 25 Jul 2024 15:09:11 -0600 Subject: [PATCH 03/11] fix declared --- compiler/semexprs.nim | 2 ++ 1 file changed, 2 insertions(+) diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim index bafeeff580de3..8b6692fd94638 100644 --- a/compiler/semexprs.nim +++ b/compiler/semexprs.nim @@ -2190,6 +2190,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 From 557c4ac5e1f54a9884aa918674804fc4f84e414a Mon Sep 17 00:00:00 2001 From: metagn Date: Thu, 25 Jul 2024 16:00:07 -0600 Subject: [PATCH 04/11] fix macros, some packages will probably still fail, maybe keep node under experimental --- lib/core/macros.nim | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/core/macros.nim b/lib/core/macros.nim index d391bf761971c..7646b165c09f2 100644 --- a/lib/core/macros.nim +++ b/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 = "" From 7a15c8aa25bfb149f2f39d52a96ba01b259ab41c Mon Sep 17 00:00:00 2001 From: metagn Date: Thu, 25 Jul 2024 16:29:52 -0600 Subject: [PATCH 05/11] fix collect, definitely needs experimental --- lib/pure/sugar.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pure/sugar.nim b/lib/pure/sugar.nim index 7833ed0633345..e3ec99b0c32dc 100644 --- a/lib/pure/sugar.nim +++ b/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, nnkClosedSymChoice, nnkOpenSymChoice} + expectKind init, {nnkCall, nnkIdent, nnkSym, nnkClosedSymChoice, nnkOpenSymChoice, nnkOpenSym} bracketExpr = newTree(nnkBracketExpr, if init.kind in {nnkCall, nnkClosedSymChoice, nnkOpenSymChoice}: freshIdentNodes(init[0]) else: freshIdentNodes(init)) From 0af93cb77aa161e052e2f61f5ecfbb14ee7725d2 Mon Sep 17 00:00:00 2001 From: metagn Date: Sat, 27 Jul 2024 18:22:09 -0600 Subject: [PATCH 06/11] adapt #23572 --- changelog.md | 11 ++++++++- compiler/ast.nim | 6 ++++- compiler/condsyms.nim | 1 + compiler/options.nim | 2 +- compiler/semexprs.nim | 38 ++++++++++++++---------------- compiler/semgnrc.nim | 22 ++++++++++++++--- doc/manual_experimental.md | 11 ++++++++- tests/generics/mopensymimport1.nim | 34 ++++++++++++++++++++++++++ tests/generics/mopensymimport2.nim | 16 +++++++++++++ tests/generics/topensymimport.nim | 5 ++++ 10 files changed, 119 insertions(+), 27 deletions(-) create mode 100644 tests/generics/mopensymimport1.nim create mode 100644 tests/generics/mopensymimport2.nim create mode 100644 tests/generics/topensymimport.nim diff --git a/changelog.md b/changelog.md index 75b848ebd1386..5056d7b3d87ba 100644 --- a/changelog.md +++ b/changelog.md @@ -78,7 +78,9 @@ is often an easy workaround. - 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 @@ -110,6 +112,13 @@ is often an easy workaround. 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 - `--nimcache` using a relative path as the argument in a config file is now relative to the config file instead of the current directory. diff --git a/compiler/ast.nim b/compiler/ast.nim index 8ea86972c7625..648bc4392857a 100644 --- a/compiler/ast.nim +++ b/compiler/ast.nim @@ -325,6 +325,9 @@ type nfFirstWrite # this node is a first write 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 + # gives warning instead TNodeFlags* = set[TNodeFlag] TTypeFlag* = enum # keep below 32 for efficiency reasons (now: 47) @@ -878,7 +881,8 @@ const nfIsRef, nfIsPtr, nfPreventCg, nfLL, nfFromTemplate, nfDefaultRefsParam, nfExecuteOnReload, nfLastRead, - nfFirstWrite, nfSkipFieldChecking} + nfFirstWrite, nfSkipFieldChecking, + nfDisabledOpenSym} namePos* = 0 patternPos* = 1 # empty except for term rewriting macros genericParamsPos* = 2 diff --git a/compiler/condsyms.nim b/compiler/condsyms.nim index aba6de49dbec3..bee9ad65fe479 100644 --- a/compiler/condsyms.nim +++ b/compiler/condsyms.nim @@ -166,4 +166,5 @@ proc initDefines*(symbols: StringTableRef) = defineSymbol("nimHasWarnStdPrefix") defineSymbol("nimHasVtables") + defineSymbol("nimHasGenericsOpenSym") defineSymbol("nimHasJsNoLambdaLifting") diff --git a/compiler/options.nim b/compiler/options.nim index b442ac5b5dd8b..de412979fed92 100644 --- a/compiler/options.nim +++ b/compiler/options.nim @@ -226,7 +226,7 @@ type strictDefs, strictCaseObjects, inferGenericTypes, - genericsOpenSym, + genericsOpenSym, # remove nfDisabledOpenSym when this switch is default vtables LegacyFeature* = enum diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim index 8b6692fd94638..51ebccfb79db1 100644 --- a/compiler/semexprs.nim +++ b/compiler/semexprs.nim @@ -182,26 +182,8 @@ proc semOpenSym(c: PContext, n: PNode, flags: TExprFlags, expectedType: PType): var o = s2.owner while o != nil: if o == c.p.owner: - if genericsOpenSym in c.features: - 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 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") - 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) - break + result = semExpr(c, id, flags, expectedType) + return o = o.owner # nothing found result = semExpr(c, n, flags, expectedType) @@ -3195,9 +3177,25 @@ 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 ident = n[0].sym.name + message(c.config, n.info, warnGenericsIgnoredInjection, + "a new symbol '" & ident.s & "' has been injected during " & + "instantiation of " & c.p.owner.name.s & ", however " & + "overloads of " & ident.s & " will be used instead; " & + "either enable --experimental:genericsOpenSym to use the " & + "injected symbol or `bind` this symbol explicitly") result = semSymChoice(c, n, flags, expectedType) of nkSym: let s = n.sym + if nfDisabledOpenSym in n.flags: + message(c.config, n.info, warnGenericsIgnoredInjection, + "a new symbol '" & s.name.s & "' has been injected during " & + "instantiation of " & c.p.owner.name.s & ", however " & + getSymRepr(c.config, s) & " captured at " & + "the proc declaration will be used instead; " & + "either enable --experimental:genericsOpenSym to use the " & + "injected symbol or `bind` this captured symbol explicitly") # 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) diff --git a/compiler/semgnrc.nim b/compiler/semgnrc.nim index b8fcfda6d7526..3f92ff22c8126 100644 --- a/compiler/semgnrc.nim +++ b/compiler/semgnrc.nim @@ -76,7 +76,11 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym, else: result = symChoice(c, n, s, scOpen) if canOpenSym(s): - result = newOpenSym(result) + if genericsOpenSym in c.features: + result = newOpenSym(result) + else: + result.flags.incl nfDisabledOpenSym + if result.kind == nkSym: result.typ = nil case s.kind of skUnknown: # Introduced in this pass! Leave it as an identifier. @@ -105,7 +109,11 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym, else: result = newSymNodeTypeDesc(s, c.idgen, n.info) if canOpenSym(result.sym): - result = newOpenSym(result) + if genericsOpenSym in c.features: + result = newOpenSym(result) + else: + result.flags.incl nfDisabledOpenSym + result.typ = nil onUse(n.info, s) of skParam: result = n @@ -115,14 +123,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 = newOpenSym(result) + 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): + 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, diff --git a/doc/manual_experimental.md b/doc/manual_experimental.md index 9b52fbd2a4702..34399e6f9d2b7 100644 --- a/doc/manual_experimental.md +++ b/doc/manual_experimental.md @@ -2528,7 +2528,9 @@ Injected symbols in generic procs With the experimental option `genericsOpenSym`, captured symbols in generic routine bodies may 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. +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 @@ -2560,6 +2562,13 @@ proc baz[T](): string = 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. + VTable for methods ================== diff --git a/tests/generics/mopensymimport1.nim b/tests/generics/mopensymimport1.nim new file mode 100644 index 0000000000000..912db13026db7 --- /dev/null +++ b/tests/generics/mopensymimport1.nim @@ -0,0 +1,34 @@ +type + Result*[T, E] = object + when T is void: + when E is void: + oResultPrivate*: bool + else: + case oResultPrivate*: bool + of false: + eResultPrivate*: E + of true: + discard + else: + when E is void: + case oResultPrivate*: bool + of false: + discard + of true: + vResultPrivate*: T + else: + case oResultPrivate*: bool + of false: + eResultPrivate*: E + of true: + vResultPrivate*: T + +template valueOr*[T: not void, E](self: Result[T, E], def: untyped): untyped = + let s = (self) # TODO avoid copy + case s.oResultPrivate + of true: + s.vResultPrivate + of false: + when E isnot void: + template error: untyped {.used, inject.} = s.eResultPrivate + def diff --git a/tests/generics/mopensymimport2.nim b/tests/generics/mopensymimport2.nim new file mode 100644 index 0000000000000..1e1cda301cd99 --- /dev/null +++ b/tests/generics/mopensymimport2.nim @@ -0,0 +1,16 @@ +{.experimental: "genericsOpenSym".} + +import mopensymimport1 + +type Xxx = enum + error + value + +proc f(): Result[int, cstring] = + Result[int, cstring](oResultPrivate: false, eResultPrivate: "f") + +proc g*(T: type): string = + let x = f().valueOr: + return $error + + "ok" diff --git a/tests/generics/topensymimport.nim b/tests/generics/topensymimport.nim new file mode 100644 index 0000000000000..a47496827b188 --- /dev/null +++ b/tests/generics/topensymimport.nim @@ -0,0 +1,5 @@ +# issue #23386 + +import mopensymimport2 + +doAssert g(int) == "f" From 00a08eb98ca2d1628bc492ba756e2404ebcfce7c Mon Sep 17 00:00:00 2001 From: metagn Date: Sat, 27 Jul 2024 20:28:53 -0600 Subject: [PATCH 07/11] preemptive fix maybe --- compiler/lookups.nim | 2 ++ 1 file changed, 2 insertions(+) diff --git a/compiler/lookups.nim b/compiler/lookups.nim index be80d59051036..f9a1c1b2cb432 100644 --- a/compiler/lookups.nim +++ b/compiler/lookups.nim @@ -803,6 +803,8 @@ proc symChoiceExtension(o: var TOverloadIter; c: PContext; n: PNode): PSym = inc o.importIdx proc nextOverloadIter*(o: var TOverloadIter, c: PContext, n: PNode): PSym = + var n = n + if n.kind == nkOpenSym: n = n[0] case o.mode of oimDone: result = nil From 0334fc794ece7617cbfca82431eb78dd146317fc Mon Sep 17 00:00:00 2001 From: metagn Date: Mon, 29 Jul 2024 13:14:12 -0600 Subject: [PATCH 08/11] remove preemptive fix for now + comment reason for line above --- compiler/lookups.nim | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/lookups.nim b/compiler/lookups.nim index f9a1c1b2cb432..d0e114a185871 100644 --- a/compiler/lookups.nim +++ b/compiler/lookups.nim @@ -704,6 +704,8 @@ proc qualifiedLookUp*(c: PContext, n: PNode, flags: set[TLookupFlag]): PSym = 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() @@ -803,8 +805,6 @@ proc symChoiceExtension(o: var TOverloadIter; c: PContext; n: PNode): PSym = inc o.importIdx proc nextOverloadIter*(o: var TOverloadIter, c: PContext, n: PNode): PSym = - var n = n - if n.kind == nkOpenSym: n = n[0] case o.mode of oimDone: result = nil From 3fe15eb1616d0eeef29f06f3dd60c24de7265676 Mon Sep 17 00:00:00 2001 From: metagn Date: Sun, 11 Aug 2024 19:12:34 +0300 Subject: [PATCH 09/11] add symchoice fix from #23939 --- compiler/semexprs.nim | 1 + compiler/semgnrc.nim | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim index 51ebccfb79db1..1c83373e3a69c 100644 --- a/compiler/semexprs.nim +++ b/compiler/semexprs.nim @@ -3185,6 +3185,7 @@ proc semExpr(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType: PType "overloads of " & ident.s & " will be used instead; " & "either enable --experimental:genericsOpenSym to use the " & "injected symbol or `bind` this symbol explicitly") + n.typ = newTypeS(tyNone, c) result = semSymChoice(c, n, flags, expectedType) of nkSym: let s = n.sym diff --git a/compiler/semgnrc.nim b/compiler/semgnrc.nim index 3f92ff22c8126..e2da56c5d7abd 100644 --- a/compiler/semgnrc.nim +++ b/compiler/semgnrc.nim @@ -80,7 +80,7 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym, result = newOpenSym(result) else: result.flags.incl nfDisabledOpenSym - if result.kind == nkSym: result.typ = nil + result.typ = nil case s.kind of skUnknown: # Introduced in this pass! Leave it as an identifier. From f0e40ed0b1c343630855f4aeeb5bba0823f5af1c Mon Sep 17 00:00:00 2001 From: metagn Date: Mon, 12 Aug 2024 15:00:11 +0300 Subject: [PATCH 10/11] fix spurious warning, change new define name --- compiler/condsyms.nim | 2 +- compiler/semexprs.nim | 48 +++++++++++++-------- tests/generics/tmacroinjectedsymwarning.nim | 5 +++ 3 files changed, 36 insertions(+), 19 deletions(-) diff --git a/compiler/condsyms.nim b/compiler/condsyms.nim index bee9ad65fe479..3119e657eb09b 100644 --- a/compiler/condsyms.nim +++ b/compiler/condsyms.nim @@ -166,5 +166,5 @@ proc initDefines*(symbols: StringTableRef) = defineSymbol("nimHasWarnStdPrefix") defineSymbol("nimHasVtables") - defineSymbol("nimHasGenericsOpenSym") + defineSymbol("nimHasGenericsOpenSym2") defineSymbol("nimHasJsNoLambdaLifting") diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim index 1c83373e3a69c..6301dd94501c5 100644 --- a/compiler/semexprs.nim +++ b/compiler/semexprs.nim @@ -157,7 +157,8 @@ 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, flags: TExprFlags, expectedType: PType): PNode = +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 @@ -182,11 +183,33 @@ proc semOpenSym(c: PContext, n: PNode, flags: TExprFlags, expectedType: PType): var o = s2.owner while o != nil: if o == c.p.owner: - result = semExpr(c, id, flags, expectedType) + if not warnDisabled: + result = semExpr(c, id, flags, expectedType) + else: + result = nil + var msg = + "a new symbol '" & ident.s & "' has been injected during " & + "instantiation of " & c.p.owner.name.s & ", 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") + else: + n.typ = newTypeS(tyNone, c) + 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) return o = o.owner # nothing found - result = semExpr(c, n, flags, expectedType) + if not warnDisabled: + result = semExpr(c, n, flags, expectedType) + else: + result = nil proc inlineConst(c: PContext, n: PNode, s: PSym): PNode {.inline.} = result = copyTree(s.astdef) @@ -3178,25 +3201,14 @@ proc semExpr(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType: PType result = semSymChoice(c, result, flags, expectedType) of nkClosedSymChoice, nkOpenSymChoice: if nfDisabledOpenSym in n.flags: - let ident = n[0].sym.name - message(c.config, n.info, warnGenericsIgnoredInjection, - "a new symbol '" & ident.s & "' has been injected during " & - "instantiation of " & c.p.owner.name.s & ", however " & - "overloads of " & ident.s & " will be used instead; " & - "either enable --experimental:genericsOpenSym to use the " & - "injected symbol or `bind` this symbol explicitly") - n.typ = newTypeS(tyNone, c) + 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 nfDisabledOpenSym in n.flags: - message(c.config, n.info, warnGenericsIgnoredInjection, - "a new symbol '" & s.name.s & "' has been injected during " & - "instantiation of " & c.p.owner.name.s & ", however " & - getSymRepr(c.config, s) & " captured at " & - "the proc declaration will be used instead; " & - "either enable --experimental:genericsOpenSym to use the " & - "injected symbol or `bind` this captured symbol explicitly") + 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) diff --git a/tests/generics/tmacroinjectedsymwarning.nim b/tests/generics/tmacroinjectedsymwarning.nim index 43d11aed712ec..4b2ae4f7e5262 100644 --- a/tests/generics/tmacroinjectedsymwarning.nim +++ b/tests/generics/tmacroinjectedsymwarning.nim @@ -46,6 +46,11 @@ proc f(): Result[int, cstring] = proc g(T: type): string = let x = f().valueOr: + {.push warningAsError[GenericsIgnoredInjection]: on.} + # test spurious error + discard true + let _ = f + {.pop.} return $error #[tt.Warning ^ a new symbol 'error' has been injected during instantiation of g, however 'error' [enumField declared in tmacroinjectedsymwarning.nim(6, 3)] captured at the proc declaration will be used instead; either enable --experimental:genericsOpenSym to use the injected symbol or `bind` this captured symbol explicitly [GenericsIgnoredInjection]]# From ccc20ad7edaaf22455c6c73195fd38b01a38d9d0 Mon Sep 17 00:00:00 2001 From: metagn Date: Mon, 12 Aug 2024 15:03:03 +0300 Subject: [PATCH 11/11] simplify diff and fix symchoice again --- compiler/semexprs.nim | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim index 6301dd94501c5..d49f115ea80b7 100644 --- a/compiler/semexprs.nim +++ b/compiler/semexprs.nim @@ -185,8 +185,8 @@ proc semOpenSym(c: PContext, n: PNode, flags: TExprFlags, expectedType: PType, if o == c.p.owner: if not warnDisabled: result = semExpr(c, id, flags, expectedType) + return else: - result = nil var msg = "a new symbol '" & ident.s & "' has been injected during " & "instantiation of " & c.p.owner.name.s & ", however " @@ -197,19 +197,21 @@ proc semOpenSym(c: PContext, n: PNode, flags: TExprFlags, expectedType: PType, "either enable --experimental:genericsOpenSym to use the " & "injected symbol or `bind` this captured symbol explicitly") else: - n.typ = newTypeS(tyNone, c) 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) - return + break o = o.owner # 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)