Reconsidering -Wall and -Wcompat

newer
[Template Haskell] Desugaring TH...

older
Potentially confusing syntax for...

Ben Gamari

14 Feb 2016 14 Feb '16

4:12 p.m.

tl;dr. GHC has a new set of warnings, -Wcompat, intended to give users advance notice of coming library changes. We want to know whether you think this set should be included in -Wall. See the Wiki [4] and voice your opinion via the linked poll. Hello everyone, GHC 8.0.1 will include a new warning group, -Wcompat, which arose out of the MonadFail proposal discussion [1] late last year. This warning set is intended to provide a means of informing users of coming changes in GHC's core libraries. We would like to solicit the community's feedback on whether this new flag set should be implied by -Wall. This proposal is motivated by concern expressed by some that -Wcompat would see little usage unless it is placed in one of the warning sets typically used during development. One such set is -Wall, which enables a generous fraction of GHC's warning collectionand is is intended [2] for use during development. Unfortunately, despite the (albeit only recently stated) intent of flag, -Wall is widely used outside of development [3], often with the expectation that the result be warning-clean across multiple GHC versions. While we hope that -Wall will see less use in this context in the future, given its current role we wouldn't want to add options to it that would cause undue burden for users. So, we are asking for your opinion: should -Wcompat be implied by -Wall? You can find a more thorough description of the precise proposal on the GHC Wiki [4]. It would be very much appreciated if you could take a few minutes familiarize yourself with the options and provide your thoughts via this quick poll, https://docs.google.com/forms/d/1BmIQvhHcnDB79LgBvaWl_xXpS1q0dMHe3Gq9JeU9odM... Feel free to discuss the issue further in this thread. Thank you for sharing, - Ben [1] https://mail.haskell.org/pipermail/ghc-devs/2015-October/010101.html [2] https://mail.haskell.org/pipermail/ghc-devs/2016-January/010955.html [3] In a rather unscientific study, nearly half of packages on Hackage include it in ghc-options, $ tar -xf ~/.cabal/packages/00-INDEX.tar $ (for pkg in $(ls); do ls $pkg/*/*.cabal | sort -r | head -n1; done) | xargs grep -L '\-Wall' | wc -l 4352 $ ls | wc -l 9347 [4] https://ghc.haskell.org/trac/ghc/wiki/Design/Warnings/Wcompat

Attachments:

signature.asc (application/pgp-signature — 472 bytes)

Show replies by date

Sven Panne

14 Feb 14 Feb

6:51 p.m.

2016-02-14 17:12 GMT+01:00 Ben Gamari :

...

[...] This proposal is motivated by concern expressed by some that -Wcompat would see little usage unless it is placed in one of the warning sets typically used during development. One such set is -Wall, which enables a generous fraction of GHC's warning collectionand is is intended [2] for use during development.

IMHO, the distinction between "during development" and "outside of it" is purely hypothetical. A typical workflow is: Develop your code locally against one GHC/set of libraries, commit to GitHub and let Travis CI do the real work of testing against a matrix of configurations. If things work well and the changes are worth it, tag your current state and release it. Where exactly in this scenario is the code leaving the "during development" state? I definitely want to enable -Wall for the Travis CI builds, because that's the place where they are most valuable. As stated on the Wiki, stuff in -Wcompat will often be non-actionable, so the only option I see if -Wcompat is included in -Wall will be -Wno-compat for all my projects. -Wcompat would be restricted to a few manual local builds to see where things are heading.

...

Unfortunately, despite the (albeit only recently stated) intent of flag, -Wall is widely used outside of development [3], often with the expectation that the result be warning-clean across multiple GHC versions. While we hope that -Wall will see less use in this context in the future, [...]

Seeing -Wall this way is quite unusual, especially for people coming from C/C++ (and the numbers quoted from Hackage seem to be a hint that others think so, too). Normally, -Wall -Wextra -pedantic etc. are life-savers and should be kept enabled all the time, unless you like endless debugging hours, of course. In a nutshell: I would consider -Wall implying -Wcompat an annoyance, but as long as it can be disabled by 2 lines in .travis.yml, I don't really care. ;-) Cheers, S.

Richard Eisenberg

15 Feb 15 Feb

3:47 a.m.

On Feb 14, 2016, at 1:51 PM, Sven Panne wrote:

...

IMHO, the distinction between "during development" and "outside of it" is purely hypothetical.

