Hi Simon,
On Tue, Aug 21, 2012 at 7:14 PM, Simon Peyton-Jones
Can you give me read/write access to your github repo? I'm simonpj on github. That way I can add comments/questions in code, and generally clean up. It would make things easier if you could merge with HEAD so that I don't have to mess around moving libraries back in time.
Done. I've merged with HEAD as of right now. The only subrepository I've forked is the testsuite repository, which you can find on https://github.com/xnyhps/testsuite. I've tried to get validate to pass all tests, I had to fix this one: https://github.com/xnyhps/testsuite/commit/5016899250ace0d2db227569073ad8357..., and I've added this https://github.com/xnyhps/testsuite/commit/47f1bb7f7c8aced6e59f8d3e2413f6c6c... as a test case for holes. It's not a very complicated example of holes, but I'm a bit unsure how to properly write a test case for it. If you have some ideas to do this better, I'd be interested. Some other tests were reporting to be too (in)efficient, but compared to running validate on HEAD without my patch it seemed the same thing happened there, so I haven't look further into those.
------------------------------ You've put "LANGUAGE Holes" in TcErrors which means I can't bootstrap.
Sorry, that was sloppy, I've fixed it.
------------------------------ You have this in your patch file, which can't be right + | CHoleCan { + cc_ev :: CtEvidence, + cc_hole_ty :: TcTauType, -- Not a Xi! See same not as above + cc_depth :: SubGoalDepth -- See Note [WorkList] + } + \end{code}
\begin{code} @@ -933,6 +940,9 @@ ctPred (CTyEqCan { cc_tyvar = tv, cc_rhs = xi }) ctPred (CFunEqCan { cc_fun = fn, cc_tyargs = xis1, cc_rhs = xi2 }) = mkTcEqPred (mkTyConApp fn xis1) xi2 ctPred (CIrredEvCan { cc_ty = xi }) = xi +ctPred (CHoleCan { cc_flavor = fl, cc_hole_ty = xi }) + = xi +
since c_flavor isn't a field of CHoleCan.
This code is gone after merging with HEAD.
-----------------------
| The 3 currently remaining issues: | | - Free type variables are not tidied consistently. For every one of | these hole warnings, the same TidyEnv is reused, without taking the | updates from the other holes into account. I'm pretty sure I know where | this happens and how I could fix it.
This should be easy. * TcErrors.reportUnsolved uses tyVarsOfWC to find the free type variables of unsolved constraints * It then uses tidyFreeTyVars to assign them names * And that gives an env used in tidying.
So it should just work. I hope you are letting the various 'tidy' calls in TcErrors do the work.
I traced the problem back to not zonking the hole constraint properly, reporting them now works as it should (I've removed the hack I was using, and cleaned up some code that didn't need to be monadic anymore). (The reason they weren't zonked properly is that I've given zonkCt a special case for CHoleCans (https://github.com/xnyhps/ghc/blob/eee4242df72cf0a7910e896c3bd6d0fb20b97e37/...), as holes themselves do not carry enough information for me to turn them from CNonCanonicals back into CHoleCans (they become CIrredEvCans). I'm still not sure this is the right thing to do here.)
| - What I thought would be the local environment doesn't actually seem | to be it. The holes store in their origin the result of `getLclTypeEnv' | at their location, but as the Note [Bindings with closed types] says, | the TopLevelFlag of these don't actually differentiate the top level | from the non-top level bindings. I think it would be more helpful to | only show the non-top level bindings at the hole's location, any hints | about how to obtain just these would be appreciated.
In this context "local" means "this module" rather than "not top level". Use isExternalName to distinguish top-level things from nested things.
This works now, thanks!
| - The holes do not have very accurate source location information, like | some other errors have. The hole has its origin, ("test2.hs:3:16"), but | somehow not something like: "In the expression: folder _ x _, In an | equation for `test': test x = foldr _ x _". Help with how that is | supposed to work would also be appreciated.
That's odd. Every Wanted constraint has a WantedLoc (TcRnTypes), which includes a [ErrCtxt], which is that stack of "In ..; In... " stuff you see.
Code looks plausible.
I've fixed the way I was constructing the CtLoc, this was wrong and was always using [] as the [ErrCtxt]. Regards, Thijs Alkemade