[PATCH] D95714: [clang-tidy] fix modernize-use-nullptr false positive with spaceship operator comparisons

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

In D95714#2532957 <https://reviews.llvm.org/D95714#2532957>, @poelmanc wrote:

> Thoughts on which option seems like the best path forward?

I think you should be able to correctly match everything. Try a matcher like (can probably be cleaned up a bit):

  auto isOrHasDescendant = [](auto InnerMatcher) {
    return anyOf(InnerMatcher, hasDescendant(InnerMatcher));
  };
  
  return traverse(
      TK_AsIs,
      anyOf(castExpr(anyOf(ImplicitCastToNull,
                           explicitCastExpr(hasDescendant(ImplicitCastToNull))),
                     unless(hasAncestor(explicitCastExpr())),
                     unless(hasAncestor(cxxRewrittenBinaryOperator())))
                .bind(CastSequence),
            cxxRewrittenBinaryOperator(
                // Match rewritten operators, but verify (in the check method)
                // that if an implicit cast is found, it is not from another
                // nested rewritten operator
                expr().bind("matchBinopOperands"),
                hasEitherOperand(isOrHasDescendant(
                    implicitCastExpr(
                        ImplicitCastToNull,
                        hasAncestor(cxxRewrittenBinaryOperator().bind(
                            "checkBinopOperands")))
                        .bind(CastSequence))))));

ie, add an `anyOf`, ignore all descendants below `cxxRewrittenBinaryOperator` in the first part of the `anyOf`, and look only and `cxxRewrittenBinaryOperator` in the second part of it.

Here's another testcase for nested rewritten operators:

  result = (a1 > ((a1 > (ptr == 0 ? a1 : a2)) ? a1 : a2));
  // CHECK-FIXES: result = (a1 > ((a1 > (ptr == nullptr ? a1 : a2)) ? a1 : a2));




CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95714/new/

https://reviews.llvm.org/D95714