Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding more HasCallStack s #494

Open
toyboot4e opened this issue Jun 8, 2024 · 3 comments
Open

Adding more HasCallStack s #494

toyboot4e opened this issue Jun 8, 2024 · 3 comments

Comments

@toyboot4e
Copy link
Contributor

toyboot4e commented Jun 8, 2024

I suggest adding more HasCallStack constraints.

1. Missing HasCallStack constraints

Some functions in non- Generic modules are missing HasCallStack constraints.

Example: VU.(!)

VU.(!) does not have HasCallStack constraint and we don't get full trace:

Example
{- stack script --resolver lts-22.24 --package vector -}

import Data.Vector.Unboxed qualified as VU

main :: IO ()
main = do
  let !_ = (VU.fromList [0 :: Int]) VU.! 42
  return ()

The call stack does not contain VU.(!) call from main:

$ stack example-u.hs
example-u.hs: index out of bounds (42,1)
CallStack (from HasCallStack):
  error, called at src/Data/Vector/Internal/Check.hs:103:12 in vector-0.13.1.0-Aqc2YUE1Egs5WmXVMbcJ5T:Data.Vector.Internal.Check
  checkError, called at src/Data/Vector/Internal/Check.hs:109:17 in vector-0.13.1.0-Aqc2YUE1Egs5WmXVMbcJ5T:Data.Vector.Internal.Check
  check, called at src/Data/Vector/Internal/Check.hs:122:5 in vector-0.13.1.0-Aqc2YUE1Egs5WmXVMbcJ5T:Data.Vector.Internal.Check
  checkIndex, called at src/Data/Vector/Generic.hs:235:11 in vector-0.13.1.0-Aqc2YUE1Egs5WmXVMbcJ5T:Data.Vector.Generic
  !, called at src/Data/Vector/Unboxed.hs:297:7 in vector-0.13.1.0-Aqc2YUE1Egs5WmXVMbcJ5T:Data.Vector.Unboxed

VG.(!) has HasCallStack constraint and we get a full trace:

Example
{- stack script --resolver lts-22.24 --package vector -}

import Data.Vector.Unboxed qualified as VU
+import Data.Vector.Generic qualified as VG

main :: IO ()
main = do
-  let !_ = (VU.fromList [0 :: Int]) VU.! 42
+  let !_ = (VU.fromList [0 :: Int]) VG.! 42
  return ()

The call stack contains VG.(!) call from main:

$ stack example-g.hs
example-g.hs: index out of bounds (42,1)
CallStack (from HasCallStack):
  error, called at src/Data/Vector/Internal/Check.hs:103:12 in vector-0.13.1.0-Aqc2YUE1Egs5WmXVMbcJ5T:Data.Vector.Internal.Check
  checkError, called at src/Data/Vector/Internal/Check.hs:109:17 in vector-0.13.1.0-Aqc2YUE1Egs5WmXVMbcJ5T:Data.Vector.Internal.Check
  check, called at src/Data/Vector/Internal/Check.hs:122:5 in vector-0.13.1.0-Aqc2YUE1Egs5WmXVMbcJ5T:Data.Vector.Internal.Check
  checkIndex, called at src/Data/Vector/Generic.hs:235:11 in vector-0.13.1.0-Aqc2YUE1Egs5WmXVMbcJ5T:Data.Vector.Generic
  !, called at /home/tbm/dev/hs/tmp/example-g.hs:8:37 in main:Main

2. head and more

I personally like to add HasCallStack to anywhere possible.

For example, the head function does not have HasCallStack constraint. But the list's head function in base doesn't have it either. What do you think? Thank you.

@toyboot4e
Copy link
Contributor Author

Oh no, it was already discussed A LOT in the Haskell community (outside of the vector repository). Closing as the impact seems bigger than I expected.

@Shimuuar
Copy link
Contributor

Could you provide links to discussions? Adding HasCallStack to functions which could fail with out of range exception seems quite sensible to me.

@Shimuuar Shimuuar reopened this Jun 15, 2024
@toyboot4e
Copy link
Contributor Author

toyboot4e commented Jun 16, 2024

Thank you, I rushed. Let me share the links.


Past PR:

Compilation time, memory usage, runtime performance:

Related:


My notes:

  • HasCallStack was not added to Data.Ix. It I guess is because we'll get global backtrace mechanism (based on HasCallStack) that doesn't need user code change in the future.

  • Adding HasCallStack to U.(!) etc. would be ok if the runtime/compile time cost is little.

  • Adding HasCallStack to head etc. depends on policies.

  • For now, we can use generic functions such as VG.(!) or VGM.write for getting backtraces. We can also define total functions such as headMaybe in the user land.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants