search

haskell / ghc-events

Library and tool for parsing .eventlog files from GHC
http://www.haskell.org/haskellwiki/ThreadScope
Other
33 stars 34 forks source link

Assertion failure but nobody notices because assertions are off by default #106

Open nh2 opened 2 weeks ago

nh2 commented 2 weeks ago

I'm building ghc-events as part of static-haskell-nix's CI, where sometimes I build with -O0.

An assertion appears:

test-versions: Assertion failed
CallStack (from HasCallStack):
  assert, called at src/GHC/RTS/Events/Binary.hs:822:5 in ghc-events-0.19.0.1-863kLHKqkAq5pQOjpORL0w:GHC.RTS.Events.Binary

That's this one:

https://github.com/haskell/ghc-events/blob/2168f610bf3580a3a4dca7b0582c6384d5433413/src/GHC/RTS/Events/Binary.hs#L822-L830

I tested also with stack test on a stack.yaml with this config:

resolver:
  url: https://raw.githubusercontent.com/commercialhaskell/stackage-snapshots/master/lts/22/25.yaml
packages:
- .

As documented in assert, it's disabled on -O, which cabal enables by default.

So probably nobody noticed that.

I recommend setting either

  ghc-options:      -Wall -fno-ignore-asserts

if assertions should be used in code, or not using assert.

Mikolaj commented 2 weeks ago

I'm all for assertions, but people usually don't like the idea. We should at least enable them in CI. And fix whichever are failing...

nh2 commented 2 weeks ago

but people usually don't like the idea

People don't want assertions in production code that make it 100x slower.

The checks implemented in that file seem to be simple integer comparisons, fast enough to enable without noticing anything.

So I think those should just be error.