Opened 16 months ago

Closed 3 months ago

#8624 closed feature request (fixed)

-ddump-splices-file

Reported by: GregWeber Owned by:
Priority: normal Milestone: 7.10.1
Component: Compiler Version: 7.6.3
Keywords: Cc: sean.leather@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Revisions: Phab:D518

Description

I am proposing an additional feature, -ddump-splices-file that generates a corresponding .hs.th file for every .hs file that uses Template Haskell.

-ddump-splices is an invaluable but a frustrating way to look at generated Haskell code. If TH generation were some kind of error message, the current output would make sense. However, TH is generating code that we rely on and would be easier to comprehend if we could see it in a way the most similar to our existing Haskell code.

There is a valid complaint that when TH defines something you can't just grep for it, you have to know what TH is defining by reading documentation and imagining something that isn't in front of you.

If you have a file Foo.hs -ddump-splices-file will generate Foo.hs.th. Then whenever someone greps after buliding they will find the declaration. If you check these files in they can grep even before building. Similarly, an IDE can show these files as the source of a declaration. Also, if the TH generation changes in some way when a TH function changes, that will be visible.

This seems like a relatively easy feature to add. Any pointers on where to get started?

Change History (26)

comment:1 Changed 16 months ago by goldfire

I think the first step would be to write up a concrete proposal for the new feature on a wiki page. A few questions I have that would need to be answered:

  • If the source file is a .lhs file, is the output .lhs.th?
  • Can a user alter the extension?
  • Can a user alter the directory where this file is created?

Then, for implementing it, I would start by looking at DynFlags and then poking around to see how -ddump-splices works. I imagine it wouldn't be difficult just to redirect the -ddump-splices output to a file. Caveat: I haven't done anything quite like this to GHC, so my suggestions may be wrong.

comment:2 Changed 16 months ago by GregWeber

Thanks for the feedback and the pointers.

1) yes. I thought appending .th would be good because it is tab-completion friendly, but it also appears to be friendly to different haskell extensions.
2) no, we would want to see feedback from users that this is necessary. If .th is too overloaded we could just make it bigger. One more character, such as .ths might help. Although the extension could easily conflict with someone's computer-wide filesystem, it is unlikely to conflict in a directory with Haskell code.
3) no, we would want to see feedback from users that this is necessary

comment:3 Changed 16 months ago by nomeata

Yay bikeshedding:

I’d recommend Foo.th, just like the .imports file that -ddump-minimal-imports generates. That is probably a feature you’d want to look at, as it is somewhat similar to what you want.

Also, it would be useful if the .th file contains precise code locations of the origin of the splices. This would allow the tools that replace TH splices by their output in the original file to use the .th file conveniently.

Bonus points (well, different and more complicated feature actually, so ignore this for now): Enable a mode where GHC will read the .th file and use that instead of actually running Template Haskell. Distributing the .th files will then allow building packages on architectures where Template Haskell is not available.

comment:4 follow-up: Changed 16 months ago by GregWeber

Thanks for the pointer to -ddump-minimal-imports

Adding location information is a great idea, and if is easy to put the locations in a comment I will do that. However, you might need to get involved with this to add in the features you want. Build-caching is a very interesting feature, but I think it will require GHC/cabal build experts and otherwise a lot more input to think about and implement correctly, so I will leave that for another ticket.

The problem with Foo.th is that now tab completion of the filename stops at Foo.. Whereas with Foo.hs.th tab completion first stops at Foo.hs, which is what you want most of the time. Also, .hs.th together makes for a more unique file extension. If this feature is extended to build caching there will probably be a desire to change how the extensions work. This will actually be a good thing since it will avoid the need to think about backwards compatible files.

comment:5 in reply to: ↑ 4 Changed 16 months ago by nomeata

Replying to GregWeber:

Adding location information is a great idea, and if is easy to put the locations in a comment I will do that. However, you might need to get involved with this to add in the features you want. Build-caching is a very interesting feature, but I think it will require GHC/cabal build experts and otherwise a lot more input to think about and implement correctly, so I will leave that for another ticket.

Yes, that was just brianstorming... :-)

The problem with Foo.th is that now tab completion of the filename stops at Foo.. Whereas with Foo.hs.th tab completion first stops at Foo.hs, which is what you want most of the time.

Maybe its different with different workflows, but I, and probably lots of developers, usually happen to have .hi and .o files around that prevent the correct completion anyways. So unless you change that to .hs.hi and .hs.o as well, for the sake of consistency, .th is what follows the principle of least surprise. (But note that this is bikeshedding, do not let such a discussion discourage you from implementing the feature in the first place.)

comment:6 Changed 16 months ago by GregWeber

