refcount: define macro-constraint HasCallStackIfDebug

[wenkokke]

@dcoutts This PR defines a HasCallStackIfDebug macro which

#ifdef NO_IGNORE_ASSERTS
#define HasCallStackIfDebug HasCallStack
#else
#define HasCallStackIfDebug ()
#endif

which does not throw a warning when used as

newRef ::
     (RefCounted obj, FinaliserM obj ~ m, PrimMonad m)
  => HasCallStackIfDebug
  => m ()
  -> (RefCounter m -> obj)
  -> m (Ref obj)

but is closer to Haskell syntax than the previous HAS_CALL_STACK macro.

[wenkokke]

Note that all other variants do throw a warning.

The following use of the above macro throws a warning:

newRef ::
     (RefCounted obj, FinaliserM obj ~ m, PrimMonad m, HasCallStackIfDebug) 
  => m ()
  -> (RefCounter m -> obj)
  -> m (Ref obj)

The following definition always throws a warning, regardless of use:

type HasCallStackIfDebug :: Constraint
#ifdef NO_IGNORE_ASSERTS
type HasCallStackIfDebug = HasCallStack
#else
type HasCallStackIfDebug = ()
#endif

[jorisdral]

I prefer the all-caps macros because it makes it clear that it is not actual haskell syntax

[wenkokke]

Do you feel that with the all-caps name, this is an improvement?

[wenkokke]

There is, of course, the risk that this is highly dependent on what is arguably a GHC bug.

[jorisdral]

What's the reason that this does not throw a warning?

[wenkokke]

By the looks of it, GHC combines successive constraint arrows, so by the time it reaches the warning subsystem, (A, B) => () => x has turned into (A, B) => x by concatenation, whereas it does no other normalisation, so (A, B, ()) => x remains unchanged.

[wenkokke]

My preferred version would be to use CPP to define a type and simply disable the warning, since we're purposefully creating an empty constraint, so disabling the warning is sensible. That way, the code we're writing actually can follow the general rule that each CPP section should be syntactically valid Haskell.

The current PR achieves that, but only by lying and possibly relying on a bug in the GHC warning system.

[dcoutts]

LGTM!

I have no problem with using a "normal" name rather than a CPP_ISH name. I did the CPP_ISH name because the thing I did was clearly not valid syntax, so it provided a hint that some evil CPP was at play.

And I don't mind relying on GHC warning quirks here.

Related: we should file a ghc bug/feature request to ask that redundant () constraints not get warned about but just get eliminated.

[dcoutts]

My preferred version would be to use CPP to define a type and simply disable the warning, since we're purposefully creating an empty constraint, so disabling the warning is sensible.

The downside of disabling the warning is we cannot do it selectively only for this constraint. It would disable all redundant constraint checking in the module, which can hide real things (e.g. during later refactoring).