[commit: ghc] master: Fix incorrect import warnings when methods with identical names are imported (1818b48)
git at git.haskell.org
git at git.haskell.org
Tue Oct 13 05:46:55 UTC 2015
Repository : ssh://git@git.haskell.org/ghc
On branch : master
Link : http://ghc.haskell.org/trac/ghc/changeset/1818b48e420fd0f689105da76721895aadee7fd6/ghc
>---------------------------------------------------------------
commit 1818b48e420fd0f689105da76721895aadee7fd6
Author: Ömer Sinan Ağacan <omeragacan at gmail.com>
Date: Tue Oct 13 00:47:28 2015 -0500
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
>---------------------------------------------------------------
1818b48e420fd0f689105da76721895aadee7fd6
compiler/rename/RnNames.hs | 10 ++++------
.../tests/warnings/should_compile/T10890/A.hs | 4 ++++
.../tests/warnings/should_compile/T10890/B.hs | 4 ++++
.../tests/warnings/should_compile/T10890/Base.hs | 6 ++++++
.../warnings/should_compile/T10890/Extends.hs | 4 ++++
.../should_compile/T10890}/Makefile | 0
.../tests/warnings/should_compile/T10890/T10890.hs | 23 ++++++++++++++++++++++
.../warnings/should_compile/T10890/T10890_1.hs | 22 +++++++++++++++++++++
.../tests/warnings/should_compile/T10890/all.T | 7 +++++++
9 files changed, 74 insertions(+), 6 deletions(-)
diff --git a/compiler/rename/RnNames.hs b/compiler/rename/RnNames.hs
index 7b067de..0cdf38c 100644
--- a/compiler/rename/RnNames.hs
+++ b/compiler/rename/RnNames.hs
@@ -1461,13 +1461,11 @@ extendImportMap :: GlobalRdrEnv -> RdrName -> ImportMap -> ImportMap
-- it into scope; choose one of them (bestImport), and record
-- the RdrName in that import decl's entry in the ImportMap
extendImportMap rdr_env rdr imp_map
- | [gre] <- lookupGRE_RdrName rdr rdr_env
- , GRE { gre_lcl = lcl, gre_imp = imps } <- gre
- , not lcl
- = add_imp gre (bestImport imps) imp_map
- | otherwise
- = imp_map
+ = foldr recordRdrName imp_map nonLocalGREs
where
+ recordRdrName gre m = add_imp gre (bestImport (gre_imp gre)) m
+ nonLocalGREs = filter (not . gre_lcl) (lookupGRE_RdrName rdr rdr_env)
+
add_imp :: GlobalRdrElt -> ImportSpec -> ImportMap -> ImportMap
add_imp gre (ImpSpec { is_decl = imp_decl_spec }) imp_map
= Map.insertWith add decl_loc [avail] imp_map
diff --git a/testsuite/tests/warnings/should_compile/T10890/A.hs b/testsuite/tests/warnings/should_compile/T10890/A.hs
new file mode 100644
index 0000000..c88ccc5
--- /dev/null
+++ b/testsuite/tests/warnings/should_compile/T10890/A.hs
@@ -0,0 +1,4 @@
+module A where
+
+class A a where
+ has :: a
diff --git a/testsuite/tests/warnings/should_compile/T10890/B.hs b/testsuite/tests/warnings/should_compile/T10890/B.hs
new file mode 100644
index 0000000..cfc2790
--- /dev/null
+++ b/testsuite/tests/warnings/should_compile/T10890/B.hs
@@ -0,0 +1,4 @@
+module B where
+
+class B a where
+ has :: a
diff --git a/testsuite/tests/warnings/should_compile/T10890/Base.hs b/testsuite/tests/warnings/should_compile/T10890/Base.hs
new file mode 100644
index 0000000..f56bd9a
--- /dev/null
+++ b/testsuite/tests/warnings/should_compile/T10890/Base.hs
@@ -0,0 +1,6 @@
+module Base (AClass(..), BClass()) where
+
+import Extends (BClass ())
+
+class AClass a where
+ has :: a
diff --git a/testsuite/tests/warnings/should_compile/T10890/Extends.hs b/testsuite/tests/warnings/should_compile/T10890/Extends.hs
new file mode 100644
index 0000000..a81013e
--- /dev/null
+++ b/testsuite/tests/warnings/should_compile/T10890/Extends.hs
@@ -0,0 +1,4 @@
+module Extends where
+
+class BClass b where
+ has :: b
diff --git a/testsuite/tests/ghci.debugger/scripts/break022/Makefile b/testsuite/tests/warnings/should_compile/T10890/Makefile
similarity index 100%
copy from testsuite/tests/ghci.debugger/scripts/break022/Makefile
copy to testsuite/tests/warnings/should_compile/T10890/Makefile
diff --git a/testsuite/tests/warnings/should_compile/T10890/T10890.hs b/testsuite/tests/warnings/should_compile/T10890/T10890.hs
new file mode 100644
index 0000000..a6c0751
--- /dev/null
+++ b/testsuite/tests/warnings/should_compile/T10890/T10890.hs
@@ -0,0 +1,23 @@
+module Main where
+
+-- Previously GHC was printing this warning:
+--
+-- 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
+
+import A (A (has))
+import B (B (has))
+
+data Blah = Blah
+
+instance A Blah where
+ has = Blah
+
+instance B Blah where
+ has = Blah
+
+main :: IO ()
+main = return ()
diff --git a/testsuite/tests/warnings/should_compile/T10890/T10890_1.hs b/testsuite/tests/warnings/should_compile/T10890/T10890_1.hs
new file mode 100644
index 0000000..7614ee3
--- /dev/null
+++ b/testsuite/tests/warnings/should_compile/T10890/T10890_1.hs
@@ -0,0 +1,22 @@
+module Main where
+
+import Base
+import Extends
+
+-- Previously GHC was giving this false positive:
+--
+-- T10890_1.hs:4:1: Warning:
+-- The import of ‘Extends’ is redundant
+-- except perhaps to import instances from ‘Extends’
+-- To import instances alone, use: import Extends()
+
+data Bar = Bar
+
+instance AClass Bar where
+ has = Bar
+
+instance BClass Bar where
+ has = Bar
+
+main :: IO ()
+main = return ()
diff --git a/testsuite/tests/warnings/should_compile/T10890/all.T b/testsuite/tests/warnings/should_compile/T10890/all.T
new file mode 100644
index 0000000..3e323f0
--- /dev/null
+++ b/testsuite/tests/warnings/should_compile/T10890/all.T
@@ -0,0 +1,7 @@
+test('T10890',
+ extra_clean(['A.o', 'A.hi', 'B.o', 'B.hi']),
+ multimod_compile, ['T10890', '-v0 -fwarn-unused-imports'])
+
+test('T10890_1',
+ extra_clean(['Base.o', 'Base.hi', 'Extends.o', 'Extends.hi']),
+ multimod_compile, ['T10890_1', '-v0 -fwarn-unused-imports'])
More information about the ghc-commits
mailing list