From d463d491cc3991df67db25d4696007bcc09b7819 Mon Sep 17 00:00:00 2001
From: Mark Spanbroek <mark@spanbroek.net>
Date: Mon, 18 Dec 2023 17:01:37 +0100
Subject: [PATCH] =?UTF-8?q?Handle=20bind=20(=3D=3F)=20errors=20in=20`witho?=
 =?UTF-8?q?ut`=20statements=20differently?=

Keeps track of the current error variable at compile time,
instead of using a pointer to the error variable at runtime.

Employs a trick with an unused type parameter to ensure that
invocations of the bindFailed() macro are expanded after
captureBindError() is expanded.
---
 questionable/private/binderror.nim | 51 ++++++++++++++++++++----------
 testmodules/results/test.nim       | 11 +++++++
 2 files changed, 46 insertions(+), 16 deletions(-)

diff --git a/questionable/private/binderror.nim b/questionable/private/binderror.nim
index 9ddb5bb..d4b7df5 100644
--- a/questionable/private/binderror.nim
+++ b/questionable/private/binderror.nim
@@ -1,24 +1,43 @@
 import std/options
+import std/macros
 
-var captures {.global, compileTime.}: int
-var errorVariable {.threadvar.}: ptr ref CatchableError
+# A stack of names of error variables. Keeps track of the error variables that
+# are given to captureBindError().
+var errorVariableNames {.global, compileTime.}: seq[string]
 
-template captureBindError*(error: var ref CatchableError, expression): auto =
-  let previousErrorVariable = errorVariable
-  errorVariable = addr error
+macro captureBindError*(error: var ref CatchableError, expression): auto =
+  ## Ensures that an error is assigned to the error variable when a binding (=?)
+  ## fails inside the expression.
 
-  static: inc captures
-  let evaluated = expression
-  static: dec captures
-
-  errorVariable = previousErrorVariable
-
-  evaluated
+  # name of the error variable as a string literal
+  let errorVariableName = newLit($error)
+  quote do:
+    # add error variable to the top of the stack
+    static: errorVariableNames.add(`errorVariableName`)
+    # evaluate the expression
+    let evaluated = `expression`
+    # pop error variable from the stack
+    static: discard errorVariableNames.pop()
+    # return the evaluated result
+    evaluated
 
 func error[T](option: Option[T]): ref CatchableError =
   newException(ValueError, "Option is set to `none`")
 
-template bindFailed*(expression) =
-  when captures > 0:
-    mixin error
-    errorVariable[] = expression.error
+macro bindFailed*(expression; _: type = void) =
+  ## Called when a binding (=?) fails.
+  ## Assigns an error to the error variable (specified in captureBindError())
+  ## when appropriate.
+
+  # This macro has a type parameter to ensure that the compiler does not
+  # expand it before it expands invocations of captureBindError().
+
+  # check that we have an error variable on the stack
+  if errorVariableNames.len > 0:
+    # create an identifier that references the current error variable
+    let errorVariable = ident errorVariableNames[^1]
+    return quote do:
+      # check that the error variable is in scope
+      when compiles(`errorVariable`):
+        # assign bind error to error variable
+        `errorVariable` = `expression`.error
diff --git a/testmodules/results/test.nim b/testmodules/results/test.nim
index 4eb27be..961ba84 100644
--- a/testmodules/results/test.nim
+++ b/testmodules/results/test.nim
@@ -412,6 +412,17 @@ suite "result":
       for i in 0..<1000:
         spawn fail(i)
 
+  test "without statement doesn't interfere with generic code called elsewhere":
+    proc foo(_: type): ?!int =
+      if error =? success(1).errorOption:
+        discard
+
+    proc bar {.used.} = # defined, but not used
+      without x =? bool.foo(), error:
+        discard error
+
+    discard bool.foo() # same type parameter 'bool' as used in bar()
+
   test "catch can be used to convert exceptions to results":
     check parseInt("42").catch == 42.success
     check parseInt("foo").catch.error of ValueError