Feedback on -Wredundant-constraints

With 8.0.1 freshly arrived, I'm taking on the task of updating a number of our larger projects here at Galois. I made a couple of these comments briefly on a relevant Trac ticket[1], but I wanted to open this discussion to a wider audience. We tend to use -Wall (and -Werror, in CI environments), and so I've had to make decisions about how to handle the new -Wredundant-constraints warnings. So far, I've come to think of it as two different warnings that happen to be combined: Warning 1: a warning for constraints made redundant by superclass relationships, and Warning 2: a warning for unused constraints Overall I'm a fan of Warning 1. It seems very much in the spirit of other warnings such as unused imports. The only stumbling block is how it affects the 3-release compatibility plan with respect to, e.g., the AMP. Most of our code targets a 2-release window, though, so in every such case it has been fine for us to simply remove the offending constraint. Warning 2 on the other hand is far more dubious to me. In the best case, it finds constraints that through oversight or non-local changes are truly no longer necessary in the codebase. This is nice, but the much more common case in our code is that we've made a deliberate decision to include that constraint as part of our API design. The most painful example of this I've hit so far is in an API of related functions, where we've put the same constraint on each function even when the implementation of that particular function might not need that constraint. This is good for consistency and forward-looking compatibility (what if we need that constraint in the next version?). The warning's advice in this case makes the API harder to understand, and less abstract (the client shouldn't care or know that f needs Functor, but g doesn't, if both will always be used in a Functor context). On another level, Warning 2 is a warning that we could have given a more general type to a definition. We quite rightly don't do this for the non-constraint parts of the type signatures, so why are we doing it for the constraints? I'm happy that Warning 1 is now around, but Warning 2 feels much more like an opinionated lint check, and I really wish it wasn't part of -Wall. [1]: https://ghc.haskell.org/trac/ghc/ticket/10635#comment:15

Agreed. Ive hit exactly these issues myself to the point where i err on
suppressing that warning in my code now. In part because i use those
unused constraints as a semantic contract.
On Friday, June 3, 2016, Adam Foltzer
With 8.0.1 freshly arrived, I'm taking on the task of updating a number of our larger projects here at Galois. I made a couple of these comments briefly on a relevant Trac ticket[1], but I wanted to open this discussion to a wider audience.
We tend to use -Wall (and -Werror, in CI environments), and so I've had to make decisions about how to handle the new -Wredundant-constraints warnings. So far, I've come to think of it as two different warnings that happen to be combined:
Warning 1: a warning for constraints made redundant by superclass relationships, and Warning 2: a warning for unused constraints
Overall I'm a fan of Warning 1. It seems very much in the spirit of other warnings such as unused imports. The only stumbling block is how it affects the 3-release compatibility plan with respect to, e.g., the AMP. Most of our code targets a 2-release window, though, so in every such case it has been fine for us to simply remove the offending constraint.
Warning 2 on the other hand is far more dubious to me. In the best case, it finds constraints that through oversight or non-local changes are truly no longer necessary in the codebase. This is nice, but the much more common case in our code is that we've made a deliberate decision to include that constraint as part of our API design.
The most painful example of this I've hit so far is in an API of related functions, where we've put the same constraint on each function even when the implementation of that particular function might not need that constraint. This is good for consistency and forward-looking compatibility (what if we need that constraint in the next version?). The warning's advice in this case makes the API harder to understand, and less abstract (the client shouldn't care or know that f needs Functor, but g doesn't, if both will always be used in a Functor context).
On another level, Warning 2 is a warning that we could have given a more general type to a definition. We quite rightly don't do this for the non-constraint parts of the type signatures, so why are we doing it for the constraints?
I'm happy that Warning 1 is now around, but Warning 2 feels much more like an opinionated lint check, and I really wish it wasn't part of -Wall.
[1]: https://ghc.haskell.org/trac/ghc/ticket/10635#comment:15

