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

Thu Sep 21 13:13:32 UTC 2023

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> 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> 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
>> 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/20230921/27e75fb9/attachment.html>