ymandel added a comment.
LGTM, but leaving for one of the other reviewers to give you the official
"Accept Revision".
================
Comment at:
clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp:72
+ binaryOperator(hasOperatorName("=="),
+ hasEitherOperand(ignoringParenImpCasts(StringNpos)),
+ hasEitherOperand(ignoringParenImpCasts(StringFind))),
----------------
tdl-g wrote:
> njames93 wrote:
> > ymandel wrote:
> > > Would `hasOperands` replace these two separate calls to
> > > `hasEitherOperand`? Same below lines 79-80 (I think it's a new matcher,
> > > fwiw).
> > Just added, I added it to remove calls of multiple hasEitherOperand calls
> > for the reason of preventing the matchers matching the same operand. I just
> > got a little lost in my previous comment
> Switched to hasOperands. I agree that expresses the intent better than two
> independent calls of hasEitherOperand.
>
> Note that it was working fine before--the test cases confirmed that it wasn't
> matching string::npos == string::npos or s.find(a) == s.find(a). I'm
> guessing that's because the matchers inside hasEitherOperand have bindings,
> and if the matcher matched but one of the bindings was missing, the
> rewriterule suppressed itself? Is this a plausible theory?
>
> (The question moot since hasOperands is better overall, but I'd like to
> understand why it was working before.)
I was wondering the same when I noticed this. I think its something like this:
one of the arguments falls into bucket 1, one of the arguments falls into
bucket 2. Since a single argument cannot simultaneously fall into two buckets
(the predicates are mutually exlusive), it must be the case that the arguments
are different.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80023/new/
https://reviews.llvm.org/D80023
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
