Add trace injection

Authored by dfeuer on Nov 6 2017, 9:56 AM.



Add support for injecting runtime calls to trace in DsM. This
allows the desugarer to add compile-time information to a runtime

Diff Detail

rGHC Glasgow Haskell Compiler
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.
dfeuer created this revision.Nov 6 2017, 9:56 AM
austin resigned from this revision.Nov 6 2017, 3:50 PM
dfeuer updated this revision to Diff 14595.Nov 7 2017, 6:33 PM
  • Simplify API using exprType
  • Move pprRuntimeTrace to DsMonad
dfeuer updated this revision to Diff 14596.Nov 7 2017, 6:35 PM
  • Move pprRuntimeTrace to DsMonad
bgamari requested changes to this revision.Nov 23 2017, 12:02 PM

This seems reasonable to me although I've suggested some cleanups.


This looks reasonable but can you make the documentation a bit more comprehensive? It's currently rather unclear what this actually does without reading the implementation.


Why is this boot file necessary?


What we drop these?

This revision now requires changes to proceed.Nov 23 2017, 12:02 PM
dfeuer added inline comments.Nov 23 2017, 2:43 PM

To allow trace to be injected into modules that Debug.Trace depends on. Those modules just need to add import {-# SOURCE #-} Debug.Trace (). I guess I should add a comment, and mention it in the pprRuntimeTrace documentation.


I'll erase those lines.

bgamari added inline comments.Nov 23 2017, 3:20 PM

In that case why do we need to export anything but trace?

Another option is to simply make trace wired-in. This would avoid all of the boot-file silliness.


What we drop these?

An impressive bit of gibberish there. Sheesh.

dfeuer added inline comments.Nov 27 2017, 4:49 PM

We might potentially want to use other tracing functions from modules with import loops. Is there any reason not to include them? Making trace wired-in to support pprRuntimeTrace sounds very reasonable; how hard is that?

dfeuer updated this revision to Diff 14850.Nov 29 2017, 5:07 PM

Clean things up a bit

dfeuer marked 4 inline comments as done.Nov 29 2017, 5:14 PM
dfeuer added inline comments.

I looked into the idea of wiring trace in, and to be honest, I don't think it's worth it. That locks down its inlining info, which seems like a potential maintenance burden. It seems likely to be better for anyone who needs to inject trace calls into Debug.Trace or its dependencies to just add the necessary imports by hand--there just aren't that many modules like that.

bgamari accepted this revision.Nov 29 2017, 6:52 PM

Just one request inline.


We need to be more explicit here; in particular point out who this is intended for.

I'm also still quite uncertain of whether we *really* need to export the entire interface.

This revision is now accepted and ready to land.Nov 29 2017, 6:52 PM
dfeuer updated this revision to Diff 14856.Nov 29 2017, 6:58 PM

Add further documentation

This revision was automatically updated to reflect the committed changes.