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

[Conrad Poelman via Phabricator via cfe-commits]

poelmanc added a comment.

@njames93 Thank you so much for the quick feedback, I made your suggested 
changes and added a test that it properly converts `result = (a1 > (ptr == 0 ? 
a1 : a2));` to `result = (a1 > (ptr == nullptr ? a1 : a2));` now.

In these examples so far, checking the grandparent works. Here's the AST:

  $ clang -std=c++20 -Xclang -ast-dump modernize-use-nullptr-cxx20.cpp
  ...
      |-BinaryOperator 0x22d30bec4e0 <line:41:3, col:20> 'bool' lvalue '=' // 
result = (a1 < a2); // result = (a1 > (ptr == 0 ? a1 : a2));
      | |-DeclRefExpr 0x22d30be8960 <col:3> 'bool' lvalue Var 0x22d30be88e0 
'result' 'bool'
      | `-ParenExpr 0x22d30bec4c0 <col:12, col:20> 'bool'
      |   `-CXXRewrittenBinaryOperator 0x22d30bec4a8 <col:13, col:18> 'bool'
      |     `-CXXOperatorCallExpr 0x22d30bec470 <col:13, col:16> 'bool' '<' adl
      |       |-ImplicitCastExpr 0x22d30bec458 <col:16> 'bool (*)(const 
std::strong_ordering, std::strong_ordering::zero_type) noexcept' 
<FunctionToPointerDecay>
      |       | `-DeclRefExpr 0x22d30bec3d8 <col:16> 'bool (const 
std::strong_ordering, std::strong_ordering::zero_type) noexcept' lvalue 
Function 0x22d30ba9f28 'operator<' 'bool (const std::strong_ordering, 
std::strong_ordering::zero_type) noexcept'
      |       |-CXXOperatorCallExpr 0x22d30bec360 <col:13, col:18> 
'std::strong_ordering':'std::strong_ordering' '<=>'
      |       | |-ImplicitCastExpr 0x22d30bec348 <col:16> 'std::strong_ordering 
(*)(const A &) const noexcept' <FunctionToPointerDecay>
      |       | | `-DeclRefExpr 0x22d30be8a08 <col:16> 'std::strong_ordering 
(const A &) const noexcept' lvalue CXXMethod 0x22d30bedcd8 'operator<=>' 
'std::strong_ordering (const A &) const noexcept'
      |       | |-ImplicitCastExpr 0x22d30be89f0 <col:13> 'const A' lvalue 
<NoOp>
      |       | | `-DeclRefExpr 0x22d30be8998 <col:13> 'A' lvalue Var 
0x22d30be8290 'a1' 'A'
      |       | `-ImplicitCastExpr 0x22d30be89d8 <col:18> 'const A' lvalue 
<NoOp>
      |       |   `-DeclRefExpr 0x22d30be89b8 <col:18> 'A' lvalue Var 
0x22d30be87f0 'a2' 'A'
      |       `-ImplicitCastExpr 0x22d30bec3c0 <col:16> 
'std::strong_ordering::zero_type':'nullptr_t' <NullToPointer> // Auto-generated 
cast we should ignore
      |         `-IntegerLiteral 0x22d30bec398 <col:16> 'int' 0
  ...
      `-BinaryOperator 0x22d30becb20 <line:48:3, col:38> 'bool' lvalue '='
        |-DeclRefExpr 0x22d30bec830 <col:3> 'bool' lvalue Var 0x22d30be88e0 
'result' 'bool'
        `-ParenExpr 0x22d30becb00 <col:12, col:38> 'bool'
          `-CXXRewrittenBinaryOperator 0x22d30becae8 <col:13, col:37> 'bool'
            `-CXXOperatorCallExpr 0x22d30becab0 <col:13, col:16> 'bool' '>' adl
              |-ImplicitCastExpr 0x22d30beca98 <col:16> 'bool (*)(const 
std::strong_ordering, std::strong_ordering::zero_type) noexcept' 
<FunctionToPointerDecay>
              | `-DeclRefExpr 0x22d30beca78 <col:16> 'bool (const 
std::strong_ordering, std::strong_ordering::zero_type) noexcept' lvalue 
Function 0x22d30baa1a0 'operator>' 'bool (const std::strong_ordering, 
std::strong_ordering::zero_type) noexcept'
              |-CXXOperatorCallExpr 0x22d30beca00 <col:13, col:37> 
'std::strong_ordering':'std::strong_ordering' '<=>'
              | |-ImplicitCastExpr 0x22d30bec9e8 <col:16> 'std::strong_ordering 
(*)(const A &) const noexcept' <FunctionToPointerDecay>
              | | `-DeclRefExpr 0x22d30bec9c8 <col:16> 'std::strong_ordering 
(const A &) const noexcept' lvalue CXXMethod 0x22d30bedcd8 'operator<=>' 
'std::strong_ordering (const A &) const noexcept'
              | |-ImplicitCastExpr 0x22d30bec9b0 <col:13> 'const A' lvalue 
<NoOp>
              | | `-DeclRefExpr 0x22d30bec850 <col:13> 'A' lvalue Var 
0x22d30be8290 'a1' 'A'
              | `-ImplicitCastExpr 0x22d30bec998 <col:18, col:37> 'const A' 
lvalue <NoOp>
              |   `-ParenExpr 0x22d30bec978 <col:18, col:37> 'A' lvalue
              |     `-ConditionalOperator 0x22d30bec948 <col:19, col:35> 'A' 
lvalue
              |       |-BinaryOperator 0x22d30bec8e8 <col:19, col:26> 'bool' 
'=='
              |       | |-ImplicitCastExpr 0x22d30bec8b8 <col:19> 'int *' 
<LValueToRValue>
              |       | | `-DeclRefExpr 0x22d30bec870 <col:19> 'int *' lvalue 
Var 0x22d30bec758 'ptr' 'int *'
              |       | `-ImplicitCastExpr 0x22d30bec8d0 <col:26> 'int *' 
<NullToPointer> // ptr == 0 cast that we want to convert to nullptr
              |       |   `-IntegerLiteral 0x22d30bec890 <col:26> 'int' 0
              |       |-DeclRefExpr 0x22d30bec908 <col:30> 'A' lvalue Var 
0x22d30be8290 'a1' 'A'
              |       `-DeclRefExpr 0x22d30bec928 <col:35> 'A' lvalue Var 
0x22d30be87f0 'a2' 'A'
              `-ImplicitCastExpr 0x22d30beca60 <col:16> 
'std::strong_ordering::zero_type':'nullptr_t' <NullToPointer> // Auto-generated 
cast we should ignore
                `-IntegerLiteral 0x22d30beca38 <col:16> 'int' 0

This looks a little bit different from your AST but I'm not sure what code your 
AST was generated from. If you have a test case I'd be happy to take a look. 
Thanks!


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

https://reviews.llvm.org/D95714

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits