#15278 closed task (fixed)

Add -Werror=compat, enable it in the testsuite

Reported by: int-index Owned by:
Priority: normal Milestone: 8.8.1
Component: Compiler Version:
Keywords: Cc: bgamari
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description (last modified by int-index)

Proposed change

Add a flag -Werror=compat to GHC which would have the effect of -Werror=x -Werror=y ... where x, y, ... are warnings from minusWcompatOpts. Enable -Werror=compat in the testsuite.

Motivation

  1. -Werror=compat would be a convenient shorthand to ensure forwards-compatibility of code. I can imagine Haskell libs enabling it on their CI.
  1. Enabling -Werror=compat in the testsuite would allow us to easily see the impact that a new warning has on code. It also means that in the period between adding the warning and making the actual breaking change, all new test cases that are being added to the testsuite will be forwards-compatible. This is good because it will make the actual breaking change contain less irrelevant testsuite updates.

The idea came up during my work on Phab:D4834 which defines a new warning and adds it to -Wcompat. Ryan Scott raised the following concern:

There are likely many tests in the testsuite that will now fail due to this change—I'm unclear if you're intending to fix them by explicitly quantifying their kind variables, or leaving the new error message as-is.

My intention was to update the test cases, but it turned out that I couldn't even locate these test cases without make EXTRA_HC_OPTS="-Werror=implicit-kind-vars". And even if I updated the testsuite, nothing would prevent new test cases that are not forwards-compatible, because the warning isn't enabled by default (and without -Werror=compat, enabling -Wcompat would not cause test failures in all circumstances).

Implementation plan

  1. Add new flags: -Werror=compat, -Wno-error=compat, -Wwarn=compat. Document these flags in the User's Guide.
  2. Enable -Werror=compat in the testsuite by default and fix all tests that break.

Change History (4)

comment:1 Changed 16 months ago by int-index

Description: modified (diff)

comment:2 Changed 16 months ago by Ben Gamari <ben@…>

In 04e9fe5/ghc:

Add -Werror=compat

Add a flag `-Werror=compat` to GHC which has the effect of `-Werror=x
-Werror=y ...`, where `x, y, ...` are warnings from the `-Wcompat`
option group.

Test Plan: ./validate

Reviewers: bgamari

Reviewed By: bgamari

Subscribers: rwbarton, thomie, carter

GHC Trac Issues: #15278

Differential Revision: https://phabricator.haskell.org/D4860

comment:3 Changed 12 months ago by Ben Gamari <ben@…>

In 165d3d5/ghc:

Enable -Wcompat=error in the testsuite

Enabling -Werror=compat in the testsuite allows us to easily see the
impact that a new warning has on code. It also means that in the period
between adding the warning and making the actual breaking change, all
new test cases that are being added to the testsuite will be
forwards-compatible. This is good because it will make the actual
breaking change contain less irrelevant testsuite updates.

Things that -Wcompat warns about are things that are going to break in
the future, so we can be proactive and keep our testsuite
forwards-compatible.

This patch consists of two main changes:

* Add `TEST_HC_OPTS += -Werror=compat` to the testsuite configuration.
* Fix all broken test cases.

Test Plan: Validate

Reviewers: hvr, goldfire, bgamari, simonpj, RyanGlScott

Reviewed By: goldfire, RyanGlScott

Subscribers: rwbarton, carter

GHC Trac Issues: #15278

Differential Revision: https://phabricator.haskell.org/D5200

comment:4 Changed 12 months ago by bgamari

Resolution: fixed
Status: newclosed

I believe that should finish this.

Note: See TracTickets for help on using tickets.