- User Since
- Sep 5 2013, 6:50 AM (293 w, 4 d)
Feb 5 2019
Sorry, this fell off my radar :). LGTM too.
Feb 3 2019
I have no objection to this, but I wonder whether all state accessible from all nodes should be part of the AST dump. Where do you think the line is? Is there anything else missing currently from other nodes?
Feb 1 2019
Jan 31 2019
Jan 30 2019
Jan 29 2019
An alternative was written and committed.
This was subsequently reverted. Is there a status update? Rebasing and committing D56959 would unblock traverser work and would allow this to progress separately in parallel.
Jan 26 2019
I was just working on this this afternoon to show you the difference when it is split up. Looks like you committed this meanwhile.
Jan 25 2019
Jan 24 2019
There's definitely a better possible ordering in two commits:
Thanks, but I don't see the factored out changes in the repo. Are you going to commit those first?
Jan 23 2019
Just in case you didn't know, you could have (and still can!) just update https://reviews.llvm.org/D57098 instead of creating a new PR.
I haven't done a full review as it's not obvious what parts of the diff relate to which separate change. Perhaps Aaron will review and approve though for you anyway.
Splitting the introduction of and porting to Create would significantly reduce the number of files touched by the 'real' change in this commit, and therefore reduce noise in the commit (following the idea of "do one thing per commit" to make the code reviewable in the future).
Thanks for doing this!
Jan 21 2019
Jan 20 2019
Jan 19 2019
Jan 18 2019
Thanks, do you need someone to commit this for you?
Jan 17 2019
Should this one now be closed? The 'Add Action...' dropdown has an action for that.
I don't know what C++ code results in the undeserialized declarations output. Can you suggest some?