Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#10752 closed feature request (fixed)

Print which warning-flag controls/enabled an emitted warning

Reported by: hvr Owned by: barrucadu
Priority: normal Milestone: 8.0.1
Component: Compiler Version:
Keywords: Cc: osa1, asr
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Incorrect warning at compile-time Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s): Phab:D1943
Wiki Page: Design/Warnings

Description (last modified by hvr)

Both gcc and clang tell which warning flag a reported warning can be controlled with, e.g.

$ clang-3.5 -Wall -c hello.c
hello.c:3:7: warning: unused variable 'x' [-Wunused-variable]
  int x;
      ^
1 warning generated.
$ gcc -Wall -c hello.c
hello.c: In function ‘main’:
hello.c:3:7: warning: unused variable ‘x’ [-Wunused-variable]
   int x;
       ^
hello.c:4:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^

With GHC however, we need to lookup the documentation (or memorise all warning flags) to find out which -fno-warn-* flag we need to use to silence a specific warning.

I propose we augment GHC's warnings in a similar style to how gcc/clang report flags in compile warnings.


As an obvious extension, (which maybe should be controlled by some -f flag, e.g. -f(no-)show-warning-groups), GHC could then also show by which warning-group a given warning is implied, e.g.

WCompatWarningsOn.hs:16:1: warning: [-Wsemigroup (in -Wcompat)]
    Local definition of ‘<>’ clashes with a future Prelude name.
    This will become an error in a future release.

WCompatWarningsOn.hs:22:3: warning: [-Wnoncanonical-monoid-instances (in -Wcompat)]
    Noncanonical ‘(<>) = mappend’ definition detected
    in the instance declaration for ‘Semi.Semigroup S’.
    Move definition from ‘mappend’ to ‘(<>)’

For the warning-sets which form a superset chain (Weverything > Wextra > Wall > Wdefault) we'd only mention the smallest one (and omit the trivial -Weverything set altogether) to keep the output concise.

Change History (26)

comment:1 Changed 3 years ago by thoughtpolice

Milestone: 7.12.18.0.1

Milestone renamed

comment:2 Changed 3 years ago by osa1

Cc: osa1 added

comment:3 Changed 3 years ago by osa1

This is a great idea and it should be just a straightforward refactoring:

  • Modify ErrUtils.makeIntoWarning to get a Maybe WarningFlag as argument.
  • Modify ErrUtils.ErrMsg to record a WarningFlag.
  • Since we want to show flags uniformly in messages, maybe modify LogAction to pass this Maybe WarningFlag. Then modify DynFlags.defaultLogAction to print these flags.
  • Fix all the compile errors by passing warning flags to makeIntoWarning.

comment:4 Changed 3 years ago by goldfire

+1 from me. I look these up with some frequency.

comment:5 Changed 3 years ago by hvr

Milestone: 8.0.18.2.1
Owner: set to quchen
Version: 7.10.2

comment:6 Changed 3 years ago by hvr

Wiki Page: Design/Warnings

comment:7 Changed 3 years ago by asr

+1

comment:8 Changed 3 years ago by asr

Cc: asr added

comment:9 Changed 3 years ago by thomie

Type of failure: None/UnknownIncorrect warning at compile-time

comment:10 Changed 3 years ago by barrucadu

Owner: changed from quchen to barrucadu

comment:11 Changed 3 years ago by barrucadu

Differential Rev(s): Phab:D1934

comment:12 Changed 3 years ago by hvr

Description: modified (diff)

