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

Proposal: Remove method (/=) from class Eq #3

Closed
nomeata opened this issue Oct 27, 2021 · 171 comments
Closed

Proposal: Remove method (/=) from class Eq #3

nomeata opened this issue Oct 27, 2021 · 171 comments
Labels
approved Approved by CLC vote awaits-merge Approved, but MR is still unmerged

Comments

@nomeata
Copy link

nomeata commented Oct 27, 2021

@Bodigrim suggested I take the proposal that I recently submitted to libraries@ here, to play guinea pig for this process.

What

I suggest to change

class Eq a where (==), (/=) :: a -> a -> Bool

to

class Eq a where (==) :: a -> a -> Bool

and turn (/=) into a normal function.

Why

  • If we’d define class Eq now, we wouldn’t include (/=).

    In general, the motivations for “redundant” methods with default implementation could be

    • The alternative method can be implemented more efficiently than the “main” method.
    • If the “main” method has a default implementation, too: The alternative method is easier or more natural to implement.

    Here, neither of these apply (justification below), and “fixing” this is worth it, for the following reasons:

  • Teaching: The Eq class is often the first, or one of the first, type classes that beginners will face. This means that educators have to explain what default methods are and why they exist, and then have to apologetically say “but this doesn't really apply here, sorry”. (The Ord class is much better in that regard, for example (<=) can be implemented more efficiently than compare.)

  • Development cost and effort: Every developer implementing an Eq instance will have to make a decision whether to instantiate (/=). They have more documentation to read. And precisely because (/=) doesn’t really make sense here, they might have to think extra long about whether to instantiate (/=) or not.

    Removing the method will save some time of future developers.

  • Similarly. whenever someone reads code and comes across an Eq instance with (/=) defined explicitly they have extra work to do, and think “Why was this defined?”, “Is it lawful?”. Removing the method from the class alltogether will mean library code gets simpler and easier to read.

    Removing the method will cause libraries to have less code, not more.

  • Lawfulness. Eq says x /= y = not (x == y), but that is only true if everybody plays by the rules. By having (/=) it is possible for instances to be unlawful (intentionally or accidentally), for little gain.

    Removing the method will guarantee that equation.

  • Code changes are backward compatible. This change will require some library maintainers to change their code. But the code change is simple (remove the method, possibly adjust the import statement), and is compatible with old versions of base. No complex migration strategy, no CPP needed.

  • Performance gains (very minor). Single method classes are currently represented more efficiently in GHC. In some higher-order cases removing the method might speed up programs. Also, compiling base and libraries with Eq instances and creating haddocks gets (very slightly) faster, because there is less to do.

  • Implementation in base is simple: See https://gitlab.haskell.org/ghc/ghc/-/merge_requests/6793.

Why not (and rebuttals)

Most of these are taken from the libraries thread; rebuttals are mine.

  • (/=) can sometimes be implemented more efficiently.

    Rebuttal: Beyond the cost of a single call to not, which GHC often optimizes away anyways, this is not possible (for law-abiding instances.)

  • (/=) can sometimes be more natural to implement.

    Rebuttal: Relatively unlikely, and possibly more error-prone and harder to read, due to the many negations.

  • There is a one-time cost for affected library authors, and it’s not worth it.

    A mildly naive hackage search shows 131 affected packages.

    Rebuttal: The change is easy, mechanical and backward compatible. But more important: Because this change simplifies both base and the code of affected libraries, it will pay off eventually, because of all the work we (as the community) save over time by no longer having to think about (/=). In fact, the evidence that 131 packages’s authors manually (and, in most cases, pointlessy) wrote an (/=) means this proposal will save the next 131 packages’s authors, and beyond, unnecessary work.

  • Some libraries use this to write unlawful instances, e.g. Non-lawful Eq Null instance? haskellari/postgresql-simple#78.

    Rebuttal: Don’t write unlawful instances. If you need (/=) to behave differently than the negation of (==), Eq is the wrong type class. I understand that this is a bit harsh, as the libraries authors might have reasons to break the lawfulness.

  • RULE matching on single method class methods doesn't work reliably, but does work for other classes.

    _ Rebuttal_: I consider that a bug in GHC and would fix one way or another before enacting this proposal.

  • We should clean up the type class hierarchy in a bigger way.

    Rebuttal: Maybe, but that should not doing this small cleanup proposed here.

@djduke
Copy link

djduke commented Oct 27, 2021

I'm -1 on this change. I've >10 years experience teaching Haskell and its never been an issue with teaching.
On the contrary Eq is a simpler case studies to introduce laws, rather than wait until students encounter
Applicative etc
I've sometimes found it simpler to think terms of dfining /=, I've never worried about performance from
this issue, ther eare usually lower hanging fruits elsewhere.

