shadowNames: Accept an OccName, not a GreName

previously, the `shadowNames` function would take `[GreName]`. This has
confused me for two reasons:

* Why `GreName` and not `Name`? Does the difference between a normal
  name and a field name matter? The code of `shadowNames` shows that it
  does not, but really its better if the type signatures says so.

* Why `Name` and not `OccName`? The point of `shadowNames` is to shadow
  _unqualified names_, at least in the two use cases I am aware of
  (names defined on the GHCI prompt or in TH splices).

  The code of `shadowNames` used to have cases that peek at the module
  of the given name and do something if that module appears in the
  `GlobalRdrElt`, but I think these cases are dead code, I don’t see
  how they could occur in the above use cases. Also, I replaced them
  with `errors` and GHC would still validate. Hence removing this code
  (yay!)

This change also allows `shadowNames` to accept an `OccSet` instead,
which allows for a faster implemenation; I’ll try that separately. This
in stead might help with !6703.

3 files changed, 13 insertions, 22 deletions

diff --git a/compiler/GHC/Rename/Names.hs b/compiler/GHC/Rename/Names.hs
index b7826509e2..9cba5f5997 100644
--- a/compiler/GHC/Rename/Names.hs
+++ b/compiler/GHC/Rename/Names.hs
@@ -660,7 +660,7 @@ extendGlobalRdrEnvRn avails new_fixities
 
               -- Deal with shadowing: see Note [GlobalRdrEnv shadowing]
               want_shadowing = isGHCi || inBracket
-              rdr_env1 | want_shadowing = shadowNames rdr_env new_names
+              rdr_env1 | want_shadowing = shadowNames rdr_env new_occs
                        | otherwise      = rdr_env
 
               lcl_env3 = lcl_env2 { tcl_th_bndrs = extendNameEnvList th_bndrs
diff --git a/compiler/GHC/Runtime/Context.hs b/compiler/GHC/Runtime/Context.hs
index 6b4a4d0624..aaf99ee9a7 100644
--- a/compiler/GHC/Runtime/Context.hs
+++ b/compiler/GHC/Runtime/Context.hs
@@ -367,7 +367,8 @@ icExtendGblRdrEnv env tythings
        | otherwise
        = foldl' extendGlobalRdrEnv env1 (concatMap localGREsFromAvail avail)
        where
-          env1  = shadowNames env (concatMap availGreNames avail)
+          new_gres = concatMap availGreNames avail
+          env1  = shadowNames env (map occName new_gres)
           avail = tyThingAvailInfo thing
 
     -- Ugh! The new_tythings may include record selectors, since they
diff --git a/compiler/GHC/Types/Name/Reader.hs b/compiler/GHC/Types/Name/Reader.hs
index 91563a53f8..426849ff2f 100644
--- a/compiler/GHC/Types/Name/Reader.hs
+++ b/compiler/GHC/Types/Name/Reader.hs
@@ -1067,7 +1067,7 @@ extendGlobalRdrEnv env gre
   = extendOccEnv_Acc insertGRE Utils.singleton env
                      (greOccName gre) gre
 
-shadowNames :: GlobalRdrEnv -> [GreName] -> GlobalRdrEnv
+shadowNames :: GlobalRdrEnv -> [OccName] -> GlobalRdrEnv
 shadowNames = foldl' shadowName
 
 {- Note [GlobalRdrEnv shadowing]
@@ -1075,7 +1075,7 @@ shadowNames = foldl' shadowName
 Before adding new names to the GlobalRdrEnv we nuke some existing entries;
 this is "shadowing".  The actual work is done by RdrEnv.shadowName.
 Suppose
-   env' = shadowName env M.f
+   env' = shadowName env f `extendGlobalRdrEnv` M.f
 
 Then:
    * Looking up (Unqual f) in env' should succeed, returning M.f,
@@ -1086,6 +1086,7 @@ Then:
 
    * Looking up (Qual X.f) in env', where X /= M, should be the same as
      looking up (Qual X.f) in env.
+
      That is, shadowName does /not/ delete earlier qualified bindings
 
 There are two reasons for shadowing:
@@ -1107,9 +1108,10 @@ There are two reasons for shadowing:
            ghci> True
            ghci> M.x           -- M.x is still in scope!
            ghci> "Hello"
+
     So when we add `x = True` we must not delete the `M.x` from the
     `GlobalRdrEnv`; rather we just want to make it "qualified only";
-    hence the `mk_fake-imp_spec` in `shadowName`.  See also Note
+    hence the `set_qual` in `shadowName`.  See also Note
     [Interactively-bound Ids in GHCi] in GHC.Runtime.Context
 
   - Data types also have External Names, like Ghci4.T; but we still want
@@ -1143,24 +1145,17 @@ There are two reasons for shadowing:
       At that stage, the class op 'f' will have an Internal name.
 -}
 
-shadowName :: GlobalRdrEnv -> GreName -> GlobalRdrEnv
+shadowName :: GlobalRdrEnv -> OccName -> GlobalRdrEnv
 -- Remove certain old GREs that share the same OccName as this new Name.
 -- See Note [GlobalRdrEnv shadowing] for details
-shadowName env new_name
-  = alterOccEnv (fmap (mapMaybe shadow)) env (occName new_name)
+shadowName = alterOccEnv (fmap (mapMaybe shadow))
   where
-    maybe_new_mod = nameModule_maybe (greNameMangledName new_name)
-
     shadow :: GlobalRdrElt -> Maybe GlobalRdrElt
     shadow
        old_gre@(GRE { gre_lcl = lcl, gre_imp = iss })
        = case greDefinitionModule old_gre of
            Nothing -> Just old_gre   -- Old name is Internal; do not shadow
            Just old_mod
-              | Just new_mod <- maybe_new_mod
-              , new_mod == old_mod   -- Old name same as new name; shadow completely
-              -> Nothing
-
               | null iss'            -- Nothing remains
               -> Nothing
 
@@ -1168,7 +1163,7 @@ shadowName env new_name
               -> Just (old_gre { gre_lcl = False, gre_imp = iss' })
 
               where
-                iss' = lcl_imp ++ mapMaybe shadow_is iss
+                iss' = lcl_imp ++ mapMaybe set_qual iss
                 lcl_imp | lcl       = [mk_fake_imp_spec old_gre old_mod]
                         | otherwise = []
 
@@ -1181,13 +1176,8 @@ shadowName env new_name
                                    , is_qual = True
                                    , is_dloc = greDefinitionSrcSpan old_gre }
 
-    shadow_is :: ImportSpec -> Maybe ImportSpec
-    shadow_is is@(ImpSpec { is_decl = id_spec })
-       | Just new_mod <- maybe_new_mod
-       , is_as id_spec == moduleName new_mod
-       = Nothing   -- Shadow both qualified and unqualified
-       | otherwise -- Shadow unqualified only
-       = Just (is { is_decl = id_spec { is_qual = True } })
+    set_qual :: ImportSpec -> Maybe ImportSpec
+    set_qual is = Just (is { is_decl = (is_decl is) { is_qual = True } })
 
 
 {-