Expose local variables in brackets to reify.
AbandonedPublic

Authored by facundominguez on Jan 23 2017, 11:08 AM.

Details

Reviewers
simonpj
goldfire
bgamari
austin
Trac Issues
#12778
Summary

The changes in https://phabricator.haskell.org/D2286 helped obtaining
type information of local variables using reify, but it only worked in
non-nested splices.

This patch comes to complete this feature allowing reify to retrieve
the type of variables bound inside brackets. E.g.

$([| let x = True

      in $(do addModFinalizer (reify (mkName "x") >>= runIO . print)
              [| x |]
		  )
   |])
Test Plan

./validate

facundominguez retitled this revision from to Expose local variables in brackets to reify..Jan 23 2017, 11:08 AM
facundominguez edited the test plan for this revision. (Show Details)
facundominguez added reviewers: simonpj, goldfire.
facundominguez updated the Trac tickets for this revision.
facundominguez updated this object.
facundominguez added subscribers: mboes, alpmestan.
facundominguez added a comment.EditedJan 23 2017, 11:30 AM

This patch addresses the intended issue but has the problems discussed in Trac #12778.

In particular, finalizers are not run if the TH ast is not inserted in the splice result. And if the result is inserted more than once, the finalizers would run as many times. E.g.

$(do 
  exp@(SplicedE here_we_carry_the_finalizers (TupE [])) <-
   [| $(addModFinalizer (runIO (putStrLn "finalizer")) >> [| () |] ) |]
  return $ TupE [exp, exp]
 )

This can be fixed by maintaining a global set of finalizers. After all they are referenced as RemoteRefs that we can compare. But before addressing this, I'd like to get some encouragement that the overall approach of the patch is good, or get some suggestions for a better solution.

facundominguez added inline comments.Jan 23 2017, 1:17 PM
libraries/template-haskell/Language/Haskell/TH/Syntax.hs
1591

This is really

data ModFinalizers = ModFinalizers [RemoteRef (Q ())]

But RemoteRef is defined in package ghci, which depends on template-haskell, and I don't know how to avoid the circularity. The current definition is undesirable because it forces using unsafeCoerce when destructing ModFinalizer values.

facundominguez added inline comments.Jan 23 2017, 4:07 PM
libraries/template-haskell/Language/Haskell/TH/Syntax.hs
1591

I'm changing it to

data ModFinalizers = ModFinalizers [Ptr (Q ())]

which is more tolerable. RemoteRefs can then be converted back and forth from the pointers.

bgamari added inline comments.Jan 23 2017, 8:49 PM
libraries/template-haskell/Language/Haskell/TH/Syntax.hs
1591

We have the ghc-boot-th packages for things needed by template-haskell and ghc. Perhaps RemoteRef could live there? It's a bit of an odd fit, but arguably better than the current situation.

Remove uses of unsafeCoerce by changing the ModFinalizers type.

I was aiming towards merging this before the freeze deadline for ghc-8.2.1. We might still be able to address the open issues and new comments on time, provided we get some feedback and we find no major blockers. That said, I understand everybody might be just too busy for this plan to work.

I'm sorry Facundo, I just don't have the bandwidth to dig into this for the next few weeks. Richard, who is in charge of TH, is in a similar bind.

I have spent 30 minutes on it, and I'm very uncomfortable with the creeping complexity that attends addModFinalizer. Now it's even invaded TH syntax too, with a new consructor with a mysterious [Ptr ()] inside it. (And no comments to explainn the Ptr () type.

It doesn't smell right... it feelis like a slippery slope that we perhpas embraked on too quickly.

Is this just a side project, or is it mission critical to some project your are on? I suppose if it really mattered to you and your colleauges we could just hold our noses and put it in -- it's not a massive patch.

But I would like to revisit the original goal and see if we can't figure out a better way to meet it.

Simon

Thanks for taking a look. We can manage without this. Let's wait until we can discuss it then.

bgamari requested changes to this revision.Jan 31 2017, 3:47 PM

Bumping out of the review queue for now

This revision now requires changes to proceed.Jan 31 2017, 3:47 PM
austin resigned from this revision.Nov 9 2017, 11:32 AM
facundominguez abandoned this revision.Nov 9 2017, 1:54 PM

I didn't need this anymore after having my use case, inline-java, use GHC plugins.