Re: Reconsidering -Wall and -Wcompat

2016-02-15 Thread Ben Gamari
"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



signature.asc
Description: PGP signature
___
ghc-devs mailing list
ghc-devs@haskell.org
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs


Re: Reconsidering -Wall and -Wcompat

2016-02-15 Thread Boespflug, Mathieu
>> * 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?
___
ghc-devs mailing list
ghc-devs@haskell.org
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs


Re: Reconsidering -Wall and -Wcompat

2016-02-15 Thread Herbert Valerio Riedel
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.
___
ghc-devs mailing list
ghc-devs@haskell.org
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs


Re: Reconsidering -Wall and -Wcompat

2016-02-15 Thread Herbert Valerio Riedel
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
___
ghc-devs mailing list
ghc-devs@haskell.org
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs


Re: Reconsidering -Wall and -Wcompat

2016-02-15 Thread Herbert Valerio Riedel
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.
___
ghc-devs mailing list
ghc-devs@haskell.org
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs


Re: Reconsidering -Wall and -Wcompat

2016-02-14 Thread Richard Eisenberg

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.
___
ghc-devs mailing list
ghc-devs@haskell.org
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs


Re: Reconsidering -Wall and -Wcompat

2016-02-14 Thread Manuel M T Chakravarty
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/viewform
> 
> 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

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


Re: Reconsidering -Wall and -Wcompat

2016-02-14 Thread Sven Panne
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.
___
ghc-devs mailing list
ghc-devs@haskell.org
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs