Refactor the GlobalRdrEnv, fixing #7672
authorSimon Peyton Jones <simonpj@microsoft.com>
Mon, 1 Jun 2015 23:02:03 +0000 (00:02 +0100)
committerSimon Peyton Jones <simonpj@microsoft.com>
Mon, 1 Jun 2015 23:50:45 +0000 (00:50 +0100)
commit9b73cb16485f331d9dc1f37826c6d503e24a5b0b
treef5f5772dc844ed925af757c1b5cd08b8fa6bd88d
parent11d8f84fd3237c3821c8f826716fc4c9adfccb8c
Refactor the GlobalRdrEnv, fixing #7672

This patch started innocently enough, by deleting a single
call from rnImportDecl, namely

    let gbl_env = mkGlobalRdrEnv (filterOut from_this_mod gres)

The 'filterOut' makes no sense, and was the cause of #7672.

But that little loose end led to into a twisty maze of little
passages, all alike, which has taken me an unreasonably long
time to straighten out. Happily, I think the result is really
much better.

In particular:

 * INVARIANT 1 of the GlobalRdrEnv type was simply not true:
   we had multiple GlobalRdrElts in a list with the same
   gre_name field. This kludgily implmented one form of
   shadowing.

 * Meanwhile, extendGlobalRdrEnvRn implemented a second form of
   shadowing, by deleting stuff from the GlobalRdrEnv.

 * In turn, much of this shadowing stuff depended on the Names of
   the Ids bound in the GHCi InteractiveContext being Internal
   names, even though the TyCons and suchlike all had External
   Names. Very confusing.

So I have made the following changes

 * I re-established INVARIANT 1 of GlobalRdrEnv.  As a result
   some strange code in RdrName.pickGREs goes away.

 * RnNames.extendGlobalRdrEnvRn now makes one call to deal with
   shadowing, where necessary, and another to extend the
   environment.  It deals separately with duplicate bindings.

   The very complicated RdrName.extendGlobalRdrEnv becomes much
   simpler; we need to export the shadowing function, now called
   RdrName.shadowNames; and we can nuke
   RdrName.findLocalDupsRdrEnv altogether.

   RdrName Note [GlobalRdrEnv shadowing] summarises the shadowing
   story

 * The Names of the Ids bound in the GHCi interactive context are
   now all External.  See Note [Interactively-bound Ids in GHCi]
   in HscTypes.

 * Names for Ids created by the debugger are now made by
   IfaceEnv.newInteractiveBinder.  This fixes a lurking bug which
   was that the debugger was using mkNewUniqueSupply 'I' to make
   uniques, which does NOT guarantee a fresh supply of uniques on
   successive calls.

 * Note [Template Haskell ambiguity] in RnEnv shows that one TH-related
   error is reported lazily (on occurrences) when it might be better
   reported when extending the environment.  In some (but not all) cases
   this was done before; but now it's uniformly at occurrences.  In
   some ways it'd be better to report when extending the environment,
   but it's a tiresome test and the error is rare, so I'm leaving it
   at the lookup site for now, with the above Note.

 * A small thing: RnNames.greAvail becomes RdrName.availFromGRE, where
   it joins the dual RdrName.gresFromAvail.
24 files changed:
compiler/basicTypes/RdrName.hs
compiler/ghci/Debugger.hs
compiler/iface/IfaceEnv.hs
compiler/main/HscTypes.hs
compiler/main/InteractiveEval.hs
compiler/rename/RnEnv.hs
compiler/rename/RnNames.hs
compiler/typecheck/TcEnv.hs
compiler/typecheck/TcRnDriver.hs
testsuite/tests/ghci.debugger/scripts/break027.stdout
testsuite/tests/ghci/scripts/T10248.stderr
testsuite/tests/ghci/scripts/T5564.stderr
testsuite/tests/module/mod110.stderr
testsuite/tests/module/mod151.stderr
testsuite/tests/module/mod152.stderr
testsuite/tests/module/mod153.stderr
testsuite/tests/rename/should_compile/T1972.stderr
testsuite/tests/rename/should_fail/T5533.stderr
testsuite/tests/rename/should_fail/T7906.stderr
testsuite/tests/rename/should_fail/rn_dup.stderr
testsuite/tests/rename/should_fail/rnfail044.stderr
testsuite/tests/th/T7241.stderr
testsuite/tests/th/T8932.stderr
testsuite/tests/typecheck/should_fail/tcfail037.stderr