#10890: Incorrect redundant import warning for type classes -------------------------------------+------------------------------------- Reporter: quchen | Owner: Type: bug | Status: new Priority: normal | Milestone: 8.0.1 Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Revisions: -------------------------------------+------------------------------------- Description changed by quchen: Old description:

Given the three files below, the import marked with (!!!) is reported as redundant. Upon deleting it, GHC reports that "has" is not a visible class method of BClass.

This came up in the context of the MFP (#10751), with the correspondences Base<->Control.Monad, Extends<->Control.Monad.Fail. We would like to have Control.Monad re-export Control.Monad.Fail.MonadFail without its (clashing) member "fail" for the time being. For future compatibility, some modules should implement MonadFail right now, requiring us to import Control.Monad.Fail explicitly in order to write an instance. Said procedure leads to the described import warning, which breaks the validation script for example.

{{{#!hs -- Base.hs module Base (AClass(..), BClass()) where

import Extends (BClass())

class AClass a where has :: a }}}

{{{#!hs -- Extends.hs module Extends where

class BClass b where has :: b }}}

{{{#!hs -- UseSite.hs module UseSite where

import Base import Extends -- (!!!)

data Bar = Bar

instance AClass Bar where has = Bar

instance BClass Bar where has = Bar }}}

New description: Given the three files below, the import marked with (!!!) is reported as redundant. Upon deleting it, GHC reports that "has" is not a visible class method of BClass. This came up in the context of the MFP (#10751), with the correspondences Base<->Control.Monad, Extends<->Control.Monad.Fail. We would like to have Control.Monad re-export Control.Monad.Fail.MonadFail without its (clashing) member "fail" for the time being. For future compatibility, some modules should implement MonadFail right now, requiring us to import Control.Monad.Fail explicitly in order to write an instance. Said procedure leads to the described import warning, which breaks the validation script for example. {{{#!hs -- Base.hs module Base (AClass(..), BClass()) where import Extends (BClass()) class AClass a where has :: a }}} {{{#!hs -- Extends.hs module Extends where class BClass b where has :: b }}} {{{#!hs -- UseSite.hs module UseSite where import Base import Extends -- (!!!) data Bar = Bar instance AClass Bar where has = Bar instance BClass Bar where has = Bar }}} We either get {{{ UseSite.hs:12:5: error: ‘has’ is not a (visible) method of class ‘BClass’ }}} or {{{ UseSite.hs:4:1: warning: The import of ‘Extends’ is redundant except perhaps to import instances from ‘Extends’ To import instances alone, use: import Extends() }}} -- -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/10890#comment:2> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler

#10890: Incorrect redundant import warning for type classes -------------------------------------+------------------------------------- Reporter: quchen | Owner: Type: bug | Status: new Priority: normal | Milestone: 8.0.1 Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Revisions: -------------------------------------+------------------------------------- Comment (by osa1): Not sure how much this helps with debugging this, but I found out that when compiling this slightly modified program: {{{#!haskell module Main where import Base (AClass (has)) import Extends (BClass (has)) data Bar = Bar instance AClass Bar where has = Bar instance BClass Bar where has = Bar }}} GHC prints this: {{{ Main.hs:3:1: warning: The import of ‘Base.has’ from module ‘Base’ is redundant Main.hs:4:1: warning: The import of ‘Extends.has’ from module ‘Extends’ is redundant Linking Main ... }}} This is a test case I generated while trying to figure out if GHC thinks both `has` are coming from the same module. -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/10890#comment:4> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler

#10890: Incorrect redundant import warning for type classes -------------------------------------+------------------------------------- Reporter: quchen | Owner: Type: bug | Status: new Priority: normal | Milestone: 8.0.1 Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Revisions: -------------------------------------+------------------------------------- Comment (by osa1): Here's more stuff: GHC actually doesn't even consider `has` as an used symbol(i.e. internally it doesn't add it to the set of symbols used in a module, `tcg_used_rdrnames` of `TcGblEnv`), so in this case it only considers importing `AClass` and `BClass`. (that why in my second example it prints warnings about both `has`) `BClass` is exported by both `Base` and `Extends`. Now let's say we have this: {{{#!haskell import Base import Extends }}} Which one should be used for importing `BClass`? GHC makes the choice based on their locations in the source file. That is, it imports it from `Base` and not from `Extends`. But if I change imports like this: {{{#!haskell import Extends import Base }}} Now GHC imports `BClass` from `Extends` because it comes first in the source code, so no warnings here. Long story short, I think the problem is that GHC doesn't consider `has` a used symbol. -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/10890#comment:6> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler

#10890: Incorrect redundant import warning for type classes -------------------------------------+------------------------------------- Reporter: quchen | Owner: Type: bug | Status: new Priority: normal | Milestone: 8.0.1 Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Revisions: -------------------------------------+------------------------------------- Changes (by quchen): * cc: quchen (added) -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/10890#comment:8> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler

#10890: Incorrect redundant import warning for type classes -------------------------------------+------------------------------------- Reporter: quchen | Owner: Type: bug | Status: patch Priority: normal | Milestone: 8.0.1 Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Revisions: D1257 -------------------------------------+------------------------------------- Changes (by osa1): * status: new => patch * differential: => D1257 Comment: Thanks to @quchen for realizing that my patch reduces to previous version if there's only one imported `RdrName` with same name. I created a patch for this and I'll be adding tests soon. -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/10890#comment:10> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler

#10890: Incorrect redundant import warning for type classes -------------------------------------+------------------------------------- Reporter: quchen | Owner: Type: bug | Status: patch Priority: normal | Milestone: 8.0.1 Component: Compiler | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D1257 -------------------------------------+------------------------------------- Comment (by Austin Seipp <austin@…>): In [changeset:"1818b48e420fd0f689105da76721895aadee7fd6/ghc" 1818b48/ghc]: {{{ #!CommitTicketReference repository="ghc" revision="1818b48e420fd0f689105da76721895aadee7fd6" Fix incorrect import warnings when methods with identical names are imported Currently, GHC's warning generation code is assuming that a name (`RdrName`) can be imported at most once. This is a correct assumption, because 1) it's OK to import same names as long as we don't use any of them 2) when we use one of them, GHC generates an error because it doesn't disambiguate it automatically. But apparently the story is different with typeclass methods. If I import two methods with same names, it's OK to use them in typeclass instance declarations, because the context specifies which one to use. For example, this is OK (where modules A and B define typeclasses A and B, both with a function has), import A import B data Blah = Blah instance A Blah where has = Blah instance B Blah where has = Blah But GHC's warning generator is not taking this into account, and so if I change import list of this program to: import A (A (has)) import B (B (has)) GHC is printing these warnings: Main.hs:5:1: Warning: The import of ‘A.has’ from module ‘A’ is redundant Main.hs:6:1: Warning: The import of ‘B.has’ from module ‘B’ is redundant Why? Because warning generation code is _silently_ ignoring multiple symbols with same names. With this patch, GHC takes this into account. If there's only one name, then this patch reduces to the previous version, that is, it works exactly the same as current GHC (thanks goes to @quchen for realizing this). Reviewed By: austin Differential Revision: https://phabricator.haskell.org/D1257 GHC Trac Issues: #10890 }}} -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/10890#comment:12> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler

#10890: Incorrect redundant import warning for type classes -------------------------------------+------------------------------------- Reporter: quchen | Owner: Type: bug | Status: closed Priority: normal | Milestone: 8.0.1 Component: Compiler | Version: 7.10.2 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D1257 Wiki Page: | -------------------------------------+------------------------------------- Comment (by Simon Peyton Jones <simonpj@…>): In [changeset:"9376249b6b78610db055a10d05f6592d6bbbea2f/ghc" 9376249/ghc]: {{{ #!CommitTicketReference repository="ghc" revision="9376249b6b78610db055a10d05f6592d6bbbea2f" Fix unused-import stuff in a better way The fix for Trac #10890 in commit 1818b48, namely Fix incorrect import warnings when methods with identical names are imported was wrong, as demonstrated by the new test T10890_2. It suppressed far too many warnings! This patch fixes the original problem in a different way, by making RdrName.greUsedRdrName a bit cleverer. But this too is not really the Right Thing. I think the Right Thing is to store the /GRE/ in the tcg_used_rdrnames, not the /RdrName/. That would be a lot simpler and more direct. But one step at a time. }}} -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/10890#comment:14> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler