NoQ added a comment. Looking good now! I still recommend eventually adding tests with casts of references.
In D65889#1620128 <https://reviews.llvm.org/D65889#1620128>, @Charusso wrote: > - May it is better to also check for `getAsCXXRecordDecl()` for obtaining a > class. In D65889#1621587 <https://reviews.llvm.org/D65889#1621587>, @Charusso wrote: > - The call as `getAsCXXRecordDecl()` sometimes run into an assertion failure, > so we need to avoid it Well, yeah, i was just about to say "good thing that you don't do this because we really don't want to get into business of modeling return-by-value `getAs` calls such as `SVal::getAs`". ================ Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:51 + + const CallDescriptionMap<CastCheck> SugarCastCDM = { + {{{"clang", "castAs"}, 0}, &CastValueChecker::evalCastAs}, ---------------- Charusso wrote: > NoQ wrote: > > I'd rather have only one map from call descriptions to (callback, kind) > > pairs. This should make the `evalCall` code much more readable. > The problem with that I really want to use something like that: > ``` > auto &&[Check, Kind, MoreData] = *Lookup; > ``` > but I cannot. Until that time it equally non-readable and difficult to scale > sadly. For now it is fine, but that C++17 feature would be more awesome. I think it turned out pretty pretty (no pun intended). ================ Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:26 class CastValueChecker : public Checker<eval::Call> { + enum class CastKind { Checking, Sugar }; + ---------------- Please explain "Checking" and "Sugar". Checking what? Sugar... you mean syntactic sugar? For what? ================ Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:220 - // If we cannot obtain both of the classes we cannot be sure how to model it. - if (!CE->getType()->getPointeeCXXRecordDecl() || - !CE->getArg(0)->getType()->getPointeeCXXRecordDecl()) - return false; + const CastCheck *Check = &Lookup->first; + CastKind Kind = Lookup->second; ---------------- What's the point of making this a raw pointer? (no pun intended) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65889/new/ https://reviews.llvm.org/D65889 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