@isovector
Copy link

I'm +1. This is a fantastic simplifying change. There's no reason not to do this, other than extremely trivial breakages. And I would really like to live in a world where we don't let the burden of already-written code get in the way of turning Haskell into the language we want it to be.

@cartazio
Copy link

this breaks code for no reason. Lets not touch this till we have better support for warning about redundantly defined type class methods such as having a "XOR" warning minimal pragma change such as I suggest in https://mail.haskell.org/pipermail/libraries/2021-October/031490.html

@mixphix
Copy link
Collaborator

mixphix commented Oct 27, 2021

I'm +1 on this. x /= y = not (x == y) can and should be defined outside of the class.

As it stands, Haskell's type class system does not have a framework for "class laws": at best, they are guidelines for how types fitting that class should behave. There is no way within a class' definition to guarantee that only those types with lawful implementations should be bestowed the class. The burden lies on the library designer to help guide their users away from footguns; unlawful instances are some of those. By removing (/=) from the class definition and forcing users only to write one method, the opportunity for bugs to arise diminishes and they are easier to spot when they do.

Also, who really writes Eq instances by hand in production code, anyway? Be honest. The vast, vast majority of users derive Eq for the datatypes they define. And by making Eq a single-method typeclass, this deriving gets even easier. You still get (/=) for free, it just so happens it's not in the class definition.

Thanks for raising this issue, @nomeata!

@Bodigrim
Copy link
Collaborator

To facilitate internal CLC discussions, I’d like to gather some additional input:

  1. If we were to design Haskell from scratch today, would we prefer one or two members of Eq? Are there arguments to prefer two today?
  2. Does this change affect educational materials or text books? Could anyone bring specific examples?
  3. Are proponents of the change willing to facilitate ecosystem’s migration by raising PRs to affected libraries? Are maintainers willing to accept such PRs? CC @phadej for postgresql-simple, @paul-rouse for mysql-simple, @treeowl for containers.
  4. What is an impact of this change on GHC and compilation times? CC @AndreasPK.

@nomeata
Copy link
Author

nomeata commented Oct 27, 2021

Does this change affect educational materials or text books? Could anyone bring specific examples?

Concrete example: In a tutorial of my own I explain the concept of default methods when I introduce the Eq class. This section would change, and default implementations only explained later when they first come up. (Unsurprisingly,) I would welcome that change; it would allow me introduce default methods with a type class where they make more sense (e.g. Ord).

@phadej
Copy link

phadej commented Oct 27, 2021

1 If we were to design Haskell from scratch today, would we prefer one or two members of Eq? Are there arguments to prefer two today?

I'd probably flip a coin

2 Does this change affect educational materials or text books? Could anyone bring specific examples?

Hutton's Programmin gin Haskell says in 3.9 basic classes section

Eq - equality types

... using following two methods

(==) :: a -> a -> Bool
(/=) :: a -> a -> Bool

However, the presentation in that chapter just tells these functions
exist, it doesn't yet speak about defining own instances.

I don't think that would trap future readers of an old edition book who use newer GHC

3 Are proponents of the change willing to facilitate ecosystem’s migration by raising PRs to affected libraries?

See below. Though I'm not proponent nor opponent of this change, I just want
to dismiss the narrative that a lot stuff would break.
I didn't believe it. Judge yourself.

Are maintainers willing to accept such PRs?

I'd accept a PR, though I'd just do it myself during the usual new-GHC-support
work. (In my case, to have a comparison example, removal of Option generates/d more
work).


I built Stackage LTS-18.8 set (which I had around for other experiment).
Out of ~2600 packages (snapshot is slightly larger, but I left out some packages
needing native dependencies etc.)
the following ~45 needed some changes:

packages: attoparsec-0.13.2.5/
packages: backprop-0.2.6.4/
packages: basement-0.0.12/
packages: basic-prelude-0.7.0/
packages: butcher-1.3.3.2/
packages: clash-lib-1.4.3/
packages: clash-prelude-1.4.3/
packages: clientsession-0.9.1.2/
packages: compdata-0.12.1/
packages: compensated-0.8.3/
packages: conduit-1.3.4.1/
packages: DBFunctor-0.1.2.1/
packages: dimensional-1.4/
packages: eventstore-1.4.1/
packages: fb-2.1.1/
packages: geojson-4.0.2/
packages: ghc-lib-parser-8.10.7.20210828/
packages: haskell-gi-base-0.25.0/
packages: HaskellNet-0.6/
packages: hOpenPGP-2.9.5/
packages: hybrid-vectors-0.2.2/
packages: intro-0.9.0.0/
packages: io-streams-1.5.2.1/
packages: loc-0.1.3.10/
packages: mixed-types-num-0.5.9.1/
packages: multistate-0.8.0.3/
packages: numbers-3000.2.0.2/
packages: numeric-prelude-0.4.3.3/
packages: postgresql-libpq-0.9.4.3/
packages: postgresql-simple-0.6.4/
packages: prelude-compat-0.0.0.2/
packages: primitive-addr-0.1.0.2/
packages: protolude-0.3.0/
packages: relude-0.7.0.0/
packages: sbv-8.15/
packages: semirings-0.6/
packages: singletons-2.7/
packages: snap-core-1.0.4.2/
packages: sqlite-simple-0.4.18.0/
packages: string-class-0.1.7.0/
packages: tfp-1.0.2/
packages: type-level-natural-number-2.0/
packages: type-natural-1.1.0.0/
packages: unboxing-vector-0.2.0.0/
packages: uncertain-0.3.1.0/
packages: unification-fd-0.11.1/
packages: vector-0.12.3.0/