I find this comment quite interesting, as I see quite a large difference between these.* For example, I use -Werror during development, but not outside it. For me, "during development" is when the author can see the output from the compiler. "Outside of it" is when the author is not looking at the output. When I'm developing, I want to see lots and lots of warnings. When I'm building something I downloaded from Hackage, I generally don't care. So I wonder if there's a way for the tooling to distinguish between development builds and non-dev builds. I know cabal better than stack, so I'll use that as my example. When I say `cabal configure --enable-tests; cabal build`, it means I want more information about how well this package works. Perhaps with --enable-tests, -Wall -Wcompat -Werror should be enabled by default. If I just say `cabal install`, I probably don't care so much about how well the package is working and can leave off the extra diagnostics. The approach of integrating this with tooling like cabal or stack also has the advantage that it is very easy to tailor custom treatment for specific compiler versions. This custom treatment could disable newly-written warnings individually if GHC introduces a new warning that, it is decided, should not be enabled by default on projects that wish to have legacy support. Would this address some of the problems that people have had in this space? Richard * By "interesting" here, I certainly don't mean "stupid". I mean "interesting", because I, like many people, tend to think that others think like I do. This is clearly not the case here, so I have to broaden my viewpoint.

Herbert Valerio Riedel

9:50 a.m.

On 2016-02-15 at 04:47:56 +0100, Richard Eisenberg wrote:

...

On Feb 14, 2016, at 1:51 PM, Sven Panne wrote:

...
IMHO, the distinction between "during development" and "outside of it" is purely hypothetical.

I find this comment quite interesting, as I see quite a large difference between these.* For example, I use -Werror during development, but not outside it. For me, "during development" is when the author can see the output from the compiler. "Outside of it" is when the author is not looking at the output.

When I'm developing, I want to see lots and lots of warnings.

When I'm building something I downloaded from Hackage, I generally don't care.

So I wonder if there's a way for the tooling to distinguish between development builds and non-dev builds.