If you use cabal there shouldn't be .hi and .o files lying around. If they are around, they should be gitignored. Tab completion can use .gitignore at least to favor .hs files. I know that doesn't happen by default in most editors though, but I think most of the community is using cabal or some other build system that they could setup to have a dist folder. .th files should not end up in dist/. Some people might want to gitignore them, but I think if you go to the effort of turning them on you want to check them in.

comment:7 Changed 16 months ago by nomeata

Besides Cabal, I often use ghc --make and ghci, both of which leave .hi files around next to .hs... but this is getting off topic, so I’ll shut up :-)

comment:8 Changed 16 months ago by GregWeber

Thanks for the feedback. What I am suggesting is better for my workflow, but I am not opposed to changing it if needed.

comment:9 Changed 16 months ago by spl

  • Cc sean.leather@… added

I think I'm at least partly to blame for Greg's proposal, so naturally, I agree with the idea.

I just wanted to add a piece of evidence for why I would prefer Foo.th.hs over Foo.hs.th or Foo.th: it allows tools/scripts to find the file when searching for *.hs. Of course, tools/scripts can be configured, so this can be worked around. But I find this issue slightly more influential than the tab-completion issue -- I'm accustomed to not having tab completion of the extension.

Whatever the decision, I'd be happy to have this feature.

comment:10 Changed 16 months ago by simonpj

This seems like a very reasonable thing to do. I'm not volunteering to do it myself, but I'll gladly support anyone who does; I know how the TH implementation works.

The "untyped" splices are expanded by the renamer, and the "typed" ones by the type checker. So if you want to see all splices expanded, you need to look at the output of the type checker. Fortunately that's not difficult: it is more or less what -ddump-tc shows you. So to a first approximation, what you want is to take the output of -ddump-tc and put it in a file.

But there are always details:

  • -ddump-tc is, as its name implies, a debugging flag. We have not taken care to ensure that the pretty-printed output is fully-parsable Haskell. It should be, but you'd need to work on the Outputable instances for HsSyn to make it fully working.
  • The type checker "elaborates" the code by adding type abstractions and applications, dictionary abstractions and applications, and so on. For debugging purposes you want to see this; but for your purposes you want to suppress all the elaboration stuff. I've been careful to use different data constructors in HsSyn for elaboration code, so it should be easy to suppress it. But to do that you need to pass a flag into the pretty printer (to tell it whether to suppress it) and we need to think about how to do that. You definitely don't want to write two pretty-printers!

The usual process is to start a GHC Trac wiki page to describe the (user-facing) specification, and sketch any implementation details or choices. And use the ticket or ghc-devs to discuss.

Simon

comment:11 Changed 16 months ago by GregWeber

Thanks for the additional pointers! I am volunteering to do it, but won't be able to start until next week.

comment:12 Changed 12 months ago by GregWeber

So next week took over 4 months, but I am going to try to do this during the hackathon 2 weeks from now.

Nathaniel Howell me about a -ddump-to-file flag. It doesn't seem to work with -ddump-splices or -ddump-tc, but Nate told be I should be looking into

    TcRnMonad.dumpTcRn should call ErrUtils.dumpIfSet_dyn

comment:13 Changed 11 months ago by GregWeber

After making the changes Nate suggested

diff --git a/compiler/typecheck/TcRnMonad.lhs b/compiler/typecheck/TcRnMonad.lhs
index 17700e7..4c4be49 100644
--- a/compiler/typecheck/TcRnMonad.lhs
+++ b/compiler/typecheck/TcRnMonad.lhs
@@ -502,7 +502,9 @@ traceOptTcRn flag doc = whenDOptM flag $ do
 dumpTcRn :: SDoc -> TcRn ()
 dumpTcRn doc = do { rdr_env <- getGlobalRdrEnv
                   ; dflags <- getDynFlags
-                  ; liftIO (printInfoForUser dflags (mkPrintUnqualified dflags rdr_env) doc) }
+                  ; liftIO $ do
+                      dumpIfSet_dyn dflags Opt_D_dump_tc "Typechecker output" doc
+                      (printInfoForUser dflags (mkPrintUnqualified dflags rdr_env) doc) }

running ghc -ddump-tc -ddump-to-file Foo.hs produces a file Foo.dump-tc:

==================== Typechecker output ====================
2014-05-19 03:56:39.777604 UTC

TYPE SIGNATURES
TYPE CONSTRUCTORS
  Foo.Foo :: *
  data Foo
    No C type associated
    RecFlag NonRecursive, Not promotable
    = Foo :: GHC.Types.Int -> Foo Stricts: _
    FamilyInstance: none
COERCION AXIOMS
Dependent modules: []
Dependent packages: [base, ghc-prim, integer-gmp]


==================== Typechecker output ====================
2014-05-19 03:56:39.78129 UTC


==================== Typechecker ====================

This only took me 10 minutes to make the code change, so that is an encouraging start. This is not what I want as an end result, but is this useful now for others that use -ddump-tc to debug? Should I add a new flag for my desired functionality?

