From a3e5f4f69c58cdfe9fcc3f3cddd84e7438c907df Mon Sep 17 00:00:00 2001 From: metagn Date: Mon, 18 Dec 2023 19:40:30 +0300 Subject: [PATCH] allow replacing captured syms in macro calls in generics (#23091) fixes #22605, separated from #22744 This marks symbol captures in macro calls in generic contexts as `nfOpenSym`, which means if there is a new symbol in the local instantiatied body during instantiation time, this symbol replaces the captured symbol. We have to be careful not to consider symbols outside of the instantiation body during instantiation, because this will leak symbols from the instantiation context scope rather than the original declaration scope. This is done by checking if the local context owner (maybe should be the symbol of the proc currently getting instantiated instead? not sure how to get this) is the same as or a parent owner of the owner of the replacement candidate symbol. This solution is distinct from the symchoice mechanisms which we originally assumed had to be related, if this assumption was wrong it would explain why this solution took so long to arrive at. (cherry picked from commit 941659581add605e8a4ddfc9ff00662aa6234d26) --- compiler/ast.nim | 4 +- compiler/ic/ic.nim | 2 +- compiler/semexprs.nim | 20 ++++++- compiler/semgnrc.nim | 13 +++++ tests/generics/tmacroinjectedsym.nim | 86 ++++++++++++++++++++++++++++ 5 files changed, 121 insertions(+), 4 deletions(-) create mode 100644 tests/generics/tmacroinjectedsym.nim diff --git a/compiler/ast.nim b/compiler/ast.nim index ecd7673f84f25..78d1e32032257 100644 --- a/compiler/ast.nim +++ b/compiler/ast.nim @@ -516,6 +516,7 @@ 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 TNodeFlags* = set[TNodeFlag] TTypeFlag* = enum # keep below 32 for efficiency reasons (now: 47) @@ -1087,7 +1088,8 @@ const nfIsRef, nfIsPtr, nfPreventCg, nfLL, nfFromTemplate, nfDefaultRefsParam, nfExecuteOnReload, nfLastRead, - nfFirstWrite, nfSkipFieldChecking} + nfFirstWrite, nfSkipFieldChecking, + nfOpenSym} namePos* = 0 patternPos* = 1 # empty except for term rewriting macros genericParamsPos* = 2 diff --git a/compiler/ic/ic.nim b/compiler/ic/ic.nim index a72db57c5f129..dfa7a2b7fa41f 100644 --- a/compiler/ic/ic.nim +++ b/compiler/ic/ic.nim @@ -421,7 +421,7 @@ proc addModuleRef(n: PNode; ir: var PackedTree; c: var PackedEncoder; m: var Pac ## add a remote symbol reference to the tree let info = n.info.toPackedInfo(c, m) ir.nodes.add PackedNode(kind: nkModuleRef, operand: 3.int32, # spans 3 nodes in total - typeId: storeTypeLater(n.typ, c, m), info: info) + typeId: storeTypeLater(n.typ, c, m), info: info, flags: n.flags) ir.nodes.add PackedNode(kind: nkInt32Lit, info: info, operand: toLitId(n.sym.itemId.module.FileIndex, c, m).int32) ir.nodes.add PackedNode(kind: nkInt32Lit, info: info, diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim index e13b6cc09132e..6314705cd008b 100644 --- a/compiler/semexprs.nim +++ b/compiler/semexprs.nim @@ -1016,7 +1016,10 @@ proc afterCallActions(c: PContext; n, orig: PNode, flags: TExprFlags; expectedTy of skMacro: result = semMacroExpr(c, result, orig, callee, flags, expectedType) of skTemplate: result = semTemplateExpr(c, result, callee, flags, expectedType) else: - semFinishOperands(c, result) + if callee.magic notin {mArrGet, mArrPut, mNBindSym}: + # calls to `[]` can be explicit generic instantiations, + # don't sem every operand now, leave it to semmagic + semFinishOperands(c, result) activate(c, result) fixAbstractType(c, result) analyseIfAddressTakenInCall(c, result) @@ -3015,9 +3018,22 @@ proc semExpr(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType: PType if result.kind == nkSym: result = semSym(c, result, result.sym, flags) of nkSym: + let s = n.sym + if nfOpenSym in n.flags: + let id = newIdentNode(s.name, n.info) + c.isAmbiguous = false + let s2 = qualifiedLookUp(c, id, {}) + if s2 != nil and s2 != s and not c.isAmbiguous: + # only consider symbols defined under current proc: + var o = s2.owner + while o != nil: + if o == c.p.owner: + result = semExpr(c, id, flags, expectedType) + return + o = o.owner # 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, n.sym, flags) + result = semSym(c, n, s, flags) of nkEmpty, nkNone, nkCommentStmt, nkType: discard of nkNilLit: diff --git a/compiler/semgnrc.nim b/compiler/semgnrc.nim index cbbb8cf556c2d..26b69c261eb62 100644 --- a/compiler/semgnrc.nim +++ b/compiler/semgnrc.nim @@ -68,6 +68,9 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym, result.transitionSonsKind(nkClosedSymChoice) else: result = symChoice(c, n, s, scOpen) + if {withinMixin, withinConcept} * flags == {withinMixin} and result.kind == nkSym: + result.flags.incl nfOpenSym + result.typ = nil case s.kind of skUnknown: # Introduced in this pass! Leave it as an identifier. @@ -95,6 +98,9 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym, result = n else: result = newSymNodeTypeDesc(s, c.idgen, n.info) + if {withinMixin, withinConcept} * flags == {withinMixin}: + result.flags.incl nfOpenSym + result.typ = nil onUse(n.info, s) of skParam: result = n @@ -103,11 +109,17 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym, if (s.typ != nil) and (s.typ.flags * {tfGenericTypeParam, tfImplicitTypeParam} == {}): result = newSymNodeTypeDesc(s, c.idgen, n.info) + if {withinMixin, withinConcept} * flags == {withinMixin}: + result.flags.incl nfOpenSym + result.typ = nil else: result = n onUse(n.info, s) else: result = newSymNode(s, n.info) + if {withinMixin, withinConcept} * flags == {withinMixin}: + result.flags.incl nfOpenSym + result.typ = nil onUse(n.info, s) proc lookup(c: PContext, n: PNode, flags: TSemGenericFlags, @@ -147,6 +159,7 @@ proc fuzzyLookup(c: PContext, n: PNode, flags: TSemGenericFlags, var s = qualifiedLookUp(c, n, luf) if s != nil: + isMacro = s.kind in {skTemplate, skMacro} result = semGenericStmtSymbol(c, n, s, ctx, flags) else: n[0] = semGenericStmt(c, n[0], flags, ctx) diff --git a/tests/generics/tmacroinjectedsym.nim b/tests/generics/tmacroinjectedsym.nim new file mode 100644 index 0000000000000..a98c1edb11c4a --- /dev/null +++ b/tests/generics/tmacroinjectedsym.nim @@ -0,0 +1,86 @@ +block: # issue #22605, normal call syntax + const error = "bad" + + template valueOr(self: int, def: untyped): untyped = + case false + of true: "" + of false: + template error: untyped {.used, inject.} = "good" + def + + proc g(T: type): string = + let x = valueOr 123: + return $error + + "ok" + + doAssert g(int) == "good" + +block: # issue #22605, method call syntax + const error = "bad" + + template valueOr(self: int, def: untyped): untyped = + case false + of true: "" + of false: + template error: untyped {.used, inject.} = "good" + def + + proc g(T: type): string = + let x = 123.valueOr: + return $error + + "ok" + + doAssert g(int) == "good" + +block: # issue #22605, original complex example + type Xxx = enum + error + value + + 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 + + proc f(): Result[int, cstring] = + Result[int, cstring](oResultPrivate: false, eResultPrivate: "f") + + proc g(T: type): string = + let x = f().valueOr: + return $error + + "ok" + + doAssert g(int) == "f"