Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Could there be a conflict with generic-lens-labels and similar libraries? #8

Open
Wizek opened this issue Mar 9, 2018 · 11 comments
Open

Comments

@Wizek
Copy link

Wizek commented Mar 9, 2018

Hey, I'm just now getting to know this library. I have been waiting/looking for a nice implementation for named parameters for Haskell for a long time, so I am glad you've created this, and I'm glad to have found it.

However, seeing labels being used as functions gives me some pause. Could this library conflict with https://github.com/duog/generic-lens-labels? According to my admittedly fuzzy and likely spotty understanding, both of these libraries define orphan instances for IsLabel (-> _), don't they? Meaning that only one of these can be used in a Haskell module (or perhaps even package?) at once, and never together, doesn't it? And if so far I am on the mark here, this mutual exclusion extends to any other libraries that define similar instances, doesn't it?

So if I'm correct above, could one relatively nice way of overcoming this issue be defining an operator that takes the being-a-function-burden off of IsLabel's shoulder?

createSymLink :: "from" % FilePath -> "to" % FilePath  -> IO ()
createSymLink (Named from) (Named to) = ...

main = createSymLink ! #from =: "/path/to/source"
                     ! #to   =: "/target/path"

E.g. above I've used =:, but could be a different one as well.
(I also wonder if the type level operator % can be re-defined on the value level as well.)

@int-index
Copy link
Contributor

both of these libraries define orphan instances for

No, named doesn't define an orphan, but generic-lens-labels does. The solution I'd prefer is that generic-lens-labels doesn't define an orphan.

@Wizek
Copy link
Author

Wizek commented Mar 9, 2018

No, named doesn't define an orphan, but generic-lens-labels does.

Ah, alright. Does that mean there is no conflict between these libraries? Or does the conflict still exist by virtue of that lib having orphans?

The solution I'd prefer is that generic-lens-labels doesn't define an orphan.

Yeah, I think that would be nice. Do you think it could be easy somehow? Any ideas how? Or would it likely be rather difficult to do?

@int-index
Copy link
Contributor

Or does the conflict still exist by virtue of that lib having orphans?

I didn’t try to use both libs together, but I predict there should be a conflict.

Do you think it could be easy somehow? Any ideas how? Or would it likely be rather difficult to do?

The lens instance could be defined for a newtype over LensLike, although you’d have to write l #mylens rather than #mylens, where l is a combinator to unwrap the newtype.

@tysonzero
Copy link

tysonzero commented Sep 30, 2019

As far as I am concerned IsLabel name (a -> Param p) is absolutely an orphan. The top-most type constructor is -> which is not owned by Named.

If we declare it to not be an orphan, then the following would also not be an orphan:

data MyVoid

instance IsLabel name (MyVoid -> a) where
    fromLabel = \case {}

At which point if I were to do:

#foo :: Void -> Param ()

Then I am going to get an overlapping instances error. Showing that our assertion that either of the above definitions are not orphans is wrong.

It's also worth noting that it would be perfectly reasonable for IsLabel to define a HasField (x :: Symbol) r a => IsLabel x (r -> a) instance. Given that HasField now exists in GHC.Records and is continuing to obtain functionality, such an instance is not overly unlikely.

@int-index
Copy link
Contributor

As far as I am concerned IsLabel name (a -> Param p) is absolutely an orphan.

Then you're using a different definition of an orphan instance, definitely not the one that e.g. -Worphans uses.

Then I am going to get an overlapping instances error.

Overlapping instances are also possible in absence of orphans, so I don't think any conclusions follow from this. IsLabel name (MyVoid -> a) isn't an orphan instance either.

Given that HasField now exists in GHC.Records and is continuing to obtain functionality, such an instance is not overly unlikely.

I'll keep it in mind, so that if someone proposes this instance, I'll mention that it would conflict with existing libraries (e.g. named).

@tysonzero
Copy link

tysonzero commented Sep 30, 2019

Then you're using a different definition of an orphan instance, definitely not the one that e.g. -Worphans uses.

Surprising that -Worphans is so lenient. But I would say that my definition is very reasonable, given that it prevents two unrelated libraries from overlapping. Which is pretty much the whole reason people even care about orphans.

Overlapping instances are also possible in absence of orphans

Under my definition of orphans, they are only possible if both overlapping instances are defined in the same module (a clear and easy to fix error), or if at least one of the definitions is an orphan.

I'll keep it in mind, so that if someone proposes this instance, I'll mention that it would conflict with existing libraries (e.g. named).

This is exactly why I don't like instances like IsLabel a (a -> Param p), as they get in the way of progress. I came to Haskell for many reasons, one of which being that I don't want the whole JavaScript fiasco of the std lib having to carefully avoid adding new features that conflict with some third party feature.

If it does come down to a head-to-head between base and named on who is more entitled to define a IsLabel name (a -> b) instance, I have a hunch for who will win. In fact I would be pretty appalled if the named instance got in the way of such an instance. It would be like defining instance Monoid (a -> Foo) and then telling base not to define instance Monoid b => Monoid (a -> b).