comment:14 Changed 11 months ago by simonpj

I think you'll want a different flag. -ddump-tc is primarily for debugging, and so it's fine for it to spit out all kinds of non-Haskell-source-code stuff.

Simon

comment:15 Changed 11 months ago by GregWeber

I submitted a patch in #9126 to get -ddump-to-file to work with more debug options. I will pursue a more ideal output with a different flag on this ticket.

comment:16 Changed 6 months ago by GregWeber

I finally made a patch for #9126 that doesn't have failing test cases.

I have a better understanding of how the printing works now. For solving this ticket (creating a file of valid Haskell code of the generated Template Haskell), I did have a question about the suggested approach of using the Typechecker output.

ddump-splices seems to contain exactly what I want, plus some extra stuff. Is the idea behind using the Typechecker output instead of refactoring the splices that I will get more generated stuff besides just Template Haskell?

comment:17 Changed 5 months ago by simonpj

I'm not sure whether you want to dump out the entire source code (after expanding splices); or just the splices (which is what -ddump-splices does.

If the former, then you need to dump the typechecker output (rather than the earlier renamer output) because type splices are expanded by the type checker.

Simon

comment:18 Changed 5 months ago by GregWeber

  • Differential Revisions set to https://phabricator.haskell.org/D518
  • Status changed from new to patch

comment:19 Changed 5 months ago by thomie

In the code review, the flag is called -dth-file. Why not include the word dump?

How about:

-ddump-th dump to stdout
-ddump-th -ddump-to-file dump to a .th.hs file

This would be more consistent with for example -ddump-hi and -ddump-hi -ddump-to-file (which dumps to a .dump-hi file).

Last edited 5 months ago by thomie (previous) (diff)

comment:20 follow-up: Changed 5 months ago by GregWeber

A big problem here is that -ddump-to-file is a global flag. I assume dumping to stdout is the default because users want that ephemeral behavior. The design for this ticket is that a user always wants to save it to a file, and that should not have to make all other dumps go to a file. If someone wants to ephemerally dump splices to stdout -ddump-splices is already there, well known, and better suited to the job.
Additionally, I want a .hs file to signify that the output is valid Haskell, not just a dump in an ad-hoc format, whereas with dumping the convention is dump-FLAG.

So I did intentionally want to get away from dumping but I am definitely open to ideas for better naming.

comment:21 in reply to: ↑ 20 Changed 5 months ago by thomie

Replying to GregWeber:

The design for this ticket is that a user always wants to save it to a file, and that should not have to make all other dumps go to a file.

So this allows -dth-file and another dump flag to be used at the same time. Why is that useful?

Additionally, I want a .hs file to signify that the output is valid Haskell, not just a dump in an ad-hoc format, whereas with dumping the convention is dump-FLAG.

This convention could be changed, -ddump-th -ddump-to-file would dump to a .th.hs file.

A better name than -dth-file or -dth-dec-file could be -dth-to-file.

comment:22 Changed 5 months ago by GregWeber

It seems like you are thinking of this as another debugging dump flag? It isn't at all, it just uses the existing dump system for implementation convenience. It is designed to be always turned on (if desired) and produce output that can be checked into source control. This should not effect whether actual dump flags are sent to stdout or to a file. I think -to- was supposed to help distinguish between dumping to stdout or to a file, so it doesn't seem necessary for something that always creates a file.

comment:23 Changed 5 months ago by thomie

Thank you, that clears things up for me.

comment:24 Changed 4 months ago by thoughtpolice

  • Differential Revisions changed from https://phabricator.haskell.org/D518 to Phab:D518
  • Milestone set to 7.10.1

comment:25 Changed 3 months ago by Austin Seipp <austin@…>

In 07ace5c221adbb1675413a0fac300a9f7913c234/ghc:

add -th-file which generates a th.hs file

Summary:
see Trac #8624

similar functionality is now available
with -ddump-to-file -ddump-splices

However, users are already accustomed to -ddump-splices
having a particular format, and this format is not completely valid code
The goal of -th-file is to dump valid Haskell code

Additionally, the convention of -ddump-to-file is to name the file after
the flag, so the file is .dump-splices
Given that the goal of the new flag is to generate valid Haskell,
The extension should be .hs

Additionally, -ddump-to-file effects all other dump flags

Test Plan:
look at the output of using the -th-file flag
and compare it to the output of using -ddump-to-file and -ddump-splices
I want to add test cases, but just need some pointers on getting started there

Reviewers: thomie, goldfire, simonpj, austin

Reviewed By: simonpj, austin

Subscribers: thomie, carter

Differential Revision: https://phabricator.haskell.org/D518

GHC Trac Issues: #8624

comment:26 Changed 3 months ago by thoughtpolice

  • Resolution set to fixed
  • Status changed from patch to closed

Merged to ghc-7.10.

Note: See TracTickets for help on using tickets.