Opened 6 years ago

Last modified 4 weeks ago

#3085 new feature request

warn about language extensions that are not used

Reported by: PVerswyvelen Owned by:
Priority: normal Milestone: 7.12.1
Component: Compiler Version: 6.10.1
Keywords: warnings extensions language Cc: ndmitchell@…, deduktionstheorem@…, conal@…, id@…, haskell@…, chevalier@…, vogt.adam@…, merehap@…, mmitar@…, shahn, hackage.haskell.org@…, pho@…, asr
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: #9757 Differential Revisions:

Description

When I put

{-# OPTIONS_GHC -Wall -Werror #-}

in my source file, I don't get compiler warnings about redundant language extensions that I enabled in that file.

For example if I write

{-# LANGUAGE GeneralizedNewtypeDeriving #-}

in the file, but I never do newtype deriving, the compiler could give me a warning.

It would be nice if the compiler gave warnings about this, since after refactoring, some language extensions might not be needed anymore, and hence should be removed since fewer language extensions mean more stable and portable code no?

Change History (39)

comment:1 Changed 6 years ago by MartijnVanSteenbergen

No idea how to vote here, so I'll just leave a comment:

+1

:-)

comment:2 Changed 6 years ago by NeilMitchell

  • Cc ndmitchell@… added

comment:3 Changed 6 years ago by igel

  • Cc deduktionstheorem@… added

dito, +1 :)

comment:4 Changed 6 years ago by simonmar

  • difficulty set to Unknown

While I agree this would be very cool, it's very difficult to implement and get right. In fact the only reasonable implementation I can think of is to remove extensions one at a time and check whether the compilation output was the same, but that ignores relationships between extensions (e.g. it might be ok to remove both A and B, but not either A or B. I'm not sure if we have any of those.)

comment:5 follow-ups: Changed 6 years ago by simonpj