(and mysql-simple which I didn't build).

Most changes are oneliners. Either removing explicit /= definition,
or adding /= import.

I can upload patches somewhere, if anyone is interested to take a closer look
at them. (I won't submit them upstream, though some are probably good even
without the change going in, i.e. removing unnecessary definitions of /=).

Few packages are more interesting:


clash-prelude

This is interesting application.
I'd ask if the change makes things less convenient.

The /= is defined to be some "primitive" (which is NOINLINE, and in future
OPAQUE I suppose) for which Clash looks in the Core. That's my
understanding.


alternative or special purpose preludes:

protolude, basic-prelude, haskell-gi-base, relude needed an amendment to keep the re-export list intact.
Similarly basement, though it also needed a fix in itself.


GHC (like) code: clash-lib and ghc-lib-parser, "I have seen this code".


complicated /= example

https://hackage.haskell.org/package/numbers
package illustrates the "are == and /= compatible, or do they differ?"
problem.

data Interval a = I a a

ival :: (Ord a) => a -> a -> Interval a
ival l h | l <= h = I l h
         | otherwise = error "Interval.ival: low > high"

instance (Ord a) => Eq (Interval a) where
    I l h == I l' h'  =  l == h' && h == l'
    I l h /= I l' h'  =  h < l' || h' < l

https://hackage.haskell.org/package/DBFunctor package has:

-- | Definition of the Relational Data Type. This is the data type of the values stored in each 'RTable'.
-- This is a strict data type, meaning whenever we evaluate a value of type 'RDataType', 
-- there must be also evaluated all the fields it contains.
data RDataType = 
        RInt { rint :: !Integer }
      | RText { rtext :: !T.Text }
      | RUTCTime { rutct :: !UTCTime}
      | RDate { 
                rdate :: !T.Text
               ,dtformat :: !Text  -- ^ e.g., "DD\/MM\/YYYY"
            }
      | RTime { rtime :: !RTimestamp  }
      | RDouble { rdouble :: !Double }
    -- RFloat  { rfloat :: Float }
      | Null



-- | We need to explicitly specify equation of RDataType due to SQL NULL logic (i.e., anything compared to NULL returns false):
-- @
-- Null == _ = False,
-- _ == Null = False,
-- Null /= _ = False,
-- _ /= Null = False.
-- @
-- IMPORTANT NOTE:
-- Of course this means that anywhere in your code where you have something like this: 
-- @
-- x == Null or x /= Null, 
-- @
-- will always return False and thus it is futile to do this comparison. 
-- You have to use the is 'isNull' function instead.
--
instance Eq RDataType where

    RInt i1 == RInt i2 = i1 == i2
    -- RInt i == _ = False
    RText t1 == RText t2 = t1 == t2
    -- RText t1 == _ = False
    RDate t1 s1 == RDate t2 s2 = toRTimestamp (unpack s1) (unpack t1) == toRTimestamp (unpack s2) (unpack t2)   -- (t1 == t1) && (s1 == s2)
    -- RDate t1 s1 == _ = False
    RTime t1 == RTime t2 = t1 == t2
    -- RTime t1 == _ = False
    RDouble d1 == RDouble d2 = d1 == d2
    -- RDouble d1 == _ = False
    -- Watch out: NULL logic (anything compared to NULL returns false)
    Null == Null = False
    _ == Null = False
    Null == _ = False
    -- anything else is just False
    _ == _ = False

    Null /= Null = False
    _ /= Null = False
    Null /= _ = False
    x /= y = not (x == y) 

I.e. it's a DSL trying to emulate SQL-like processing in Haskell.
Not able to do weird thing SQL does with NULl equality would not be nice.
(a lot more inconvenient then for postgresql-simple, where I actually have
no idea what Eq Null instance is useful for).

However, when removing the /= none of the tests of DBFunctor failed.


  • Oleg

@paul-rouse
Copy link

paul-rouse commented Oct 28, 2021

It would be good to get away from the blatantly unlawful instance in mysql-simple! This predates my time as maintainer, so I can only assume the idea was along the lines of the comment in DBFunctor mentioned above. However, it is a poor reflection of modern SQL semantics, and could be very confusing. I would probably just remove the Eq instance on Null, but it is, unfortunately, exposed to users of the library. I doubt if anybody actually uses it, and I've checked that persistent-mysql doesn't, but it's possible that something somewhere would break (I haven't yet tried compiling all of stackage against this change).

