Fixing type synonyms to Uniq(D)FM newtypes

[Ben Gamari]

Ömer Sinan Ağacan <omeragacan at gmail.com> writes:

> Hi,
>
> UniqFM and UniqDFM types are basically maps from Uniques to other stuff. Most of
> the time we don't actually map Uniques but instead map things like Vars or
> Names. For those we have types like VarEnv, NameEnv, FastStringEnv, ... which
> are defined as type synonyms to UniqFM or UniqDFM, and operations are defined
> like
>
>     extendFsEnv = addToUFM
>     extendNameEnv = addToUFM
>     extendVarEnv = addToUFM
>
> This causes problems when I have multiple Uniquables in scope and use the wrong
> one to update an environment because the program type checks and does the wrong
> thing in runtime. An example is #17667 where I did
>
>     delVarEnv env name
>
> where `name :: Name`. I shouldn't be able to remove a Name from a Var env but
> this currently type checks.
>
At first I was a bit confused at how this could possibly typecheck.
Afterall, delVarEnv has type,

    VarEnv a -> Var -> VarEnv a

which seems quite reasonable and should correctly reject the application
to a Name as given in Omer's example. However, the mistake in #17667
is actually that he wrote,

    delVarEnv env name

instead of

    delNameEnv env (varName var)

That is, because `VarEnv a ~ NameEnv a` one can easily mix up a
NameEnv with a VarEnv and not get a compile-time error. I can see how
this could be a nasty bug to track down.


> Two solutions proposed:
>
> - Make these env types newtypes instead of type synonyms.
> - Add a phantom type parameter to UniqFM/UniqDFM.
>
IIRC this has been suggested before. I, for one, see the value in this
and certainly wouldn't be opposed to either of these options, although
would weakly favor the former over the latter.

Cheers,

- Ben
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 487 bytes
Desc: not available
URL: <http://mail.haskell.org/pipermail/ghc-devs/attachments/20200114/9e641f25/attachment.sig>