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

Fri Sep 22 11:16:01 UTC 2023

Moritz, can I pass on some excellent advice I got from Richard on first joining the list, namely don't try to 'read the room' here but call things as you see them. Thanks to some excellent management from the usual suspects we are quite good at searching out a consensus when needed.

I am strongly in agreement with Simon Marlow here -- I really don't think we should be introducing breaking changes unless it is really, really necessary, and I just don't think "this is what we think developers should be doing" and "nobody reads manuals any more" clears the threshold for me. And, while I take Richard's point about the value of deprecation periods, I still think we should only introduce breaking changes in exceptional circumstances.

Again, I fully support the introduction of the Severe category with the proposed warnings included. 

The way I would approach this is to predicate the change of behaviour on a language extension with a view to getting it included in (say) GHC2024. People that really care about stability can tie things down by specifying the language they are working with at the appropriate points. Folks are now used to the GHC selecting a new default once a new Haskell* or GHC* standard is agreed.

Chris

> On 21 Sep 2023, at 14:13, Moritz Angermann <moritz.angermann at gmail.com> wrote:
> 
> Thank you Simon! 
> 
> I think this raises a good point, in that _any_ breaking change should stand well on its own wrt to a cost/benefit analysis.
> 
> I’m not sure if we disagree or not.  I’ve assumed that the majority wants to make these changes.  I also often do not feel sufficiently capable enough to make a convincing argument against a specific change. 
> 
> Thus under the assumption that we are going to make a breaking change (because the majority want this), I feel very strongly about informing end users beforehand as well as the quality control aspect of being able to run regression tests by just swapping out the compiler trivially.
> 
> Thus *if* we are making a breaking change, I do not believe that a migration phase reduces the costs much (you still need to adjust your code), but I consider a breaking change without migration phase to be significantly worse to a breaking change with migration phase.
> 
> The difference for me practically lies in the following:
> If we migrate to a new compiler, a team of engineers start doing the initial migration for us (this means adding the new compiler to CI, and making sure existing code at least builds), this happens for multiple projects/components in dependency order.
> 
> After this has happened the project/component teams use the new compiler and fix their code.
> 
> If we have migration warnings for upcoming breaking changes, the work shifts from the new-compiler-across-multiple-projects team into the project teams.
> 
> This makes the whole add new compiler process significantly faster, and means less friction. It also means the project teams who have much better knowledge of their specific project can address the warnings better than the migration team which has to dig deep into existing code bases, get familiar with the code to make the necessity adjustments (or request pair sessions that take time from other tasks).
> 
> It also gives one more opportunity for the community to raise their hand and say, oh hell no, that’s a stupid idea; I’m going to complain about this! (I’ve been told this more than once; migration phases could also be seen as community inquiery, to get feedback from actual practitioners).
> 
> After writing this all down, I guess my push for “if breaking change, then only with migration phase” is orthogonal to “should we have breaking changes to begin with?”
> 
> Cheers,
>   Moritz
> 
> On Thu, 21 Sep 2023 at 8:35 PM, Simon Marlow <marlowsd at gmail.com <mailto:marlowsd at gmail.com>> wrote:
>> I might be an outlier here, but deferring the change to a later release and adding an intermediate warning doesn't make it any less bad.
>> 
>> This is one cultural aspect of our community I'd like to shift: the expectation that it's OK to make breaking changes as long as you warn about them or go through a migration cycle. It just isn't! (and I speak as someone who used to make lots of changes, I'm now fully repentant!). That's not to say that we shouldn't ever change anything, but when considering the cost/benefit tradeoff adding a migration cycle doesn't reduce the cost, it just defers it.
>> 
>> Cheers
>> Simon
>> 
>> 
>> 
>> On Wed, 20 Sept 2023 at 08:25, Adam Gundry <adam at well-typed.com <mailto:adam at well-typed.com>> wrote:
>>> 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
>>> 
>>> _______________________________________________
>>> ghc-steering-committee mailing list
>>> ghc-steering-committee at haskell.org <mailto:ghc-steering-committee at haskell.org>
>>> https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee
>> _______________________________________________
>> ghc-steering-committee mailing list
>> ghc-steering-committee at haskell.org <mailto:ghc-steering-committee at haskell.org>
>> https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee
> _______________________________________________
> ghc-steering-committee mailing list
> ghc-steering-committee at haskell.org
> https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.haskell.org/pipermail/ghc-steering-committee/attachments/20230922/4a1635be/attachment-0001.html>