Opened 2 years ago

Last modified 8 months ago

#10853 new bug

Refine addTopDecls

Reported by: goldfire Owned by:
Priority: normal Milestone: 8.4.1
Component: Template Haskell Version: 7.10.2
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description (last modified by goldfire)

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

Change History (8)

comment:1 Changed 2 years ago by thoughtpolice

Milestone: 7.12.18.0.1

Milestone renamed

comment:2 Changed 2 years ago by osa1

Sorry if this is completely unrelated, but I realized this weird behavior while implementing #10891. Let's say I have this:

{-# LANGUAGE TemplateHaskell #-}

module Main where

import Language.Haskell.TH
import System.IO

data D = A Int | B Bool

test = $(... reify ''D ...)

main :: IO ()
main = return ()

This fails with:

Main.hs:11:5:
    ‘D’ is not in the type environment at a reify

But if I add this line between the splice and D:

$(return [])

Suddenly it works.

I couldn't find anything about regions in the wiki, but how do we split this program into regions and how do we explain this seemingly weird behavior? It seems like we can improve the way we evaluate TH splices.

comment:3 Changed 2 years ago by goldfire

Description: modified (diff)

The behavior in comment:2 is documented: See the last bullet point under section 7.17.1 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?

comment:4 Changed 2 years ago by goldfire

It turns out that my proposal to change the treatment of regions is a bad idea. This is because addTopDecls is useful for generating new definitions that get used right away. If region D isn't available when type-checking region A, this isn't possible. Note that the fact that region D isn't available when renaming A is OK, as Template Haskell can use "exact" names.

But this still could be cleaned up.

comment:5 Changed 2 years ago by siddhanathan

Owner: set to siddhanathan

comment:6 Changed 21 months ago by siddhanathan

Owner: siddhanathan deleted

I haven't been able to keep up the TH changes in master, and I'm getting some weird behavior on current master with the restriction removed. I think someone else would be able to do this faster than I could.

comment:7 in reply to:  6 Changed 20 months ago by thoughtpolice

Milestone: 8.0.18.2.1

Replying to siddhanathan:

I haven't been able to keep up the TH changes in master, and I'm getting some weird behavior on current master with the restriction removed. I think someone else would be able to do this faster than I could.

Thanks for the update. I'm of the opinion to punt this to the 8.2.x series, so if you'd still like to take a swing at this (and have more breathing room) or something, feel free. Otherwise just feel free to leave it unassigned, thanks!

comment:8 Changed 8 months ago by bgamari

Milestone: 8.2.18.4.1

Given that 8.2.1-rc1 is imminent, I'm bumping these off to the 8.4

Note: See TracTickets for help on using tickets.