This is not to say that I am 100% in favor of the HasField => IsLabel instance, just that I think it should be considered on its own merits and with no consideration for third party orphans.

@3noch
Copy link

3noch commented Sep 30, 2019

Which is pretty much the whole reason people even care about orphans.

I don't think that's the whole reason people care about orphans. The real danger of orphans is that you can end up with different instances in scope in different parts of your code base and therefore get different behavior depending on where in the code the instance is chosen.

@tysonzero
Copy link

tysonzero commented Sep 30, 2019

The real danger of orphans is that you can end up with different instances in scope in different parts of your code base and therefore get different behavior depending on where in the code the instance is chosen.

Thanks for bringing that up. I should have mentioned that as well for sure.

To be clear the above style of instance absolutely allows for different behavior depending on where in the code the instance is chosen.

So you bring up another very good reason why we should absolutely not have the above instance, and why -Worphans should absolutely complain about it.

I was able to break Data.Set using only lawful Ord instances by using the above style of orphan instance as follows:

module Orphans where

data Tuple a b = Tuple a b
    deriving (Eq, Show)
{-# LANGUAGE FlexibleInstances #-}

module OrphansFoo where

import Data.Monoid
import Data.Set (Set)
import qualified Data.Set as S

import Orphans

data Foo = Foo1 | Foo2
    deriving (Eq, Ord, Show)

instance Ord b => Ord (Tuple Foo b) where
    compare (Tuple a1 a2) (Tuple b1 b2) = compare a1 b1 <> compare a2 b2

foo :: Ord b => b -> b -> Set (Tuple Foo b)
foo a b = S.fromList [Tuple Foo1 a, Tuple Foo1 b, Tuple Foo2 a, Tuple Foo2 b]
{-# LANGUAGE FlexibleInstances #-}

module OrphansBar where

import Data.Monoid
import Data.Set (Set)
import qualified Data.Set as S

import Orphans

data Bar = Bar1 | Bar2
    deriving (Eq, Ord, Show)

instance Ord a => Ord (Tuple a Bar) where
    compare (Tuple a1 a2) (Tuple b1 b2) = compare a2 b2 <> compare a1 b1

bar :: Ord a => a -> a -> Set (Tuple a Bar) -> Set (Tuple a Bar)
bar a b s = S.fromList [Tuple a Bar1, Tuple a Bar2, Tuple b Bar1, Tuple b Bar2] <> s
module OrphansBaz where

import Data.Set (Set)

import Orphans
import OrphansFoo
import OrphansBar

baz :: Set (Tuple Foo Bar)
baz = bar Foo1 Foo2 $ foo Bar1 Bar2

If you run the above code you will notice that baz has 2 duplicate elements for a total of 6. Although it may depend on what version of containers that you have installed. Not to mention the fact that we somehow have a non-empty value of type Set (Tuple Foo Bar) even though Tuple Foo Bar does not have a single coherent Ord instance, and without using any "escape hatch" functions from Data.Set.

You will also notice that ghc (IMO incorrectly) does not complain about any of it, even with -Worphans enabled.

@int-index
Copy link
Contributor

int-index commented Oct 1, 2019

Under my definition of orphans, they are only possible if both overlapping instances are defined in the same module (a clear and easy to fix error), or if at least one of the definitions is an orphan.

And what is, exactly, your definition? What do you think of the example below?

{-# LANGUAGE MultiParamTypeClasses, FlexibleInstances #-}

import Data.Proxy

class C a b where
  f :: Proxy a -> Proxy b
  f Proxy = Proxy

data A
data B

instance C A b   -- orphan?
instance C a B   -- orphan?

k :: Proxy A -> Proxy B
k = f   -- overlapping!

Imagine this being split into several modules, with A and B in separate modules.

@tysonzero
Copy link

tysonzero commented Oct 1, 2019

I actually made a reddit post here right after I made my comment addressing that exact situation.

I would absolutely say that either instance C A b or instance C a B is an orphan, or both, the answer to which one is the orphan is going to require documentation in class C declaring which type variables you must "own". So due to a weakness in Haskell/-Worphan we can only informally declare which is the orphan, and not formally or with the verification of the compiler.

Regardless if we do not include MultiParamTypeClasses the answer is generally simpler. If you do not own the top level type constructor or the class, then it should be considered an orphan, and it can cause the same issues that orphans can cause.

@tysonzero
Copy link

I wanted to bring attention to ghc-proposals/ghc-proposals#279 where a lot of the above is being discussed.

On a personal note I still maintain that these instances are orphans, and that any IsLabel (a -> b) instance should be considered on its own merits. But I will say I now personally think b should be the deciding type and not a, so that #foo can be used for constructors and solving the sum/variant equivalent of the record namespacing problem.

If my desire for b in a -> b to be the deciding type were to go through, then the instances you have defined would no longer be orphans. Although depending on the implementation they may initially overlap and break and have to be moved over to some sort of IsLabelOutput class defined by base.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants