Type constraint changes

[sjakobi]

Say there are two classes:

class A a where
  ...

class A a => B a where
  ...

Assuming that the behaviour of f doesn't change, are any of these type changes permissible for a minor version bump?

1. Constraint relaxation

-f :: B a => a -> X
+f :: A a => a -> X

2. Removing a redundant constraint

-f :: (A a, B a) => a -> X
+f :: B a => a -> X

[phadej]

if A a => B a implication always existed, probably ok, I can't think of anything breaking. But if that implication wasn't there always (like Applicative f => Monad f) then I'd do major bump.

[sjakobi]

Thanks for the quick feedback, @phadej! :)

But if that implication wasn't there always (like Applicative f => Monad f) then I'd do major bump.

I'm curious about your reasoning in this case.

Say, the package exposing f requires base >= 4.8, so post-AMP, is there still anything to worry about that would require a major bump?

[phadej]

worry about that would require a major bump?

Probably not in AMP case, as that is so old, but generally in such cases I'd default to major bump; just to be resilient against issues. Someone might use allow-newer / allow-older, and while if something breaks it's their fault, having "more" constraints to guide solver to not pick broken configurations won't hurt.

TL;DR, if you think "can this change break something downstream", do a major bump. Bumping dependency bounds can be annoying, but it's never the end of the world.

[phadej]

FWIW, I think PVP spec doesn't mention these kind of somewhat special case scenarios at all (non breaking changes are essentially only additions of different kinds), as judging whether

-f :: (A a, B a) => a -> X
+f :: B a => a -> X

is a non-breaking change requires knowing non-locally available information. You have to know the relationship between A and B (in some sense for all configurations). An automatic tool would not be able to tell that this is not breaking change by just by looking at the diff alone.

And I think that having "locally decidable" (and at least in theory automatically checkable) rules is a nice property for a specification.

[parsonsmatt]

I think these changes would be fine for a patch level bump.

I do think it depends on the provenance of A and B and f. If they're defined in the same package, definitely fine as patch/minor. If B is defined in the same package as f, also fine. If A and B are defined in a separate package classes, then you would want to express a lower bound on the classes package that satisfied the relationship you want here - but, of course, there's nothing to stop classes from breaking the A a => B a relationship in an upcoming release.

ie for AMP, you'd be fine to make a patch-level bump to remove the redundant/implied constraint if you have a lower bound on base that includes that information.

[phadej]

I definitely disagree for patch level change.

Other changes. Otherwise, e.g. if change consist only of corrected documentation, non-visible change to allow different dependency range etc. A.B.C MAY remain the same (other version components may change).

The minor change is the least acceptable. Major change preferred.

You should error on the cautious side. Patch level is non-sense.

[phadej]

In particular, PVP is setup in a way, that if you depend on pkg >=x.y.z.v && <some-upper-bound, you can pick any pkg version x.y.z.w (i.e. any w, even less then v) and package should still compile. The API of all x.y.z.w versions should be compatible, up and down.

I hope this insight helps you understand what can or cannot be a patch-level change.

