Hoopl collection classes could use enummapset?

[hth313]

I have found myself using https://hackage.haskell.org/package/enummapset a lot in my code. It provides wrappers around IntSet/IntMap that uses Enum rather than Int, which allows for better type safety.

Now I am doing more work with Hoopl in my project and it lacks certain useful functions from IntMap. I am looking at restrictKeys at the moment. I am trying to wrap my head around how to implement it, but I also realize that in some way this collection code in Hoopl is just the same thing as is also provided by enummapset. I was thinking that maybe it would better to migrate the code in Hoopl to make use of enummapset rather than rolling its own duplicate that tends to be out of date (I have added some missing functions before).

I wonder what the maintainer and people using Hoopl think about this?

[mlite]

Without knowing why EnumMapSet is good for Hoopl's general use cases, I can provide the following information for you to consider.

1. What does adding the new dependencies mean to the build process of GHC? (GHC team is the stakeholder)
2. hoopl is used in the performance critical part of compilers. What is performance and memory usage impact? (GHC team again is the major stakeholder as well as all GHC users. Some evaluations would be great to convince them)

Sometimes code duplication is a necessary evil.

On Sat, Oct 31, 2020 at 3:19 PM Håkan Thörngren notifications@github.com wrote:

I have found myself using https://hackage.haskell.org/package/enummapset a lot in my code. It provides wrappers around IntSet/IntMap that uses Enum rather than Int, which allows for better type safety.

Now I am doing more work with Hoopl in my project and it lacks certain useful functions from IntMap. I am looking at restrictKeys at the moment. I am trying to wrap my head around how to implement it, but I also realize that in some way this collection code in Hoopl is just the same thing as is also provided by enummapset. I was thinking that maybe it would better to migrate the code in Hoopl to make use of enummapset rather than rolling its own duplicate that tends to be out of date (I have added some missing functions before).

I wonder what the maintainer and people using Hoopl think about this?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/haskell/hoopl/issues/53, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAYXSFNIZPJPFYHUI3ZDFCDSNSEO3ANCNFSM4TGEHZNQ .

[simonpj]

If it's just a question of boilerplate newtype wrappers, I'd be inclined to copy the necessary ones into hoopl, rather than take a dependency on another library. hoopl probably only uses half a dozen such functions.

Let's see what the hoopl maintainers say.

As to performance, I think that only newtype wrappers are involved, so perf impact should be zero. But worth checking anyway.

[hth313]

I added some missing functions in the past, but restricyKeys requires connecting the IsSet with the IsMap somehow, and I have yet to figure out how to do it. I tried with fundeps, but I am not a type wizard by any means, so I am kind of stuck going in that direction.

[mlite]

I support reimplementing the newtype wrappers in hoopl.

On Mon, Nov 2, 2020 at 12:59 AM simonpj notifications@github.com wrote:

If it's just a question of boilerplate newtype wrappers, I'd be inclined to copy the necessary ones into hoopl, rather than take a dependency on another library. hoopl probably only uses half a dozen such functions.

Let's see what the hoopl maintainers say.

As to performance, I think that only newtype wrappers are involved, so perf impact should be zero. But worth checking anyway.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/haskell/hoopl/issues/53#issuecomment-720335597, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAYXSFJDCR7JMVMTTBLSV73SNZYHLANCNFSM4TGEHZNQ .

[hth313]

I support reimplementing the newtype wrappers in hoopl.

So, how do you make it work?

src/Compiler/Hoopl/Label.hs:106:27: error:
    • Couldn't match expected type ‘set’ with actual type ‘LabelSet’
      ‘set’ is a rigid type variable bound by
        the type signature for:
          mapRestrictKeys :: forall set a.
                             IsSet set =>
                             LabelMap a -> set -> LabelMap a
        at src/Compiler/Hoopl/Label.hs:106:3-17
    • In the pattern: LS s
      In an equation for ‘mapRestrictKeys’:
          mapRestrictKeys (LM m) (LS s) = LM (restrictKeys m s)
      In the instance declaration for ‘IsMap LabelMap’
    • Relevant bindings include
        mapRestrictKeys :: LabelMap a -> set -> LabelMap a
          (bound at src/Compiler/Hoopl/Label.hs:106:3)

[mlite]

Michal Terepeta who is also a hoopl maintainer should be the one making the call as I'm supposed to step down from the maintainer position.

On Mon, Nov 2, 2020 at 12:46 PM Håkan Thörngren notifications@github.com wrote:

I support reimplementing the newtype wrappers in hoopl.

So, how do you make it work?

src/Compiler/Hoopl/Label.hs:106:27: error:

• Couldn't match expected type ‘set’ with actual type ‘LabelSet’

  ‘set’ is a rigid type variable bound by

    the type signature for:

      mapRestrictKeys :: forall set a.

                         IsSet set =>

                         LabelMap a -> set -> LabelMap a

    at src/Compiler/Hoopl/Label.hs:106:3-17

• In the pattern: LS s

  In an equation for ‘mapRestrictKeys’:

      mapRestrictKeys (LM m) (LS s) = LM (restrictKeys m s)

  In the instance declaration for ‘IsMap LabelMap’

• Relevant bindings include

    mapRestrictKeys :: LabelMap a -> set -> LabelMap a

      (bound at src/Compiler/Hoopl/Label.hs:106:3)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/haskell/hoopl/issues/53#issuecomment-720714783, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAYXSFK7L5JT3WGJUP7UPS3SN4LB7ANCNFSM4TGEHZNQ .