You may be interested in the upcoming `cabal.project`-file feature which besides allowing to specify which .cabal files your multi-package project is made up of (and tells cabal where your project-root is), allows to set additional `ghc-options`, specify whether tests/benchmarks are to be enabled, or request specific cabal flag settings (basically most things you can set via the CLI as well), or even which GHC compiler executable to use (rather than picking up whatever's in PATH). This can be specified for all packages in the project or on per-package granularity. `cabal.project` files have only an effect when you're triggering cabal invocations from within a source-tree, i.e. when you're in the "during development" mode. So that would be a good place to specify your development-mode GHC warning-flag configuration and other development-centric configurations.

...

I know cabal better than stack, so I'll use that as my example. When I say `cabal configure --enable-tests; cabal build`, it means I want more information about how well this package works. Perhaps with --enable-tests, -Wall -Wcompat -Werror should be enabled by default. If I just say `cabal install`, I probably don't care so much about how well the package is working and can leave off the extra diagnostics.

Herbert Valerio Riedel

10:43 a.m.

On 2016-02-14 at 19:51:19 +0100, Sven Panne wrote: [...]

...

As stated on the Wiki, stuff in -Wcompat will often be non-actionable, so the only option I see if -Wcompat is included in -Wall will be -Wno-compat for all my projects.

This depends on what we mean by "actionable". I'm not sure I'd consider the current -Wcompat warnings to be "non-actionable". For instance, `aeson-0.11` happens to be free of -Wall -Wcompat -Wnoncanonical-monad-instances -Wnoncanonical-monadfail-instances warnings, which didn't require any CPP, see e.g. - https://github.com/bos/aeson/pull/337/files - https://github.com/bos/aeson/pull/338/files Also, as an example for a larger code-base, {Cabal,cabal-install} HEAD in Git aspire to be warning-free as well[1]; so depending on your definition, -Wcompat *is* actionable.

...

-Wcompat would be restricted to a few manual local builds to see where things are heading.

[1]: See .e.g. https://github.com/haskell/cabal/blob/master/Cabal/Cabal.cabal#L191-L193 if impl(ghc >= 8.0) ghc-options: -Wcompat -Wnoncanonical-monad-instances -Wnoncanonical-monadfail-instances and the Travis job enables -Werror

Manuel M T Chakravarty

14 Feb 14 Feb

11:04 p.m.

Just as one data point, the Swift compiler is by default showing warnings about upcoming changes. Just like deprecation warnings, I do find that helpful. Based on that experience, including -Wcompat in -Wall seems like a good plan to me. Manuel

...

Ben Gamari :

tl;dr. GHC has a new set of warnings, -Wcompat, intended to give users advance notice of coming library changes. We want to know whether you think this set should be included in -Wall. See the Wiki [4] and voice your opinion via the linked poll.

Hello everyone,

GHC 8.0.1 will include a new warning group, -Wcompat, which arose out of the MonadFail proposal discussion [1] late last year. This warning set is intended to provide a means of informing users of coming changes in GHC's core libraries.

We would like to solicit the community's feedback on whether this new flag set should be implied by -Wall.

This proposal is motivated by concern expressed by some that -Wcompat would see little usage unless it is placed in one of the warning sets typically used during development. One such set is -Wall, which enables a generous fraction of GHC's warning collectionand is is intended [2] for use during development.

Unfortunately, despite the (albeit only recently stated) intent of flag, -Wall is widely used outside of development [3], often with the expectation that the result be warning-clean across multiple GHC versions. While we hope that -Wall will see less use in this context in the future, given its current role we wouldn't want to add options to it that would cause undue burden for users.

So, we are asking for your opinion: should -Wcompat be implied by -Wall? You can find a more thorough description of the precise proposal on the GHC Wiki [4]. It would be very much appreciated if you could take a few minutes familiarize yourself with the options and provide your thoughts via this quick poll,

https://docs.google.com/forms/d/1BmIQvhHcnDB79LgBvaWl_xXpS1q0dMHe3Gq9JeU9odM...

Feel free to discuss the issue further in this thread.

Thank you for sharing,

- Ben

[1] https://mail.haskell.org/pipermail/ghc-devs/2015-October/010101.html

[2] https://mail.haskell.org/pipermail/ghc-devs/2016-January/010955.html

[3] In a rather unscientific study, nearly half of packages on Hackage include it in ghc-options,

$ tar -xf ~/.cabal/packages/00-INDEX.tar $ (for pkg in $(ls); do ls $pkg/*/*.cabal | sort -r | head -n1; done) | xargs grep -L '\-Wall' | wc -l 4352 $ ls | wc -l 9347

[4] https://ghc.haskell.org/trac/ghc/wiki/Design/Warnings/Wcompat _______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs

Boespflug, Mathieu

15 Feb 15 Feb

10:33 a.m.

Hi Ben, could we enlarge the options a bit? I feel that we're in a false dichotomy currently. I think the issue isn't so much what warnings you see from the compiler with common settings, so much as "what warnings will cause my build to fail?". i.e. the issue isn't what's in -Wall, it's what goes in -Werror... If we don't include -Wcompat in -Wall, then I suspect Ben is quite right that many will ignore / not be aware of -Wcompat hence rendering the 3 release policy rather moot (it's much less useful if developers don't /know/ they're incompatible until the last minute). If we do include it then packages with -Wall -Werror will fail far too early: i.e. way before the compat change actually happens, 3 compiler releases down the line. So may I suggest the following: * include -Wcompat and -Wall, but make it "magic" so that -Wcompat never causes a build to fail even when users set -Wall -Werror. This should address the concerns I've been hearing by folks at large companies who say that at their company a warning is equivalent to an error, because they always compile with -Wall -Werror. Otherwise warnings just accumulate, hence no one pays attention to them, hence warnings become useless. As an aside: I believe -Wall should do what it says on the tin: enable all warnings. When GHC comes up with new warnings, we should pretty much never hesitate to add them to the -Wall set. At Tweag I/O we've had one instance of a serious bug that shipped into production and several days worth of wasted effort, because a constraint maintaining a key invariant was mistakenly unused. -fwarn-redundant-constraints would of caught this bug very early and saved us all the trouble (a test would have worked too, but as it happens a test did exist but had a bug too). Best, -- Mathieu Boespflug Founder at http://tweag.io. On 14 February 2016 at 17:12, Ben Gamari wrote:

...

tl;dr. GHC has a new set of warnings, -Wcompat, intended to give users advance notice of coming library changes. We want to know whether you think this set should be included in -Wall. See the Wiki [4] and voice your opinion via the linked poll.

Hello everyone,

GHC 8.0.1 will include a new warning group, -Wcompat, which arose out of the MonadFail proposal discussion [1] late last year. This warning set is intended to provide a means of informing users of coming changes in GHC's core libraries.

We would like to solicit the community's feedback on whether this new flag set should be implied by -Wall.

This proposal is motivated by concern expressed by some that -Wcompat would see little usage unless it is placed in one of the warning sets typically used during development. One such set is -Wall, which enables a generous fraction of GHC's warning collectionand is is intended [2] for use during development.

Unfortunately, despite the (albeit only recently stated) intent of flag, -Wall is widely used outside of development [3], often with the expectation that the result be warning-clean across multiple GHC versions. While we hope that -Wall will see less use in this context in the future, given its current role we wouldn't want to add options to it that would cause undue burden for users.

So, we are asking for your opinion: should -Wcompat be implied by -Wall? You can find a more thorough description of the precise proposal on the GHC Wiki [4]. It would be very much appreciated if you could take a few minutes familiarize yourself with the options and provide your thoughts via this quick poll,

https://docs.google.com/forms/d/1BmIQvhHcnDB79LgBvaWl_xXpS1q0dMHe3Gq9JeU9odM...

Feel free to discuss the issue further in this thread.

Thank you for sharing,

- Ben

[1] https://mail.haskell.org/pipermail/ghc-devs/2015-October/010101.html

[2] https://mail.haskell.org/pipermail/ghc-devs/2016-January/010955.html

[3] In a rather unscientific study, nearly half of packages on Hackage include it in ghc-options,

$ tar -xf ~/.cabal/packages/00-INDEX.tar $ (for pkg in $(ls); do ls $pkg/*/*.cabal | sort -r | head -n1; done) | xargs grep -L '\-Wall' | wc -l 4352 $ ls | wc -l 9347

[4] https://ghc.haskell.org/trac/ghc/wiki/Design/Warnings/Wcompat

_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs

Herbert Valerio Riedel

10:48 a.m.

On 2016-02-15 at 11:33:09 +0100, Boespflug, Mathieu wrote: [...]

...

* include -Wcompat and -Wall, but make it "magic" so that -Wcompat never causes a build to fail even when users set -Wall -Werror.

Tbh, I don't like the "magic" part at all. In fact, I currently rely on `-Wcompat -Werror` triggering an error in my builds. To address the concern more generally is what https://ghc.haskell.org/trac/ghc/ticket/11219 aims to, but without any magic. Then you'd say -Wall -Werror -Wno-error=compat if -Wcompat implied by -Wall. But we ran out of time for GHC 8.0, so #11219 will have to wait till GHC 8.2.

Boespflug, Mathieu

11:04 a.m.

...

...
* include -Wcompat and -Wall, but make it "magic" so that -Wcompat never causes a build to fail even when users set -Wall -Werror.

Tbh, I don't like the "magic" part at all. In fact, I currently rely on `-Wcompat -Werror` triggering an error in my builds.

Point is - if -Wall implies -Wcompat that's simply not an option for many companies, who have a huge codebase, turn on -Wall -Werror as a matter of policy yet need the time granted by the 3 release policy to adapt their codebase.

...

To address the concern more generally is what

https://ghc.haskell.org/trac/ghc/ticket/11219

aims to, but without any magic. Then you'd say

-Wall -Werror -Wno-error=compat

if -Wcompat implied by -Wall.

That sounds good. -Wno-error=compat should work fine.

...

But we ran out of time for GHC 8.0, so #11219 will have to wait till GHC 8.2.

Then is it reasonable to prevent -Wall -Werror from failing builds because of -Wcompat through some kind of special casing in GHC 8.0, to be generalized via -Wno-error in GHC 8.2?

Ben Gamari

2:09 p.m.

"Boespflug, Mathieu" writes:

...

Hi Ben,

could we enlarge the options a bit? I feel that we're in a false dichotomy currently. I think the issue isn't so much what warnings you see from the compiler with common settings, so much as "what warnings will cause my build to fail?". i.e. the issue isn't what's in -Wall, it's what goes in -Werror...

I can certainly see this point. As pointed out by Herbert, this is best provided by #11219 which unfortunately didn't make the 8.0 release.

...

If we don't include -Wcompat in -Wall, then I suspect Ben is quite right that many will ignore / not be aware of -Wcompat hence rendering the 3 release policy rather moot (it's much less useful if developers don't /know/ they're incompatible until the last minute). If we do include it then packages with -Wall -Werror will fail far too early: i.e. way before the compat change actually happens, 3 compiler releases down the line. So may I suggest the following:

