Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
LeventErkok opened this issue Jan 26, 2019 · 12 comments
Closed

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

LeventErkok opened this issue Jan 26, 2019 · 12 comments

Comments

@LeventErkok
Copy link

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
Copy link
Owner

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
Copy link
Contributor

yairchu commented Jan 28, 2019

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
Copy link
Owner

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

@ndmitchell
Copy link
Owner

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
Copy link
Owner

Fixed in hlint-2.1.14.

@LeventErkok
Copy link
Author

Thanks for the quick fix!

@moll
Copy link

moll commented Feb 5, 2020

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
Copy link
Owner

ndmitchell commented Feb 7, 2020

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
Copy link
Collaborator

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

  • Get it patched upstream so we can use it from ghc-lib-parser;
  • We can copy the implementation to ghc-lib-parser-ex and use from there it in hlint today;
  • In the fullness of time, when it's generally available in ghc-lib-parser, the ghc-lib-parser-ex reimplementation can be removed and redirect.

@ndmitchell
Copy link
Owner

@shayne-fletcher that sounds a great plan

@shayne-fletcher
Copy link
Collaborator

@shayne-fletcher
Copy link
Collaborator

Update : MR approved; milestone set for 8.12.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants