vrnithinkumar added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:147
- if (!move::isMovedFrom(State, ThisR)) {
- // TODO: Model this case as well. At least, avoid invalidation of
- // globals.
- return false;
+ if (ModelSmartPtrDereference) {
+ handleBoolOperation(Call, C);
----------------
xazax.hun wrote:
> vrnithinkumar wrote:
> > This seems little messy here.
> > I guess once we model the `std::move` for smart ptr it will be less messy
> I agree. `isNullAfterMoveMethod` is especially confusing as it does not do
> what the name of the function says. It checks if the `Call` is a boolean
> conversion operator. I'd suggest renaming that method to make this code a bit
> more sensible.
>
> Moreover, when `ModelSmartPtrDereference` is true, we no longer model moves
> below right? I think a comment that this logic is duplicated
> `handleBoolOperation` might make the code cleaner.
>
> But yeah, this needs to be cleaned up, hopefully really soon.
- Renamed the method to `isBooleanOpMethod`
- Added comments and FIXME
Hopefully we can clean up once `std::move` is modeled.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:389
+ }
+ C.addTransition(State);
+ } else if (move::isMovedFrom(State, ThisRegion)) {
----------------
xazax.hun wrote:
> This looks fine for now, but we often prefer adding a return after each case
> to avoid executing multiple `addTransition`s accidentally after refactoring.
Added return
================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:411
+ if (NotNullState) {
+ auto NonNullVal = C.getSValBuilder().makeTruthVal(true);
+ NotNullState =
----------------
NoQ wrote:
> vrnithinkumar wrote:
> > Since the inner pointer value can be any non-null value, I am not sure what
> > should be the value to be added to the map for tracking.
> >
> Don't add values, constrain existing values.
Made changes to create the conjured symbol and use that to constrain.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86027/new/
https://reviews.llvm.org/D86027
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
