hokein created this revision. Herald added a subscriber: JDevlieghere. The false positive happens on two neighbour CXXDefaultArgExpr AST nodes. like below:
CXXFunctionalCastExpr 0x85c9670 <col:7, col:23> 'struct ZZ' functional cast to struct ZZ <ConstructorConversion> `-CXXConstructExpr 0x85c9518 <col:7, col:23> 'struct ZZ' 'void (uint64, const uint64 *)' | -CallExpr 0x85a0a90 <col:10, col:22> 'uint64':'unsigned long long' | | | -ImplicitCastExpr 0x85a0a78 <col:10> 'uint64 (*)(uint64)' <FunctionToPointerDecay> | | | `-DeclRefExpr 0x85a09f0 <col:10> 'uint64 (uint64)' lvalue Function 0x85a06a0 'Hash' 'uint64 (uint64)' | | `-CXXDefaultArgExpr 0x85a0ac8 <<invalid sloc>> 'uint64':'unsigned long long' | `-CXXDefaultArgExpr 0x85c94f8 <<invalid sloc>> 'const uint64 *' For each particular CXXDefaultArgExpr node, we need to reset FirstSubExpr, otherwise FirstSubExpr will refer to an incorrect expr. https://reviews.llvm.org/D30412 Files: clang-tidy/modernize/UseNullptrCheck.cpp test/clang-tidy/modernize-use-nullptr.cpp Index: test/clang-tidy/modernize-use-nullptr.cpp =================================================================== --- test/clang-tidy/modernize-use-nullptr.cpp +++ test/clang-tidy/modernize-use-nullptr.cpp @@ -228,3 +228,19 @@ void test_default_argument() { D(nullptr); } + +// Test on two neighbour CXXDefaultArgExprs nodes. +typedef unsigned long long uint64; +struct ZZ { + explicit ZZ(uint64, const uint64* = NULL) {} +// CHECK-MESSAGES: :[[@LINE-1]]:39: warning: use nullptr +// CHECK-FIXES: explicit ZZ(uint64, const uint64* = nullptr) {} + operator bool() { return true; } +}; + +uint64 Hash(uint64 seed = 0) { return 0; } + +void f() { + bool a; + a = ZZ(Hash()); +} Index: clang-tidy/modernize/UseNullptrCheck.cpp =================================================================== --- clang-tidy/modernize/UseNullptrCheck.cpp +++ clang-tidy/modernize/UseNullptrCheck.cpp @@ -190,8 +190,10 @@ bool VisitStmt(Stmt *S) { auto *C = dyn_cast<CastExpr>(S); // Catch the castExpr inside cxxDefaultArgExpr. - if (auto *E = dyn_cast<CXXDefaultArgExpr>(S)) + if (auto *E = dyn_cast<CXXDefaultArgExpr>(S)) { C = dyn_cast<CastExpr>(E->getExpr()); + FirstSubExpr = nullptr; + } if (!C) { FirstSubExpr = nullptr; return true;
Index: test/clang-tidy/modernize-use-nullptr.cpp =================================================================== --- test/clang-tidy/modernize-use-nullptr.cpp +++ test/clang-tidy/modernize-use-nullptr.cpp @@ -228,3 +228,19 @@ void test_default_argument() { D(nullptr); } + +// Test on two neighbour CXXDefaultArgExprs nodes. +typedef unsigned long long uint64; +struct ZZ { + explicit ZZ(uint64, const uint64* = NULL) {} +// CHECK-MESSAGES: :[[@LINE-1]]:39: warning: use nullptr +// CHECK-FIXES: explicit ZZ(uint64, const uint64* = nullptr) {} + operator bool() { return true; } +}; + +uint64 Hash(uint64 seed = 0) { return 0; } + +void f() { + bool a; + a = ZZ(Hash()); +} Index: clang-tidy/modernize/UseNullptrCheck.cpp =================================================================== --- clang-tidy/modernize/UseNullptrCheck.cpp +++ clang-tidy/modernize/UseNullptrCheck.cpp @@ -190,8 +190,10 @@ bool VisitStmt(Stmt *S) { auto *C = dyn_cast<CastExpr>(S); // Catch the castExpr inside cxxDefaultArgExpr. - if (auto *E = dyn_cast<CXXDefaultArgExpr>(S)) + if (auto *E = dyn_cast<CXXDefaultArgExpr>(S)) { C = dyn_cast<CastExpr>(E->getExpr()); + FirstSubExpr = nullptr; + } if (!C) { FirstSubExpr = nullptr; return true;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits