[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

This revision was automatically updated to reflect the committed changes. Closed by commit rL368383: [analyzer] CastValueChecker: Model castAs(), getAs() (authored by Charusso, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https:

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso added a comment. Thanks you for the review! 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/lis

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:26 class CastValueChecker : public Checker { + enum class CastKind { Checking, Sugar }; + NoQ wrote: > Charusso wrote: > > NoQ wrote: > > > Please explain "Checki

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso updated this revision to Diff 214277. Charusso marked 6 inline comments as done. Charusso added a comment. - Better naming. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65889/new/ https://reviews.llvm.org/D65889 Files: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Aha, ok! Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:26 class CastValueChecker : public Checker { + enum class CastKind { Checking, Sugar }; +

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:26 class CastValueChecker : public Checker { + enum class CastKind { Checking, Sugar }; + NoQ wrote: > Please explain "Checking" and "Sugar". Checking what? Sugar

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso updated this revision to Diff 214234. Charusso marked 7 inline comments as done. Charusso added a comment. - Added a test case for casting *to* a reference. - Better naming. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65889/new/ https://reviews.llvm.org/D65889 Files: clang

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:94 + [CastFromName, CastToName, IsNullReturn, + IsSimpleCast](BugReport &) -> std::string { SmallString<128> Msg; `IsDynamicCast`.

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

NoQ added a comment. Looking good now! I still recommend eventually adding tests with casts of references. In D65889#1620128 , @Charusso wrote: > - May it is better to also check for `getAsCXXRecordDecl()` for obtaining a > class. In D65889#1621587

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso updated this revision to Diff 214186. Charusso added a comment. - The call as `getAsCXXRecordDecl()` sometimes run into an assertion failure, so we need to avoid it: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:333: ExprEngine::createTemporaryRegionIfNeeded(): Assertion `!InitValW

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso updated this revision to Diff 214041. Charusso added a comment. - May it is better to also check for `getAsCXXRecordDecl()` for obtaining a class. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65889/new/ https://reviews.llvm.org/D65889 Files: clang/lib/StaticAnalyzer/Checke

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso updated this revision to Diff 214038. Charusso added a comment. - Fix a comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65889/new/ https://reviews.llvm.org/D65889 Files: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp clang/test/Analysis/cast-value.cpp Index

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:51 + + const CallDescriptionMap SugarCastCDM = { + {{{"clang", "castAs"}, 0}, &CastValueChecker::evalCastAs}, NoQ wrote: > I'd rather have only one map from c

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso updated this revision to Diff 214037. Charusso marked 9 inline comments as done. Charusso added a comment. - Make it usable with references. - Test references. - Better messages on simple `cast<>`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65889/new/ https://reviews.llvm.or

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:51 + + const CallDescriptionMap SugarCastCDM = { + {{{"clang", "castAs"}, 0}, &CastValueChecker::evalCastAs}, I'd rather have only one map from call descriptions t

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Szelethus added a comment. Cheers! :) 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-commi

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:111 +QualType Ty = CE->getCallReturnType(C.getASTContext()); +V = C.getSValBuilder().makeTruthVal(true, Ty); + } NoQ wrote: > Charusso wrote: > > NoQ wrote

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso updated this revision to Diff 213952. Charusso marked 5 inline comments as done. Charusso added a comment. - Remove symbol-conjuring. - Add a forgotten test case. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65889/new/ https://reviews.llvm.org/D65889 Files: clang/lib/Static

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:111 +QualType Ty = CE->getCallReturnType(C.getASTContext()); +V = C.getSValBuilder().makeTruthVal(true, Ty); + } Charusso wrote: > NoQ wrote: > > Charusso wrote

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:111 +QualType Ty = CE->getCallReturnType(C.getASTContext()); +V = C.getSValBuilder().makeTruthVal(true, Ty); + } NoQ wrote: > Charusso wrote: > > That is a

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:111 +QualType Ty = CE->getCallReturnType(C.getASTContext()); +V = C.getSValBuilder().makeTruthVal(true, Ty); + } Charusso wrote: > That is a very lame way to co

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso marked 4 inline comments as done. Charusso added a comment. This is a little-bit WIP as the symbol conjuring is very naive. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:111 +QualType Ty = CE->getCallReturnType(C.getASTContext()); +V = C.ge

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Thanks to Kristóf Umann for the great idea! Repository: rC