Exactly the same code appears in postgresql-simple, so I assume the same applies there.

@phadej
Copy link

phadej commented Oct 28, 2021

However, it is a poor reflection of modern SQL semantics

The poorness there is try to model 3VL with two-valued Bool. But I think 3VL way of presenting the NULL comparisons is not particularly new.

https://en.wikipedia.org/wiki/Null_(SQL)#Comparisons_with_NULL_and_the_three-valued_logic_(3VL)

@AndreasPK
Copy link

AndreasPK commented Oct 28, 2021 via email

@Bodigrim
Copy link
Collaborator

CC @christiaanb for clash-prelude, @nkarag for DBFunctor.

@Icelandjack
Copy link

Using type class backends we could transparently evolve Eq to BackendEq with only one method

@phadej
Copy link

phadej commented Oct 29, 2021

Using type class backends we could transparently evolve Eq to BackendEq with only one method

Something like Eq should be a simple as can be imaginable. Original motivation says:

Teaching: The Eq class is often the first, or one of the first, type classes that beginners will face.

@gwils
Copy link
Contributor

gwils commented Oct 30, 2021

I find it hard to form a particularly strong opinion on this either way. It seems like a fairly minor breakage for very minor wins. Some thoughts:
I don't weight teaching very heavily in decision-making about Prelude. I think most educational material would be better served defining a custom prelude, with less polymorphic types, fewer and simpler classes, etc. I'm also perfectly happy to tell students useful lies, for example, I could just present Eq as having one method, and students writing their own instances would be able to do so happily without knowing (/=) is a class method, or even exists!
I'm somewhat sympathetic to the argument that a single method makes law-breaking instances harder, but Eq is rather weak in its laws; in deference to the practically defunct Haskell Report, the Eq laws are explicitly merely "encouraged" (if you'll allow me to side-step Melanie's point that all laws in Haskell are effectively suggestions anyway). I would be more sympathetic to this change if we took a firmer stance on Eqs lawfulness.
Inequality checks are available as an instruction for some types, aren't they? Wouldn't that mean some types will have worse performance with this change (an unnecessary function call to not vs. a single instruction). I confess I'm not very knowledgeable at that level of abstraction.

Right now I'm a very weak -1 on this.

@int-index
Copy link

int-index commented Oct 30, 2021

As a Haskell user I probably wouldn’t notice this (I always derive Eq), but back when I was learning Haskell I thought it was pretty neat that the language doesn’t take a stance whether == or /= is the fundamental operation. It’s a nice way to introduce the notion of minimal complete definition (i.e. define a method of your choice and get the other one for free).

Really, why is your proposal to remove /= and keep == rather than to remove == and keep /=? What makes one more fundamental than the other? I used to find it quite elegant that we don’t have to make these arbitrary choices (nowadays I don’t care).

To summarise, I’m leaning slightly against this proposal, but wouldn’t lose my sleep over it.

@ksqsf
Copy link

ksqsf commented Oct 30, 2021

Does the same reasoning apply to (<$)? It is currently defined in the Functor class. If we remove (/=) from Eq, should we remove (<$) from Functor too?

@gbaz
Copy link

gbaz commented Oct 30, 2021

Nope. As the docs to that say: " The default definition is fmap . const, but this may be overridden with a more efficient version." The argument for removing /= is that because negation of booleans is cheap, there is no more efficient version.

@Bodigrim
Copy link
Collaborator

When we add a type class member, which can be expressed via others, there is only one possible excuse to do so: one can provide a more efficient implementation than default. Since this is not the case for (/=), its presence in Eq just opens a window for incoherence without any benefit. Neither (==) nor (/=) is more fundamental than another, but having them both is asking for troubles.

Given that the migration path is straightforward and backward compatible and that maintainers of SQL libraries above are on board with cleaning infelicities in instance Eq Null, I'm +1 on this change.

@phadej
Copy link

phadej commented Oct 31, 2021

In some settings /= is a primitive operation and == is derived from it.

