add -Wderiving-defaults for enabling/suppressing strategy defaulting warnings
AbandonedPublic

Authored by chessai on Dec 14 2018, 11:37 AM.

Details

Reviewers
RyanGlScott
bgamari
Trac Issues
#15839
Summary

add a flag for enabling/suppressing the warning emitted by having both DeriveAnyClass and GeneralizedNewtypeDeriving on.

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Branch
arcpatch-D5455
Lint
Lint WarningsExcuse: It looks ugly if done another way. The crazy long indent also contributes to line length.
SeverityLocationCodeMessage
Warningcompiler/typecheck/TcDeriv.hs:1637TXT3Line Too Long
Unit
No Unit Test Coverage
Build Status
Buildable 25488
Build 64739: [GHC] Linux/amd64: Continuous Integration
Build 64738: [GHC] OSX/amd64: Continuous Integration
Build 64737: [GHC] Windows/amd64: Continuous Integration
Build 64736: arc lint + arc unit
chessai created this revision.Dec 14 2018, 11:37 AM

Make sure to add an entry for this flag in docs/users_guide/using-warnings.rst (see Note [Documenting warning flags]).

compiler/main/DynFlags.hs
4761

Since -Wderiving-defaults is already in standardWarnings, putting it in here is redundant (note that we prepend standardWarnings to the front of this list), so this should be removed.

docs/users_guide/8.8.1-notes.rst
85

Currently, this users' guide entry is under the "Compiler" section, but I feel that it might be a better fit in the "Language" section above.

100

s/New -Wderiving-defaults that/New -Wderiving-defaults flag that/

s/DerivingNewtype/GeneralizedNewtypeDeriving/

101

This is slightly misleading, since enabling GND and DeriveAnyClass simultaneously isn't enough to trigger this warning—you also need to derive an instance of a type class for a newtype (specifically, one for a non-stock class, although we probably don't need to go into that level of detail in the release notes). It might help to give a specific example of some code that would trigger this warning.

102

s/but not a flag/but not behind a flag/

testsuite/tests/typecheck/should_compile/T15839a.hs
8

Surely this test case is going to emit a warning? Make sure to run make accept TEST=T15839a (somewhere in the testsuite directory) to accept the stderr that this produces.

RyanGlScott requested changes to this revision.Dec 15 2018, 8:29 AM
This revision now requires changes to proceed.Dec 15 2018, 8:29 AM
chessai updated this revision to Diff 19172.Dec 17 2018, 9:31 AM
  • make changes suggested by Ryan Scott

Don't forget to add an entry for this flag in docs/users_guide/using-warnings.rst!

compiler/main/DynFlags.hs
4760

Does this compile? The trailing comma here looks suspect.

docs/users_guide/8.8.1-notes.rst
74

s/a newtype.,/a newtype/