For the most part, the fact that GHC consults a flag is enough to indicate that changing its state would affect compilation. (That's not quite true -- in some places we read flags and pass on a couple of booleans that may or may not be needed -- but it is largely true.)

So while it's not entirely trivial, I suspect we could get close to what you want, at least for the language extensions. If someone wants to volunteer to do this, I can advise. Unless I'm missing something.

Simon

comment:6 Changed 6 years ago by MartijnVanSteenbergen

I can imagine it depends entirely on the specific flag. GeneralizedNewtypeDeriving seems easy enough. RelaxedPolyRec on the other hand...

I think warnings for the easier ones is already awesome.

comment:7 Changed 6 years ago by conal

  • Cc conal@… added

I'd love to have this warning also. I go back every so often and try removing each of the extensions listed in my LANGUAGE pragma. It hadn't occurred to me that the compiler could be doing this job.

comment:8 in reply to: ↑ 5 Changed 6 years ago by dons

Seems better suited to a separate 'lint' tool to sanitize .cabal files / pragma use and other common issues. No need to bake it into the compiler, given the implementation problems.

comment:9 in reply to: ↑ 5 ; follow-up: Changed 6 years ago by simonmar

Replying to simonpj:

For the most part, the fact that GHC consults a flag is enough to indicate that changing its state would affect compilation. (That's not quite true -- in some places we read flags and pass on a couple of booleans that may or may not be needed -- but it is largely true.)

So while it's not entirely trivial, I suspect we could get close to what you want, at least for the language extensions. If someone wants to volunteer to do this, I can advise. Unless I'm missing something.

Perhaps I'm being pessimistic, but I have a bad feeling about this if we have to implement it for every language extension, everywhere that language extension is used. It'll give rise to a slew of bug reports for the places we don't quite get it right. This is a feature that requires changes all over the compiler, and introduces a new tax on language extensions.

In contrast, the "try removing the extension and see if it still works" method is foolproof and doesn't require any changes to the compiler. Someone could implement that using the GHC API...

comment:10 follow-up: Changed 6 years ago by simonpj

The most common way to consult the flags is thus:

dopt :: DynFlag -> Tc Bool

What I was suggesting is that dopt could record on the side that the flag was consulted. The tax is in dopt only.

Not all flags are consulted in this way, but most are.

comment:11 in reply to: ↑ 9 Changed 6 years ago by guest

  • Cc id@… added

Replying to simonmar:

In contrast, the "try removing the extension and see if it still works" method is foolproof and doesn't require any changes to the compiler. Someone could implement that using the GHC API...

I'm not so sure that it's foolproof. For example Template-Haskell can change the meaning of programs

foo = bar $(baz)  -- ($) or TH?

but they still may compile; and there are probably more subtle extensions with meaning-changes.

Perhaps you can avoid that issue by comparing the output to make sure it's exactly the same. But I've heard that GHC is not designed to be deterministic (that it sometimes produces different ABIs from the same source-code and options), so I guess that wouldn't even work.

However we know, as a conservative estimate, that if an extension flag is not even consulted, then the extension must not have been used; so GHC could warn sometimes. (Although the merits of having an "unreliable" warning like this may be in question.)

comment:12 Changed 6 years ago by augustss

What I usually do by hand is to remove the extensions one by one and see if it still compiles.
As Simon M says, this is a way to implement it. And I think it's a perfectly fine way. It would save me doing it by hand.

comment:13 in reply to: ↑ 10 Changed 6 years ago by simonmar

Replying to simonpj:

The most common way to consult the flags is thus:

dopt :: DynFlag -> Tc Bool

What I was suggesting is that dopt could record on the side that the flag was consulted. The tax is in dopt only.

Not all flags are consulted in this way, but most are.

Yes, for some flags it's easy. But there are some that aren't at all easy - just looking down the list I see NoMonomorphismRestriction, NoImplicitPrelude, NoMonoPatBinds, ExtendedDefaultRules, RelaxedPolyRec, CPP, OverloadedStringLiterals, and the ones that affect the lexer are not so straightforward.

So the question is whether it's worthwhile to just do the easy ones. We'd have to clearly document the flags to which the warning doesn't apply.

comment:14 Changed 6 years ago by guest

  • Cc haskell@… added

comment:15 Changed 6 years ago by tim

  • Cc chevalier@… added

comment:16 Changed 6 years ago by igloo

  • Milestone set to 6.12.1

comment:17 Changed 5 years ago by igloo

  • Milestone changed from 6.12.1 to 6.14.1
  • Type of failure set to None/Unknown

comment:18 follow-up: Changed 5 years ago by NeilMitchell

HLint 1.6.6 above above can warn on unused language extensions: http://community.haskell.org/~ndm/hlint

The hints are limited to those extensions which enable new syntax, but have spotted unused extensions for me.

comment:19 Changed 5 years ago by aavogt

  • Cc vogt.adam@… added

comment:20 Changed 5 years ago by merehap

  • Cc merehap@… added

comment:21 Changed 5 years ago by mitar

  • Cc mmitar@… added

comment:22 Changed 5 years ago by shahn

  • Cc shahn added

comment:23 Changed 4 years ago by liyang

  • Cc hackage.haskell.org@… added

comment:24 Changed 4 years ago by igloo

  • Milestone changed from 7.0.1 to 7.0.2

comment:25 Changed 4 years ago by igloo

  • Milestone changed from 7.0.2 to 7.2.1

comment:26 Changed 4 years ago by igloo

  • Milestone changed from 7.2.1 to 7.4.1

comment:27 Changed 3 years ago by PHO

  • Cc pho@… added

comment:28 Changed 3 years ago by igloo

  • Milestone changed from 7.4.1 to 7.6.1

comment:29 Changed 3 years ago by igloo

  • Milestone changed from 7.6.1 to 7.6.2

comment:30 Changed 11 months ago by asr

  • Cc asr@… added

comment:31 in reply to: ↑ 18 Changed 11 months ago by asr

Replying to NeilMitchell:

HLint 1.6.6 above above can warn on unused language extensions: http://community.haskell.org/~ndm/hlint

I used HLint 1.8.61 for removing unused language extensions from Agda. HLint made a great job.

comment:32 Changed 10 months ago by thoughtpolice

  • Milestone changed from 7.6.2 to 7.10.1

Moving to 7.10.1.

comment:33 follow-ups: Changed 5 months ago by thomie

To cite rwbarton from #9757:

I think the purpose of compiler warnings should be to warn about code that is probably bad, not code that is probably fine that could be written in a (potentially) better way. (After all, we don't warn about excess parentheses in code either.) That is the domain of hlint, as Richard notes.

Anyone disagree with a wontfix here as well?

comment:34 in reply to: ↑ 33 Changed 5 months ago by Lemming

Replying to thomie:

To cite rwbarton from #9757:

I think the purpose of compiler warnings should be to warn about code that is probably bad, not code that is probably fine that could be written in a (potentially) better way. (After all, we don't warn about excess parentheses in code either.) That is the domain of hlint, as Richard notes.

Anyone disagree with a wontfix here as well?

GHC already warns about unused variables and unused imports. Is the warning about unused instances much different? If you think, that the warning is not essential it may not be enabled by -Wall. And there are already non-Wall warnings that pointed me to bad code, like the warning about incomplete case analysis of pattern matches in lambda expressions. (I think it's -fwarn-incomplete-uni-patterns.)

comment:35 Changed 5 months ago by goldfire

Though it seems impossible to draw an obvious boundary between this idea and the one posed in #9757, I tend to think the warning proposed here does belong within the domain of GHC. I don't have an obvious way to implement this in mind, though, and I'm struggling to explain why I feel this is something GHC should do.

comment:36 Changed 5 months ago by simonpj

I think it might be difficult, or at least tiresome, to implement this.

In lots of places we test the language extension flags, to say

 if <extn-needed> and not extn-flag then raise error

To implement this feature, every such test would need to look more like

 if <extn-needed> then
    if <extn-flag-on> then  record extn-flag-needed
                      else  raise error

Presumably the "record extn-flag-needed" is some kind of mutable state carried in the monad, accumulating which extensions are used.

I'm sure it's possible, but the are many such tests, so you'd have to touch a lot of code.

Simonb

comment:37 in reply to: ↑ 33 Changed 5 months ago by simonmar

Replying to thomie:

To cite rwbarton from #9757:

I think the purpose of compiler warnings should be to warn about code that is probably bad, not code that is probably fine that could be written in a (potentially) better way. (After all, we don't warn about excess parentheses in code either.) That is the domain of hlint, as Richard notes.

Anyone disagree with a wontfix here as well?

This is an argument that the warning shouldn't be on by default, or perhaps that it shouldn't be in -Wall, but not an argument that we shouldn't implement it at all. We have several warnings that are not part of -Wall and fall into the category of "a matter of taste".

(FWIW, HLint already does this for a subset of extensions).

comment:38 Changed 4 months ago by thoughtpolice

  • Milestone changed from 7.10.1 to 7.12.1

Moving to 7.12.1 milestone; if you feel this is an error and should be addressed sooner, please move it back to the 7.10.1 milestone.

comment:39 Changed 4 weeks ago by asr

  • Cc asr added; asr@… removed
Note: See TracTickets for help on using tickets.