- User Since
- Apr 6 2020, 5:32 AM (33 w, 4 d)
Wed, Nov 25
Add another comment
Add more comments and fix another test.
Tue, Nov 24
Fix tests and a typo
Mon, Nov 23
LGTM! But I'd like other folks to take a look at it first.
Oct 20 2020
Oct 19 2020
Great job, I think it's awesome to have more control over the analyzer with the language itself.
Sep 21 2020
Great job! I also wanted to support more operations for range-based logic.
After this "accepted" and a huge thank you 😄
I came up with exactly the same fix! Great job!
I just wanted to refactor it and not having
Sep 7 2020
Sep 2 2020
Can we cover __builtin_unique_stable_name as well?
Sep 1 2020
Aug 28 2020
Aug 27 2020
Not that it would matter much, but I was just wondering why there is a slightly bigger memory usage in some of the docker/small projects? The most conspicuous is oatpp.
The measurements fluctuate quite significantly and as you can see the means for both old and new experiments for oatpp are pretty close. So, I would say that the memory differences are not conclusive.
This being said, the performance measurements should also be taken with a grain of salt. We can't really say that we get 5-10% performance improvement for sure, but that analysis tends to be faster instead.
Fix review remarks
Aug 26 2020
Here are some benchmarking results.
Natively on my Mac (no memory measurements due to this issue during psutil installation):
Aug 25 2020
I'll add the charts with performance in the next couple of days
WDYT about moving introducing a generic "SmallImmutableSet" in llvm/ADT to back your implementation?
Oof, it is not really possible. First of all, existing RangeSet didn't really ensure that ranges do not intersect (add is/was not checking for that), and this is left for the user. Additionally, it is designed and optimized specifically for RangeSet queries and operations. Like intersect and contains in a more generic Set sense mean entirely different things.
Aug 24 2020
Awesome! I will submit the patch!
Aug 13 2020
Awesome, looks great!
Off-topic: I really think that we should have some sort of DSL for that kind of stuff.
Add "no crash" comments
Simplify the main test
Aug 12 2020
Awesome, thanks! Buuuut, maybe we can use \ref form, it looks like it's a preferred form in the codebase.
Aug 7 2020
Aug 6 2020
It is an interesting checker, but it seems that this kind of checker is extremely hard in single TU/top-down analysis. It feels like it's going to produce hell a lot of false positives in the wild.
Also mutex is usually a global variable - the nemesis of any static analysis tool.
Keep SATest.py Python2 compatible
Aug 5 2020
Hey, thanks again for cleaning up the analyzer's docs 😄
Great job, I'm on board with this change!
Other than parameter names, it looks totally reasonable to me.
Aug 4 2020
Aug 3 2020
Ready! Sorry, I remember, you've already told me.
Jul 31 2020
Hey, nice catch!
However, I'm going to complain about commit messages again 😅 I would prefer having imperative mood in the message, something like "Refactor ..." or "Introduce minor refactoring..."
Thanks for working on improving the quality of the codebase!
I again have to nitpick about the commit message, can you please change it to "Simplify ..."?
Jul 30 2020
Other then the naming issue, I don't see any problems with this change!
LGTM! Thanks for all the investigative work!
Jul 29 2020
LGTM! Thanks for the fast fix!
Jul 28 2020
Jul 23 2020
LGTM, thanks for taking care of this!
Jul 22 2020
Make --projects and --max-size compatible
Jul 20 2020
Jul 16 2020
The same thing with the commit message here, it would be better as just "Change"
It is a good practice in many projects to make commit messages into imperative (i.e. "Improve" instead of "Improving" or "Improved"). Again, not everyone follows it, but it is good to keep it consistent, right?
Additionally, I would prefer commit message to be imperative. It is sorta like a rule of a good commit message.