#10853: Refine addTopDecls -------------------------------------+------------------------------------- Reporter: goldfire | Owner: Type: bug | Status: new Priority: normal | Milestone: 8.0.1 Component: Template Haskell | Version: 7.10.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: | Unknown/Multiple Type of failure: None/Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Revisions: -------------------------------------+------------------------------------- Old description:
This suggested refactoring/documentation task is spurred on by #10486. Readers do not have to read that ticket to understand this task.
Currently, `addTopDecls` is restricted in what it can add. In an email exchange with Geoff Mainland (the original implementor of `addTopDecls`), it was unclear exactly why this restriction is in place. So, I propose lifting the restriction.
We do have to be careful about ordering. As it is currently implemented, here is what happens:
{{{ -- region A, including everything up to but not including a top-level splice $( {- region B, the contents of a top-level splice -} ) -- region C, everything after the splice }}}
In addition to those regions, we also have
{{{ -- region D, consisting of all declarations added with `addTopDecls`. }}}
By reading the code (`TcRnDriver.tc_rn_src_decls`), it seems that the expected behavior should be this:
1. Rename region A. Anything defined in other regions is not in scope. 2. Rename region D, with declarations from region A in scope. 3. Type-check regions A and D together. 4. When running TH code in region B, all declarations from A and D are in scope and can be accessed via `reify` and friends. 5. Combine regions B and C together, and recur to step 1 (where the combined B and C is now a new region A).
This all means that A and D are mutually recursive w.r.t. the type- checker but not the renamer. This is a bit peculiar, but not terrible. For example, this means that there could be a function whose definition is in A and type signature in D. Proper mutual recursion would be hard (impossible?) without better renaming support.
In any case, I don't see a reason to restrict what's allowed in `addTopDecls`.
So, at a minimum, I propose: * Remove restrictions in `addTopDecls` * Document `addTopDecls` generally * Document the above behavior, detailing the interaction between these regions
Perhaps better, we could do: * Remove restrictions in `addTopDecls` * Rejigger the implementation of `addTopDecls` to lump region D in with B and C. This looks quite straightforward, and it makes for a simpler story about mutual recursion. This is a potentially-breaking change, if existing code has, say, a function defined in A with its type signature in D. * Document the new, simpler behavior
Regardless of which proposal we go with, some testing is in order, to make sure that this is all correct. I've done no testing in formulating this ticket, just reading code.
New description: This suggested refactoring/documentation task is spurred on by #10486. Readers do not have to read that ticket to understand this task. Currently, `addTopDecls` is restricted in what it can add. In an email exchange with Geoff Mainland (the original implementor of `addTopDecls`), it was unclear exactly why this restriction is in place. So, I propose lifting the restriction. We do have to be careful about ordering. As it is currently implemented, here is what happens: {{{ -- region A, including everything up to but not including a top-level splice $( {- region B, the contents of a top-level splice -} ) -- region C, everything after the splice }}} In addition to those regions, we also have {{{ -- region D, consisting of all declarations added with `addTopDecls`, -- as called from splices in region A }}} By reading the code (`TcRnDriver.tc_rn_src_decls`), it seems that the expected behavior should be this: 1. Rename region A. Anything defined in other regions is not in scope. 2. Rename region D, with declarations from region A in scope. 3. Type-check regions A and D together. 4. When running TH code in region B, all declarations from A and D are in scope and can be accessed via `reify` and friends. 5. Combine regions B and C together, and recur to step 1 (where the combined B and C is now a new region A). This all means that A and D are mutually recursive w.r.t. the type-checker but not the renamer. This is a bit peculiar, but not terrible. For example, this means that there could be a function whose definition is in A and type signature in D. Proper mutual recursion would be hard (impossible?) without better renaming support. In any case, I don't see a reason to restrict what's allowed in `addTopDecls`. So, at a minimum, I propose: * Remove restrictions in `addTopDecls` * Document `addTopDecls` generally * Document the above behavior, detailing the interaction between these regions Perhaps better, we could do: * Remove restrictions in `addTopDecls` * Rejigger the implementation of `addTopDecls` to lump region D in with B and C. This looks quite straightforward, and it makes for a simpler story about mutual recursion. This is a potentially-breaking change, if existing code has, say, a function defined in A with its type signature in D. * Document the new, simpler behavior Regardless of which proposal we go with, some testing is in order, to make sure that this is all correct. I've done no testing in formulating this ticket, just reading code. EDIT: Clarify that region D is created from splices from region A -- Comment (by goldfire): The behavior in comment:2 is documented: See the last bullet point under section 7.17.1 [https://downloads.haskell.org/~ghc/latest/docs/html/users_guide/template- haskell.html here]. Mind you, I didn't say this is documented well or in an obvious place (though I think we could do worse). If you have a concrete suggestion of how to make this better, I'm happy to take it. Does this clarify? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/10853#comment:3 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler