omtcyfz created this revision.
omtcyfz added reviewers: alexfh, klimek, ioeric.
omtcyfz added subscribers: cfe-commits, Eugene.Zelenko.
This patch extends clang-tidy's readability-redundant-smartptr-get to produce
warnings for previously unsupported cases:
```
std::unique_ptr<void> ptr;
if (ptr.get())
if (ptr.get() == NULL)
if (ptr.get() != NULL)
```
This is intended to fix https://llvm.org/bugs/show_bug.cgi?id=25804, a bug
report opened by @eugene.zelenko.
However, there still are cases not detected by the check. They can be found in
`void Negative()` function defined in
test/clang-tidy/readability-redundant-smartptr-get.cpp.
https://reviews.llvm.org/D24893
Files:
clang-tidy/readability/RedundantSmartptrGetCheck.cpp
test/clang-tidy/readability-redundant-smartptr-get.cpp
Index: test/clang-tidy/readability-redundant-smartptr-get.cpp
===================================================================
--- test/clang-tidy/readability-redundant-smartptr-get.cpp
+++ test/clang-tidy/readability-redundant-smartptr-get.cpp
@@ -113,6 +113,37 @@
// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant get() call
// CHECK-MESSAGES: i = *PointerWithOverloadedGet().get();
// CHECK-FIXES: i = *PointerWithOverloadedGet();
+
+ bb = std::unique_ptr<int>().get() == NULL;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant get() call
+ // CHECK-MESSAGES: bb = std::unique_ptr<int>().get() == NULL;
+ // CHECK-FIXES: bb = std::unique_ptr<int>() == NULL;
+ bb = ss->get() == NULL;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant get() call
+ // CHECK-MESSAGES: bb = ss->get() == NULL;
+ // CHECK-FIXES: bb = *ss == NULL;
+
+ std::unique_ptr<int> x, y;
+ if (x.get() == nullptr);
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant get() call
+ // CHECK-MESSAGES: if (x.get() == nullptr);
+ // CHECK-FIXES: if (x == nullptr);
+ if (nullptr == y.get());
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: redundant get() call
+ // CHECK-MESSAGES: if (nullptr == y.get());
+ // CHECK-FIXES: if (nullptr == y);
+ if (x.get() == NULL);
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant get() call
+ // CHECK-MESSAGES: if (x.get() == NULL);
+ // CHECK-FIXES: if (x == NULL);
+ if (NULL == x.get());
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: redundant get() call
+ // CHECK-MESSAGES: if (NULL == x.get());
+ // CHECK-FIXES: if (NULL == x);
+ if (x.get());
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant get() call
+ // CHECK-MESSAGES: if (x.get());
+ // CHECK-FIXES: if (x);
}
void Negative() {
@@ -137,7 +168,5 @@
(*Fail2().get()).Do();
int_ptr ip;
- bool bb = std::unique_ptr<int>().get() == NULL;
- bb = ip.get() == nullptr;
- bb = u->get() == NULL;
+ bool bb = ip.get() == nullptr;
}
Index: clang-tidy/readability/RedundantSmartptrGetCheck.cpp
===================================================================
--- clang-tidy/readability/RedundantSmartptrGetCheck.cpp
+++ clang-tidy/readability/RedundantSmartptrGetCheck.cpp
@@ -65,16 +65,22 @@
// Matches against nullptr.
Finder->addMatcher(
- binaryOperator(
- anyOf(hasOperatorName("=="), hasOperatorName("!=")),
- hasEitherOperand(ignoringImpCasts(cxxNullPtrLiteralExpr())),
- hasEitherOperand(callToGet(IsAKnownSmartptr))),
+ binaryOperator(anyOf(hasOperatorName("=="), hasOperatorName("!=")),
+ hasEitherOperand(ignoringImpCasts(
+ anyOf(cxxNullPtrLiteralExpr(), gnuNullExpr(),
+ integerLiteral(equals(0))))),
+ hasEitherOperand(callToGet(IsAKnownSmartptr))),
+ Callback);
+
+ // Matches against if(ptr.get())
+ Finder->addMatcher(
+ ifStmt(hasCondition(ignoringImpCasts(callToGet(IsAKnownSmartptr)))),
Callback);
- // TODO: Catch ptr.get() == other_ptr.get()
-}
+ // FIXME: Match and fix if (l.get() == r.get()).
+}
-} // namespace
+} // namespace
void RedundantSmartptrGetCheck::registerMatchers(MatchFinder *Finder) {
// Only register the matchers for C++; the functionality currently does not
@@ -102,10 +108,11 @@
Result.Nodes.getNodeAs<Type>("getType")->getUnqualifiedDesugaredType();
return OpArrowType == OpStarType && OpArrowType == GetType;
}
-} // namespace
+} // namespace
void RedundantSmartptrGetCheck::check(const MatchFinder::MatchResult &Result) {
- if (!allReturnTypesMatch(Result)) return;
+ if (!allReturnTypesMatch(Result))
+ return;
bool IsPtrToPtr = Result.Nodes.getNodeAs<Decl>("ptr_to_ptr") != nullptr;
bool IsMemberExpr = Result.Nodes.getNodeAs<Expr>("memberExpr") != nullptr;
Index: test/clang-tidy/readability-redundant-smartptr-get.cpp
===================================================================
--- test/clang-tidy/readability-redundant-smartptr-get.cpp
+++ test/clang-tidy/readability-redundant-smartptr-get.cpp
@@ -113,6 +113,37 @@
// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant get() call
// CHECK-MESSAGES: i = *PointerWithOverloadedGet().get();
// CHECK-FIXES: i = *PointerWithOverloadedGet();
+
+ bb = std::unique_ptr<int>().get() == NULL;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant get() call
+ // CHECK-MESSAGES: bb = std::unique_ptr<int>().get() == NULL;
+ // CHECK-FIXES: bb = std::unique_ptr<int>() == NULL;
+ bb = ss->get() == NULL;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant get() call
+ // CHECK-MESSAGES: bb = ss->get() == NULL;
+ // CHECK-FIXES: bb = *ss == NULL;
+
+ std::unique_ptr<int> x, y;
+ if (x.get() == nullptr);
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant get() call
+ // CHECK-MESSAGES: if (x.get() == nullptr);
+ // CHECK-FIXES: if (x == nullptr);
+ if (nullptr == y.get());
+ // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: redundant get() call
+ // CHECK-MESSAGES: if (nullptr == y.get());
+ // CHECK-FIXES: if (nullptr == y);
+ if (x.get() == NULL);
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant get() call
+ // CHECK-MESSAGES: if (x.get() == NULL);
+ // CHECK-FIXES: if (x == NULL);
+ if (NULL == x.get());
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: redundant get() call
+ // CHECK-MESSAGES: if (NULL == x.get());
+ // CHECK-FIXES: if (NULL == x);
+ if (x.get());
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant get() call
+ // CHECK-MESSAGES: if (x.get());
+ // CHECK-FIXES: if (x);
}
void Negative() {
@@ -137,7 +168,5 @@
(*Fail2().get()).Do();
int_ptr ip;
- bool bb = std::unique_ptr<int>().get() == NULL;
- bb = ip.get() == nullptr;
- bb = u->get() == NULL;
+ bool bb = ip.get() == nullptr;
}
Index: clang-tidy/readability/RedundantSmartptrGetCheck.cpp
===================================================================
--- clang-tidy/readability/RedundantSmartptrGetCheck.cpp
+++ clang-tidy/readability/RedundantSmartptrGetCheck.cpp
@@ -65,16 +65,22 @@
// Matches against nullptr.
Finder->addMatcher(
- binaryOperator(
- anyOf(hasOperatorName("=="), hasOperatorName("!=")),
- hasEitherOperand(ignoringImpCasts(cxxNullPtrLiteralExpr())),
- hasEitherOperand(callToGet(IsAKnownSmartptr))),
+ binaryOperator(anyOf(hasOperatorName("=="), hasOperatorName("!=")),
+ hasEitherOperand(ignoringImpCasts(
+ anyOf(cxxNullPtrLiteralExpr(), gnuNullExpr(),
+ integerLiteral(equals(0))))),
+ hasEitherOperand(callToGet(IsAKnownSmartptr))),
+ Callback);
+
+ // Matches against if(ptr.get())
+ Finder->addMatcher(
+ ifStmt(hasCondition(ignoringImpCasts(callToGet(IsAKnownSmartptr)))),
Callback);
- // TODO: Catch ptr.get() == other_ptr.get()
-}
+ // FIXME: Match and fix if (l.get() == r.get()).
+}
-} // namespace
+} // namespace
void RedundantSmartptrGetCheck::registerMatchers(MatchFinder *Finder) {
// Only register the matchers for C++; the functionality currently does not
@@ -102,10 +108,11 @@
Result.Nodes.getNodeAs<Type>("getType")->getUnqualifiedDesugaredType();
return OpArrowType == OpStarType && OpArrowType == GetType;
}
-} // namespace
+} // namespace
void RedundantSmartptrGetCheck::check(const MatchFinder::MatchResult &Result) {
- if (!allReturnTypesMatch(Result)) return;
+ if (!allReturnTypesMatch(Result))
+ return;
bool IsPtrToPtr = Result.Nodes.getNodeAs<Decl>("ptr_to_ptr") != nullptr;
bool IsMemberExpr = Result.Nodes.getNodeAs<Expr>("memberExpr") != nullptr;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
