[GHC] #11824: GHC error in desugarer lookup

[Simon Peyton Jones]

|  If the latter there is no reason to fear: TcBinds.addTypecheckedBinds
|  (which TcTypeable uses) ensures that no bindings are added to the
|  environment if we are compiling a boot module,

Ah, I had missed that; it's pretty deeply hidden.

Suggestion:

* Move tcRecSelBinds from tcAddImplicits, to mkTypeableBinds.
  More precisely, make a new function addDerivedBindings, which
    * add record-selector bindings (via tcRecSelBinds)
    * adds typeable bindings (via mkTypableBinds)
  and call it where we currently call mkTypeableBinds.

* Instead of burying the "drop the bindings" deep in addTypecheckedBinds,
  inline it into addDerivedBindings:
   - make mkTypeableBinds *return* bindings rather than extending gbl-env
   - ditto tcRecSelBinds
  Then addDerivedBinds can test hs-boot, and extend the tcg_binds if necessary

That would put the two together nicely, they really are very similar.


I'm not bothered about the cost of generating bindings and then discarding them; it only happens for hs-boot files.  I _am_ concerned about clarity!

Thanks!

Simon

|  -----Original Message-----
|  From: Ben Gamari [mailto:ben at well-typed.com]
|  Sent: 13 April 2016 14:53
|  To: Simon Peyton Jones <simonpj at microsoft.com>
|  Cc: GHC developers <ghc-devs at haskell.org>
|  Subject: RE: [GHC] #11824: GHC error in desugarer lookup
|  
|  Simon Peyton Jones <simonpj at microsoft.com> writes:
|  
|  > Ben
|  >
|  > I'm offline, so can't reply on Phab.
|  >
|  > Calling mkTypeableBinds in tcRnHsBootDecls isn't quite right, because
|  it'll generate lots of *bindings* whereas all we want is the *Ids* in
|  the GlobalValEnv.
|  >
|  > (It's possible that tcg_binds is ignored in the boot-module case, but
|  > it's unsavoury to generate the bindings at all.  They are generated
|  > when we process the main module.)
|  >
|  > I suggest the easiest fix is, in mkModIdBinding and
|  mkTypeableTyConBinds, guard the binding-generation with a test for
|  whether isHsBootOrSig is true.
|  >
|  Are you concerned about the bindings being generated at all or merely
|  that they are being added to the typechecking environment?
|  
|  If the latter there is no reason to fear: TcBinds.addTypecheckedBinds
|  (which TcTypeable uses) ensures that no bindings are added to the
|  environment if we are compiling a boot module,
|  
|      addTypecheckedBinds :: TcGblEnv -> [LHsBinds Id] -> TcGblEnv
|      addTypecheckedBinds tcg_env binds
|        | isHsBootOrSig (tcg_src tcg_env) = tcg_env
|          -- Do not add the code for record-selector bindings
|          -- when compiling hs-boot files
|        | otherwise = tcg_env { tcg_binds = foldr unionBags
|                                                  (tcg_binds tcg_env)
|                                                  binds }
|  
|  Of course, we are still doing the work of producing the bindings; if it
|  is this that you are concerned about then I can be more careful in
|  guarding their production.
|  
|  Cheers,
|  
|  - Ben