ndmitchell / hlint

Haskell source code suggestions
Other
1.47k stars 196 forks source link

I cannot get GHC and HLint to agree.. #587

Closed LeventErkok closed 5 years ago

LeventErkok commented 5 years ago

Versions:

$ ghc --version
The Glorious Glasgow Haskell Compilation System, version 8.6.3
$ hlint --version
HLint v2.1.13, (C) Neil Mitchell 2006-2019

For this module:

{-# LANGUAGE ImplicitParams       #-}
{-# LANGUAGE TypeSynonymInstances #-}
{-# LANGUAGE FlexibleInstances    #-}

module Test where

data X a = X
type XI = X Int

class Foo a where
  foo :: (?p :: Int) => a -> Int

instance Foo XI where foo = undefined

I don't seem to be able to make both GHC and HLint happy at the same time:

$ hlint Test.hs
Test.hs:2:1: Warning: Unused LANGUAGE pragma
Found:
  {-# LANGUAGE TypeSynonymInstances #-}
Perhaps you should remove it.
Note: Extension TypeSynonymInstances is implied by FlexibleInstances

Test.hs:3:1: Warning: Unused LANGUAGE pragma
Found:
  {-# LANGUAGE FlexibleInstances #-}
Perhaps you should remove it.
Note: Extension FlexibleInstances is implied by ImplicitParams

2 hints

Of the 3 LANGUAGE extensions in the module, I tried all 8 possible combinations. (Having none to having all of them) and there isn't a single instance where both GHC accept the program and HLint not complain about it.

In fact, it seems that GHC requires all three pragmas to be present to compile it; and HLint isn't happy about that.

Is there some other trick I'm missing to make both happy? Or is this an hlint oversight?

ndmitchell commented 5 years ago

Thanks very much for the wonderful test case, really appreciated! The fact that GHC and HLint disagree is always an HLint bug.

Looking at the code, removing TypeSynonymInstances still compiles, so that seems a legitimate warning. However, FlexibleInstances is required, and I have no idea why GHC would make ImplicitParams imply that. @yairchu - how did you get the original list of extensions that implied others? It seems like ImplicitParams does not imply the extensions that are claimed. I can remove that, but might there be any other bugs in it?

Given the bug, I'll make a release tomorrow.

yairchu commented 5 years ago

I got it from https://downloads.haskell.org/~ghc/master/users-guide/glasgow_exts.html#language-options But translation into code was a manual process so an error may have slipped in

ndmitchell commented 5 years ago

Thanks @yairchu - the GHC docs are buggy so I filed a ticket at https://ghc.haskell.org/trac/ghc/ticket/16248.

ndmitchell commented 5 years ago

So seems a GHC doc bug, that we transcribed into an HLint actual bug. I've stubbed out ImplicitParams implications and I'll make a new release.

ndmitchell commented 5 years ago

Fixed in hlint-2.1.14.

LeventErkok commented 5 years ago

Thanks for the quick fix!

moll commented 4 years ago

Just for the sake of future documentation, I think https://gitlab.haskell.org/ghc/ghc/blob/e8004e5d1e993363a89b0107380604c5bc02be6b/compiler/main/DynFlags.hs#L4668 is a better source for implied flags.

ndmitchell commented 4 years ago

Ah, thanks for that link @moll. I've added docs to the code. Unfortunately the GHC notion of extension doesn't quite match that of Haskell extensions, but it's certainly the ground truth.

Sadly that function isn't exposed in GHC, or else we could use it directly. @shayne-fletcher-da - do you think it's worth patching GHC (upstream) to expose it?

shayne-fletcher commented 4 years ago

@ndmitchell Yes. There's a couple of things we can do:

ndmitchell commented 4 years ago

@shayne-fletcher that sounds a great plan

shayne-fletcher commented 4 years ago
shayne-fletcher commented 4 years ago

Update : MR approved; milestone set for 8.12.1.