I've been bitten by this too and had to disable the warning. Let me propose an alternative; * -Wredundant-constraints becomes only your Warning 1. That is, it reports when a user writes a constraint that is fully equivalent to some other, strictly smaller constraint, like suggesting simplifying (Eq a, Ord a) to (Ord a). * -Wtype-overly-specific takes on Warning 2, and adds the ability to catch any type signature that's more specific than it needs to be. (Whether or not to add this to -Wall is for others to decide.) This is indeed a more lint-like warning, but HLint would be hard-pressed to figure this one out. * We really need a way of disabling/enabling warnings per declaration. I propose something like this:
{-# WARNINGS foo -Wno-type-overly-specific #-} foo :: Int -> Int foo x = x
Richard
On Jun 4, 2016, at 9:11 AM, Carter Schonwald
Agreed. Ive hit exactly these issues myself to the point where i err on suppressing that warning in my code now. In part because i use those unused constraints as a semantic contract.
On Friday, June 3, 2016, Adam Foltzer
wrote: With 8.0.1 freshly arrived, I'm taking on the task of updating a number of our larger projects here at Galois. I made a couple of these comments briefly on a relevant Trac ticket[1], but I wanted to open this discussion to a wider audience. We tend to use -Wall (and -Werror, in CI environments), and so I've had to make decisions about how to handle the new -Wredundant-constraints warnings. So far, I've come to think of it as two different warnings that happen to be combined:
Warning 1: a warning for constraints made redundant by superclass relationships, and Warning 2: a warning for unused constraints
Overall I'm a fan of Warning 1. It seems very much in the spirit of other warnings such as unused imports. The only stumbling block is how it affects the 3-release compatibility plan with respect to, e.g., the AMP. Most of our code targets a 2-release window, though, so in every such case it has been fine for us to simply remove the offending constraint.
Warning 2 on the other hand is far more dubious to me. In the best case, it finds constraints that through oversight or non-local changes are truly no longer necessary in the codebase. This is nice, but the much more common case in our code is that we've made a deliberate decision to include that constraint as part of our API design.
The most painful example of this I've hit so far is in an API of related functions, where we've put the same constraint on each function even when the implementation of that particular function might not need that constraint. This is good for consistency and forward-looking compatibility (what if we need that constraint in the next version?). The warning's advice in this case makes the API harder to understand, and less abstract (the client shouldn't care or know that f needs Functor, but g doesn't, if both will always be used in a Functor context).
On another level, Warning 2 is a warning that we could have given a more general type to a definition. We quite rightly don't do this for the non-constraint parts of the type signatures, so why are we doing it for the constraints?
I'm happy that Warning 1 is now around, but Warning 2 feels much more like an opinionated lint check, and I really wish it wasn't part of -Wall.
[1]: https://ghc.haskell.org/trac/ghc/ticket/10635#comment:15 _______________________________________________ Glasgow-haskell-users mailing list Glasgow-haskell-users@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/glasgow-haskell-users

Strong emphatic agreement. Both that it should split up thusly and that the
latter shouldn't be in WALL. Otherwise it poisons the well for all be the
most sophisticated users of type level programming.
I don't suppose there's any hope of having this resolved prior to GHC 8.2
because it is a real usability regression? Because I think we can all agree
that the proposed change would not break any 8.0 series code, and
positively impact everyone.
On Jun 6, 2016 9:40 AM, "Richard Eisenberg"
I've been bitten by this too and had to disable the warning.
Let me propose an alternative; * -Wredundant-constraints becomes only your Warning 1. That is, it reports when a user writes a constraint that is fully equivalent to some other, strictly smaller constraint, like suggesting simplifying (Eq a, Ord a) to (Ord a).
* -Wtype-overly-specific takes on Warning 2, and adds the ability to catch any type signature that's more specific than it needs to be. (Whether or not to add this to -Wall is for others to decide.) This is indeed a more lint-like warning, but HLint would be hard-pressed to figure this one out.
* We really need a way of disabling/enabling warnings per declaration. I propose something like this:
{-# WARNINGS foo -Wno-type-overly-specific #-} foo :: Int -> Int foo x = x
Richard
On Jun 4, 2016, at 9:11 AM, Carter Schonwald
wrote: Agreed. Ive hit exactly these issues myself to the point where i err on suppressing that warning in my code now. In part because i use those unused constraints as a semantic contract.
On Friday, June 3, 2016, Adam Foltzer
wrote: With 8.0.1 freshly arrived, I'm taking on the task of updating a number of our larger projects here at Galois. I made a couple of these comments briefly on a relevant Trac ticket[1], but I wanted to open this discussion to a wider audience.
We tend to use -Wall (and -Werror, in CI environments), and so I've had to make decisions about how to handle the new -Wredundant-constraints warnings. So far, I've come to think of it as two different warnings that happen to be combined:
Warning 1: a warning for constraints made redundant by superclass relationships, and Warning 2: a warning for unused constraints
Overall I'm a fan of Warning 1. It seems very much in the spirit of other warnings such as unused imports. The only stumbling block is how it affects the 3-release compatibility plan with respect to, e.g., the AMP. Most of our code targets a 2-release window, though, so in every such case it has been fine for us to simply remove the offending constraint.
Warning 2 on the other hand is far more dubious to me. In the best case, it finds constraints that through oversight or non-local changes are truly no longer necessary in the codebase. This is nice, but the much more common case in our code is that we've made a deliberate decision to include that constraint as part of our API design.
The most painful example of this I've hit so far is in an API of related functions, where we've put the same constraint on each function even when the implementation of that particular function might not need that constraint. This is good for consistency and forward-looking compatibility (what if we need that constraint in the next version?). The warning's advice in this case makes the API harder to understand, and less abstract (the client shouldn't care or know that f needs Functor, but g doesn't, if both will always be used in a Functor context).
On another level, Warning 2 is a warning that we could have given a more general type to a definition. We quite rightly don't do this for the non-constraint parts of the type signatures, so why are we doing it for the constraints?
I'm happy that Warning 1 is now around, but Warning 2 feels much more like an opinionated lint check, and I really wish it wasn't part of -Wall.
[1]: https://ghc.haskell.org/trac/ghc/ticket/10635#comment:15
_______________________________________________ Glasgow-haskell-users mailing list Glasgow-haskell-users@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/glasgow-haskell-users

On Mon, Jun 6, 2016, at 07:37, Carter Schonwald wrote:
I don't suppose there's any hope of having this resolved prior to GHC 8.2 because it is a real usability regression? Because I think we can all agree that the proposed change would not break any 8.0 series code, and positively impact everyone.
Do you mean prior to 8.0.2?

It's a tall ask, but why not?
On Jun 6, 2016 12:16 PM, "Eric Seidel"
On Mon, Jun 6, 2016, at 07:37, Carter Schonwald wrote:
I don't suppose there's any hope of having this resolved prior to GHC 8.2 because it is a real usability regression? Because I think we can all agree that the proposed change would not break any 8.0 series code, and positively impact everyone.
Do you mean prior to 8.0.2? _______________________________________________ Glasgow-haskell-users mailing list Glasgow-haskell-users@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/glasgow-haskell-users

To better elaborate : I definitely want this for the next major version
i.e. 8.2.* , but I'm also wondering if perhaps the "overly constrained
type" warning should be flat out removed from Wall even in ghc 8.0.2,
On Monday, June 6, 2016, Carter Schonwald
It's a tall ask, but why not? On Jun 6, 2016 12:16 PM, "Eric Seidel"
javascript:_e(%7B%7D,'cvml','eric@seidel.io');> wrote: On Mon, Jun 6, 2016, at 07:37, Carter Schonwald wrote:
I don't suppose there's any hope of having this resolved prior to GHC 8.2 because it is a real usability regression? Because I think we can all agree that the proposed change would not break any 8.0 series code, and positively impact everyone.
Do you mean prior to 8.0.2? _______________________________________________ Glasgow-haskell-users mailing list Glasgow-haskell-users@haskell.org javascript:_e(%7B%7D,'cvml','Glasgow-haskell-users@haskell.org'); http://mail.haskell.org/cgi-bin/mailman/listinfo/glasgow-haskell-users

Either way, do we have a strong agreement that for ghc 8.2 that this
warning should be split up and the noiser part should be moved out of the
default wall set?
On Monday, June 6, 2016, Carter Schonwald
To better elaborate : I definitely want this for the next major version i.e. 8.2.* , but I'm also wondering if perhaps the "overly constrained type" warning should be flat out removed from Wall even in ghc 8.0.2,
On Monday, June 6, 2016, Carter Schonwald
javascript:_e(%7B%7D,'cvml','carter.schonwald@gmail.com');> wrote: It's a tall ask, but why not? On Jun 6, 2016 12:16 PM, "Eric Seidel"
wrote: On Mon, Jun 6, 2016, at 07:37, Carter Schonwald wrote:
I don't suppose there's any hope of having this resolved prior to GHC 8.2 because it is a real usability regression? Because I think we can all agree that the proposed change would not break any 8.0 series code, and positively impact everyone.
Do you mean prior to 8.0.2? _______________________________________________ Glasgow-haskell-users mailing list Glasgow-haskell-users@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/glasgow-haskell-users

I don't have any personal experience with the new warning, but it does sound to me like there are two separate warnings (redundant vs unused constraints) combined under a single flag. So I would support separating them! On Sat, Jun 11, 2016, at 09:38, Carter Schonwald wrote:
Either way, do we have a strong agreement that for ghc 8.2 that this warning should be split up and the noiser part should be moved out of the default wall set?
On Monday, June 6, 2016, Carter Schonwald
wrote: To better elaborate : I definitely want this for the next major version i.e. 8.2.* , but I'm also wondering if perhaps the "overly constrained type" warning should be flat out removed from Wall even in ghc 8.0.2,
On Monday, June 6, 2016, Carter Schonwald
javascript:_e(%7B%7D,'cvml','carter.schonwald@gmail.com');> wrote: It's a tall ask, but why not? On Jun 6, 2016 12:16 PM, "Eric Seidel"
wrote: On Mon, Jun 6, 2016, at 07:37, Carter Schonwald wrote:
I don't suppose there's any hope of having this resolved prior to GHC 8.2 because it is a real usability regression? Because I think we can all agree that the proposed change would not break any 8.0 series code, and positively impact everyone.
Do you mean prior to 8.0.2? _______________________________________________ Glasgow-haskell-users mailing list Glasgow-haskell-users@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/glasgow-haskell-users

I strongly agree with per-declaration warning suppression. But I'd like to
leave both warnings on by default in -Wall.
1. Sometimes an upstream library will drop a constraint. The warning lets
me know I can drop it too.
2. Sometimes an implementation evolves from a draft that requires a
constraint to a final form that does not; it's easy to forget to check
whether any constraints have become redundant.
It's true that this is somewhat analogous to a function that is less
polymorphic than it can be. But my answer to that is the opposite: I'd be
very happy to be warned when I write a top-level function with a specific
type when it could be parametrically polymorphic in that type.
On Jun 6, 2016 9:40 AM, "Richard Eisenberg"
I've been bitten by this too and had to disable the warning.
Let me propose an alternative; * -Wredundant-constraints becomes only your Warning 1. That is, it reports when a user writes a constraint that is fully equivalent to some other, strictly smaller constraint, like suggesting simplifying (Eq a, Ord a) to (Ord a).
* -Wtype-overly-specific takes on Warning 2, and adds the ability to catch any type signature that's more specific than it needs to be. (Whether or not to add this to -Wall is for others to decide.) This is indeed a more lint-like warning, but HLint would be hard-pressed to figure this one out.
* We really need a way of disabling/enabling warnings per declaration. I propose something like this:
{-# WARNINGS foo -Wno-type-overly-specific #-} foo :: Int -> Int foo x = x
Richard
On Jun 4, 2016, at 9:11 AM, Carter Schonwald
wrote: Agreed. Ive hit exactly these issues myself to the point where i err on suppressing that warning in my code now. In part because i use those unused constraints as a semantic contract.
On Friday, June 3, 2016, Adam Foltzer
wrote: With 8.0.1 freshly arrived, I'm taking on the task of updating a number of our larger projects here at Galois. I made a couple of these comments briefly on a relevant Trac ticket[1], but I wanted to open this discussion to a wider audience.
We tend to use -Wall (and -Werror, in CI environments), and so I've had to make decisions about how to handle the new -Wredundant-constraints warnings. So far, I've come to think of it as two different warnings that happen to be combined:
Warning 1: a warning for constraints made redundant by superclass relationships, and Warning 2: a warning for unused constraints
Overall I'm a fan of Warning 1. It seems very much in the spirit of other warnings such as unused imports. The only stumbling block is how it affects the 3-release compatibility plan with respect to, e.g., the AMP. Most of our code targets a 2-release window, though, so in every such case it has been fine for us to simply remove the offending constraint.
Warning 2 on the other hand is far more dubious to me. In the best case, it finds constraints that through oversight or non-local changes are truly no longer necessary in the codebase. This is nice, but the much more common case in our code is that we've made a deliberate decision to include that constraint as part of our API design.
The most painful example of this I've hit so far is in an API of related functions, where we've put the same constraint on each function even when the implementation of that particular function might not need that constraint. This is good for consistency and forward-looking compatibility (what if we need that constraint in the next version?). The warning's advice in this case makes the API harder to understand, and less abstract (the client shouldn't care or know that f needs Functor, but g doesn't, if both will always be used in a Functor context).
On another level, Warning 2 is a warning that we could have given a more general type to a definition. We quite rightly don't do this for the non-constraint parts of the type signatures, so why are we doing it for the constraints?
I'm happy that Warning 1 is now around, but Warning 2 feels much more like an opinionated lint check, and I really wish it wasn't part of -Wall.
[1]: https://ghc.haskell.org/trac/ghc/ticket/10635#comment:15
_______________________________________________ Glasgow-haskell-users mailing list Glasgow-haskell-users@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/glasgow-haskell-users
_______________________________________________ Glasgow-haskell-users mailing list Glasgow-haskell-users@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/glasgow-haskell-users
participants (5)
-
Adam Foltzer
-
Carter Schonwald
-
David Feuer
-
Eric Seidel
-
Richard Eisenberg