From 469a6044c00ce657d2f543f292678b3c71e0b037 Mon Sep 17 00:00:00 2001 From: metagn Date: Thu, 25 Jul 2024 14:10:15 -0600 Subject: [PATCH] implement genericsOpenSym for symchoices (#23873) fixes #23865 The node flag `nfOpenSym` implemented in #23091 for sym nodes is now also implemented for open symchoices. This means the intended behavior is still achieved when multiple overloads are in scope to be captured, so the issue is fixed. The code for the flag is documented and moved into a helper proc and the experimental switch is now enabled for the compiler test suite. --- compiler/ast.nim | 2 + compiler/semexprs.nim | 75 +++++++++++++++------ compiler/semgnrc.nim | 4 +- tests/config.nims | 1 + tests/generics/tmacroinjectedsym.nim | 59 ++++++++++++++++ tests/generics/tmacroinjectedsymwarning.nim | 6 +- 6 files changed, 122 insertions(+), 25 deletions(-) diff --git a/compiler/ast.nim b/compiler/ast.nim index c74861bd87..624bc32f9c 100644 --- a/compiler/ast.nim +++ b/compiler/ast.nim @@ -326,6 +326,8 @@ type 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) diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim index f727fcbfda..54a8440351 100644 --- a/compiler/semexprs.nim +++ b/compiler/semexprs.nim @@ -157,6 +157,51 @@ 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 + ## 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 ident = n.getPIdent + assert ident != nil + let id = newIdentNode(ident, n.info) + c.isAmbiguous = false + let s2 = qualifiedLookUp(c, id, {}) + # for `nkSym`, the first found symbol being different and unambiguous is + # enough to replace the original + # for `nkOpenSymChoice`, the first found symbol must be non-overloadable, + # since otherwise we have to use regular `nkOpenSymChoice` functionality + if s2 != nil and not c.isAmbiguous and + ((s == nil and s2.kind notin OverloadableSyms) or + (s != nil and s2 != s)): + # only consider symbols defined under current proc: + 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 s == nil: + msg.add( + "overloads of " & ident.s & " will be used instead; " & + "either enable --experimental:genericsOpenSym to use the " & + "injected symbol or `bind` this symbol explicitly") + else: + msg.add( + 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") + message(c.config, n.info, warnGenericsIgnoredInjection, msg) + break + o = o.owner + proc inlineConst(c: PContext, n: PNode, s: PSym): PNode {.inline.} = result = copyTree(s.astdef) if result.isNil: @@ -3141,31 +3186,17 @@ proc semExpr(c: PContext, n: PNode, flags: TExprFlags = {}, expectedType: PType if isSymChoice(result): result = semSymChoice(c, result, flags, expectedType) of nkClosedSymChoice, nkOpenSymChoice: - result = semSymChoice(c, result, flags, expectedType) + 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: - 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: - if genericsOpenSym in c.features: - result = semExpr(c, id, flags, expectedType) - return - else: - 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") - break - o = o.owner + 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) diff --git a/compiler/semgnrc.nim b/compiler/semgnrc.nim index 638b4311bd..e914cf00d3 100644 --- a/compiler/semgnrc.nim +++ b/compiler/semgnrc.nim @@ -72,9 +72,9 @@ proc semGenericStmtSymbol(c: PContext, n: PNode, s: PSym, result.transitionSonsKind(nkClosedSymChoice) else: result = symChoice(c, n, s, scOpen) - if result.kind == nkSym and canOpenSym(result.sym): + if canOpenSym(s): result.flags.incl nfOpenSym - result.typ = nil + if result.kind == nkSym: result.typ = nil case s.kind of skUnknown: # Introduced in this pass! Leave it as an identifier. diff --git a/tests/config.nims b/tests/config.nims index 690123c4a3..91934cc74b 100644 --- a/tests/config.nims +++ b/tests/config.nims @@ -45,3 +45,4 @@ switch("define", "nimPreviewNonVarDestructor") switch("warningAserror", "UnnamedBreak") switch("legacy", "verboseTypeMismatch") switch("experimental", "vtables") +switch("experimental", "genericsOpenSym") diff --git a/tests/generics/tmacroinjectedsym.nim b/tests/generics/tmacroinjectedsym.nim index d36d34cdd8..5271a667f1 100644 --- a/tests/generics/tmacroinjectedsym.nim +++ b/tests/generics/tmacroinjectedsym.nim @@ -113,3 +113,62 @@ block: # issue #22605, original complex example "ok" doAssert g2(int) == "error" + +block: # issue #23865 + 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 + + func error[T, E](self: Result[T, E]): E = + ## Fetch error of result if set, or raise Defect + case self.oResultPrivate + of true: + when T isnot void: + raiseResultDefect("Trying to access error when value is set", self.vResultPrivate) + else: + raiseResultDefect("Trying to access error when value is set") + of false: + when E isnot void: + self.eResultPrivate + + 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) == "error" diff --git a/tests/generics/tmacroinjectedsymwarning.nim b/tests/generics/tmacroinjectedsymwarning.nim index 7adb759e88..43d11aed71 100644 --- a/tests/generics/tmacroinjectedsymwarning.nim +++ b/tests/generics/tmacroinjectedsymwarning.nim @@ -1,3 +1,7 @@ +discard """ + matrix: "--skipParentCfg --filenames:legacyRelProj" +""" + type Xxx = enum error value @@ -43,7 +47,7 @@ proc f(): Result[int, cstring] = proc g(T: type): string = let x = f().valueOr: return $error #[tt.Warning - ^ a new symbol 'error' has been injected during instantiation of g, however 'error' [enumField declared in tmacroinjectedsymwarning.nim(2, 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]]# + ^ 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]]# "ok"