Convert XException to ErrorCall

[christiaanb]

To make it easier to report XException when pack would normally hide them.

[leonschoorl]

We could also use HasCallStack and friends to get the location of the xToErrorCtx
In addition to or instead of the string

*Main> h  undefined 3
*** Exception: a is X
CallStack (from HasCallStack):
  xToErrorCtx, called at xerror.hs:6:4 in main:Main
X: undefined
CallStack (from HasCallStack):
  errorX, called at src/Clash/XException.hs:876:13 in clash-prelude-1.3.0-inplace:Clash.XException
  undefined, called at <interactive>:1:4 in interactive:Ghci2

Details

diff --git a/clash-prelude/src/Clash/XException.hs b/clash-prelude/src/Clash/XException.hs
index 7c702e3f..b5566331 100644
--- a/clash-prelude/src/Clash/XException.hs
+++ b/clash-prelude/src/Clash/XException.hs
@@ -125,9 +125,9 @@ errorX msg = throw (XException ("X: " ++ msg ++ "\n" ++ prettyCallStack callStac
 -- > f (xToErrorCtx "a is X" -> !a) (xToErrorCtx "b is X" -> !b) = ...
 --
 -- __NB:__ Fully synthesisable, so doesn't have to be removed before synthesis
-xToErrorCtx :: String -> a -> a
+xToErrorCtx :: HasCallStack => String -> a -> a
 xToErrorCtx ctx a = unsafeDupablePerformIO
-  (catch (evaluate a >> return a) (\(XException msg) -> throw (ErrorCall (ctx <> "\n" <> msg))))
+  (catch (evaluate a >> return a) (\(XException msg) -> throw (ErrorCall (ctx <> "\n" <> prettyCallStack callStack <> "\n" <> msg))))
 {-# NOINLINE xToErrorCtx #-}

[christiaanb]

I doesn't have to be added to the primitive though

[leonschoorl]

Right, we can keep the primitive as it is and create a wrapper that passed the rendered callstack as a string

[leonschoorl]

The callstack can also be used to trace further back before h:

f, g, h :: HasCallStack => Bool -> BitVector 8 -> BitVector 8
f = g
g = h
h (xToErrorCtx "a is X" -> a) (xToErrorCtx "b is X" -> b) = slice d7 d0 (pack a ++# b)

*Main> f undefined 3
*** Exception: a is X
CallStack (from HasCallStack):
  xToErrorCtx, called at xerror.hs:8:4 in main:Main
  h, called at xerror.hs:7:5 in main:Main
  g, called at xerror.hs:6:5 in main:Main
  f, called at <interactive>:12:1 in interactive:Ghci1
X: undefined
CallStack (from HasCallStack):
  errorX, called at src/Clash/XException.hs:876:13 in clash-prelude-1.3.0-inplace:Clash.XException
  undefined, called at <interactive>:12:3 in interactive:Ghci1

[martijnbastiaan]

Looks good, have you tested it? Could be good to add a couple of tests testing the synthesizability / whether it actually displays usable error messages.

[martijnbastiaan]

synthesizable 🇺🇸

[christiaanb]

@leonschoorl @martijnbastiaan could you review again to see whether it looks good now?

[leonschoorl]

Please add the callstack argument to the comment

[leonschoorl]

And the correct primitive name

[christiaanb]

@leonschoorl I think I've addressed all your points now

[leonschoorl]

Why do you popCallStack?
Doesn't that remove the removes the call that will tell me whether it was the a or the b argument to h that contained the XException?