- User Since
- Sep 27 2019, 4:27 AM (113 w, 5 d)
Oct 21 2021
Oct 6 2021
Pavel, could you take another look? Interestingly the null-deref problem was already fixed in the follow-up patch, maybe I just did not land quickly enough :-)).
Avoid nullptr deref/ref.
Oct 4 2021
Addressed Pavel's comments.
Oct 1 2021
Thank you for the great comments, Pavel. I took a stab at merging the parameters without interleaving them with the locals. Let me know what you think; I can certainly put this back to the original state if you think this is a change for the worse.
Addressed reviewer comments, separated merging of the abstract parameters into a function, prevented interleaving of parameters with locals.
Sep 30 2021
Cache only global variables.
Improved the comment, as Greg suggested.
Sep 29 2021
First of all, thank you, Greg and Pavel, for all the great feedback and discussion. I have followed all your suggestions (use plain method, use image lookup, added the additional tests). I have also packaged the C test, but as Greg notes I am not convinced it will keep testing what it's supposed to.
Changed the test to avoid running the process and use image lookup instead of frame variable.
Added a C test (I have also verified that the C test fails without the SymbolFileDWARF patch).
Rewrote the recursive parser to use a plain method.
Sep 28 2021
Just a few replies below; I am hoping to put the relevant code changes together tomorrow.
Thank you for the review! Does the separate method look more reasonable?
Added the /*can_create=*/ comments.
Separated ParseVariablesInFunctionContext's lambda into a method.
Sep 27 2021
Thank you for taking a look, shafik@. I hope you don't mind if I address your comments later, once a full review arrives from Pavel (or Johannes).
Hi, could you take a look at this change?
Hi, could you possibly take a look at this change?
Aug 4 2021
Thanks for getting back to this, Raphael!
Nov 10 2020
This is looking great, thanks!
Oct 28 2020
The code change looks good to me and it is in line with the change that Raphael and Greg wanted in https://reviews.llvm.org/D72133. As far as I remember, https://reviews.llvm.org/D72133 did not change the previous behavior because I felt that changing API semantics was out of scope of what I wanted to do with the formatters.
Sep 14 2020
This makes sense, thanks!
Aug 13 2020
Aug 11 2020
This looks great! Thank you for the extensive test suite.
Jul 16 2020
Jul 15 2020
Undo the infinite loop detection.
Addressed review comments.
Jun 9 2020
Added a comment.
Jun 8 2020
Jun 7 2020
Jun 4 2020
Exclude UUID strings ending with "-".
Jun 1 2020
Added a test for missing nibble in UUID.
Removed size restrictions on UUIDs.
May 30 2020
Change SetFromStringRef to return bool.
May 29 2020
May 28 2020
May 25 2020
Raphael, could you land this for me?
May 22 2020
May 20 2020
Merged the ObjC pointer case with the reference case, simplified the test.
May 19 2020
Added more assertions, reformatted the test code.
Pavel, could you possibly land this for us?
May 12 2020
May 6 2020
Jim, do you think this is good to go?
May 5 2020
Reopening for further investigation.
... now also fixed the 'volatile'. It took only three patches to copy four lines of code by hand. Not bad, huh?
... and remove the extra braces.
Addressed reviewer comments.
Do you think Raphael would want to review this as well? If you think it is not necessary, could you land the patch for me?
May 4 2020
Simplify the test based on the suggestion from labath@.
May 3 2020
Apr 22 2020
Apr 11 2020
Abandoning the patch since we cannot reach agreement on how this should be tested.
Apr 10 2020
Regarding the callback idea, I have bad experience with callbacks because they break if the code is not crafted for re-entrancy and they are much harder to understand. That change feels out of scope for just adding a test and fixing an unrelated bug.
Apr 9 2020
As labath@ suggested, I have teased apart the test and the testability refactoring into a separate patch. The patch lives at https://reviews.llvm.org/D77790, could you please take a look?
Addressed reviewer comments.
I rewrote parts of the test, hopefully making it a bit clearer. Please let me know if this made it better.
Addressed some of the reviewer comments.
Apr 8 2020
Apr 7 2020
Looking at the code for flushing L1 cache, it appears broken. I am guessing that is the reason for the failure of my patch on the bots.