@barrucadu This looks very promising... but have you talked to David (quchen) before you took the ticket from him? He had already started working on a warning refactoring in his Github repo afaik (see .e.g. https://github.com/quchen/ghc/commits/warn-refactoring)...

PS: I've extended the ticket description with a bonus task

comment:13 Changed 3 years ago by barrucadu

I had not, no. I assumed that since nothing had been updated here in a few months nobody was actively working on this.

comment:14 Changed 3 years ago by barrucadu

Differential Rev(s): Phab:D1934Phab:D1943

comment:15 Changed 3 years ago by Herbert Valerio Riedel <hvr@…>

In bb5afd3/ghc:

Print which warning-flag controls an emitted warning

Both gcc and clang tell which warning flag a reported warning can be
controlled with, this patch makes ghc do the same. More generally, this
allows for annotated compiler output, where an optional annotation is
displayed in brackets after the severity.

This also adds a new flag `-f(no-)show-warning-groups` to control
whether to show which warning-group (such as `-Wall` or `-Wcompat`)
a warning belongs to. This flag is on by default.

This implements #10752

Reviewed By: quchen, bgamari, hvr

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

comment:16 Changed 3 years ago by Herbert Valerio Riedel <hvr@…>

In b9c697ee/ghc:

Print which flag controls emitted desugaring warnings

This is extends bb5afd3c274011c5ea302210b4c290ec1f83209c to cover
warnings emitted during the desugaring phase.

This implements another part of #10752

Reviewed-by: quchen, bgamari

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

comment:17 Changed 3 years ago by Herbert Valerio Riedel <hvr@…>

In 869d9c62/ghc:

Print which flag controls emitted lexer warnings

This is extends bb5afd3c274011c5ea302210b4c290ec1f83209c to cover
warnings emitted during lexing.

This implements another part of #10752

comment:18 Changed 3 years ago by Herbert Valerio Riedel <hvr@…>

In 82f200b/ghc:

Annotate `[-Wredundant-constraints]` in warnings (re #10752)

This was missed in bb5afd3c274011c5ea302210b4c290ec1f83209c

comment:19 Changed 3 years ago by Herbert Valerio Riedel <hvr@…>

In b6c61e37/ghc:

Print which flag controls emitted SafeHaskell warnings

This is extends bb5afd3c274011c5ea302210b4c290ec1f83209c to cover
SafeHaskell warnings.

This implements yet another part of #10752

comment:20 Changed 3 years ago by Herbert Valerio Riedel <hvr@…>

In 3cd4c9ca/ghc:

Annotate `[-Wdeferred-type-errors]` in warnings (re #10752)

This was missed in bb5afd3c274011c5ea302210b4c290ec1f83209c

comment:21 Changed 3 years ago by Herbert Valerio Riedel <hvr@…>

In 46f3775c/ghc:

Default to -fno-show-warning-groups (re #10752)

As `-fno-show-warning-groups` shows associated warning groups regardless
of whether the respective warning group flag as been passed on the CLI,
the warning-group information may be confusing to users.

At this point, `-fshow-warning-groups` is useful mostly to GHC
developers and possibly GHC users who want to see which warning groups
an emitted warning is part of. (Btw, this is particularly interesting in
combination with `-Weverything` which enables *every* warning flag known
to GHC.)

Consequently, starting with this commit, one has to opt-in via
`-fshow-warning-groups` for GHC to show warning groups.

In order to reduce the testsuite delta in this commit, the
`-fshow-warning-groups` flag has been added to TEST_HC_OPTS.

comment:22 Changed 3 years ago by hvr

Milestone: 8.2.18.0.1
Resolution: fixed
Status: newclosed

Let's call it a day and declare this ticket's scope done... at least I didn't notice any other warning-flags not being annotated in warnings (but there could still be!).

There'll likely be follow-up tickets for evolving this feature in the future.


Merged to ghc-8.0 via

comment:23 Changed 3 years ago by Thomas Miedema <thomasmiedema@…>

In f8a5dd05/ghc:

Only add -fshow-warning-groups for ghc >= 7.11 (#10752)

comment:24 Changed 3 years ago by bgamari

Status: closedmerge

This introduced a small bug which is fixed by Phab:D2077. This will need merging.

comment:25 Changed 3 years ago by Ben Gamari <ben@…>

In 1e6ec12/ghc:

Fix misattribution of `-Wunused-local-binds` warnings

This fixes a bug where warnings actually controlled by

- `Opt_WarnUnusedMatches`
- `Opt_WarnUnusedTypePatterns`
- `Opt_WarnUnusedTopBinds`

were incorrectly reported as being controlled by
`Opt_WarnUnusedLocalBinds` as well

This bug was introduced in bb5afd3c274011c5ea302210b4c290ec1f83209c
while implementing #10752

Test Plan: ./validate still running -- testsuite output wiggles expected

Reviewers: barrucadu, quchen, austin, bgamari

Reviewed By: bgamari

Subscribers: thomie

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

comment:26 Changed 3 years ago by bgamari

Status: mergeclosed
Last edited 3 years ago by bgamari (previous) (diff)
Note: See TracTickets for help on using tickets.