[clang-tidy] make readability-redundant-smartptr-get report get() usage in conditions

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.

Reviewers: alexfh

Differential Revision: https://reviews.llvm.org/D24893

llvm-svn: 282386
This commit is contained in:
Kirill Bobyrev 2016-09-26 07:22:37 +00:00
parent 839d15a194
commit 9559f04b9d
2 changed files with 50 additions and 14 deletions

View File

@ -60,21 +60,27 @@ void registerMatchersForGetEquals(MatchFinder *Finder,
// might be on global namespace or found by ADL, might be a template, etc.
// For now, lets keep a list of known standard types.
const auto IsAKnownSmartptr = recordDecl(
anyOf(hasName("::std::unique_ptr"), hasName("::std::shared_ptr")));
const auto IsAKnownSmartptr =
recordDecl(hasAnyName("::std::unique_ptr", "::std::shared_ptr"));
// 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);
// TODO: Catch ptr.get() == other_ptr.get()
// Matches against if(ptr.get())
Finder->addMatcher(
ifStmt(hasCondition(ignoringImpCasts(callToGet(IsAKnownSmartptr)))),
Callback);
// 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 @@ bool allReturnTypesMatch(const MatchFinder::MatchResult &Result) {
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;

View File

@ -113,6 +113,37 @@ void Positive() {
// 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 @@ void Negative() {
(*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;
}