From 341b04d3d8861f971fd686a792559d1ef6f75ad2 Mon Sep 17 00:00:00 2001 From: metagn <10591326+metagn@users.noreply.github.com> Date: Mon, 18 Dec 2023 12:08:57 +0300 Subject: [PATCH 1/3] allow replacing captured syms in macro calls in generics fixes #22605, separated from #22744 --- compiler/ast.nim | 4 +- compiler/semexprs.nim | 10 +++- compiler/semgnrc.nim | 24 +++++++- tests/generics/tmacroinjectedsym.nim | 86 ++++++++++++++++++++++++++++ 4 files changed, 121 insertions(+), 3 deletions(-) create mode 100644 tests/generics/tmacroinjectedsym.nim diff --git a/compiler/ast.nim b/compiler/ast.nim index 06b6cd3573a0a..732763f0fe61b 100644 --- a/compiler/ast.nim +++ b/compiler/ast.nim @@ -520,6 +520,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) @@ -1095,7 +1096,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/semexprs.nim b/compiler/semexprs.nim index b3b8c27fc405a..d6d9c1adf7a30 100644 --- a/compiler/semexprs.nim +++ b/compiler/semexprs.nim @@ -3063,9 +3063,17 @@ proc semExpr(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType: PType of nkClosedSymChoice, nkOpenSymChoice: result = semSymChoice(c, result, flags, expectedType) 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 and s2.owner == c.p.owner: + result = semExpr(c, id, flags, expectedType) + 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, 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 cae823a8f0cae..3236ee0855ed5 100644 --- a/compiler/semgnrc.nim +++ b/compiler/semgnrc.nim @@ -69,6 +69,9 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym, result.transitionSonsKind(nkClosedSymChoice) else: result = symChoice(c, n, s, scOpen) + if withinMixin in flags 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. @@ -96,6 +99,9 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym, result = n else: result = newSymNodeTypeDesc(s, c.idgen, n.info) + if withinMixin in flags: + result.flags.incl nfOpenSym + result.typ = nil onUse(n.info, s) of skParam: result = n @@ -104,11 +110,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 in flags: + result.flags.incl nfOpenSym + result.typ = nil else: result = n onUse(n.info, s) else: result = newSymNode(s, n.info) + if withinMixin in flags: + result.flags.incl nfOpenSym + result.typ = nil onUse(n.info, s) proc lookup(c: PContext, n: PNode, flags: TSemGenericFlags, @@ -148,6 +160,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) @@ -224,7 +237,16 @@ proc semGenericStmt(c: PContext, n: PNode, var dummy: bool result = fuzzyLookup(c, n, flags, ctx, dummy) of nkSym: - let a = n.sym + var a = n.sym + if nfOpenSym in n.flags: + let id = newIdentNode(a.name, n.info) + c.isAmbiguous = false + let s2 = qualifiedLookUp(c, id, {}) + if s2 != nil and s2 != a and not c.isAmbiguous and s2.owner == c.p.owner: + n.sym = s2 + a = s2 + if withinMixin notin flags: + n.flags.excl nfOpenSym let b = getGenSym(c, a) if b != a: n.sym = b of nkEmpty, succ(nkSym)..nkNilLit, nkComesFrom: 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" From 98f71e19779623e0d6c3be37ea9797fe952f80fe Mon Sep 17 00:00:00 2001 From: metagn <10591326+metagn@users.noreply.github.com> Date: Mon, 18 Dec 2023 13:37:08 +0300 Subject: [PATCH 2/3] should fix CI --- compiler/semexprs.nim | 5 ++++- compiler/semgnrc.nim | 10 +++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim index d6d9c1adf7a30..5e75a23d1953e 100644 --- a/compiler/semexprs.nim +++ b/compiler/semexprs.nim @@ -1024,7 +1024,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) diff --git a/compiler/semgnrc.nim b/compiler/semgnrc.nim index 3236ee0855ed5..5a20bbd5e000d 100644 --- a/compiler/semgnrc.nim +++ b/compiler/semgnrc.nim @@ -69,7 +69,7 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym, result.transitionSonsKind(nkClosedSymChoice) else: result = symChoice(c, n, s, scOpen) - if withinMixin in flags and result.kind == nkSym: + if {withinMixin, withinConcept} * flags == {withinMixin} and result.kind == nkSym: result.flags.incl nfOpenSym result.typ = nil case s.kind @@ -99,7 +99,7 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym, result = n else: result = newSymNodeTypeDesc(s, c.idgen, n.info) - if withinMixin in flags: + if {withinMixin, withinConcept} * flags == {withinMixin}: result.flags.incl nfOpenSym result.typ = nil onUse(n.info, s) @@ -110,7 +110,7 @@ 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 in flags: + if {withinMixin, withinConcept} * flags == {withinMixin}: result.flags.incl nfOpenSym result.typ = nil else: @@ -118,7 +118,7 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym, onUse(n.info, s) else: result = newSymNode(s, n.info) - if withinMixin in flags: + if {withinMixin, withinConcept} * flags == {withinMixin}: result.flags.incl nfOpenSym result.typ = nil onUse(n.info, s) @@ -245,7 +245,7 @@ proc semGenericStmt(c: PContext, n: PNode, if s2 != nil and s2 != a and not c.isAmbiguous and s2.owner == c.p.owner: n.sym = s2 a = s2 - if withinMixin notin flags: + if {withinMixin, withinConcept} * flags != {withinMixin}: n.flags.excl nfOpenSym let b = getGenSym(c, a) if b != a: n.sym = b From fd76586ff88611cbaf764d9a64e15bf52a88f556 Mon Sep 17 00:00:00 2001 From: metagn <10591326+metagn@users.noreply.github.com> Date: Mon, 18 Dec 2023 17:00:58 +0300 Subject: [PATCH 3/3] fix IC, remove redundant logic from semgnrc, consider nested owners --- compiler/ic/ic.nim | 10 ++++++---- compiler/semexprs.nim | 11 ++++++++--- compiler/semgnrc.nim | 11 +---------- 3 files changed, 15 insertions(+), 17 deletions(-) diff --git a/compiler/ic/ic.nim b/compiler/ic/ic.nim index e856219107a91..c9f546c76a952 100644 --- a/compiler/ic/ic.nim +++ b/compiler/ic/ic.nim @@ -433,11 +433,11 @@ proc addModuleRef(n: PNode; ir: var PackedTree; c: var PackedEncoder; m: var Pac let info = n.info.toPackedInfo(c, m) if n.typ != n.sym.typ: ir.addNode(kind = nkModuleRef, operand = 3.int32, # spans 3 nodes in total - info = info, + info = info, flags = n.flags, typeId = storeTypeLater(n.typ, c, m)) else: ir.addNode(kind = nkModuleRef, operand = 3.int32, # spans 3 nodes in total - info = info) + info = info, flags = n.flags) ir.addNode(kind = nkNone, info = info, operand = toLitId(n.sym.itemId.module.FileIndex, c, m).int32) ir.addNode(kind = nkNone, info = info, @@ -829,7 +829,8 @@ proc loadNodes*(c: var PackedDecoder; g: var PackedModuleGraph; thisModule: int; result.ident = getIdent(c.cache, g[thisModule].fromDisk.strings[n.litId]) of nkSym: result.sym = loadSym(c, g, thisModule, PackedItemId(module: LitId(0), item: tree[n].soperand)) - if result.typ == nil: result.typ = result.sym.typ + if result.typ == nil and nfOpenSym notin result.flags: + result.typ = result.sym.typ of externIntLit: result.intVal = g[thisModule].fromDisk.numbers[n.litId] of nkStrLit..nkTripleStrLit: @@ -842,7 +843,8 @@ proc loadNodes*(c: var PackedDecoder; g: var PackedModuleGraph; thisModule: int; assert n2.kind == nkNone transitionNoneToSym(result) result.sym = loadSym(c, g, thisModule, PackedItemId(module: n1.litId, item: tree[n2].soperand)) - if result.typ == nil: result.typ = result.sym.typ + if result.typ == nil and nfOpenSym notin result.flags: + result.typ = result.sym.typ else: for n0 in sonsReadonly(tree, n): result.addAllowNil loadNodes(c, g, thisModule, tree, n0) diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim index 5e75a23d1953e..93574e217ffea 100644 --- a/compiler/semexprs.nim +++ b/compiler/semexprs.nim @@ -3071,9 +3071,14 @@ proc semExpr(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType: PType 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 and s2.owner == c.p.owner: - result = semExpr(c, id, flags, expectedType) - return + 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, s, flags) diff --git a/compiler/semgnrc.nim b/compiler/semgnrc.nim index 5a20bbd5e000d..a96bc484bb294 100644 --- a/compiler/semgnrc.nim +++ b/compiler/semgnrc.nim @@ -237,16 +237,7 @@ proc semGenericStmt(c: PContext, n: PNode, var dummy: bool result = fuzzyLookup(c, n, flags, ctx, dummy) of nkSym: - var a = n.sym - if nfOpenSym in n.flags: - let id = newIdentNode(a.name, n.info) - c.isAmbiguous = false - let s2 = qualifiedLookUp(c, id, {}) - if s2 != nil and s2 != a and not c.isAmbiguous and s2.owner == c.p.owner: - n.sym = s2 - a = s2 - if {withinMixin, withinConcept} * flags != {withinMixin}: - n.flags.excl nfOpenSym + let a = n.sym let b = getGenSym(c, a) if b != a: n.sym = b of nkEmpty, succ(nkSym)..nkNilLit, nkComesFrom: