From dd37bb88febafba67c264ba715398aae8863fd5c 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. (cherry picked from commit 469a6044c00ce657d2f543f292678b3c71e0b037) --- compiler/ast.nim | 2 + compiler/semexprs.nim | 75 +++++++++++++++------ compiler/semgnrc.nim | 4 +- tests/config.nims | 2 +- tests/generics/tmacroinjectedsym.nim | 59 ++++++++++++++++ tests/generics/tmacroinjectedsymwarning.nim | 6 +- 6 files changed, 122 insertions(+), 26 deletions(-) diff --git a/compiler/ast.nim b/compiler/ast.nim index d0cccdca4c..bb9443ecf8 100644 --- a/compiler/ast.nim +++ b/compiler/ast.nim @@ -518,6 +518,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 6612b5b219..8c778409a2 100644 --- a/compiler/semexprs.nim +++ b/compiler/semexprs.nim @@ -148,6 +148,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: @@ -3089,31 +3134,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 e8b1872c73..b1de4133c0 100644 --- a/compiler/semgnrc.nim +++ b/compiler/semgnrc.nim @@ -71,9 +71,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 e0de65fc42..51aa4e8f1e 100644 --- a/tests/config.nims +++ b/tests/config.nims @@ -44,4 +44,4 @@ switch("define", "nimPreviewNonVarDestructor") switch("warningAserror", "UnnamedBreak") switch("legacy", "verboseTypeMismatch") - +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"