fgross created this revision.
fgross added reviewers: aaron.ballman, alexfh, sbenza.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
The MSVC STL defines smart pointer `operator*` and `operator->` as method
templates. The existing duck typing implementation doesn't catch this
implementation. To keep the duck typing simple, I added the already existing
list of known standard to the matchers in `registerMatchersForGetArrowStart`.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D61209
Files:
clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp
clang-tools-extra/test/clang-tidy/readability-redundant-smartptr-get-msvc.cpp
Index: clang-tools-extra/test/clang-tidy/readability-redundant-smartptr-get-msvc.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/readability-redundant-smartptr-get-msvc.cpp
@@ -0,0 +1,95 @@
+// RUN: %check_clang_tidy %s readability-redundant-smartptr-get %t
+
+#define NULL __null
+
+namespace std {
+
+// MSVC headers define operator templates instead of plain operators.
+
+template <typename T>
+struct unique_ptr {
+ template <typename T2 = T>
+ T2& operator*() const;
+ template <typename T2 = T>
+ T2* operator->() const;
+ T* get() const;
+ explicit operator bool() const noexcept;
+};
+
+template <typename T>
+struct shared_ptr {
+ template <typename T2 = T>
+ T2& operator*() const;
+ template <typename T2 = T>
+ T2* operator->() const;
+ T* get() const;
+ explicit operator bool() const noexcept;
+};
+
+} // namespace std
+
+struct Bar {
+ void Do();
+ void ConstDo() const;
+};
+
+void Positive() {
+
+ std::unique_ptr<Bar>* up;
+ (*up->get()).Do();
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant get() call
+ // CHECK-MESSAGES: (*up->get()).Do();
+ // CHECK-FIXES: (**up).Do();
+
+ std::unique_ptr<int> uu;
+ std::shared_ptr<double> *ss;
+ bool bb = uu.get() == nullptr;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: redundant get() call
+ // CHECK-MESSAGES: uu.get() == nullptr;
+ // CHECK-FIXES: bool bb = uu == nullptr;
+
+ if (up->get());
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant get() call
+ // CHECK-MESSAGES: if (up->get());
+ // CHECK-FIXES: if (*up);
+ if ((uu.get()));
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant get() call
+ // CHECK-MESSAGES: if ((uu.get()));
+ // CHECK-FIXES: if ((uu));
+ bb = !ss->get();
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant get() call
+ // CHECK-MESSAGES: bb = !ss->get();
+ // CHECK-FIXES: bb = !*ss;
+
+ bb = nullptr != ss->get();
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: redundant get() call
+ // CHECK-MESSAGES: nullptr != ss->get();
+ // CHECK-FIXES: bb = nullptr != *ss;
+
+ 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);
+}
\ No newline at end of file
Index: clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp
@@ -30,6 +30,10 @@
.bind("redundant_get");
}
+internal::Matcher<Decl> knownSmartptr() {
+ return recordDecl(hasAnyName("::std::unique_ptr", "::std::shared_ptr"));
+}
+
void registerMatchersForGetArrowStart(MatchFinder *Finder,
MatchFinder::MatchCallback *Callback) {
const auto QuacksLikeASmartptr = recordDecl(
@@ -39,21 +43,21 @@
has(cxxMethodDecl(hasName("operator*"), returns(qualType(references(
type().bind("op*Type")))))));
+ // Make sure we are not missing the known standard types
+ const auto Smartptr = anyOf(knownSmartptr(), QuacksLikeASmartptr);
+
// Catch 'ptr.get()->Foo()'
Finder->addMatcher(memberExpr(expr().bind("memberExpr"), isArrow(),
- hasObjectExpression(ignoringImpCasts(
- callToGet(QuacksLikeASmartptr)))),
+ hasObjectExpression(ignoringImpCasts(callToGet(Smartptr)))),
Callback);
// Catch '*ptr.get()' or '*ptr->get()'
Finder->addMatcher(
- unaryOperator(hasOperatorName("*"),
- hasUnaryOperand(callToGet(QuacksLikeASmartptr))),
+ unaryOperator(hasOperatorName("*"), hasUnaryOperand(callToGet(Smartptr))),
Callback);
// Catch '!ptr.get()'
- const auto CallToGetAsBool = ignoringParenImpCasts(callToGet(recordDecl(
- QuacksLikeASmartptr, has(cxxConversionDecl(returns(booleanType()))))));
+ const auto CallToGetAsBool = ignoringParenImpCasts(callToGet(recordDecl(Smartptr, has(cxxConversionDecl(returns(booleanType()))))));
Finder->addMatcher(
unaryOperator(hasOperatorName("!"), hasUnaryOperand(CallToGetAsBool)),
Callback);
@@ -71,10 +75,7 @@
// This one is harder to do with duck typing.
// The operator==/!= that we are looking for might be member or non-member,
// 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(hasAnyName("::std::unique_ptr", "::std::shared_ptr"));
+ // For now, lets keep it to the known standard types.
// Matches against nullptr.
Finder->addMatcher(
@@ -82,7 +83,7 @@
hasEitherOperand(ignoringImpCasts(
anyOf(cxxNullPtrLiteralExpr(), gnuNullExpr(),
integerLiteral(equals(0))))),
- hasEitherOperand(callToGet(IsAKnownSmartptr))),
+ hasEitherOperand(callToGet(knownSmartptr()))),
Callback);
// FIXME: Match and fix if (l.get() == r.get()).
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