(Note: bug changes could be in patch version, as "compiles" and "works" are not the same; but I'd still suggest that true bug-fixes are indicated with minor bumps, because downstream could use CPP to work around buggy versions if they cannot simply bump lower bounds).

[parsonsmatt]

In particular, PVP is setup in a way, that if you depend on pkg >=x.y.z.v && <some-upper-bound, you can pick any pkg version x.y.z.w (i.e. any w, even less then v) and package should still compile.

Can you link a source for that? That's a really strange requirement, and I cannot find a relevant clause in the PVP Specification.

I'm also curious what scenario might cause a failure-to-compile here, even with that requirement. Especially in the case that package foo defines all of class A a, class (A a) => B a, and f.

[phadej]

an you link a source for that? That's a really strange requirement,

It's not a requirement, it's a consequence (but also a design guide) of current spec. Patch level changes don't change API in any way, so you can (in theory) downgrade.

EDIT: As a concrete example transformers-compat used to release multiple patch versions (with different bounds on base and transformers) without relying on automatic flags. That is a valid (even though a bit obscure) model. But https://hackage.haskell.org/package/transformers-compat-0.6.0.2 and https://hackage.haskell.org/package/transformers-compat-0.6.0.3 and ... are "the same version". .3 is not "better" or "newer" than .2.

[phadej]

I'm also curious what scenario might cause a failure-to-compile here

Going from B a => ... to (A a, B b) => is definitely a breaking change. If traverse :: Applicative f => ..... suddenly required Monad f => ... instead, everything would break.

[parsonsmatt]

Going from B a => ... to (A a, B b) => is definitely a breaking change. If traverse :: Applicative f => ..... suddenly required Monad f => ... instead, everything would break.

The OP has an assumption that class (A a) => B a - so B already implies A. It is more like:

 traverse ::
-   (Monad f) =>
+  (Applicative f) =>
   (a -> f b) ->
   t a ->
   f (t b)

Which - in a post-AMP world - is not a breaking change because all Monad are Applicative.

Additionally, I believe OP is asking about going the other direction:

-f :: (A a, B a) => ...
+f :: (B a) => ...

In other words, fixing a "redundant constraint" warning, rather than introducing one.

I definitely agree that adding a constraint is a breaking change. But removing a constraint can only break -Werror users by triggering -Wredundant-constraints, and removing an implied constraint can't break anything.

Constraint relaxation (ie f :: (B a) => _ to f :: (A a) => _ also would not trigger -Wredundant-constraints - existing callsites would have B a => _ which satisfies A a and GHC currently does not warn if your constraints are "too powerful" but not "redundant."

I suppose it is possible that a library consumer already write (A a, B a) => _ - and so removing B would trigger a redundant constraint warning. However, they'd already have a redundant constraint warning, so they could not possibly be using -Werror and building, so I don't think it is possible to break a build here.

[phadej]

As I said

The OP has an assumption that

is irrelevant. PVP rules are currently "local", i.e. doesn't require knowing any context to deduce whether the change requires major or minor bump.

If you want to advice otherwise, change the spec. (I'll be against).

I think this is discussed enough.

[parsonsmatt]

I don't understand how the spec supports your interpretation. Furthermore, your recommendation has actually changed in this thread from "minor bump is permissible depending on class details" to "always major bump due to 'non-local reasoning'".

The spec says "1. Breaking change. If any entity was removed, or the types of any entities or the definitions of datatypes or classes were changed, or orphan instances were added or any instances were removed, then the new A.B MUST be greater than the previous A.B." This does support "changing type signature syntactically is always a breaking change" - but it also means that, by strictest interpretation, f :: (A a, B b) => _ into f :: (B a, A a) => _ is also a breaking change, which seems silly - they're equivalent.

If we allow "equivalent but syntactically distinct types" to be non-breaking, the question becomes: are they a "non-breaking change" or an "other change"?

Non-breaking change. Otherwise, if only new bindings, types, classes, non-orphan instances or modules (but see below) were added to the interface, then A.B MAY remain the same but the new C MUST be greater than the old C. Note that modifying imports or depending on a newer version of another package may cause extra non-orphan instances to be exported and thus force a minor version change.

Other changes. Otherwise, e.g. if change consist only of corrected documentation, non-visible change to allow different dependency range etc. A.B.C MAY remain the same (other version components may change).

I also don't see anything in the spec to support the "non-local" interpretation you're giving here.

I'm also inclined to say that @sjakobi or myself should be the arbiter on whether or not this has been discussed enough. I respect that you feel confident in your interpretation, but I am confused by it. If you do not wish to discuss it or justify your opinion/interpretation, that's totally fine, and I won't hold you to it, but to close the issue and say "it's been discussed enough" feels unnecessarily aggressive.