Parameterize the simplifier over the counting mechanism
Motivation
Counting is done in the simplifier using the SimplCount
type. This type however is the combination of two independent and incompatible counting mechanisms: VerySimplCount
which just wraps an integer and the more elaborate SimplCount
.
Both are incompatible with each other, for example GHC will panic if one did plusSimpleCount (VerySimplCount 0) (SimplCount ...)
as the plusSimplCount
function is only defined if we pass values for the same counting mechanism as arguments.
Additionally, every time we need a identity value (zeroSimplCount
) we need to pass an option telling which mechanism we want.
Proposal
We propose to parameterize the simplifier over the counting mechanism used, i.e. replace the concrete SimplCount
type with an abstract one that is an instance of a new class Counting
. This class is a subclass of Semigroup
/Monoid
and provides additional methods like the creation of a non-zero count from a Tick
. Then VerySimplCount
becomes an own type and we provide the necessary instances for both types. For example, right now we have the following:
data SimplCount
= VerySimplCount Int
| SimplCount {...}
-- The identity element
zeroSimplCount :: DynFlags -> SimplCount
zeroSimplCount dflags
| dopt Opt_D_dump_simpl_stats dflags = SimplCount {...}
| otherwise = VerySimplCount 0
-- Add a Tick to a count
doSimplTick :: DynFlags -- ^ Used to get the history size
-> Tick -> SimplCount -> SimplCount
-- Addition of counts
plusSimplCount :: SimplCount -> SimplCount -> SimplCount
plusSimplCount (SimplCount ...) (SimplCount ...) = ...
plusSimplCount (VerySimplCount n) (VerySimplCount m) = VerySimplCount (n+m)
plusSimplCount lhs rhs = throwGhcException ...
-- Some function in the simplifier
simplifyPgmIO :: ... -> IO (SimplCount, ModGuts)
After the refactoring we have the following:
class Monoid count => Counting count where
addTick :: Int -- ^ History size
-> Tick -> count -> count
data SimplCount = SimplCount {...}
instance Semigroup SimplCount where {...}
instance Monoid SimplCount where {...}
instance Counting SimplCount where {...}
newtype VerySimplCount = VerySimplCount Int
deriving (Semigroup, Monoid) via Int
instance Counting SimplCount where {...}
simplifyPgmIO :: Counting counts => ... -> IO (counts, ModGuts)
-- Somewhere in the driver
foo :: DynFlags -> ...
foo dflags
| dopt Opt_D_dump_simpl_stats dflags = runTheSimplifier (mempty :: SimplCount) ...
| otherwise = runTheSimplifier (mempty :: VerySimplCount) ...
This way we get rid of the partiality and open other possibilities like using a simple WriterT
for the accumulation of the counts or other counting mechanisms like NoCount
.
Results
If this is done after #21812, we get the following benefits:
No partiality! Easier for newcomers!
@Ericson3214 really values this.
As a reader of unfamiliar code whenever I (@Ericson2314) see partiality I groan, because it means there is some non-trivial invariant that must upheld by any user of this code I need to learn about.
Conversely, if I see a Monoid
instance (what this would be), not only can be I not worry about any sneaky invariants, I probably don't even need to read the instance! Monoid
tells me everything I need to know, and I can "conserve brain cells" and not worry about the details.
This makes the code much more approachable and easier to work with. Fancier types (which are merely internal to SimpleM
!) are well worth the cost of being able to trust the type system to do more work so, I the programmer new to the code, am saddled fewer responsibilities.
Get rid of a duplication
Prior to #21812, there is no SimplM
because more stuff uses CoreM
. After #21812 but without this issue, there is a SimplM
but will duplicate part of the definition of CoreM
because the use of DynFlags
in zeroSimplCount
complicates things.
With this change, DynFlags
is no longer needed for zeroSimplCount
, and thus the Monad
and Application
definitions don't need DynFlags
either. This greatly simplifies keeping things modular so we can more easily deduplicate CoreM
and SimplM
, using the former in the definition of the latter.
CoreM
is abstract, so this refactor would not impose any breakage on plugin consumers of CoreM
.