This is speculation. @nomeata's test indicate that it's not an issue. (Compiling Cabal, see the mailing post https://mail.haskell.org/pipermail/libraries/2021-October/031484.html). @nomeata have you already run nofib too?

@tomjaguarpaw
Copy link
Member

Many people with good judgement believe there is some merit to this proposal. I confess I cannot see what they see. I am very much against churn for the sake of minor improvement. The Haskell community already has a reputation of insufficiently valuing stability.

On the other hand, perhaps this change will have minimal impact. Indeed the only way I could countenance it is if there is no impact on maintainers. That seems to imply that someone must take responsibility for ensuring that the entirety of Hackage builds with this change before the change is actually applied. If that happens then I won't oppose the proposal (not that my voice carries any weight) but I still can't understand the motivation.

@nomeata
Copy link
Author

nomeata commented Oct 31, 2021

@nomeata have you already run nofib too?

No, I did not. I shy away from performance measurements because it's so hard to get them meaningfully right, at least for runtime changes in sub percentage space.

@phadej
Copy link

phadej commented Oct 31, 2021

@nomeata check nofib perf options. These are good as they don't count wall clock:

Usage: nofib-run [-c|--clean] [TEST] [-j|--threads N] [-w|--compiler HC] 
                 [--compiler-arg ARG] (-o|--output DIR) [--cachegrind] 
                 [--cachegrind-arg ARG] [--perf] [[--perf-arg ARG]] 
                 [-s|--speed ARG] [--rts-arg ARG] [-t|--times ARG] 
                 [--keep-going] [--head] [-V|-v|--verbosity ARG]
  nofib Benchmark Suite

Available options:
  -h,--help                Show this help text
...

  --perf                   Run the tests under `perf stat`

@santiweight
Copy link

santiweight commented Oct 31, 2021

I've seen a number of proposals on and off like this recently. I think the reason is likely that there's a an energy to "fixing" Haskell, which I think is a great symptom and energy in the community. However, I advocate against this change, not because the change is bad in isolation, but because accepting this proposal would be symptomatic of a poor and inconsiderate design process, and a lack of unifying philosophy in the CLC.

My understanding is that accepting such a proposal would mean that the CLC is saying "we will accept any improvement to the Haskell core libraries that has a minor cost and is an improvement on the past situation."

For example, we can take a similar proposal here from @isovector which aimed to export liftA2 and join from Prelude which is a seemingly innocent change, just like this proposal from @nomeata. That proposal seems to satisfy a similar place to this Eq proposal, namely it is small and not-too-breaking.

But let's assume we accept this change. Now I come along and look at Ord, and I say "huh", why include anything other than <=? I won't argue for or against the content of such a proposal... But note now that we have 3 breaking changes, each come up with "on a whim" (which is not bad), and 3 breaking changes to Prelude that seem:

  1. Unaware of each other
  2. Unclear whether they actually address the principle they attempt to uphold (for example this Eq proposal which attempts to prevent unlawful Eq does not do so for `Ord... Why is that?)
  3. Minor changes that alone are not a big deal but start to add up... I could probably find 10 such proposals if I tried...
  4. Potentially happening across different base releases

Note also that the change to Eq might cause a worse situation... What if the breakage caused by the change to Ord is actually too substantial (I personally once had a unlawful Ord instance) and we decide that that typeclass cannot be broken. Now we have one typeclass that is "right" and one typeclass that is "wrong". That situation is worse than today! Next year we'll have someone propose including /= in Eq ;)

The point being, we shouldn't accept that these proposals would each require more files to have CPP code in them without really auditing base and doing such a change in one fell swoop. Plus we should really consider developing a migration tool for Haskell (and non-text-based conditional compilation), because these problems will keep coming up and library maintainers will eventually be unable to use linters if we don't give them the attention they deserve.

In sum, such a change cannot and should not be an isolated change. The real proposal as I see it is "Remove default typeclass methods from typeclasses where their implementation can upset the class's laws". That is a very reasonable proposal, whose actual costs we can discuss fully by inspecting all unlawful classes and a subsequent migration strategy. That way, we can also talk about three or four "principles" being fixed in base in a major version instead of a hodge-podge of unrelated, but each seemingly reasonable, whack-a-moles updates!

@isovector
Copy link

@santiweight's comment is in line with my recent thoughts here. There are lots of really good changes we should make to base, all of which keep getting shot down for exactly the same reason of "too much churn." But that doesn't mean we shouldn't do them! It just means we should pick some particular time/release and put all of our big breaking changes into that basket.

ofc a better solution would be to unprivilege base so that it's unrelated to GHC versions, and then just use semver for it like we do with literally every other package.

@phadej
Copy link

phadej commented Oct 31, 2021

In sum, such a change cannot and should be an isolated change. The real proposal as I see it is "Remove default typeclass methods from typeclasses where their implementation can upset the class's laws".

But it is not*. The proposal says

(The Ord class is much better in that regard, for example (<=) can be implemented more efficiently than compare.)

And as other have pointed out, it's difficult to figure out where /= is more efficient then ==.

The examples brought by @effectfully are all with a disclaimer: "(I don't really use the Eq class there)".

@phadej
Copy link

phadej commented Oct 31, 2021

and then just use semver for it like we do with literally every other package.

PVP is semantic versioning scheme. There are problems:

  • base (and template-haskell) are the only ways to imply GHC version support in cabal files. There are changes which are not library ones, (like recent changes in how RankNTypes work, simplified subsumption).
  • base is huge API surface, in 6 months it almost by necessity gets some breaking change in a corner like

Add a Typeable constraint to fromStaticPtr in the class GHC.StaticPtr.IsStatic.

Otherwise I agree, base which is decoupled from GHC release cycle would be great, but the infrastructure is not there yet. It cannot be done any time soon.

It would greatly reduce the compatibility burden. it would be enough to make a version of base to be compatible with 3 latest GHCs, and not worry about that the downstream code can be written against three different bases.

But that all just wishes. If you don't like the churn, please push for solving the underlying problems, not blocking changes in the mean time.

@santiweight
Copy link

@phadej I don't I made my point totally clear in that regard, the point being that the removing <= or compare would each have their arguments, and the fact that the removal of one or the other could be a reasonable proposal means that we will always be catching up (addressing new proposals etc.) if we don't initially decide on what principle guides the design of such proposals, so we can easily say "this proposal fails on this principle" for example. As it stands, each time a proposal comes out we start from 0 and the discussion is repeating itself.

@Ericson2314
Copy link
Contributor

I would like some large scale coordination on how to make decoupled base a reality. Otherwise we get these massive threads with no easy answers and the CLC can only easily do things at the margins.

I think we can do it today with CPP, and then we will want to invest in things like better orphans and better backpack to make the interfaces cleaner.

@santiweight
Copy link

But that all just wishes. If you don't like the churn, please push for solving the underlying problems, not blocking changes in the mean time.

I am pushing for the underlying problems! I am currently trying to come up with a non-text-based pre-processor/conditional-compilation extension that would allow a migration tool to be written for changes to namespace etc. I posted in the slack channel yesterday in fact and (once some current personal life stuff is done) I will try to get some implementation over Christmas. But the point also stands that this proposal's acceptance is a worrying and a real_world slippery slope (slippery slopes are valid arguments if they happen), and I am personally not trying to "block changes" but instead to protect users, which is, after-all the whole point of the core libraries. The core principle here is not "make states unrepresentable, but make users happy and productive - the second principle implies the first only some of the time, for the reasons outlined above and in the proposal

@phadej
Copy link

phadej commented Oct 31, 2021

@santiweight

I don't I made my point totally clear in that regard, the point being that the removing <= or compare would each have their arguments,

No they won't. compare and <= has more efficient implementations. And GHC derives them:

{-# OPTIONS -ddump-deriv #-}
module MyMaybe where

data MyMaybe a  =  MyNothing | MyJust a
  deriving ( Eq, Ord )
==================== Derived instances ====================
Derived class instances:
  instance GHC.Classes.Eq a =>
           GHC.Classes.Eq (MyMaybe.MyMaybe a) where
    (GHC.Classes.==) (MyMaybe.MyNothing) (MyMaybe.MyNothing)
      = GHC.Types.True
    (GHC.Classes.==) (MyMaybe.MyJust a1_a2cD) (MyMaybe.MyJust b1_a2cE)
      = ((a1_a2cD GHC.Classes.== b1_a2cE))
    (GHC.Classes.==) _ _ = GHC.Types.False
  
  instance GHC.Classes.Ord a =>
           GHC.Classes.Ord (MyMaybe.MyMaybe a) where
    GHC.Classes.compare a_a2cF b_a2cG
      = case a_a2cF of
          MyMaybe.MyNothing
            -> case b_a2cG of
                 MyMaybe.MyNothing -> GHC.Types.EQ
                 _ -> GHC.Types.LT
          MyMaybe.MyJust a1_a2cH
            -> case b_a2cG of
                 MyMaybe.MyJust b1_a2cI -> (a1_a2cH `GHC.Classes.compare` b1_a2cI)
                 _ -> GHC.Types.GT
    (GHC.Classes.<) a_a2cJ b_a2cK
      = case a_a2cJ of
          MyMaybe.MyNothing
            -> case b_a2cK of
                 MyMaybe.MyNothing -> GHC.Types.False
                 _ -> GHC.Types.True
          MyMaybe.MyJust a1_a2cL
            -> case b_a2cK of
                 MyMaybe.MyJust b1_a2cM -> (a1_a2cL GHC.Classes.< b1_a2cM)
                 _ -> GHC.Types.False
    (GHC.Classes.<=) a_a2cN b_a2cO
      = GHC.Classes.not ((GHC.Classes.<) b_a2cO a_a2cN)
    (GHC.Classes.>) a_a2cP b_a2cQ = (GHC.Classes.<) b_a2cQ a_a2cP
    (GHC.Classes.>=) a_a2cR b_a2cS
      = GHC.Classes.not ((GHC.Classes.<) a_a2cR b_a2cS)

Note: there is no /= definition. Default is used. There is both compare and <

@JakobBruenker
Copy link

That would seem to be the consistent way to handle this

@augustss
Copy link

I think a clear statement in the Haskell Report on how class laws are supposed to be interpreted would be useful.
If the class laws are required to be fulfilled then it would allow better reasoning, but it would also make certain instances impossible, like Eq and Ord for floating point (using the normal semantics for IEEE754).
If the laws are just encouraged, then removing (/=) from Eq will break code that does not abide by the laws.

@nomeata
Copy link
Author

nomeata commented Nov 22, 2021

I agree: if the sentiment is that law-breaking instances are not just an unavoidable possibility that falls under “just don't do that, and if you do, you get to keep both pieces” (a bit like orphan instances), but rather something we want to support developers in doing, then I may not have proposed this (because then there is a justification for having (/=), e.g. to play tricks like the SQL libraries). It is not my sentiment, but maybe worth discussing and documenting in its own proposal somewhere to see where the community really stands on this?

@treeowl
Copy link

treeowl commented Nov 22, 2021

I am all in favor of removing the bogus floating point representation instances.

@tomjaguarpaw
Copy link
Member

maybe worth discussing and documenting in its own proposal somewhere to see where the community really stands on this?

Probably worth having had that discussion before this proposal was accepted (or maybe even before it was proposed). It's much easier to get community buy-in on a change that adheres to already agreed and understood principles! It's much harder to get community buy-in when one is implicitly trying to establish principles during and after discussion on a specific proposal!

@nomeata
Copy link
Author

nomeata commented Nov 22, 2021

Absolutely! But sometimes you first have to discover that some principles are not universally held…

@augustss
Copy link

I think the fact that x/=x for floating point numbers is a small trap for the unwary compared with, e.g., the fact that associativity doesn't hold for + and *. Floating point numbers should not be a member of any of the standard classes.
But that would be a shame.

@treeowl
Copy link

treeowl commented Nov 22, 2021

@augustss, why would that be a shame? Floating point representations have a completely different mathematical structure than most things traditionally considered numbers.

@augustss
Copy link

@treeowl One of the major reasons to introduce type classes in Haskell was to be able to write + both for integers and floating point numbers. This what people expect, and I think this is what Haskell should allow.

@davean
Copy link

davean commented Nov 22, 2021 via email

@augustss
Copy link

I think would be better to separate the lawful instances from the "raw instances".
So, for instance, we would have
class (Eq a) => LawfulEq a;
The LawfulEq class will have no methods (until we have full dependent types and can make proofs), but will indicate that the instance actually obeys the laws.
So for code that requires the laws to hold, e.g., Data.Set, the constraint should be LawfulEq rather than Eq.

Alternatively, have Eq be lawful, and UnlawfulEq be Eq without laws.

@svenpanne
Copy link

I think would be better to separate the lawful instances from the "raw instances". So, for instance, we would have class (Eq a) => LawfulEq a; The LawfulEq class will have no methods (until we have full dependent types and can make proofs), but will indicate that the instance actually obeys the laws. [...]

This is effectively what Rust does:

While not perfect from a theoretical POV, I think it is in a very nice "sweet spot" between theory and practice. This would have been a step into the right direction for Haskell, not some random shuffling of methods/functions for no good reason.

@treeowl
Copy link

treeowl commented Nov 22, 2021

Classes with only laws make it hard to read code. Every time you see a method, your have to figure out:

  1. Does this refer to a concrete type? If so, which one? Does that have a lawful instance?
  2. If you figure out that it refers to one or more type variables, then you have to look to the (possibly distant) type signature to figure out how those are constrained.

This sounds awful. An alternative is to copy all the methods, so we have lawlessFmap along with fmap. This sounds pretty bad too!

@augustss
Copy link

I'm sorry, I don't understand at all why a class with only laws would make code harder to read. The code could look exactly like it does today, and very little would change when looking up methods.

@treeowl
Copy link

treeowl commented Nov 23, 2021

@augustss, suppose I'm reading some code and I come across an expression 3 * 1_000_000 * 7. Does that mean the same thing as 21 * 10^6? Maybe. Maybe not. A wild Num instance could define fromInteger and (*) as utterly arbitrary functions. Another example: if I see fmap f (fmap g x), can I change that to fmap (f . g) x? Or do I have to first perform a careful investigation of whether fmap for this type deletes some of the line breaks? My ability to understand code locally goes out the window.

@bfrk
Copy link

bfrk commented Dec 3, 2021

-1 for the reasons given by others. This proposal is missing a clear design principle. With the same argument we could eliminate < and > from Ord (x < y = not (x >= y) etc). Or should we eliminate >= and <=? I am not saying that the choice to include both == and /= is strictly better than having just one of them, but it is consistent with the choices made for Ord: it expresses a preference for symmetry in favour of minimality. The existing defaults already nudge people who write instances manually toward abiding the laws, you'd have to go out of your way to write a non-abiding instance. IMO this is solving a non-issue.

@ekmett
Copy link
Member

ekmett commented Dec 3, 2021 via email

@Ericson2314
Copy link
Contributor

Ericson2314 commented Dec 3, 2021

In addition to implementation explosion, type inference will happily supply
the LawlessFoo constraint

This is the main thing for me. When correctness because the enemy of automation. correctness looses.

(In this case, the automation in question is "type tetris", semi-consciously changing things just thinking about the error messages and not what meaning of what one is writing)


For NaN, I honestly think the right thing to do is teach GHC that:

  • there is some kinda of unboxed maybe monad going on
  • the primops are actually liftA2# (==) and liftA2# (/=) over the "true" floats. ((==) and (/=) either need to become reuntime-rep polymorphic, or we create variants there which are.
  • True floats and maybe floats have the same runtime rep

Done right, there should be no performance cost to this, at least one one "overuses" the FloatMaybe# Float# version. With enough optimizations and Float#.do, perhaps something that looks more idiomatic might also be fast.

@phadej
Copy link

phadej commented Dec 18, 2022

@nomeata and @Bodigrim is there are any implementation plan for this? or is this proposal scraped and won't be implemented in foreseeable future?

@phadej
Copy link

phadej commented Dec 18, 2022

In other words won't be implemented in foreseeable future.

Thanks.

EDIT: https://github.com/haskell/core-libraries-committee/blob/main/guides/no-noneq-in-eq.md is outdated. It speaks about GHC-9.6, but as there is no progress on either of GHC issues, I'd not promise any GHC version.

@nomeata
Copy link
Author

nomeata commented Dec 18, 2022

Indeed progress has stalled on some internals of GHC here, in a way. In addition, the manurestorm that this proposal caused made it tempting to push this through. 🤷‍♂️

@chshersh
Copy link
Member

I'm trying to summarise the state of this proposal as part of my volunteering effort to track the progress of all approved CLC proposals.

Field Value
Author @nomeata
Status not merged
base version Unknown
Merge Request (MR) https://gitlab.haskell.org/ghc/ghc/-/merge_requests/6793
Blocked by GHC Issue #20535, GHC Issue #20897
CHANGELOG entry missing
Migration guide https://github.com/haskell/core-libraries-committee/blob/main/guides/no-noneq-in-eq.md

Please, let me know if you find any mistakes 🙂


@nomeata could you share a status update on the implementation and what are next steps if anything changed since Dec 2022? Also, please do let CLC know if you need any help, so we can coordinate and prioritise approved proposals accordingly!

@chshersh chshersh added the awaits-merge Approved, but MR is still unmerged label Mar 15, 2023
@nomeata
Copy link
Author

nomeata commented Mar 16, 2023

Thanks for the nudge! Nothing new really. To make this change with the least possible disruption, GHC first has to stop treating single-method classes different from multi-method classes (https://gitlab.haskell.org/ghc/ghc/-/issues/20897). The whole thing got entangled in a confusing mess including how to implement IsLabel and IP then, and interaction with the reflection library (https://gitlab.haskell.org/ghc/ghc/-/issues/21568).

Eventually I ran out of steam, and the partial work now just sits there, waiting for someone to pick it up. These changes to GHC are likely desirable even without the Eq-of-no-neq proposal, but it’s not fully clear that they are good.

I was a bit burned by the outcry after the Eq-of-no-neq proposal and am not going to push for it.

So basically there is some low priority work on the GHC side blocking this, so this proposal will likely just have to sit until that is done.

@Bodigrim
Copy link
Collaborator

Bodigrim commented Jul 5, 2023

As a historical note, with this proposal we made a full circle back to Haskell Report 1988 which defines Eq and Ord as follows (p. 34):

class Eq a where 
  (==) :: a -> a -> Bool
class Eq a => Ord a where 
  (<), (<=) :: a -> a -> Bool

(/=) :: Eq a => a -> a -> Bool
(>), (>=) :: Ord a => a -> a -> Bool
max, min :: Ord a => a -> a -> a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Approved by CLC vote awaits-merge Approved, but MR is still unmerged
Projects
None yet
Development

No branches or pull requests