vsavchenko added a comment. In D100852#2704282 <https://reviews.llvm.org/D100852#2704282>, @NoQ wrote:
> I think this is already way better than before. I see that `OSDynamicCast` > isn't supported yet and we seem to hit the same wall as D97183 > <https://reviews.llvm.org/D97183> because inter-operation between > `trackExpressionValue` and the checkers isn't implemented yet. Let's say that `OSDynamicCast` isn't **fully** supported. If we have a chain of bindings `var1 -> var2 -> ... -> varN` and `varM -> varM+1` is a dynamic cast assignment, we will create a note that `varM+1` is assigned/initialized here (as you can see in tests), but not further down the chain. `check_dynamic_cast_alias_intermediate` is a good example here. We still have work to do, but in the most common case where `var1 -> var2` is the whole chain, we work as expected. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp:636 if (isa<NonParamVarRegion>(R)) - Result.push_back(R); + Result.emplace_back(R, Val); ---------------- NoQ wrote: > Because we already know that `Val.getAsLocSymbol()` is equal to `Sym`, we can > be certain that `Val` is either a `&SymRegion{Sym}` (i.e., literally this > symbol in its pristine representation) or, in some rare cases, a > `LocAsInteger` over that (which is a case i'm not sure we even want to > handle). I dunno if we really need to return it here. Maybe it's easier to > re-construct it in place as `SValBuilder.makeLoc(Sym)`. Oh, that was my original solution, but it doesn't work for dynamic casts. This check compares symbols, but `FindLastStore` compares `SVal`s. `&Derived{&SymRegion}` is not the same as `&SymRegion` and we fail. So, instead of hacking on the result from `SValBuilder` trying to make something that will indeed match the last stored value, I save that last stored value here, so we're guaranteed that `FindLastStore` will work as intended. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp:978 + + if (auto KV = AllVarBindings[0].second.getAs<KnownSVal>()) + // Because 'AllocBindingToReport' is not the the same as ---------------- NoQ wrote: > In particular, this check is always true and you can use `castAs`. Yep, makes sense! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100852/new/ https://reviews.llvm.org/D100852 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
