arc diff linter looping / stuck
Hi all, I've prepared a bunch of commits to fix several tickets. After pushing these commits to branch wip/rae (to save my place and to get validate running on Travis), I then `git checkout`ed back to a point where `git diff origin/master` gave me a patch for precisely one bug (instead of the several unrelated ones I had fixed). I wanted to post to Differential. `arc diff` allowed me to fill out a description message (which mentioned, in its comments, the right set of commits), but then hung on the "linting..." stage. I suppose I could skip the linter, but it's more likely I've done something wrong here... Any advice? In the meantime, please consider this to be a request for feedback on everything in wip/rae! The bugs fixed are #9200, #9415, #9404, and #9371. Thanks! Richard
I'm off on holiday for a week, but you and I have discussed most of these changes, some at length. If you are happy with your implementation, then go ahead and commit, from my pov. I did take a quick look though. For #9200 and TcTyClsDecls, I think you have implemented "Possible new strategy" on https://ghc.haskell.org/trac/ghc/wiki/GhcKinds/KindInference, but not "A possible variation" (same page). correct? If so, worth a note in the source code. And actually I'd transfer the algorithm itself, including the definition of CUSK, into the code. kcStrategy seems a very odd name for a predicate on HsDecls that is just a Bool saying whether or not it has a CUSK. Also odd is that every call to kcHsTyVarBndrs has a corresponding call to kcStrategy, and both functions are in TcHsType; why not just combine them into one? Thanks for doing this Simon | -----Original Message----- | From: ghc-devs [mailto:ghc-devs-bounces@haskell.org] On Behalf Of Richard | Eisenberg | Sent: 07 August 2014 19:34 | To: ghc-devs | Subject: arc diff linter looping / stuck | | Hi all, | | I've prepared a bunch of commits to fix several tickets. After pushing | these commits to branch wip/rae (to save my place and to get validate | running on Travis), I then `git checkout`ed back to a point where `git | diff origin/master` gave me a patch for precisely one bug (instead of the | several unrelated ones I had fixed). I wanted to post to Differential. | `arc diff` allowed me to fill out a description message (which mentioned, | in its comments, the right set of commits), but then hung on the | "linting..." stage. I suppose I could skip the linter, but it's more | likely I've done something wrong here... | | Any advice? | | In the meantime, please consider this to be a request for feedback on | everything in wip/rae! The bugs fixed are #9200, #9415, #9404, and #9371. | | Thanks! | Richard | _______________________________________________ | ghc-devs mailing list | ghc-devs@haskell.org | http://www.haskell.org/mailman/listinfo/ghc-devs
On Aug 7, 2014, at 5:51 PM, Simon Peyton Jones
I'm off on holiday for a week, but you and I have discussed most of these changes, some at length. If you are happy with your implementation, then go ahead and commit, from my pov.
OK.
I did take a quick look though. For #9200 and TcTyClsDecls, I think you have implemented "Possible new strategy" on https://ghc.haskell.org/trac/ghc/wiki/GhcKinds/KindInference, but not "A possible variation" (same page). correct? If so, worth a note in the source code. And actually I'd transfer the algorithm itself, including the definition of CUSK, into the code.
I've made a new ticket, #9427, for the "variation", which is properly a feature request, not a bug. I can cycle back around to this in a little while.
kcStrategy seems a very odd name for a predicate on HsDecls that is just a Bool saying whether or not it has a CUSK. Also odd is that every call to kcHsTyVarBndrs has a corresponding call to kcStrategy, and both functions are in TcHsType; why not just combine them into one?
There is enough variation in how kcHsTyVarBndrs is called to make this a little inconvenient (it's sometimes called on a FamilyDecl, not a TyClDecl, and with existentials in a data constructor, with no clear declaration at all). Instead, I've decided that CUSKness a property of the declaration itself, and put the CUSK-checking code in HsDecls. There is a Note there as well. And, I've removed the last vestiges of KindCheckingStrategy. I hope this is OK. If it validates, I'll push, and we can revisit refactoring later if necessary. Thanks, Richard
Thanks for doing this
Simon
| -----Original Message----- | From: ghc-devs [mailto:ghc-devs-bounces@haskell.org] On Behalf Of Richard | Eisenberg | Sent: 07 August 2014 19:34 | To: ghc-devs | Subject: arc diff linter looping / stuck | | Hi all, | | I've prepared a bunch of commits to fix several tickets. After pushing | these commits to branch wip/rae (to save my place and to get validate | running on Travis), I then `git checkout`ed back to a point where `git | diff origin/master` gave me a patch for precisely one bug (instead of the | several unrelated ones I had fixed). I wanted to post to Differential. | `arc diff` allowed me to fill out a description message (which mentioned, | in its comments, the right set of commits), but then hung on the | "linting..." stage. I suppose I could skip the linter, but it's more | likely I've done something wrong here... | | Any advice? | | In the meantime, please consider this to be a request for feedback on | everything in wip/rae! The bugs fixed are #9200, #9415, #9404, and #9371. | | Thanks! | Richard | _______________________________________________ | ghc-devs mailing list | ghc-devs@haskell.org | http://www.haskell.org/mailman/listinfo/ghc-devs
participants (2)
-
Richard Eisenberg -
Simon Peyton Jones