serokell / universum

:milky_way: Prelude written in @Serokell
MIT License
176 stars 26 forks source link

Make `error` print callstack of its call site rather than its definition #203

Closed Martoon-00 closed 5 years ago

Martoon-00 commented 5 years ago

Description

As we know, our error is funny:

*** Exception: 123
CallStack (from HasCallStack):
  error, called at /home/martoon/serokell/universum/src/Universum/Debug.hs:62:11 in main:Universum.Debug
  error, called at <interactive>:1:1 in interactive:Ghci1

I hope I fixed it to work as expected:

Related issues(s)

No corresponding issue exist.

✓ Checklist for your Pull Request

Ideally a PR has all of the checkmarks set.

If something in this list is irrelevant to your PR, you should still set this checkmark indicating that you are sure it is dealt with (be that by irrelevance).

Related changes (conditional)

Stylistic guide (mandatory)

gromakovsky commented 5 years ago

I think it makes sense to add a test where you call error, then catch ErrorCall and check that the second String in ErrorCallWithLocation contains expected number of newline characters (AFAIU, 2, if the string passed to error doesn't have them).

Martoon-00 commented 5 years ago

It seems to me, that examining the number of newlines would be more fragile than useful. Our error implementation will hardly ever change since now (unless GHC improves error again). On the other hand, if GHC changes the text of the error, that test case would need to catch up. I'm not completely sure though, @gromakovsky if you disagree with this point I'll add the test.

There seems to be no way to extract CallStack from the exception thrown by the error unfortunately.

Martoon-00 commented 5 years ago

Thanks, done.