User Details
- User Since
- Mar 28 2022, 9:12 AM (52 w, 1 d)
Thu, Mar 9
Mon, Mar 6
Nice, thank you.
Fri, Mar 3
I'm happy to help fix up that one remaining issue (@sunho I might ask you for help and will definitely ask you for a review), would love to get this in :)
Thu, Mar 2
Tue, Feb 28
Nov 18 2022
Nov 15 2022
Nov 13 2022
Nov 12 2022
Nov 11 2022
Oct 14 2022
No consensus, closing this.
No consensus, closing this.
Oct 13 2022
LGTM with river's comment applied about the code comment. Thanks nick!
LGTM - I might clarify get to be getOldValue or something to make it easier on the reader at the callsite, but I don't have strong feelings about it.
Oct 12 2022
Thanks Nick :)
Oct 3 2022
LGTM, thanks River :)
Sep 28 2022
Thanks river :)
Sep 20 2022
Thanks River :)
Sep 6 2022
Sep 3 2022
Sep 2 2022
Sep 1 2022
Discourse thread here for folks that want to discuss more long-form: https://discourse.llvm.org/t/psa-swapping-out-or-null-with-if-present/65018
It's also worth noting that the _or_null variants are soft-deprecated (see comments on _or_null) and this is in preparation for actually marking them deprecated. IMO, we want one way to do the "if this value exists, do the dyn_cast/cast" and the if_present naming can apply equally well to pointers and optionals.
Aug 31 2022
Aug 30 2022
Aug 21 2022
Thanks for fixing the or_null to if_present too (where it makes sense). LGTM as long as everything passes CI :)
Aug 19 2022
Aug 17 2022
+1 to the concept, many thanks for this! I think it makes more sense to implement the member versions with the llvm-style versions, it should simplify the member style code because then you won't have to manually implement the classof/isa/cast innards it'd just be cast<T>(*this) or something.
Aug 10 2022
Aug 9 2022
Aug 8 2022
This is great, thank you for doing this! I'm not a competent reviewer for the actual clang-tidy code changes but the +1 for the idea :)
Jul 15 2022
Jul 13 2022
Jul 12 2022
Jul 10 2022
I had also considered a version of this patch that iterated the block with a set of refcounts for each value - that also works but this version felt a little closer to the way that the rest of the analysis is performed. I'm totally fine either way, happy to update the patch to use the other method.
Jun 28 2022
Thanks River :)
Jun 8 2022
Jun 6 2022
Jun 3 2022
Someone else already did this! great.
Jun 2 2022
May 23 2022
May 17 2022
Not a qualified reviewer for anything other than the llvm::isa usage and that line looks good to me!
Currently, we expect that casts result in regular SVal objects, instead of pointer-like objects , thus this code to compile:
NonLoc N = llvm::cast<NonLoc>(V), where V is of type SVal. I believe that is why I decided to make that change.
May 16 2022
@ftynse https://reviews.llvm.org/D125590 should fix this - can you please confirm and let me know if not?
May 15 2022
Thank you for working on this!