whisperity added a comment.
In D81272#2218050 <https://reviews.llvm.org/D81272#2218050>, @aaron.ballman
wrote:
> Thanks to the new info, I think the check basically LGTM. Can you add some
> negative tests and documentation wording to make it clear that the check
> doesn't currently handle all logically equivalent predicates, like:
>
> if (foo) {
> } else {
> if (!foo) {
> }
> }
>
> // or
> if (foo > 5) {
> if (foo > 3) {
> }
> }
>
> // or
> if (foo > 5) {
> if (5 < foo) {
> }
> }
>
> (I'm assuming these cases aren't handled currently and that handling them
> isn't necessary to land the patch.)
I'm afraid handling such cases will eventually invoke the same problems the
RangeConstraintSolver has, and then the discussion about whether this is good
in Tidy instead of CSA will be resurrected... 😦
One could come up with even more elaborate cases. Just hit something like below
a few minutes ago while working:
const CallExpr* const C;
if (auto* CalledFun = dyn_cast_or_null<FunctionDecl>(C->getCalleeDecl())) {
// ...
if (const auto* Fun = dyn_cast_or_null<FunctionDecl>(C->getCalleeDecl())) {
// ...
}
// ...
}
and the rabbit hole goes deeper. But it's pretty interesting how many cases the
current check found as-is.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81272/new/
https://reviews.llvm.org/D81272
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
