| 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@well-typed.com] | Sent: 13 April 2016 14:53 | To: Simon Peyton Jones <simonpj@microsoft.com> | Cc: GHC developers <ghc-devs@haskell.org> | Subject: RE: [GHC] #11824: GHC error in desugarer lookup | | Simon Peyton Jones <simonpj@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