Implement fmap/coerce rules #66
Conversation
|
Thanks. Does the "This is not tested" mean we should wait before you test it? Also, in Data.Map.Strict and Data.Map.Base -- we are providing trivial MIN_VERSION_base if it is unavailable. In Data.Map.Base, could you move the existing MIN_VERSION_base definition just before comments before module definitions (as in Data.Sequence and Data.Tree; currently it is defined after you use it to guard import of Data.Coerce) and add it to Data.Map.Strict? |
|
It actually means I haven't set up the bleeding-edge cabal to be able to test it. If someone else wants to try (compiling with a recent GHC 7.9 to activate the special code) that would be great. Otherwise, I'll get to it when I have a chance. I can definitely move and add |
treeowl
force-pushed
the
seqfmapcoerce
branch
2 times, most recently
from
bb6a34a to
34e82aa
Compare
|
I've tested the new code for |
| @@ -1660,6 +1662,14 @@ mapEitherWithKey f0 t0 = toPair $ go f0 t0 | |||
| map :: (a -> b) -> Map k a -> Map k b | |||
| map _ Tip = Tip | |||
| map f (Bin sx kx x l r) = Bin sx kx (f x) (map f l) (map f r) | |||
| #if MIN_VERSION_base(4,8,0) | |||
| -- Safe coercions were introduced in 4.7.0, but I am not sure if they played | |||
| -- well enough with RULES to do what we want. | |||
There was a problem hiding this comment.
I don’t think much changed with regard to RULES and coercion since 7.8, and even if the rule does not fire it is not a problem, I think.
There was a problem hiding this comment.
I stand corrected. There were changes, and "map/coere" was also only added in base 4.8. So this can stay as it is.
treeowl
force-pushed
the
seqfmapcoerce
branch
from
34e82aa to
43b745d
Compare
Implement fmap/coerce rules for Map, Sequence, and Tree. One concern: unfortunately, implementing the RULES forces the LANGUAGE to be turned from Safe to Trustworthy. This is rather sad. An alternative would be to do this in another module, but orphan rules are not so lovely either.
treeowl
force-pushed
the
seqfmapcoerce
branch
from
43b745d to
ad24ce6
Compare
|
OK, I think this is ready to go, for what it is. I've tested all three containers implemented. I'll open a new pull request later, once I've done something with |
|
Thanks. |
NOTE: This is not tested yet!
Implement fmap/coerce rules for Map, Sequence, and Tree. One concern:
unfortunately, implementing the RULES requires importing Data.Coerce,
which forces the LANGUAGE to be turned from Safe to Trustworthy. This
is rather sad. An alternative would be to do this in another module, but
orphan rules are not so lovely either.