[ghc-steering-committee] #571: -Wsevere, Shepherd: Adam (rec: accept)

[Adam Gundry]

I agree with Joachim's position here.

Having a -Wsevere group at all would be a good start, but the problem 
with stopping at "best-practice guidance" rather than changing the 
defaults is that it benefits only those who learn about and follow the 
guidance. So it remains a footgun for newcomers.

Regarding migration, given that GHC has issued these warnings by default 
for some time, I don't think we need be too shy about upgrading their 
severity. We could have -Wcompat imply -Werror=severe, but that 
complicates the semantics of warning groups, and will help only those 
who use -Wcompat but don't fix warnings that occur with -Wdefault.

I suppose as Moritz suggests we could introduce and advertise -Wsevere 
in 9.10, and mention in the warning message that this will be an error 
in the future, but only enable -Werror=severe in 9.14 (or 9.12?), so 
9.10 would give something like:

M.hs:8:10: warning: [-Wmissing-methods]
     • No explicit implementation for
         ‘<>’
     • In the instance declaration for ‘Semigroup T’
     • Warning: this may lead to an infinite loop or runtime exception.
     • This will become an error by default in a future GHC release;
       use -Werror=severe to make severe warnings into errors now.

Then in a future release:

M.hs:8:10: error: [-Wmissing-methods, -Werror=missing-methods]
     • No explicit implementation for
         ‘<>’
     • In the instance declaration for ‘Semigroup T’
     • Warning: this may lead to an infinite loop or runtime exception.
     • Use -Wwarn=missing-methods to permit this anyway.

Would that be a reasonable compromise?

Adam


On 19/09/2023 22:13, Joachim Breitner wrote:
> Hi,
> 
> Am Dienstag, dem 19.09.2023 um 15:26 +0100 schrieb Simon Peyton Jones:
>> Maybe implementing this "severe" category, but not changing its
>> default to error, would get us some of the way there?  Then "best-
>> practice guidance" could be "use -Werror=severe", and job done.
>> That's a bit easier to say than saying "use -Werrror=missing-methods
>> -Werror= ..." etc.
> 
> anyone using `-Werror` would already get this behavior. So what is the
> useful for using `-Werror=severe` instead? Presumably the rationale is:
> 
>   -Werror, while great _during_ development or in leaf packages, is not
>   is not good idea in _released_ code, i.e. code that is compiled by
>   others, as that code needs to keep working even as compilers and
>   dependencies change, such as libraries on hackage, or executables
>   built by distro packagers.
>   That’s why -Werror is frowned upon there.
> 
>   But some changes in upstream packages _ought_ to cause compilation to
>   fail, because they really need downstream code changes. These will
>   cause severe warnings, and therefore -Werror=severe is desirable
>   even for released code.
> 
> Is that a good way of phrasing your thoughts there?
> 
> It looks reasonable to me; if we think of deferable type error as
> severe warnings, it totally fits this description: It would be
> _possible_ to keep compiling the downstream code, but it would not be
> desirable. That's why compilation fails, and that’s why we don’t defer
> type errors by default.
> 
> But if -Werror=severe is desirable generally, it would be unfortunate
> if we cannot make it the default. If not right away, then maybe with a
> suitable migration strategy? (Although I wonder if there are many users
> out there that pay attention to deprecation warnings, e.g. watch
> -Wdeprecation, that would not have already fixed -Wdefault warnings
> about missing fields/methods already…)
> 
> 
> Cheers,
> Joachim

-- 
Adam Gundry, Haskell Consultant
Well-Typed LLP, https://www.well-typed.com/

Registered in England & Wales, OC335890
27 Old Gloucester Street, London WC1N 3AX, England