Add Maybe instance

[JonathanLorimer]

No description provided.

[tfausak]

I'm generally opposed to providing instances that distribute over containers. See #37 some discussion about tuples. In short, I don't think from @(Maybe a) @(Maybe b) is really any better than fmap @Maybe (from @a @b).

[JonathanLorimer]

@tfausak makes sense. I thought about it a bit too, and weighed the pros / cons, and I agree that its not a straight forward win. I just figured there was only really one sensible implementation of from maybeA maybeB and it saves an fmap in a lot of places which can make things a lot more readable (I find when you start having to nest fmaps, or nest an fmap in a bind, things get ugly pretty quickly).

Anyways, please feel free to close

[tfausak]

Setting aside the question of if this is a desirable instance or not, I think it will trip up instance selection when a ~ b:

ghci> :set -XTypeApplications

ghci> class From a b where from :: a -> b

ghci> instance From a a where from = id

ghci> instance From a b => From (Maybe a) (Maybe b) where from = fmap from

ghci> from @Int @Int 0
0

ghci> from @(Maybe Int) @(Maybe Int) Nothing

<interactive>:6:1: error: [GHC-43085]
    • Overlapping instances for From (Maybe Int) (Maybe Int)
        arising from a use of ‘from’
      Matching instances:
        instance [safe] From a a -- Defined at <interactive>:2:10
        instance [safe] From a b => From (Maybe a) (Maybe b)
          -- Defined at <interactive>:3:10
    • In the expression: from @(Maybe Int) @(Maybe Int) Nothing
      In an equation for ‘it’:
          it = from @(Maybe Int) @(Maybe Int) Nothing

[JonathanLorimer]

Yeah, that makes sense. IMO instance From a a where from = id shouldn't exist (i've already been tripped up in my codebase where I had unnecessary from's after a refactor, I think that is the default coerce instance tho). But I feel like I am really overstepping with my opinions here!

[tfausak]

Hmm well the tests passed, and there's a test for the identity instance:

witch/source/test-suite/Main.hs

Line 121 in f497644

Witch.from @Int @Int 0 `shouldBe` 0

So perhaps my minimal example was too minimal.

The identity instance (From a a) is a bit unfortunate. It would be nice to have a way to spot calls to from that are using the instance because typically they're useless and could be removed. However sometimes you actually do want them. For example you could have some function f :: From s String => s -> whatever. It's reasonable to call that function as f ("..." :: String), which needs the From a a instance to "convert" from From s String into a String.

[JonathanLorimer]

@tfausak I think its not triggering the error because there is no test for from @Maybe a @Maybe a I think you are still right about overlapping instances

[JonathanLorimer]

I have been convinced, and I think the From a a overlapping instances makes this a non-starter due to overlapping instances. Also I think From a a should exist based on arguments in this issue #46