[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

Charusso added a comment. Well, the rebase is a little-bit weird, sorry for the inconvenience. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67079/new/ https://reviews.llvm.org/D67079 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

Charusso updated this revision to Diff 222932. Charusso added a comment. - Rebase properly. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67079/new/ https://reviews.llvm.org/D67079 Files: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h clang/include/clang/

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

Charusso updated this revision to Diff 222931. Charusso added a comment. - Rebase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67079/new/ https://reviews.llvm.org/D67079 Files: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h clang/include/clang/StaticAna

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

Charusso updated this revision to Diff 222685. Charusso marked an inline comment as done. Charusso added a comment. - Use the TU's Decls instead of the gathered casts' Decls. - The math is still missing. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67079/new/ https://reviews.llvm.org/D

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

Charusso marked 4 inline comments as done. Charusso added inline comments. Comment at: clang/test/Analysis/cast-value-hierarchy-fp-suppression.cpp:25-27 + if (isa(a)) +if (isa(a)) + clang_analyzer_warnIfReached(); // no-warning NoQ wrote: > NoQ wrote: >

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

NoQ added inline comments. Comment at: clang/test/Analysis/cast-value-hierarchy-fp-suppression.cpp:25-27 + if (isa(a)) +if (isa(a)) + clang_analyzer_warnIfReached(); // no-warning NoQ wrote: > Charusso wrote: > > NoQ wrote: > > > Charusso wrote: > > > >

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

NoQ added inline comments. Comment at: clang/test/Analysis/cast-value-hierarchy-fp-suppression.cpp:25-27 + if (isa(a)) +if (isa(a)) + clang_analyzer_warnIfReached(); // no-warning Charusso wrote: > NoQ wrote: > > Charusso wrote: > > > NoQ wrote: > > > >

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

Charusso marked 3 inline comments as done. Charusso added inline comments. Comment at: clang/test/Analysis/cast-value-hierarchy-fp-suppression.cpp:25-27 + if (isa(a)) +if (isa(a)) + clang_analyzer_warnIfReached(); // no-warning NoQ wrote: > Charusso wro

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

NoQ added inline comments. Comment at: clang/test/Analysis/cast-value-hierarchy-fp-suppression.cpp:25-27 + if (isa(a)) +if (isa(a)) + clang_analyzer_warnIfReached(); // no-warning Charusso wrote: > NoQ wrote: > > Why is `(isa(a) && isa(a))` deemed possi

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

Charusso marked 2 inline comments as done. Charusso added a comment. In D67079#1688648 , @NoQ wrote: > In D67079#1687577 , @Charusso wrote: > > > - `CastVisitor` is the new facility which decides whether the assumpt

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

NoQ added a comment. In D67079#1687577 , @Charusso wrote: > - `CastVisitor` is the new facility which decides whether the assumptions are > appropriate. > - The math is still WIP. You need the math to decide whether delaying the decision to a visitor is

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

Charusso updated this revision to Diff 222370. Charusso marked 2 inline comments as done. Charusso added a comment. - When we have no information whether the cast succeeds or not we assume both. - `CastVisitor` is the new facility which decides whether the assumptions are appropriate. - The math

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/DynamicType.cpp:44 + +bool isDerivedFrom(QualType X, QualType Y) { + const CXXRecordDecl *XRD = X->getPointeeCXXRecordDecl(); Szelethus wrote: > Hmm, I think this function answers the ques

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/DynamicType.cpp:44 + +bool isDerivedFrom(QualType X, QualType Y) { + const CXXRecordDecl *XRD = X->getPointeeCXXRecordDecl(); Hmm, I think this function answers the question, at least in

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:126 + + // If the casts have a common anchestor it could not be a succeeded downcast. + for (const auto &PreviousBase : PreviousRD->bases()) NoQ wrote: > Charusso

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

Charusso updated this revision to Diff 221070. Charusso marked 9 inline comments as done. Charusso added a comment. - Try to do the math. - Create a consistent dynamic type API. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67079/new/ https://reviews.llvm.org/D67079 Files: clang/incl

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:126 + + // If the casts have a common anchestor it could not be a succeeded downcast. + for (const auto &PreviousBase : PreviousRD->bases()) Charusso wrote: > NoQ wrot

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:174 + +constexpr llvm::StringLiteral Vowels = "aeiou"; + Charusso wrote: > NoQ wrote: > > Omg lol nice. Did you try to figure out how do other people normally do it? > T

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

Charusso added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeInfo.h:24 + DynamicTypeInfo(QualType ty, bool CanBeSub = true) + : Ty(ty), CanBeASubClass(CanBeSub) {} NoQ wrote: > `Ty(Ty)` is the idiom here. G

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

Charusso updated this revision to Diff 219391. Charusso marked 5 inline comments as done. Charusso added a comment. - Fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67079/new/ https://reviews.llvm.org/D67079 Files: clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicCast

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/DynamicType.cpp:42 + return MR->StripCasts(); +} + NoQ wrote: > Charusso wrote: > > Probably a consistent way of `MemRegion` handling is more appropriate. > Pointer casts are quite a can

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/DynamicType.cpp:42 + return MR->StripCasts(); +} + Charusso wrote: > Probably a consistent way of `MemRegion` handling is more appropriate. Pointer casts are quite a can of worms. I suggest ign

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

NoQ added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeInfo.h:24 + DynamicTypeInfo(QualType ty, bool CanBeSub = true) + : Ty(ty), CanBeASubClass(CanBeSub) {} `Ty(Ty)` is the idiom here.

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

Charusso marked 2 inline comments as done. Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:211 }, - /*IsPrunable=*/true); + /*IsPrunable=*/!CastCtx.CastSucceeds); } That could be helpful, but I

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

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. The key is `CXXRecordDecl::isDerivedFrom()`. Repository: rC