Add newtype 'AsMovable' to derive Consumable and Dupable from Movable #357
Conversation
| instance Movable a => Consumable (AsMovable a) where | ||
| consume (AsMovable x) = | ||
| move x & \case | ||
| Ur x' -> () | ||
|
|
||
| instance Movable a => Dupable (AsMovable a) where | ||
| dupV (AsMovable x) = | ||
| move x & \case | ||
| Ur x' -> Data.pure (AsMovable x') |
There was a problem hiding this comment.
Just to outline an unimportant alternative: you could have given an instance instance Movable a => Movable (AsMovable a), and used it to save a little space in the Consumable and Dupable instances.
It doesn't change the generated code, and the code is still very short as it stands; I just wanted to point out that the possibility exists.
There was a problem hiding this comment.
Oh, I didn't know it was possible to do so, and it looks clearer to me with instance Movable a => Movable (AsMovable a), so I'll add it
EDIT: I tried to implement it, but now I don't see what can be removed from Consumable and Dupable instances after adding a Movable instance for AsMovable 🤔
|
|
||
| instance Dupable () where | ||
| dupV () = Data.pure () | ||
| deriving via (AsMovable ()) instance Dupable () |
There was a problem hiding this comment.
Since I'm doing trivial comments: I'm voting removing the empty line between two deriving instance one-liners.
| @@ -426,24 +313,36 @@ instance Prelude.Traversable Ur where | |||
|
|
|||
| -- Some stock instances | |||
| deriving instance Consumable a => Consumable (Sum a) | |||
|
|
|||
There was a problem hiding this comment.
Ah, but I see some such lines have been added. Is that an Ormolu thing?
There was a problem hiding this comment.
As you guessed it, it is an ormolu thing. Maybe I could rewrite it in the "packed" style (so as to make the diff cleaner), as PR #355 will ormolize the whole codebase later?
There was a problem hiding this comment.
It would have been preferable. But it's probably not worth the time investment now.
a770788 to
dd11d76
Compare
No description provided.