From 5ffb18458a49ef32adcf2315312eb38a57437ce6 Mon Sep 17 00:00:00 2001 From: Jacek Sieka Date: Mon, 4 Sep 2023 11:35:20 +0200 Subject: [PATCH 01/14] Workaround for #34 See also https://github.com/nim-lang/Nim/issues/22605 --- results.nim | 123 +++++++++++++++++++++++++++++++++++++---- tests/test_results.nim | 36 +++++++++--- 2 files changed, 140 insertions(+), 19 deletions(-) diff --git a/results.nim b/results.nim index 5d41448..4be4c54 100644 --- a/results.nim +++ b/results.nim @@ -340,6 +340,20 @@ type Opt*[T] = Result[T, void] +const + resultsGenericBindingWorkaround* {.booldefine.} = true + ## Enable a workaround for the template injection problem in the issue + ## linked below where injected templates get bound differently depending + ## on whether we're in a generic context or not - this leads to surprising + ## errors where random symbols from outer scopes get bound to the name + ## instead of the intended value. + ## However, this ugly hack might introduce more damage than it's worth so + ## it can be disabled at compile-time - hopefully an upstream solution + ## can be found. + # TODO https://github.com/nim-lang/Nim/issues/22605 + # TODO https://github.com/arnetheduck/nim-results/issues/34 + + func raiseResultOk[T, E](self: Result[T, E]) {.noreturn, noinline.} = # noinline because raising should take as little space as possible at call # site @@ -955,6 +969,67 @@ func get*[T, E](self: Result[T, E], otherwise: T): T {.inline.} = of false: otherwise +when resultsGenericBindingWorkaround: + import macros + + proc containsHack(n: NimNode): bool = + if n.len == 0: + n.eqIdent("isOkOr") or n.eqIdent("isErrOr") or n.eqIdent("valueOr") or + n.eqIdent("errorOr") + else: + for child in n: + if containsHack(child): + return true + false + + proc replace(n: NimNode, what: string, with: NimNode): NimNode = + if n.eqIdent(what): + result = with + else: + case n.kind + of nnkCallKinds: + # `error(...)` - replace args but not function name + if n[0].containsHack(): + result = n + else: + result = copyNimNode(n) + result.add n[0] + for i in 1.. Date: Mon, 4 Sep 2023 13:21:15 +0200 Subject: [PATCH 02/14] more exceptions --- results.nim | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/results.nim b/results.nim index 4be4c54..2e24937 100644 --- a/results.nim +++ b/results.nim @@ -982,7 +982,20 @@ when resultsGenericBindingWorkaround: return true false + proc containsIdent(n: NimNode, what: string): bool = + if n.kind == nnkIdent and n.eqIdent(what): + true + else: + for child in n: + if containsIdent(child, what): + return true + + false + proc replace(n: NimNode, what: string, with: NimNode): NimNode = + if not containsIdent(n, what): # Avoid lots of node copies if not needed + return n + if n.eqIdent(what): result = with else: @@ -997,20 +1010,12 @@ when resultsGenericBindingWorkaround: for i in 1.. Date: Mon, 4 Sep 2023 16:02:04 +0200 Subject: [PATCH 03/14] match nnkSym also for pre-ident-check --- results.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/results.nim b/results.nim index 2e24937..1a497d1 100644 --- a/results.nim +++ b/results.nim @@ -983,7 +983,7 @@ when resultsGenericBindingWorkaround: false proc containsIdent(n: NimNode, what: string): bool = - if n.kind == nnkIdent and n.eqIdent(what): + if n.eqIdent(what): true else: for child in n: From 87a712318c4b3169e8f4a83565b0fe50be1665a4 Mon Sep 17 00:00:00 2001 From: Jacek Sieka Date: Thu, 7 Sep 2023 10:30:40 +0200 Subject: [PATCH 04/14] fix hack name --- results.nim | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/results.nim b/results.nim index 1a497d1..9052bda 100644 --- a/results.nim +++ b/results.nim @@ -1036,7 +1036,7 @@ when resultsGenericBindingWorkaround: for i in 0.. Date: Mon, 19 Feb 2024 11:01:53 +0100 Subject: [PATCH 05/14] format --- results.nim | 265 +++++++++++++++++++--------------- tests/test_results.nim | 308 +++++++++++++++++++++++++++++----------- tests/test_results2.nim | 1 + 3 files changed, 381 insertions(+), 193 deletions(-) diff --git a/results.nim b/results.nim index 9052bda..5ad17f8 100644 --- a/results.nim +++ b/results.nim @@ -340,19 +340,17 @@ type Opt*[T] = Result[T, void] -const - resultsGenericBindingWorkaround* {.booldefine.} = true - ## Enable a workaround for the template injection problem in the issue - ## linked below where injected templates get bound differently depending - ## on whether we're in a generic context or not - this leads to surprising - ## errors where random symbols from outer scopes get bound to the name - ## instead of the intended value. - ## However, this ugly hack might introduce more damage than it's worth so - ## it can be disabled at compile-time - hopefully an upstream solution - ## can be found. - # TODO https://github.com/nim-lang/Nim/issues/22605 - # TODO https://github.com/arnetheduck/nim-results/issues/34 - +const resultsGenericBindingWorkaround* {.booldefine.} = true + ## Enable a workaround for the template injection problem in the issue + ## linked below where injected templates get bound differently depending + ## on whether we're in a generic context or not - this leads to surprising + ## errors where random symbols from outer scopes get bound to the name + ## instead of the intended value. + ## However, this ugly hack might introduce more damage than it's worth so + ## it can be disabled at compile-time - hopefully an upstream solution + ## can be found. + # TODO https://github.com/nim-lang/Nim/issues/22605 + # TODO https://github.com/arnetheduck/nim-results/issues/34 func raiseResultOk[T, E](self: Result[T, E]) {.noreturn, noinline.} = # noinline because raising should take as little space as possible at call @@ -360,7 +358,9 @@ func raiseResultOk[T, E](self: Result[T, E]) {.noreturn, noinline.} = when T is void: raise (ref ResultError[void])(msg: "Trying to access error with value") else: - raise (ref ResultError[T])(msg: "Trying to access error with value", error: self.vResultPrivate) + raise (ref ResultError[T])( + msg: "Trying to access error with value", error: self.vResultPrivate + ) func raiseResultError[T, E](self: Result[T, E]) {.noreturn, noinline.} = # noinline because raising should take as little space as possible at call @@ -376,15 +376,18 @@ func raiseResultError[T, E](self: Result[T, E]) {.noreturn, noinline.} = elif compiles(toException(self.eResultPrivate)): raise toException(self.eResultPrivate) elif compiles($self.eResultPrivate): - raise (ref ResultError[E])( - error: self.eResultPrivate, msg: $self.eResultPrivate) + raise (ref ResultError[E])(error: self.eResultPrivate, msg: $self.eResultPrivate) else: - raise (ref ResultError[E])(msg: "Trying to access value with err", error: self.eResultPrivate) + raise (ref ResultError[E])( + msg: "Trying to access value with err", error: self.eResultPrivate + ) func raiseResultDefect(m: string, v: auto) {.noreturn, noinline.} = mixin `$` - when compiles($v): raise (ref ResultDefect)(msg: m & ": " & $v) - else: raise (ref ResultDefect)(msg: m) + when compiles($v): + raise (ref ResultDefect)(msg: m & ": " & $v) + else: + raise (ref ResultDefect)(msg: m) func raiseResultDefect(m: string) {.noreturn, noinline.} = raise (ref ResultDefect)(msg: m) @@ -450,21 +453,30 @@ template err*[T](self: var Result[T, void]) = ## Example: `result.err()` self = err(type self) -template ok*(v: auto): auto = ok(typeof(result), v) -template ok*(): auto = ok(typeof(result)) +template ok*(v: auto): auto = + ok(typeof(result), v) + +template ok*(): auto = + ok(typeof(result)) -template err*(v: auto): auto = err(typeof(result), v) -template err*(): auto = err(typeof(result)) +template err*(v: auto): auto = + err(typeof(result), v) -template isOk*(self: Result): bool = self.oResultPrivate -template isErr*(self: Result): bool = not self.oResultPrivate +template err*(): auto = + err(typeof(result)) + +template isOk*(self: Result): bool = + self.oResultPrivate + +template isErr*(self: Result): bool = + not self.oResultPrivate when not defined(nimHasEffectsOfs): template effectsOf(f: untyped) {.pragma, used.} func map*[T0: not void, E; T1: not void]( - self: Result[T0, E], f: proc(x: T0): T1): - Result[T1, E] {.inline, effectsOf: f.} = + self: Result[T0, E], f: proc(x: T0): T1 +): Result[T1, E] {.inline, effectsOf: f.} = ## Transform value using f, or return error ## ## ``` @@ -485,8 +497,8 @@ func map*[T0: not void, E; T1: not void]( result.err(self.eResultPrivate) func map*[T: not void, E]( - self: Result[T, E], f: proc(x: T)): - Result[void, E] {.inline, effectsOf: f.} = + self: Result[T, E], f: proc(x: T) +): Result[void, E] {.inline, effectsOf: f.} = ## Transform value using f, or return error ## ## ``` @@ -504,8 +516,8 @@ func map*[T: not void, E]( result.err(self.eResultPrivate) func map*[E; T1: not void]( - self: Result[void, E], f: proc(): T1): - Result[T1, E] {.inline, effectsOf: f.} = + self: Result[void, E], f: proc(): T1 +): Result[T1, E] {.inline, effectsOf: f.} = ## Transform value using f, or return error case self.oResultPrivate of true: @@ -516,9 +528,7 @@ func map*[E; T1: not void]( else: result.err(self.eResultPrivate) -func map*[E]( - self: Result[void, E], f: proc()): - Result[void, E] {.inline, effectsOf: f.} = +func map*[E](self: Result[void, E], f: proc()): Result[void, E] {.inline, effectsOf: f.} = ## Call f if `self` is ok case self.oResultPrivate of true: @@ -531,8 +541,8 @@ func map*[E]( result.err(self.eResultPrivate) func flatMap*[T0: not void, E, T1]( - self: Result[T0, E], f: proc(x: T0): Result[T1, E]): - Result[T1, E] {.inline, effectsOf: f.} = + self: Result[T0, E], f: proc(x: T0): Result[T1, E] +): Result[T1, E] {.inline, effectsOf: f.} = case self.oResultPrivate of true: f(self.vResultPrivate) @@ -543,8 +553,8 @@ func flatMap*[T0: not void, E, T1]( Result[T1, E].err(self.eResultPrivate) func flatMap*[E, T1]( - self: Result[void, E], f: proc(): Result[T1, E]): - Result[T1, E] {.inline, effectsOf: f.} = + self: Result[void, E], f: proc(): Result[T1, E] +): Result[T1, E] {.inline, effectsOf: f.} = case self.oResultPrivate of true: f() @@ -554,9 +564,9 @@ func flatMap*[E, T1]( else: Result[T1, E].err(self.eResultPrivate) -func mapErr*[T; E0: not void; E1: not void]( - self: Result[T, E0], f: proc(x: E0): E1): - Result[T, E1] {.inline, effectsOf: f.} = +func mapErr*[T; E0: not void, E1: not void]( + self: Result[T, E0], f: proc(x: E0): E1 +): Result[T, E1] {.inline, effectsOf: f.} = ## Transform error using f, or leave untouched case self.oResultPrivate of true: @@ -568,8 +578,8 @@ func mapErr*[T; E0: not void; E1: not void]( result.err(f(self.eResultPrivate)) func mapErr*[T; E1: not void]( - self: Result[T, void], f: proc(): E1): - Result[T, E1] {.inline, effectsOf: f.} = + self: Result[T, void], f: proc(): E1 +): Result[T, E1] {.inline, effectsOf: f.} = ## Transform error using f, or return value case self.oResultPrivate of true: @@ -581,8 +591,8 @@ func mapErr*[T; E1: not void]( result.err(f()) func mapErr*[T; E0: not void]( - self: Result[T, E0], f: proc(x: E0)): - Result[T, void] {.inline, effectsOf: f.} = + self: Result[T, E0], f: proc(x: E0) +): Result[T, void] {.inline, effectsOf: f.} = ## Transform error using f, or return value case self.oResultPrivate of true: @@ -595,8 +605,8 @@ func mapErr*[T; E0: not void]( result.err() func mapErr*[T]( - self: Result[T, void], f: proc()): - Result[T, void] {.inline, effectsOf: f.} = + self: Result[T, void], f: proc() +): Result[T, void] {.inline, effectsOf: f.} = ## Transform error using f, or return value case self.oResultPrivate of true: @@ -609,7 +619,8 @@ func mapErr*[T]( result.err() func mapConvert*[T0: not void, E]( - self: Result[T0, E], T1: type): Result[T1, E] {.inline.} = + self: Result[T0, E], T1: type +): Result[T1, E] {.inline.} = ## Convert result value to A using an conversion # Would be nice if it was automatic... case self.oResultPrivate @@ -624,8 +635,7 @@ func mapConvert*[T0: not void, E]( else: result.err(self.eResultPrivate) -func mapCast*[T0: not void, E]( - self: Result[T0, E], T1: type): Result[T1, E] {.inline.} = +func mapCast*[T0: not void, E](self: Result[T0, E], T1: type): Result[T1, E] {.inline.} = ## Convert result value to A using a cast ## Would be nice with nicer syntax... case self.oResultPrivate @@ -640,8 +650,7 @@ func mapCast*[T0: not void, E]( else: result.err(self.eResultPrivate) -func mapConvertErr*[T, E0]( - self: Result[T, E0], E1: type): Result[T, E1] {.inline.} = +func mapConvertErr*[T, E0](self: Result[T, E0], E1: type): Result[T, E1] {.inline.} = ## Convert result error to E1 using an conversion # Would be nice if it was automatic... when E0 is E1: @@ -658,8 +667,7 @@ func mapConvertErr*[T, E0]( else: result.err(E1(self.eResultPrivate)) -func mapCastErr*[T, E0]( - self: Result[T, E0], E1: type): Result[T, E1] {.inline.} = +func mapCastErr*[T, E0](self: Result[T, E0], E1: type): Result[T, E1] {.inline.} = ## Convert result value to A using a cast ## Would be nice with nicer syntax... if self.oResultPrivate: @@ -719,7 +727,7 @@ template orErr*[T, E0, E1](self: Result[T, E0], error: E1): Result[T, E1] = ## ``` ## ## ** Experimental, may be removed ** - let s = (self) # TODO avoid copy + let s = (self) # TODO avoid copy type R = Result[T, E1] case s.oResultPrivate of true: @@ -733,7 +741,6 @@ template orErr*[T, E0, E1](self: Result[T, E0], error: E1): Result[T, E1] = of false: err(R, error) - template catch*(body: typed): Result[type(body), ref CatchableError] = ## Catch exceptions for body and store them in the Result ## @@ -772,10 +779,9 @@ template capture*[E: Exception](T: type, someExceptionExpr: ref E): Result[T, re ret = R.err(caught) ret -func `==`*[ - T0: not void, E0: not void, - T1: not void, E1: not void]( - lhs: Result[T0, E0], rhs: Result[T1, E1]): bool {.inline.} = +func `==`*[T0: not void, E0: not void, T1: not void, E1: not void]( + lhs: Result[T0, E0], rhs: Result[T1, E1] +): bool {.inline.} = if lhs.oResultPrivate != rhs.oResultPrivate: false else: @@ -785,8 +791,7 @@ func `==`*[ of false: lhs.eResultPrivate == rhs.eResultPrivate -func `==`*[E0, E1]( - lhs: Result[void, E0], rhs: Result[void, E1]): bool {.inline.} = +func `==`*[E0, E1](lhs: Result[void, E0], rhs: Result[void, E1]): bool {.inline.} = if lhs.oResultPrivate != rhs.oResultPrivate: false else: @@ -796,8 +801,7 @@ func `==`*[E0, E1]( of false: lhs.eResultPrivate == rhs.eResultPrivate -func `==`*[T0, T1]( - lhs: Result[T0, void], rhs: Result[T1, void]): bool {.inline.} = +func `==`*[T0, T1](lhs: Result[T0, void], rhs: Result[T1, void]): bool {.inline.} = if lhs.oResultPrivate != rhs.oResultPrivate: false else: @@ -822,8 +826,12 @@ func value*[T: not void, E](self: var Result[T, E]): var T {.inline.} = ## Fetch value of result if set, or raise Defect ## Exception bridge mode: raise given Exception instead ## See also: Option.get - (block: - withAssertOk(self): addr self.vResultPrivate)[] + + ( + block: + withAssertOk(self): + addr self.vResultPrivate + )[] template `[]`*[T: not void, E](self: Result[T, E]): T = ## Fetch value of result if set, or raise Defect @@ -877,24 +885,30 @@ func expect*[T, E](self: Result[T, E], m: string): T = self.vResultPrivate func expect*[T: not void, E](self: var Result[T, E], m: string): var T = - (case self.oResultPrivate - of false: - when E isnot void: - raiseResultDefect(m, self.eResultPrivate) - else: - raiseResultDefect(m) - of true: - addr self.vResultPrivate)[] + ( + case self.oResultPrivate + of false: + when E isnot void: + raiseResultDefect(m, self.eResultPrivate) + else: + raiseResultDefect(m) + of true: + addr self.vResultPrivate + )[] func `$`*[T, E](self: Result[T, E]): string = ## Returns string representation of `self` case self.oResultPrivate of true: - when T is void: "ok()" - else: "ok(" & $self.vResultPrivate & ")" + when T is void: + "ok()" + else: + "ok(" & $self.vResultPrivate & ")" of false: - when E is void: "none()" - else: "err(" & $self.eResultPrivate & ")" + when E is void: + "none()" + else: + "err(" & $self.eResultPrivate & ")" func error*[T, E](self: Result[T, E]): E = ## Fetch error of result if set, or raise Defect @@ -946,28 +960,36 @@ func optError*[T, E](self: Result[T, E]): Opt[E] = Opt.some(self.eResultPrivate) # Alternative spellings for `value`, for `options` compatibility -template get*[T: not void, E](self: Result[T, E]): T = self.value() -template get*[E](self: Result[void, E]) = self.value() +template get*[T: not void, E](self: Result[T, E]): T = + self.value() + +template get*[E](self: Result[void, E]) = + self.value() + +template tryGet*[T: not void, E](self: Result[T, E]): T = + self.tryValue() + +template tryGet*[E](self: Result[void, E]) = + self.tryValue() -template tryGet*[T: not void, E](self: Result[T, E]): T = self.tryValue() -template tryGet*[E](self: Result[void, E]) = self.tryValue() +template unsafeGet*[T: not void, E](self: Result[T, E]): T = + self.unsafeValue() -template unsafeGet*[T: not void, E](self: Result[T, E]): T = self.unsafeValue() -template unsafeGet*[E](self: Result[void, E]) = self.unsafeValue() +template unsafeGet*[E](self: Result[void, E]) = + self.unsafeValue() # `var` overloads should not be needed but result in invalid codegen (!): # TODO https://github.com/nim-lang/Nim/issues/22049 -func get*[T: not void, E](self: var Result[T, E]): var T = self.value() +func get*[T: not void, E](self: var Result[T, E]): var T = + self.value() func get*[T, E](self: Result[T, E], otherwise: T): T {.inline.} = ## Fetch value of result if set, or return the value `otherwise` ## See `valueOr` for a template version that avoids evaluating `otherwise` ## unless necessary case self.oResultPrivate - of true: - self.vResultPrivate - of false: - otherwise + of true: self.vResultPrivate + of false: otherwise when resultsGenericBindingWorkaround: import macros @@ -1007,17 +1029,17 @@ when resultsGenericBindingWorkaround: else: result = copyNimNode(n) result.add n[0] - for i in 1..= (1, 6): - {.warning[BareExcept]:off.} + {.warning[BareExcept]: off.} try: echo rErr[] @@ -118,7 +139,7 @@ block: discard when (NimMajor, NimMinor) >= (1, 6): - {.warning[BareExcept]:on.} + {.warning[BareExcept]: on.} # Comparisons doAssert (rOk == rOk) @@ -126,13 +147,32 @@ block: doAssert (rOk != rErr) # Mapping - doAssert (rOk.map(func(x: int): string = $x)[] == $rOk.value) - doAssert (rOk.map(func(x: int) = discard)).isOk() - - doAssert (rOk.flatMap( - proc(x: int): Result[string, string] = Result[string, string].ok($x))[] == $rOk.value) - - doAssert (rErr.mapErr(func(x: string): string = x & "no!").error == (rErr.error & "no!")) + doAssert ( + rOk.map( + func (x: int): string = + $x + )[] == $rOk.value + ) + doAssert ( + rOk.map( + func (x: int) = + discard + ) + ).isOk() + + doAssert ( + rOk.flatMap( + proc(x: int): Result[string, string] = + Result[string, string].ok($x) + )[] == $rOk.value + ) + + doAssert ( + rErr.mapErr( + func (x: string): string = + x & "no!" + ).error == (rErr.error & "no!") + ) # Casts and conversions doAssert rOk.mapConvert(int64)[] == int64(42) @@ -186,7 +226,8 @@ block: func testQn2(): Result[int, string] = # looks like we can even use it creatively like this - if ?fails() == 42: raise (ref ValueError)(msg: "shouldn't happen") + if ?fails() == 42: + raise (ref ValueError)(msg: "shouldn't happen") func testQn3(): Result[bool, string] = # different T but same E @@ -198,7 +239,8 @@ block: doAssert testQn3()[] proc heterOr(): Result[int, int] = - let value = ? (rErr or err(42)) # TODO ? binds more tightly than `or` - can that be fixed? + let value = ?(rErr or err(42)) + # TODO ? binds more tightly than `or` - can that be fixed? doAssert value + 1 == value, "won't reach, ? will shortcut execution" ok(value) @@ -209,9 +251,18 @@ block: doAssert Result[R, string].ok(rErr).flatten() == rErr # Filter - doAssert rOk.filter(proc(x: int): auto = Result[void, string].ok()) == rOk - doAssert rOk.filter(proc(x: int): auto = Result[void, string].err("filter")).error == "filter" - doAssert rErr.filter(proc(x: int): auto = Result[void, string].err("filter")) == rErr + doAssert rOk.filter( + proc(x: int): auto = + Result[void, string].ok() + ) == rOk + doAssert rOk.filter( + proc(x: int): auto = + Result[void, string].err("filter") + ).error == "filter" + doAssert rErr.filter( + proc(x: int): auto = + Result[void, string].err("filter") + ) == rErr # Collections block: @@ -249,10 +300,12 @@ type AnEnum = enum anEnumA anEnumB + AnException = ref object of CatchableError v: AnEnum -func toException(v: AnEnum): AnException = AnException(v: v) +func toException(v: AnEnum): AnException = + AnException(v: v) func testToException(): int = try: @@ -263,10 +316,9 @@ func testToException(): int = doAssert testToException() == 42 -type - AnEnum2 = enum - anEnum2A - anEnum2B +type AnEnum2 = enum + anEnum2A + anEnum2B func testToString(): int = try: @@ -280,13 +332,19 @@ doAssert testToString() == 42 block: # Result[void, E] type VoidRes = Result[void, int] - func worksVoid(): VoidRes = VoidRes.ok() - func worksVoid2(): VoidRes = result.ok() - func worksVoid3(): VoidRes = ok() + func worksVoid(): VoidRes = + VoidRes.ok() + func worksVoid2(): VoidRes = + result.ok() + func worksVoid3(): VoidRes = + ok() - func failsVoid(): VoidRes = VoidRes.err(42) - func failsVoid2(): VoidRes = result.err(42) - func failsVoid3(): VoidRes = err(42) + func failsVoid(): VoidRes = + VoidRes.err(42) + func failsVoid2(): VoidRes = + result.err(42) + func failsVoid3(): VoidRes = + err(42) let vOk = worksVoid() @@ -317,13 +375,38 @@ block: # Result[void, E] doAssert (vOk != vErr) # Mapping - doAssert vOk.map(proc (): int = 42).get() == 42 - vOk.map(proc () = discard).get() - - vOk.mapErr(proc(x: int): int = 10).get() - vOk.mapErr(proc(x: int) = discard).get() - - doAssert vErr.mapErr(proc(x: int): int = 10).error() == 10 + doAssert vOk + .map( + proc(): int = + 42 + ) + .get() == 42 + vOk + .map( + proc() = + discard + ) + .get() + + vOk + .mapErr( + proc(x: int): int = + 10 + ) + .get() + vOk + .mapErr( + proc(x: int) = + discard + ) + .get() + + doAssert vErr + .mapErr( + proc(x: int): int = + 10 + ) + .error() == 10 # string conversion doAssert $vOk == "ok()" @@ -334,7 +417,7 @@ block: # Result[void, E] ok() func voidF2(): Result[int, int] = - ? voidF() + ?voidF() ok(42) @@ -345,20 +428,35 @@ block: # Result[void, E] doAssert Result[VoidRes, int].ok(vErr).flatten() == vErr # Filter - doAssert vOk.filter(proc(): auto = Result[void, int].ok()) == vOk - doAssert vOk.filter(proc(): auto = Result[void, int].err(100)).error == 100 - doAssert vErr.filter(proc(): auto = Result[void, int].err(100)) == vErr + doAssert vOk.filter( + proc(): auto = + Result[void, int].ok() + ) == vOk + doAssert vOk.filter( + proc(): auto = + Result[void, int].err(100) + ).error == 100 + doAssert vErr.filter( + proc(): auto = + Result[void, int].err(100) + ) == vErr block: # Result[T, void] aka `Opt` type OptInt = Result[int, void] - func worksOpt(): OptInt = OptInt.ok(42) - func worksOpt2(): OptInt = result.ok(42) - func worksOpt3(): OptInt = ok(42) + func worksOpt(): OptInt = + OptInt.ok(42) + func worksOpt2(): OptInt = + result.ok(42) + func worksOpt3(): OptInt = + ok(42) - func failsOpt(): OptInt = OptInt.err() - func failsOpt2(): OptInt = result.err() - func failsOpt3(): OptInt = err() + func failsOpt(): OptInt = + OptInt.err() + func failsOpt2(): OptInt = + result.err() + func failsOpt3(): OptInt = + err() let oOk = worksOpt() @@ -388,20 +486,45 @@ block: # Result[T, void] aka `Opt` oErr.unsafeError() # Mapping - doAssert oOk.map(proc(x: int): string = $x).get() == $oOk.get() - oOk.map(proc(x: int) = discard).get() - - doAssert oOk.mapErr(proc(): int = 10).get() == oOk.get() - doAssert oOk.mapErr(proc() = discard).get() == oOk.get() - - doAssert oErr.mapErr(proc(): int = 10).error() == 10 + doAssert oOk + .map( + proc(x: int): string = + $x + ) + .get() == $oOk.get() + oOk + .map( + proc(x: int) = + discard + ) + .get() + + doAssert oOk + .mapErr( + proc(): int = + 10 + ) + .get() == oOk.get() + doAssert oOk + .mapErr( + proc() = + discard + ) + .get() == oOk.get() + + doAssert oErr + .mapErr( + proc(): int = + 10 + ) + .error() == 10 # string conversion doAssert $oOk == "ok(42)" doAssert $oErr == "none()" proc optQuestion(): OptInt = - let v = ? oOk + let v = ?oOk ok(v) doAssert optQuestion().isOk() @@ -411,13 +534,35 @@ block: # Result[T, void] aka `Opt` doAssert Result[OptInt, void].ok(oErr).flatten() == oErr # Filter - doAssert oOk.filter(proc(x: int): auto = Result[void, void].ok()) == oOk - doAssert oOk.filter(proc(x: int): auto = Result[void, void].err()).isErr() - doAssert oErr.filter(proc(x: int): auto = Result[void, void].err()) == oErr - - doAssert oOk.filter(proc(x: int): bool = true) == oOk - doAssert oOk.filter(proc(x: int): bool = false).isErr() - doAssert oErr.filter(proc(x: int): bool = true) == oErr + doAssert oOk.filter( + proc(x: int): auto = + Result[void, void].ok() + ) == oOk + doAssert oOk + .filter( + proc(x: int): auto = + Result[void, void].err() + ) + .isErr() + doAssert oErr.filter( + proc(x: int): auto = + Result[void, void].err() + ) == oErr + + doAssert oOk.filter( + proc(x: int): bool = + true + ) == oOk + doAssert oOk + .filter( + proc(x: int): bool = + false + ) + .isErr() + doAssert oErr.filter( + proc(x: int): bool = + true + ) == oErr doAssert Opt.some(42).get() == 42 doAssert Opt.none(int).isNone() @@ -449,7 +594,13 @@ block: # `cstring` dangling reference protection block: # Experiments # Can formalise it into a template (https://github.com/arnetheduck/nim-result/issues/8) template `?=`(v: untyped{nkIdent}, vv: Result): bool = - (let vr = vv; template v: auto {.used.} = unsafeGet(vr); vr.isOk) + ( + let vr = vv + template v(): auto {.used.} = + unsafeGet(vr) + + vr.isOk + ) if f ?= Result[int, string].ok(42): doAssert f == 42 @@ -487,14 +638,13 @@ block: # Experiments block: # Constants # TODO https://github.com/nim-lang/Nim/issues/20699 - type - WithOpt = object - opt: Opt[int] - const - noneWithOpt = - WithOpt(opt: Opt.none(int)) + type WithOpt = object + opt: Opt[int] + + const noneWithOpt = WithOpt(opt: Opt.none(int)) proc checkIt(v: WithOpt) = doAssert v.opt.isNone() + checkIt(noneWithOpt) block: # TODO https://github.com/nim-lang/Nim/issues/22049 @@ -519,7 +669,7 @@ block: doAssert y == 1234 - when (NimMajor, NimMinor) >= (1,6): + when (NimMajor, NimMinor) >= (1, 6): # pre 1.6 nim vm have worse bug static: var z = bug() @@ -547,4 +697,4 @@ block: doAssert $error == $Breaking.error 33 - discard genericFunc(int) \ No newline at end of file + discard genericFunc(int) diff --git a/tests/test_results2.nim b/tests/test_results2.nim index 4897b82..b815d28 100644 --- a/tests/test_results2.nim +++ b/tests/test_results2.nim @@ -9,5 +9,6 @@ template repeater(b: Opt[int]): untyped = # Check that Result can be used inside a template - this fails # sometimes with field access errors as noted in above issue b + let x = repeater(Opt.none(int)) doAssert x.isNone() From 6ffc63e5aba6b5324ff87a3e77774efc8c8431ae Mon Sep 17 00:00:00 2001 From: Jacek Sieka Date: Mon, 19 Feb 2024 11:06:43 +0100 Subject: [PATCH 06/14] fmt --- tests/test_results.nim | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_results.nim b/tests/test_results.nim index 74861a7..c717b09 100644 --- a/tests/test_results.nim +++ b/tests/test_results.nim @@ -381,6 +381,7 @@ block: # Result[void, E] 42 ) .get() == 42 + vOk .map( proc() = @@ -394,6 +395,7 @@ block: # Result[void, E] 10 ) .get() + vOk .mapErr( proc(x: int) = From 23429eb7d33d79bb717256230c44f11f115adbbe Mon Sep 17 00:00:00 2001 From: Jacek Sieka Date: Mon, 19 Feb 2024 11:08:22 +0100 Subject: [PATCH 07/14] add devel test --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e6aaff1..3f4c816 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,7 +17,7 @@ jobs: - os: linux cpu: amd64 TEST_LANG: cpp - branch: [version-1-2, version-1-4, version-1-6, version-2-0] + branch: [version-1-2, version-1-4, version-1-6, version-2-0, devel] include: - target: os: linux From c95e0ba7f9960b4ac769e98df5929d3cd3ba914e Mon Sep 17 00:00:00 2001 From: Jacek Sieka Date: Sun, 10 Mar 2024 10:03:02 +0100 Subject: [PATCH 08/14] format, work around when bugs --- results.nim | 337 ++++++++++++++++++++++++++++++++----------------- results.nimble | 3 +- 2 files changed, 221 insertions(+), 119 deletions(-) diff --git a/results.nim b/results.nim index e7a1aa9..60aaf38 100644 --- a/results.nim +++ b/results.nim @@ -351,6 +351,8 @@ const resultsGenericBindingWorkaround* {.booldefine.} = true ## can be found. # TODO https://github.com/nim-lang/Nim/issues/22605 # TODO https://github.com/arnetheduck/nim-results/issues/34 + # TODO https://github.com/nim-lang/Nim/issues/23386 + # TODO https://github.com/nim-lang/Nim/issues/23385 func raiseResultOk[T, E](self: Result[T, E]) {.noreturn, noinline.} = # noinline because raising should take as little space as possible at call @@ -528,7 +530,9 @@ func map*[E; T1: not void]( else: result.err(self.eResultPrivate) -func map*[E](self: Result[void, E], f: proc()): Result[void, E] {.inline, effectsOf: f.} = +func map*[E]( + self: Result[void, E], f: proc() +): Result[void, E] {.inline, effectsOf: f.} = ## Call f if `self` is ok case self.oResultPrivate of true: @@ -635,7 +639,9 @@ func mapConvert*[T0: not void, E]( else: result.err(self.eResultPrivate) -func mapCast*[T0: not void, E](self: Result[T0, E], T1: type): Result[T1, E] {.inline.} = +func mapCast*[T0: not void, E]( + self: Result[T0, E], T1: type +): Result[T1, E] {.inline.} = ## Convert result value to A using a cast ## Would be nice with nicer syntax... case self.oResultPrivate @@ -1066,153 +1072,248 @@ when resultsGenericBindingWorkaround: # of injecting a template and likely doesn't cover all applicable cases replace(body, $what, with) -template isOkOr*[T, E](self: Result[T, E], body: untyped) = - ## Evaluate `body` iff result has been assigned an error - ## `body` is evaluated lazily. - ## - ## Example: - ## ``` - ## let - ## v = Result[int, string].err("hello") - ## x = v.isOkOr: echo "not ok" - ## # experimental: direct error access using an unqualified `error` symbol - ## z = v.isOkOr: echo error - ## ``` - ## - ## `error` access: - ## - ## TODO experimental, might change in the future - ## - ## The template contains a shortcut for accessing the error of the result, - ## it can only be used outside of generic code, - ## see https://github.com/status-im/nim-stew/issues/161#issuecomment-1397121386 +when resultsGenericBindingWorkaround: + template isOkOr*[T, E](self: Result[T, E], body: untyped) = + ## Evaluate `body` iff result has been assigned an error + ## `body` is evaluated lazily. + ## + ## Example: + ## ``` + ## let + ## v = Result[int, string].err("hello") + ## x = v.isOkOr: echo "not ok" + ## # experimental: direct error access using an unqualified `error` symbol + ## z = v.isOkOr: echo error + ## ``` + ## + ## `error` access: + ## + ## TODO experimental, might change in the future + ## + ## The template contains a shortcut for accessing the error of the result, + ## it can only be used outside of generic code, + ## see https://github.com/status-im/nim-stew/issues/161#issuecomment-1397121386 - let s = (self) # TODO avoid copy - case s.oResultPrivate - of false: - when E isnot void: - when resultsGenericBindingWorkaround: + let s = (self) # TODO avoid copy + case s.oResultPrivate + of false: + when E isnot void: template error(): E {.used, gensym.} = s.eResultPrivate replaceHack(body, "error", error) else: - template error(): E {.used, inject.} = - s.eResultPrivate - body - else: - body - of true: - discard + of true: + discard -template isErrOr*[T, E](self: Result[T, E], body: untyped) = - ## Evaluate `body` iff result has been assigned a value - ## `body` is evaluated lazily. - ## - ## Example: - ## ``` - ## let - ## v = Result[int, string].ok(42) - ## x = v.isErrOr: echo "not err" - ## # experimental: direct value access using an unqualified `value` symbol - ## z = v.isErrOr: echo value - ## ``` - ## - ## `value` access: - ## - ## TODO experimental, might change in the future - ## - ## The template contains a shortcut for accessing the value of the result, - ## it can only be used outside of generic code, - ## see https://github.com/status-im/nim-stew/issues/161#issuecomment-1397121386 + template isErrOr*[T, E](self: Result[T, E], body: untyped) = + ## Evaluate `body` iff result has been assigned a value + ## `body` is evaluated lazily. + ## + ## Example: + ## ``` + ## let + ## v = Result[int, string].ok(42) + ## x = v.isErrOr: echo "not err" + ## # experimental: direct value access using an unqualified `value` symbol + ## z = v.isErrOr: echo value + ## ``` + ## + ## `value` access: + ## + ## TODO experimental, might change in the future + ## + ## The template contains a shortcut for accessing the value of the result, + ## it can only be used outside of generic code, + ## see https://github.com/status-im/nim-stew/issues/161#issuecomment-1397121386 - let s = (self) # TODO avoid copy - case s.oResultPrivate - of true: - when T isnot void: - when resultsGenericBindingWorkaround: + let s = (self) # TODO avoid copy + case s.oResultPrivate + of true: + when T isnot void: template value(): T {.used, gensym.} = s.vResultPrivate replaceHack(body, "value", value) else: - template value(): T {.used, inject.} = - s.vResultPrivate - body - else: - body - of false: - discard + of false: + discard -template valueOr*[T: not void, E](self: Result[T, E], def: untyped): T = - ## Fetch value of result if set, or evaluate `def` - ## `def` is evaluated lazily, and must be an expression of `T` or exit - ## the scope (for example using `return` / `raise`) - ## - ## See `isOkOr` for a version that works with `Result[void, E]`. - ## - ## Example: - ## ``` - ## let - ## v = Result[int, string].err("hello") - ## x = v.valueOr: 42 # x == 42 now - ## y = v.valueOr: raise (ref ValueError)(msg: "v is an error, gasp!") - ## # experimental: direct error access using an unqualified `error` symbol - ## z = v.valueOr: raise (ref ValueError)(msg: error) - ## ``` - ## - ## `error` access: - ## - ## TODO experimental, might change in the future - ## - ## The template contains a shortcut for accessing the error of the result, - ## it can only be used outside of generic code, - ## see https://github.com/status-im/nim-stew/issues/161#issuecomment-1397121386 - ## - let s = (self) # TODO avoid copy - case s.oResultPrivate - of true: - s.vResultPrivate - of false: - when E isnot void: - when resultsGenericBindingWorkaround: + template valueOr*[T: not void, E](self: Result[T, E], def: untyped): T = + ## Fetch value of result if set, or evaluate `def` + ## `def` is evaluated lazily, and must be an expression of `T` or exit + ## the scope (for example using `return` / `raise`) + ## + ## See `isOkOr` for a version that works with `Result[void, E]`. + ## + ## Example: + ## ``` + ## let + ## v = Result[int, string].err("hello") + ## x = v.valueOr: 42 # x == 42 now + ## y = v.valueOr: raise (ref ValueError)(msg: "v is an error, gasp!") + ## # experimental: direct error access using an unqualified `error` symbol + ## z = v.valueOr: raise (ref ValueError)(msg: error) + ## ``` + ## + ## `error` access: + ## + ## TODO experimental, might change in the future + ## + ## The template contains a shortcut for accessing the error of the result, + ## it can only be used outside of generic code, + ## see https://github.com/status-im/nim-stew/issues/161#issuecomment-1397121386 + ## + let s = (self) # TODO avoid copy + case s.oResultPrivate + of true: + s.vResultPrivate + of false: + when E isnot void: template error(): E {.used, gensym.} = s.eResultPrivate replaceHack(def, "error", error) else: - template error(): E {.used, inject.} = - s.eResultPrivate - def - else: - def -template errorOr*[T; E: not void](self: Result[T, E], def: untyped): E = - ## Fetch error of result if not set, or evaluate `def` - ## `def` is evaluated lazily, and must be an expression of `T` or exit - ## the scope (for example using `return` / `raise`) - ## - ## See `isErrOr` for a version that works with `Result[T, void]`. - let s = (self) # TODO avoid copy - case s.oResultPrivate - of false: - s.eResultPrivate - of true: - when T isnot void: - when resultsGenericBindingWorkaround: + template errorOr*[T; E: not void](self: Result[T, E], def: untyped): E = + ## Fetch error of result if not set, or evaluate `def` + ## `def` is evaluated lazily, and must be an expression of `T` or exit + ## the scope (for example using `return` / `raise`) + ## + ## See `isErrOr` for a version that works with `Result[T, void]`. + let s = (self) # TODO avoid copy + case s.oResultPrivate + of false: + s.eResultPrivate + of true: + when T isnot void: template value(): T {.used, gensym.} = s.vResultPrivate replaceHack(def, "value", value) else: + def + +else: + template isOkOr*[T, E](self: Result[T, E], body: untyped) = + ## Evaluate `body` iff result has been assigned an error + ## `body` is evaluated lazily. + ## + ## Example: + ## ``` + ## let + ## v = Result[int, string].err("hello") + ## x = v.isOkOr: echo "not ok" + ## # experimental: direct error access using an unqualified `error` symbol + ## z = v.isOkOr: echo error + ## ``` + ## + ## `error` access: + ## + ## TODO experimental, might change in the future + ## + ## The template contains a shortcut for accessing the error of the result, + ## it can only be used outside of generic code, + ## see https://github.com/status-im/nim-stew/issues/161#issuecomment-1397121386 + + let s = (self) # TODO avoid copy + case s.oResultPrivate + of false: + when E isnot void: + template error(): E {.used, inject.} = + s.eResultPrivate + + body + of true: + discard + + template isErrOr*[T, E](self: Result[T, E], body: untyped) = + ## Evaluate `body` iff result has been assigned a value + ## `body` is evaluated lazily. + ## + ## Example: + ## ``` + ## let + ## v = Result[int, string].ok(42) + ## x = v.isErrOr: echo "not err" + ## # experimental: direct value access using an unqualified `value` symbol + ## z = v.isErrOr: echo value + ## ``` + ## + ## `value` access: + ## + ## TODO experimental, might change in the future + ## + ## The template contains a shortcut for accessing the value of the result, + ## it can only be used outside of generic code, + ## see https://github.com/status-im/nim-stew/issues/161#issuecomment-1397121386 + + let s = (self) # TODO avoid copy + case s.oResultPrivate + of true: + when T isnot void: + template value(): T {.used, inject.} = + s.vResultPrivate + + body + of false: + discard + + template valueOr*[T: not void, E](self: Result[T, E], def: untyped): T = + ## Fetch value of result if set, or evaluate `def` + ## `def` is evaluated lazily, and must be an expression of `T` or exit + ## the scope (for example using `return` / `raise`) + ## + ## See `isOkOr` for a version that works with `Result[void, E]`. + ## + ## Example: + ## ``` + ## let + ## v = Result[int, string].err("hello") + ## x = v.valueOr: 42 # x == 42 now + ## y = v.valueOr: raise (ref ValueError)(msg: "v is an error, gasp!") + ## # experimental: direct error access using an unqualified `error` symbol + ## z = v.valueOr: raise (ref ValueError)(msg: error) + ## ``` + ## + ## `error` access: + ## + ## TODO experimental, might change in the future + ## + ## The template contains a shortcut for accessing the error of the result, + ## it can only be used outside of generic code, + ## see https://github.com/status-im/nim-stew/issues/161#issuecomment-1397121386 + ## + let s = (self) # TODO avoid copy + case s.oResultPrivate + of true: + s.vResultPrivate + of false: + when E isnot void: + template error(): E {.used, inject.} = + s.eResultPrivate + + def + + template errorOr*[T; E: not void](self: Result[T, E], def: untyped): E = + ## Fetch error of result if not set, or evaluate `def` + ## `def` is evaluated lazily, and must be an expression of `T` or exit + ## the scope (for example using `return` / `raise`) + ## + ## See `isErrOr` for a version that works with `Result[T, void]`. + let s = (self) # TODO avoid copy + case s.oResultPrivate + of false: + s.eResultPrivate + of true: + when T isnot void: template value(): T {.used, inject.} = s.vResultPrivate - def - else: def func flatten*[T, E](self: Result[Result[T, E], E]): Result[T, E] = diff --git a/results.nimble b/results.nimble index 806162f..bb52fab 100644 --- a/results.nimble +++ b/results.nimble @@ -20,7 +20,8 @@ proc test(env, path: string) = task test, "Runs the test suite": for f in ["test_results.nim", "test_results2.nim"]: - test "", "tests/" & f + for opt in ["", "-d:resultsGenericBindingWorkaround=false"]: + test opt, "tests/" & f task bench, "Run benchmark": test "-d:release", "benchmarks/benchmark.nim" From ecf9d1eaa7f1025011f5c74f2af5c425a6bf6a65 Mon Sep 17 00:00:00 2001 From: Jacek Sieka Date: Mon, 11 Mar 2024 13:39:23 +0100 Subject: [PATCH 09/14] hack zero-arg call --- results.nim | 9 +++++++-- tests/test_results.nim | 12 +++++++++--- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/results.nim b/results.nim index 60aaf38..13a221c 100644 --- a/results.nim +++ b/results.nim @@ -1029,10 +1029,15 @@ when resultsGenericBindingWorkaround: else: case n.kind of nnkCallKinds: - # `error(...)` - replace args but not function name if n[0].containsHack(): + # Don't replace inside nested expansion result = n + elif n.len == 1 and n[0].eqIdent(what): + # No arguments - replace call symbol + result = copyNimNode(n) + result.add with else: + # `error(...)` - replace args but not function name result = copyNimNode(n) result.add n[0] for i in 1 ..< n.len: @@ -1070,7 +1075,7 @@ when resultsGenericBindingWorkaround: # This hack replaces the `what` identifier with `with` except where # this replacing is not expected - this is an approximation of the intent # of injecting a template and likely doesn't cover all applicable cases - replace(body, $what, with) + result = replace(body, $what, with) when resultsGenericBindingWorkaround: template isOkOr*[T, E](self: Result[T, E], body: untyped) = diff --git a/tests/test_results.nim b/tests/test_results.nim index d13aa50..a06ebff 100644 --- a/tests/test_results.nim +++ b/tests/test_results.nim @@ -99,9 +99,15 @@ block: doAssert value == rOk.value() doAssert rOk.valueOr(failFast()) == rOk.value() - let rErrV = rErr.valueOr: - ord(error[0]) - doAssert rErrV == ord(rErr.error[0]) + block: # plain syntax: `error` + let rErrV = rErr.valueOr: + ord(error[0]) + doAssert rErrV == ord(rErr.error[0]) + + block: # call syntax: `error()` + let rErrV = rErr.valueOr: + ord(error()[0]) + doAssert rErrV == ord(rErr.error()[0]) block: # nested valueOr binds to the inner error let rInnerV = rErr.valueOr: From 5955e992026cfe83683e6d791f7e5e9985f891f0 Mon Sep 17 00:00:00 2001 From: Jacek Sieka Date: Mon, 12 Aug 2024 10:59:35 +0200 Subject: [PATCH 10/14] update to latest upstream --- results.nim | 63 +++++++++++++++++++++++++++++------------- tests/test_results.nim | 2 +- 2 files changed, 45 insertions(+), 20 deletions(-) diff --git a/results.nim b/results.nim index 3547402..ac32971 100644 --- a/results.nim +++ b/results.nim @@ -340,19 +340,37 @@ type Opt*[T] = Result[T, void] -const resultsGenericBindingWorkaround* {.booldefine.} = true - ## Enable a workaround for the template injection problem in the issue - ## linked below where injected templates get bound differently depending - ## on whether we're in a generic context or not - this leads to surprising - ## errors where random symbols from outer scopes get bound to the name - ## instead of the intended value. - ## However, this ugly hack might introduce more damage than it's worth so - ## it can be disabled at compile-time - hopefully an upstream solution - ## can be found. - # TODO https://github.com/nim-lang/Nim/issues/22605 - # TODO https://github.com/arnetheduck/nim-results/issues/34 - # TODO https://github.com/nim-lang/Nim/issues/23386 - # TODO https://github.com/nim-lang/Nim/issues/23385 +const + resultsGenericsOpenSym* {.booldefine.} = true + ## Enable the experimental `genericsOpenSym` feature or a workaround for the + ## template injection problem in the issue linked below where scoped symbol + ## resolution works differently for expanded bodies in templates depending on + ## whether we're in a generic context or not. + ## + ## This leads to surprising errors where symbols from outer scopes get bound + ## instead of the symbol created in the template scope which should be seen + ## as a better candidate. + ## + ## On Nim versions that do not support `genericsOpenSym`, a macro is used + ## instead to reassign symbol matches which may or may not work depending on + ## the complexity of the code. + # TODO https://github.com/nim-lang/Nim/issues/22605 + # TODO https://github.com/arnetheduck/nim-results/issues/34 + # TODO https://github.com/nim-lang/Nim/issues/23386 + # TODO https://github.com/nim-lang/Nim/issues/23385 + # + # Related PR:s (there's more probably, but this gives an overview) + # https://github.com/nim-lang/Nim/pull/23102 + # https://github.com/nim-lang/Nim/pull/23572 + # https://github.com/nim-lang/Nim/pull/23873 + # https://github.com/nim-lang/Nim/pull/23892 + # https://github.com/nim-lang/Nim/pull/23939 + + resultsGenericsOpenSymWorkaround* {.booldefine.} = + resultsGenericsOpenSym and not defined(nimHasGenericsOpenSym) + ## Prefer macro workaround to solve genericsOpenSym issue + + pushGenericsOpenSym = defined(nnimHasGenericsOpenSym) and resultsGenericsOpenSym func raiseResultOk[T, E](self: Result[T, E]) {.noreturn, noinline.} = # noinline because raising should take as little space as possible at call @@ -1000,7 +1018,7 @@ func get*[T, E](self: Result[T, E], otherwise: T): T {.inline.} = of true: self.vResultPrivate of false: otherwise -when resultsGenericBindingWorkaround: +when resultsGenericsOpenSymWorkaround: import macros proc containsHack(n: NimNode): bool = @@ -1080,7 +1098,6 @@ when resultsGenericBindingWorkaround: # of injecting a template and likely doesn't cover all applicable cases result = replace(body, $what, with) -when resultsGenericBindingWorkaround: template isOkOr*[T, E](self: Result[T, E], body: untyped) = ## Evaluate `body` iff result has been assigned an error ## `body` is evaluated lazily. @@ -1232,7 +1249,9 @@ else: case s.oResultPrivate of false: when E isnot void: - template error(): E {.used, inject.} = + when resultsGenericsOpenSym: + {.push experimental: "genericsOpenSym".} + template error(): E {.used.} = s.eResultPrivate body @@ -1264,7 +1283,9 @@ else: case s.oResultPrivate of true: when T isnot void: - template value(): T {.used, inject.} = + when resultsGenericsOpenSym: + {.push experimental: "genericsOpenSym".} + template value(): T {.used.} = s.vResultPrivate body @@ -1302,7 +1323,9 @@ else: s.vResultPrivate of false: when E isnot void: - template error(): E {.used, inject.} = + when resultsGenericsOpenSym: + {.push experimental: "genericsOpenSym".} + template error(): E {.used.} = s.eResultPrivate def @@ -1319,7 +1342,9 @@ else: s.eResultPrivate of true: when T isnot void: - template value(): T {.used, inject.} = + when resultsGenericsOpenSym: + {.push experimental: "genericsOpenSym".} + template value(): T {.used.} = s.vResultPrivate def diff --git a/tests/test_results.nim b/tests/test_results.nim index a06ebff..472eb2b 100644 --- a/tests/test_results.nim +++ b/tests/test_results.nim @@ -699,7 +699,7 @@ block: proc genericFunc(T: type): int = let rErr = Result[int, string].err("abc") rErr.valueOr: - when resultsGenericBindingWorkaround: + when resultsGenericsOpenSym: doAssert $error == $rErr.error() else: doAssert $error == $Breaking.error From aee3e242af3ca7bc8b7039a4ed4a9db1c6dd4193 Mon Sep 17 00:00:00 2001 From: Jacek Sieka Date: Mon, 12 Aug 2024 11:33:22 +0200 Subject: [PATCH 11/14] `nimHasGenericsOpenSym` already declared even though it's not working --- results.nim | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/results.nim b/results.nim index ac32971..1db6e2d 100644 --- a/results.nim +++ b/results.nim @@ -367,10 +367,11 @@ const # https://github.com/nim-lang/Nim/pull/23939 resultsGenericsOpenSymWorkaround* {.booldefine.} = - resultsGenericsOpenSym and not defined(nimHasGenericsOpenSym) + resultsGenericsOpenSym and not defined(nimHasGenericsOpenSym2) ## Prefer macro workaround to solve genericsOpenSym issue + # TODO https://github.com/nim-lang/Nim/pull/23892#discussion_r1713434311 - pushGenericsOpenSym = defined(nnimHasGenericsOpenSym) and resultsGenericsOpenSym + resultsGenericsOpenSymWorkaroundHint* {.booldefine.} = true func raiseResultOk[T, E](self: Result[T, E]) {.noreturn, noinline.} = # noinline because raising should take as little space as possible at call @@ -1057,6 +1058,8 @@ when resultsGenericsOpenSymWorkaround: # No arguments - replace call symbol result = copyNimNode(n) result.add with + when resultsGenericsOpenSymWorkaroundHint: + hint("Replaced node with injected symbol " & what, n[0]) else: # `error(...)` - replace args but not function name result = copyNimNode(n) @@ -1224,6 +1227,9 @@ when resultsGenericsOpenSymWorkaround: def else: + # TODO https://github.com/nim-lang/Nim/pull/23892#discussion_r1713434311 + const pushGenericsOpenSym = defined(nimHasGenericsOpenSym2) and resultsGenericsOpenSym + template isOkOr*[T, E](self: Result[T, E], body: untyped) = ## Evaluate `body` iff result has been assigned an error ## `body` is evaluated lazily. @@ -1249,7 +1255,7 @@ else: case s.oResultPrivate of false: when E isnot void: - when resultsGenericsOpenSym: + when pushGenericsOpenSym: {.push experimental: "genericsOpenSym".} template error(): E {.used.} = s.eResultPrivate @@ -1283,7 +1289,7 @@ else: case s.oResultPrivate of true: when T isnot void: - when resultsGenericsOpenSym: + when pushGenericsOpenSym: {.push experimental: "genericsOpenSym".} template value(): T {.used.} = s.vResultPrivate @@ -1323,7 +1329,7 @@ else: s.vResultPrivate of false: when E isnot void: - when resultsGenericsOpenSym: + when pushGenericsOpenSym: {.push experimental: "genericsOpenSym".} template error(): E {.used.} = s.eResultPrivate @@ -1342,7 +1348,7 @@ else: s.eResultPrivate of true: when T isnot void: - when resultsGenericsOpenSym: + when pushGenericsOpenSym: {.push experimental: "genericsOpenSym".} template value(): T {.used.} = s.vResultPrivate From f7206ffecdd657b23241e01dcb5fd46064d113a6 Mon Sep 17 00:00:00 2001 From: Jacek Sieka Date: Mon, 12 Aug 2024 11:37:55 +0200 Subject: [PATCH 12/14] comments --- results.nim | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/results.nim b/results.nim index 1db6e2d..d29f876 100644 --- a/results.nim +++ b/results.nim @@ -347,13 +347,17 @@ const ## resolution works differently for expanded bodies in templates depending on ## whether we're in a generic context or not. ## - ## This leads to surprising errors where symbols from outer scopes get bound - ## instead of the symbol created in the template scope which should be seen - ## as a better candidate. + ## The issue leads to surprising errors where symbols from outer scopes get + ## bound instead of the symbol created in the template scope which should be + ## seen as a better candidate, breaking access to `error` in `valueOr` and + ## friends. ## - ## On Nim versions that do not support `genericsOpenSym`, a macro is used + ## In Nim versions that do not support `genericsOpenSym`, a macro is used ## instead to reassign symbol matches which may or may not work depending on ## the complexity of the code. + ## + ## Nim 2.0.8 was released with an incomplete fix but already declares + ## `nimHasGenericsOpenSym`. # TODO https://github.com/nim-lang/Nim/issues/22605 # TODO https://github.com/arnetheduck/nim-results/issues/34 # TODO https://github.com/nim-lang/Nim/issues/23386 From ec158d3527fcd8b9b7f0b9df6b9aa9361e3f7240 Mon Sep 17 00:00:00 2001 From: Jacek Sieka Date: Mon, 12 Aug 2024 13:05:27 +0200 Subject: [PATCH 13/14] avoid node replace dance when the right node is already found --- results.nim | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/results.nim b/results.nim index d29f876..158d4b2 100644 --- a/results.nim +++ b/results.nim @@ -1036,34 +1036,47 @@ when resultsGenericsOpenSymWorkaround: return true false - proc containsIdent(n: NimNode, what: string): bool = - if n.eqIdent(what): + proc containsIdent(n: NimNode, what: string, with: NimNode): bool = + if n == with: + false # Don't replace if the right symbol is already being used + elif n.eqIdent(what): true else: for child in n: - if containsIdent(child, what): + if containsIdent(child, what, with): return true false proc replace(n: NimNode, what: string, with: NimNode): NimNode = - if not containsIdent(n, what): # Avoid lots of node copies if not needed + if not containsIdent(n, what, with): # Fast path that avoids copies altogether return n - if n.eqIdent(what): + if n == with: + result = with + elif n.eqIdent(what): + when resultsGenericsOpenSymWorkaroundHint: + hint("Replaced conflicting external symbol " & what, n) result = with else: case n.kind of nnkCallKinds: - if n[0].containsHack(): + if n[0].kind == nnkDotExpr: + result = copyNimNode(n) + for i in 0 ..< n.len: + result.add replace(n[i], what, with) + elif n[0].containsHack(): # Don't replace inside nested expansion result = n elif n.len == 1 and n[0].eqIdent(what): - # No arguments - replace call symbol - result = copyNimNode(n) - result.add with - when resultsGenericsOpenSymWorkaroundHint: - hint("Replaced node with injected symbol " & what, n[0]) + if n[0] == with: + result = n + else: + # No arguments - replace call symbol + result = copyNimNode(n) + result.add with + when resultsGenericsOpenSymWorkaroundHint: + hint("Replaced conflicting external symbol " & what, n[0]) else: # `error(...)` - replace args but not function name result = copyNimNode(n) @@ -1167,7 +1180,7 @@ when resultsGenericsOpenSymWorkaround: template value(): T {.used, gensym.} = s.vResultPrivate - replaceHack(body, "value", value) + replaceHack(body, "value", s.vResultPrivate) else: body of false: From 30afea1266abc6847aa64e508971531bbc42748b Mon Sep 17 00:00:00 2001 From: Jacek Sieka Date: Mon, 12 Aug 2024 14:17:30 +0200 Subject: [PATCH 14/14] better dot replacer --- results.nim | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/results.nim b/results.nim index 158d4b2..dda33fc 100644 --- a/results.nim +++ b/results.nim @@ -1061,27 +1061,28 @@ when resultsGenericsOpenSymWorkaround: else: case n.kind of nnkCallKinds: - if n[0].kind == nnkDotExpr: - result = copyNimNode(n) - for i in 0 ..< n.len: - result.add replace(n[i], what, with) - elif n[0].containsHack(): + if n[0].containsHack(): # Don't replace inside nested expansion result = n - elif n.len == 1 and n[0].eqIdent(what): - if n[0] == with: - result = n + elif n[0].eqIdent(what): + if n.len == 1: + if n[0] == with: + result = n + else: + # No arguments - replace call symbol + result = copyNimNode(n) + result.add with + when resultsGenericsOpenSymWorkaroundHint: + hint("Replaced conflicting external symbol " & what, n[0]) else: - # No arguments - replace call symbol + # `error(...)` - replace args but not function name result = copyNimNode(n) - result.add with - when resultsGenericsOpenSymWorkaroundHint: - hint("Replaced conflicting external symbol " & what, n[0]) + result.add n[0] + for i in 1 ..< n.len: + result.add replace(n[i], what, with) else: - # `error(...)` - replace args but not function name result = copyNimNode(n) - result.add n[0] - for i in 1 ..< n.len: + for i in 0 ..< n.len: result.add replace(n[i], what, with) of nnkExprEqExpr: # "error = xxx" - function call with named parameters and other weird stuff