* include -Wcompat and -Wall, but make it "magic" so that -Wcompat never causes a build to fail even when users set -Wall -Werror.

This should address the concerns I've been hearing by folks at large companies who say that at their company a warning is equivalent to an error, because they always compile with -Wall -Werror. Otherwise warnings just accumulate, hence no one pays attention to them, hence warnings become useless.

As an aside: I believe -Wall should do what it says on the tin: enable all warnings.

I am sympathetic to this argument. For better or for worse, however, there seems to be precedent for excluding some warnings in -Wall. Clang, for instance, uses -Weverything for "give me everything". Up until this point we have followed this model (although don't have a -Weverything); for instance, we disable -Wimplicit-prelude in -Wall since it is inappropriate for most users. I'm not sure whether we should break with this tradition as users are quite used to invoking their compiler with `-Wall` during development. Throwing warnings about implicit Prelude imports at them would be surprising to say the least.

...

When GHC comes up with new warnings, we should pretty much never hesitate to add them to the -Wall set.

I agree. This is why Simon wrote an explicit declaration that -Wall is not stable to the list a few weeks ago. We really don't want to worry about breaking builds of (potentially broken) user code when adding warnings. Cheers, - Ben

3438

Age (days ago)

3439

Last active (days ago)

List overview

Download

9 comments

6 participants

participants (6)

Ben Gamari
Boespflug, Mathieu
Herbert Valerio Riedel
Manuel M T Chakravarty
Richard Eisenberg
Sven Panne