[PATCH] D33827: [clang-tidy] misc-static-assert shouldn't flag assert(!"msg")
fgross created this revision.
Herald added a subscriber: xazax.hun.
Added negated string literals to the set of IsAlwaysFalse expressions to avoid
flagging of
assert(!"msg")
https://reviews.llvm.org/D33827
Files:
clang-tidy/misc/StaticAssertCheck.cpp
test/clang-tidy/misc-static-assert.cpp
Index: test/clang-tidy/misc-static-assert.cpp
===
--- test/clang-tidy/misc-static-assert.cpp
+++ test/clang-tidy/misc-static-assert.cpp
@@ -76,6 +76,9 @@
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: found assert() that could be
// CHECK-FIXES: {{^ }}static_assert(ZERO_MACRO, "");
+ assert(!"Don't report me!");
+ // CHECK-FIXES: {{^ }}assert(!"Don't report me!");
+
assert(0 && "Don't report me!");
// CHECK-FIXES: {{^ }}assert(0 && "Don't report me!");
Index: clang-tidy/misc/StaticAssertCheck.cpp
===
--- clang-tidy/misc/StaticAssertCheck.cpp
+++ clang-tidy/misc/StaticAssertCheck.cpp
@@ -33,9 +33,11 @@
if (!(getLangOpts().CPlusPlus11 || getLangOpts().C11))
return;
+ auto NegatedString = unaryOperator(
+ hasOperatorName("!"),
hasUnaryOperand(ignoringImpCasts(stringLiteral(;
auto IsAlwaysFalse =
expr(anyOf(cxxBoolLiteral(equals(false)), integerLiteral(equals(0)),
- cxxNullPtrLiteralExpr(), gnuNullExpr()))
+ cxxNullPtrLiteralExpr(), gnuNullExpr(), NegatedString))
.bind("isAlwaysFalse");
auto IsAlwaysFalseWithCast = ignoringParenImpCasts(anyOf(
IsAlwaysFalse, cStyleCastExpr(has(ignoringParenImpCasts(IsAlwaysFalse)))
Index: test/clang-tidy/misc-static-assert.cpp
===
--- test/clang-tidy/misc-static-assert.cpp
+++ test/clang-tidy/misc-static-assert.cpp
@@ -76,6 +76,9 @@
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: found assert() that could be
// CHECK-FIXES: {{^ }}static_assert(ZERO_MACRO, "");
+ assert(!"Don't report me!");
+ // CHECK-FIXES: {{^ }}assert(!"Don't report me!");
+
assert(0 && "Don't report me!");
// CHECK-FIXES: {{^ }}assert(0 && "Don't report me!");
Index: clang-tidy/misc/StaticAssertCheck.cpp
===
--- clang-tidy/misc/StaticAssertCheck.cpp
+++ clang-tidy/misc/StaticAssertCheck.cpp
@@ -33,9 +33,11 @@
if (!(getLangOpts().CPlusPlus11 || getLangOpts().C11))
return;
+ auto NegatedString = unaryOperator(
+ hasOperatorName("!"), hasUnaryOperand(ignoringImpCasts(stringLiteral(;
auto IsAlwaysFalse =
expr(anyOf(cxxBoolLiteral(equals(false)), integerLiteral(equals(0)),
- cxxNullPtrLiteralExpr(), gnuNullExpr()))
+ cxxNullPtrLiteralExpr(), gnuNullExpr(), NegatedString))
.bind("isAlwaysFalse");
auto IsAlwaysFalseWithCast = ignoringParenImpCasts(anyOf(
IsAlwaysFalse, cStyleCastExpr(has(ignoringParenImpCasts(IsAlwaysFalse)))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33827: [clang-tidy] misc-static-assert shouldn't flag assert(!"msg")
This revision was automatically updated to reflect the committed changes.
Closed by commit rL304657: [clang-tidy] Make misc-static-assert accept
assert(!"msg") (authored by fgross).
Changed prior to commit:
https://reviews.llvm.org/D33827?vs=101197&id=101311#toc
Repository:
rL LLVM
https://reviews.llvm.org/D33827
Files:
clang-tools-extra/trunk/clang-tidy/misc/StaticAssertCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/misc-static-assert.cpp
Index: clang-tools-extra/trunk/test/clang-tidy/misc-static-assert.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/misc-static-assert.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/misc-static-assert.cpp
@@ -76,6 +76,9 @@
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: found assert() that could be
// CHECK-FIXES: {{^ }}static_assert(ZERO_MACRO, "");
+ assert(!"Don't report me!");
+ // CHECK-FIXES: {{^ }}assert(!"Don't report me!");
+
assert(0 && "Don't report me!");
// CHECK-FIXES: {{^ }}assert(0 && "Don't report me!");
Index: clang-tools-extra/trunk/clang-tidy/misc/StaticAssertCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/misc/StaticAssertCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/misc/StaticAssertCheck.cpp
@@ -33,9 +33,11 @@
if (!(getLangOpts().CPlusPlus11 || getLangOpts().C11))
return;
+ auto NegatedString = unaryOperator(
+ hasOperatorName("!"),
hasUnaryOperand(ignoringImpCasts(stringLiteral(;
auto IsAlwaysFalse =
expr(anyOf(cxxBoolLiteral(equals(false)), integerLiteral(equals(0)),
- cxxNullPtrLiteralExpr(), gnuNullExpr()))
+ cxxNullPtrLiteralExpr(), gnuNullExpr(), NegatedString))
.bind("isAlwaysFalse");
auto IsAlwaysFalseWithCast = ignoringParenImpCasts(anyOf(
IsAlwaysFalse, cStyleCastExpr(has(ignoringParenImpCasts(IsAlwaysFalse)))
Index: clang-tools-extra/trunk/test/clang-tidy/misc-static-assert.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/misc-static-assert.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/misc-static-assert.cpp
@@ -76,6 +76,9 @@
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: found assert() that could be
// CHECK-FIXES: {{^ }}static_assert(ZERO_MACRO, "");
+ assert(!"Don't report me!");
+ // CHECK-FIXES: {{^ }}assert(!"Don't report me!");
+
assert(0 && "Don't report me!");
// CHECK-FIXES: {{^ }}assert(0 && "Don't report me!");
Index: clang-tools-extra/trunk/clang-tidy/misc/StaticAssertCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/misc/StaticAssertCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/misc/StaticAssertCheck.cpp
@@ -33,9 +33,11 @@
if (!(getLangOpts().CPlusPlus11 || getLangOpts().C11))
return;
+ auto NegatedString = unaryOperator(
+ hasOperatorName("!"), hasUnaryOperand(ignoringImpCasts(stringLiteral(;
auto IsAlwaysFalse =
expr(anyOf(cxxBoolLiteral(equals(false)), integerLiteral(equals(0)),
- cxxNullPtrLiteralExpr(), gnuNullExpr()))
+ cxxNullPtrLiteralExpr(), gnuNullExpr(), NegatedString))
.bind("isAlwaysFalse");
auto IsAlwaysFalseWithCast = ignoringParenImpCasts(anyOf(
IsAlwaysFalse, cStyleCastExpr(has(ignoringParenImpCasts(IsAlwaysFalse)))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61209: Fix readability-redundant-smartptr-get for MSVC STL
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
+struct unique_ptr {
+ template
+ T2& operator*() const;
+ template
+ T2* operator->() const;
+ T* get() const;
+ explicit operator bool() const noexcept;
+};
+
+template
+struct shared_ptr {
+ template
+ T2& operator*() const;
+ template
+ 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* 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 uu;
+ std::shared_ptr *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().get() == NULL;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant get() call
+ // CHECK-MESSAGES: bb = std::unique_ptr().get() == NULL;
+ // CHECK-FIXES: bb = std::unique_ptr() == 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 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 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(
-
[PATCH] D61209: [clang-tidy] Fix readability-redundant-smartptr-get for MSVC STL
fgross updated this revision to Diff 196911.
fgross added a comment.
Fixed format
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61209/new/
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
+struct unique_ptr {
+ template
+ T2& operator*() const;
+ template
+ T2* operator->() const;
+ T* get() const;
+ explicit operator bool() const noexcept;
+};
+
+template
+struct shared_ptr {
+ template
+ T2& operator*() const;
+ template
+ 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* 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 uu;
+ std::shared_ptr *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().get() == NULL;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant get() call
+ // CHECK-MESSAGES: bb = std::unique_ptr().get() == NULL;
+ // CHECK-FIXES: bb = std::unique_ptr() == 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 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 knownSmartptr() {
+ return recordDecl(hasAnyName("::std::unique_ptr", "::std::shared_ptr"));
+}
+
void registerMatchersForGetArrowStart(MatchFinder *Finder,
MatchFinder::MatchCallback *Callback) {
const auto QuacksLikeASmartptr = recordDecl(
@@ -39,21 +43,23 @@
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,
- Callback);
+ Finder->addMatcher(
+ memberExpr(expr().bind("memberExpr"), isArrow(),
+ hasObjectExpression(ignoringImpCasts(callToGet(Smartptr,
+ Callback);
// Catch '*ptr.get()' or '*ptr->get()'
Finder->addMatcher(
-
[PATCH] D61209: [clang-tidy] Fix readability-redundant-smartptr-get for MSVC STL
fgross updated this revision to Diff 196960.
fgross added a comment.
Fixed test file format
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61209/new/
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,94 @@
+// 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
+struct unique_ptr {
+ template
+ T2& operator*() const;
+ template
+ T2* operator->() const;
+ T* get() const;
+ explicit operator bool() const noexcept;
+};
+
+template
+struct shared_ptr {
+ template
+ T2& operator*() const;
+ template
+ 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* 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 uu;
+ std::shared_ptr *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().get() == NULL;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant get() call
+ // CHECK-MESSAGES: bb = std::unique_ptr().get() == NULL;
+ // CHECK-FIXES: bb = std::unique_ptr() == 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 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);
+}
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 knownSmartptr() {
+ return recordDecl(hasAnyName("::std::unique_ptr", "::std::shared_ptr"));
+}
+
void registerMatchersForGetArrowStart(MatchFinder *Finder,
MatchFinder::MatchCallback *Callback) {
const auto QuacksLikeASmartptr = recordDecl(
@@ -39,21 +43,23 @@
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,
- Callback);
+ Finder->addMatcher(
+ memberExpr(expr().bind("memberExpr"), isArrow(),
+ hasObjectExpression(ignoringImpCasts(callToGet(Smartptr,
+ Callback);
// Catch '*ptr.get()' or '*ptr->get()'
Finder->addMatcher(
- unaryOperator(hasO
[PATCH] D61209: [clang-tidy] Fix readability-redundant-smartptr-get for MSVC STL
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE359801: Fixed: Duck-typing in
readability-redundant-smartptr-get didn't catch MSVC STL… (authored by
fgross, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D61209?vs=196960&id=197809#toc
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61209/new/
https://reviews.llvm.org/D61209
Files:
clang-tidy/readability/RedundantSmartptrGetCheck.cpp
test/clang-tidy/readability-redundant-smartptr-get-msvc.cpp
Index: test/clang-tidy/readability-redundant-smartptr-get-msvc.cpp
===
--- test/clang-tidy/readability-redundant-smartptr-get-msvc.cpp
+++ test/clang-tidy/readability-redundant-smartptr-get-msvc.cpp
@@ -0,0 +1,94 @@
+// 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
+struct unique_ptr {
+ template
+ T2& operator*() const;
+ template
+ T2* operator->() const;
+ T* get() const;
+ explicit operator bool() const noexcept;
+};
+
+template
+struct shared_ptr {
+ template
+ T2& operator*() const;
+ template
+ 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* 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 uu;
+ std::shared_ptr *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().get() == NULL;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant get() call
+ // CHECK-MESSAGES: bb = std::unique_ptr().get() == NULL;
+ // CHECK-FIXES: bb = std::unique_ptr() == 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 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);
+}
Index: clang-tidy/readability/RedundantSmartptrGetCheck.cpp
===
--- clang-tidy/readability/RedundantSmartptrGetCheck.cpp
+++ clang-tidy/readability/RedundantSmartptrGetCheck.cpp
@@ -30,6 +30,10 @@
.bind("redundant_get");
}
+internal::Matcher knownSmartptr() {
+ return recordDecl(hasAnyName("::std::unique_ptr", "::std::shared_ptr"));
+}
+
void registerMatchersForGetArrowStart(MatchFinder *Finder,
MatchFinder::MatchCallback *Callback) {
const auto QuacksLikeASmartptr = recordDecl(
@@ -39,21 +43,23 @@
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,
- Callback);
+ Finder->addMatcher(
+ memberExpr(expr().bind("memberExpr"), isArrow(),
+
[PATCH] D30610: [clang-tidy] Added options to cppcoreguidelines-special-member-functions check
fgross added a comment.
> If you've written your own copy functions then you probably want to write
> your own move functions to allow moving, so `AllowMissingMoveFunctions`
> doesn't make sense.
The scenario I had in mind was legacy code which was written back when it still
was the "rule of three" instead of the "rule of five". Those classes with
defined destructor and copy operations are still perfectly safe, because moving
them falls back to copying. They may not be perfectly tuned for performance,
but having no move operations is not an indication for some resoure management
error. That's why I do think this options makes sense.
> I'd like an `AllowDeletedCopyFunctions` option that allows move and
> destructor functions to be missing when copying is disabled.
>
> struct A {
> A(const A&) = delete;
> A& operator=(const A&) = delete;
> }
>
My 2 cents on this one: Even with `AllowMissingMoveFunctions=1` at least the
missing destructor definition should be diagnosed, because it violates the
classic rule of three. If you delete your copy operations, you likely have some
resources that need to be taken care of in your destructor, so this piece of
code would worry me. Better be clear about it and explicitly default the
destructor.
https://reviews.llvm.org/D30610
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36308: Add handling for DeducedType to HasDeclarationMatcher
fgross created this revision.
HasDeclarationMatcher did not handle DeducedType, it always returned false for
deduced types.
So with code like this:
struct X{};
auto x = X{};
This did no longer match:
varDecl(hasType(recordDecl(hasName("X"
Because HasDeclarationMatcher didn't resolve the DeducedType of `x`.
This came up because some checks in clang-tidy didn't match as expected anymore.
https://reviews.llvm.org/D36308
Files:
include/clang/ASTMatchers/ASTMatchersInternal.h
unittests/ASTMatchers/ASTMatchersNodeTest.cpp
Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1184,6 +1184,10 @@
EXPECT_TRUE(matches("int v[] = { 2, 3 }; void f() { for (int i : v) {} }",
autoType()));
+ EXPECT_TRUE(matches("auto i = 2;", varDecl(hasType(isInteger();
+ EXPECT_TRUE(matches("struct X{}; auto x = X{};",
+ varDecl(hasType(recordDecl(hasName("X"));
+
// FIXME: Matching against the type-as-written can't work here, because the
//type as written was not deduced.
//EXPECT_TRUE(matches("auto a = 1;",
Index: include/clang/ASTMatchers/ASTMatchersInternal.h
===
--- include/clang/ASTMatchers/ASTMatchersInternal.h
+++ include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -741,24 +741,33 @@
/// matcher matches on it.
bool matchesSpecialized(const Type &Node, ASTMatchFinder *Finder,
BoundNodesTreeBuilder *Builder) const {
+
+// For deduced types, use the deduced type
+const Type *EffectiveType = &Node;
+if (const auto *S = dyn_cast(&Node)) {
+ EffectiveType = S->getDeducedType().getTypePtrOrNull();
+ if (!EffectiveType)
+return false;
+}
+
// First, for any types that have a declaration, extract the declaration
and
// match on it.
-if (const auto *S = dyn_cast(&Node)) {
+if (const auto *S = dyn_cast(EffectiveType)) {
return matchesDecl(S->getDecl(), Finder, Builder);
}
-if (const auto *S = dyn_cast(&Node)) {
+if (const auto *S = dyn_cast(EffectiveType)) {
return matchesDecl(S->getDecl(), Finder, Builder);
}
-if (const auto *S = dyn_cast(&Node)) {
+if (const auto *S = dyn_cast(EffectiveType)) {
return matchesDecl(S->getDecl(), Finder, Builder);
}
-if (const auto *S = dyn_cast(&Node)) {
+if (const auto *S = dyn_cast(EffectiveType)) {
return matchesDecl(S->getDecl(), Finder, Builder);
}
-if (const auto *S = dyn_cast(&Node)) {
+if (const auto *S = dyn_cast(EffectiveType)) {
return matchesDecl(S->getDecl(), Finder, Builder);
}
-if (const auto *S = dyn_cast(&Node)) {
+if (const auto *S = dyn_cast(EffectiveType)) {
return matchesDecl(S->getInterface(), Finder, Builder);
}
@@ -770,14 +779,14 @@
// template struct X { T t; } class A {}; X a;
// The following matcher will match, which otherwise would not:
// fieldDecl(hasType(pointerType())).
-if (const auto *S = dyn_cast(&Node)) {
+if (const auto *S = dyn_cast(EffectiveType)) {
return matchesSpecialized(S->getReplacementType(), Finder, Builder);
}
// For template specialization types, we want to match the template
// declaration, as long as the type is still dependent, and otherwise the
// declaration of the instantiated tag type.
-if (const auto *S = dyn_cast(&Node)) {
+if (const auto *S = dyn_cast(EffectiveType)) {
if (!S->isTypeAlias() && S->isSugared()) {
// If the template is non-dependent, we want to match the instantiated
// tag type.
@@ -796,7 +805,7 @@
// FIXME: We desugar elaborated types. This makes the assumption that users
// do never want to match on whether a type is elaborated - there are
// arguments for both sides; for now, continue desugaring.
-if (const auto *S = dyn_cast(&Node)) {
+if (const auto *S = dyn_cast(EffectiveType)) {
return matchesSpecialized(S->desugar(), Finder, Builder);
}
return false;
Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1184,6 +1184,10 @@
EXPECT_TRUE(matches("int v[] = { 2, 3 }; void f() { for (int i : v) {} }",
autoType()));
+ EXPECT_TRUE(matches("auto i = 2;", varDecl(hasType(isInteger();
+ EXPECT_TRUE(matches("struct X{}; auto x = X{};",
+ varDecl(hasType(recordDecl(hasName("X"));
+
// FIXME: Matching against the type-as-written can't work here, because the
//type as written was not deduced.
//EXPECT_TRUE(matches("auto a = 1;",
Index: incl
[PATCH] D36308: Add handling for DeducedType to HasDeclarationMatcher
fgross updated this revision to Diff 109795.
fgross added a comment.
Changed comment, added some clang-tidy test cases.
https://reviews.llvm.org/D36308
Files:
include/clang/ASTMatchers/ASTMatchersInternal.h
test/clang-tidy/misc-use-after-move.cpp
test/clang-tidy/performance-inefficient-string-concatenation.cpp
test/clang-tidy/readability-redundant-smartptr-get.cpp
test/clang-tidy/readability-uniqueptr-delete-release.cpp
unittests/ASTMatchers/ASTMatchersNodeTest.cpp
Index: test/clang-tidy/readability-uniqueptr-delete-release.cpp
===
--- test/clang-tidy/readability-uniqueptr-delete-release.cpp
+++ test/clang-tidy/readability-uniqueptr-delete-release.cpp
@@ -24,6 +24,11 @@
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer '= nullptr' to 'delete x.release()' to reset unique_ptr<> objects [readability-uniqueptr-delete-release]
// CHECK-FIXES: {{^}} P = nullptr;
+ auto P2 = P;
+ delete P2.release();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer '= nullptr' to 'delete x.release()' to reset unique_ptr<> objects [readability-uniqueptr-delete-release]
+ // CHECK-FIXES: {{^}} P2 = nullptr;
+
std::unique_ptr Array[20];
delete Array[4].release();
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer '= nullptr' to 'delete
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
@@ -97,6 +97,12 @@
// CHECK-MESSAGES: int i = *ip.get();
// CHECK-FIXES: int i = *ip;
+ auto ip2 = ip;
+ i = *ip2.get();
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant get() call
+ // CHECK-MESSAGES: i = *ip2.get();
+ // CHECK-FIXES: i = *ip2;
+
std::unique_ptr uu;
std::shared_ptr *ss;
bool bb = uu.get() == nullptr;
Index: test/clang-tidy/performance-inefficient-string-concatenation.cpp
===
--- test/clang-tidy/performance-inefficient-string-concatenation.cpp
+++ test/clang-tidy/performance-inefficient-string-concatenation.cpp
@@ -19,6 +19,8 @@
int main() {
std::string mystr1, mystr2;
std::wstring mywstr1, mywstr2;
+ auto myautostr1 = mystr1;
+ auto myautostr2 = mystr2;
for (int i = 0; i < 10; ++i) {
f(mystr1 + mystr2 + mystr1);
@@ -33,6 +35,8 @@
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: string concatenation
mywstr1 = mywstr2 + mywstr2 + mywstr2;
// CHECK-MESSAGES: :[[@LINE-1]]:33: warning: string concatenation
+myautostr1 = myautostr1 + myautostr2;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: string concatenation
mywstr1 = mywstr2 + mywstr2;
mystr1 = mystr2 + mystr2;
Index: test/clang-tidy/misc-use-after-move.cpp
===
--- test/clang-tidy/misc-use-after-move.cpp
+++ test/clang-tidy/misc-use-after-move.cpp
@@ -723,6 +723,11 @@
std::move(container);
container.clear();
container.empty();
+
+auto container2 = container;
+std::move(container2);
+container2.clear();
+container2.empty();
}
{
std::deque container;
Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1184,6 +1184,10 @@
EXPECT_TRUE(matches("int v[] = { 2, 3 }; void f() { for (int i : v) {} }",
autoType()));
+ EXPECT_TRUE(matches("auto i = 2;", varDecl(hasType(isInteger();
+ EXPECT_TRUE(matches("struct X{}; auto x = X{};",
+ varDecl(hasType(recordDecl(hasName("X"));
+
// FIXME: Matching against the type-as-written can't work here, because the
//type as written was not deduced.
//EXPECT_TRUE(matches("auto a = 1;",
Index: include/clang/ASTMatchers/ASTMatchersInternal.h
===
--- include/clang/ASTMatchers/ASTMatchersInternal.h
+++ include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -741,24 +741,34 @@
/// matcher matches on it.
bool matchesSpecialized(const Type &Node, ASTMatchFinder *Finder,
BoundNodesTreeBuilder *Builder) const {
+
+// DeducedType does not have declarations of its own, so
+// match the deduced type instead.
+const Type *EffectiveType = &Node;
+if (const auto *S = dyn_cast(&Node)) {
+ EffectiveType = S->getDeducedType().getTypePtrOrNull();
+ if (!EffectiveType)
+return false;
+}
+
// First, for any types that have a declaration, extract the declaration and
// match on it.
-if (const auto *S = dyn_cast(&Node)) {
+if (const auto *S = dyn_cast(EffectiveType)) {
return matchesDecl(S->getDecl(), Finder, Builder);
}
-if (const auto
[PATCH] D36308: Add handling for DeducedType to HasDeclarationMatcher
This revision was automatically updated to reflect the committed changes.
Closed by commit rL310095: [ASTMatcher] Add handling for DeducedType to
HasDeclarationMatcher (authored by fgross).
Changed prior to commit:
https://reviews.llvm.org/D36308?vs=109795&id=109796#toc
Repository:
rL LLVM
https://reviews.llvm.org/D36308
Files:
cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
cfe/trunk/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
Index: cfe/trunk/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1184,6 +1184,10 @@
EXPECT_TRUE(matches("int v[] = { 2, 3 }; void f() { for (int i : v) {} }",
autoType()));
+ EXPECT_TRUE(matches("auto i = 2;", varDecl(hasType(isInteger();
+ EXPECT_TRUE(matches("struct X{}; auto x = X{};",
+ varDecl(hasType(recordDecl(hasName("X"));
+
// FIXME: Matching against the type-as-written can't work here, because the
//type as written was not deduced.
//EXPECT_TRUE(matches("auto a = 1;",
Index: cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
===
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -741,24 +741,34 @@
/// matcher matches on it.
bool matchesSpecialized(const Type &Node, ASTMatchFinder *Finder,
BoundNodesTreeBuilder *Builder) const {
+
+// DeducedType does not have declarations of its own, so
+// match the deduced type instead.
+const Type *EffectiveType = &Node;
+if (const auto *S = dyn_cast(&Node)) {
+ EffectiveType = S->getDeducedType().getTypePtrOrNull();
+ if (!EffectiveType)
+return false;
+}
+
// First, for any types that have a declaration, extract the declaration
and
// match on it.
-if (const auto *S = dyn_cast(&Node)) {
+if (const auto *S = dyn_cast(EffectiveType)) {
return matchesDecl(S->getDecl(), Finder, Builder);
}
-if (const auto *S = dyn_cast(&Node)) {
+if (const auto *S = dyn_cast(EffectiveType)) {
return matchesDecl(S->getDecl(), Finder, Builder);
}
-if (const auto *S = dyn_cast(&Node)) {
+if (const auto *S = dyn_cast(EffectiveType)) {
return matchesDecl(S->getDecl(), Finder, Builder);
}
-if (const auto *S = dyn_cast(&Node)) {
+if (const auto *S = dyn_cast(EffectiveType)) {
return matchesDecl(S->getDecl(), Finder, Builder);
}
-if (const auto *S = dyn_cast(&Node)) {
+if (const auto *S = dyn_cast(EffectiveType)) {
return matchesDecl(S->getDecl(), Finder, Builder);
}
-if (const auto *S = dyn_cast(&Node)) {
+if (const auto *S = dyn_cast(EffectiveType)) {
return matchesDecl(S->getInterface(), Finder, Builder);
}
@@ -770,14 +780,14 @@
// template struct X { T t; } class A {}; X a;
// The following matcher will match, which otherwise would not:
// fieldDecl(hasType(pointerType())).
-if (const auto *S = dyn_cast(&Node)) {
+if (const auto *S = dyn_cast(EffectiveType)) {
return matchesSpecialized(S->getReplacementType(), Finder, Builder);
}
// For template specialization types, we want to match the template
// declaration, as long as the type is still dependent, and otherwise the
// declaration of the instantiated tag type.
-if (const auto *S = dyn_cast(&Node)) {
+if (const auto *S = dyn_cast(EffectiveType)) {
if (!S->isTypeAlias() && S->isSugared()) {
// If the template is non-dependent, we want to match the instantiated
// tag type.
@@ -796,7 +806,7 @@
// FIXME: We desugar elaborated types. This makes the assumption that users
// do never want to match on whether a type is elaborated - there are
// arguments for both sides; for now, continue desugaring.
-if (const auto *S = dyn_cast(&Node)) {
+if (const auto *S = dyn_cast(EffectiveType)) {
return matchesSpecialized(S->desugar(), Finder, Builder);
}
return false;
Index: cfe/trunk/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1184,6 +1184,10 @@
EXPECT_TRUE(matches("int v[] = { 2, 3 }; void f() { for (int i : v) {} }",
autoType()));
+ EXPECT_TRUE(matches("auto i = 2;", varDecl(hasType(isInteger();
+ EXPECT_TRUE(matches("struct X{}; auto x = X{};",
+ varDecl(hasType(recordDecl(hasName("X"));
+
// FIXME: Matching against the type-as-written can't work here, because the
//type as written was not deduced
[PATCH] D30841: [clang-tidy] readability-misleading-indentation: fix chained if
fgross added a comment. I don't have commit access, so could someone please commit this for me? Thanks! https://reviews.llvm.org/D30841 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30841: [clang-tidy] readability-misleading-indentation: fix chained if
fgross updated this revision to Diff 92026.
fgross added a comment.
Now using `ASTContext::getParents` instead of `ChainedIfs` map.
For some reason I thought of `getParents` as an expensive function to call...
https://reviews.llvm.org/D30841
Files:
clang-tidy/readability/MisleadingIndentationCheck.cpp
clang-tidy/readability/MisleadingIndentationCheck.h
test/clang-tidy/readability-misleading-indentation.cpp
Index: test/clang-tidy/readability-misleading-indentation.cpp
===
--- test/clang-tidy/readability-misleading-indentation.cpp
+++ test/clang-tidy/readability-misleading-indentation.cpp
@@ -76,5 +76,31 @@
{
}
+ if(cond1) {
+ }
+ else if (cond2) {
+ }
+ else {
+ }
+
+ if(cond1) {
+ }
+ else if (cond2) {
+ }
+ else {
+ }
+ // CHECK-MESSAGES: :[[@LINE-2]]:8: warning: different indentation for 'if'
and corresponding 'else' [readability-misleading-indentation]
+
+ if (cond1) {
+if (cond1) {
+}
+else if (cond2) {
+}
+else {
+}
+ }
+ else if (cond2) {
+ }
+
BLOCK
}
Index: clang-tidy/readability/MisleadingIndentationCheck.h
===
--- clang-tidy/readability/MisleadingIndentationCheck.h
+++ clang-tidy/readability/MisleadingIndentationCheck.h
@@ -30,7 +30,8 @@
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
private:
- void danglingElseCheck(const SourceManager &SM, const IfStmt *If);
+ void danglingElseCheck(const SourceManager &SM, ASTContext *Context,
+ const IfStmt *If);
void missingBracesCheck(const SourceManager &SM, const CompoundStmt *CStmt);
};
Index: clang-tidy/readability/MisleadingIndentationCheck.cpp
===
--- clang-tidy/readability/MisleadingIndentationCheck.cpp
+++ clang-tidy/readability/MisleadingIndentationCheck.cpp
@@ -17,7 +17,22 @@
namespace tidy {
namespace readability {
+static const IfStmt *getPrecedingIf(const SourceManager &SM,
+ASTContext *Context, const IfStmt *If) {
+ auto parents = Context->getParents(*If);
+ if (parents.size() != 1)
+return nullptr;
+ if (const auto *PrecedingIf = parents[0].get()) {
+SourceLocation PreviousElseLoc = PrecedingIf->getElseLoc();
+if (SM.getExpansionLineNumber(PreviousElseLoc) ==
+SM.getExpansionLineNumber(If->getIfLoc()))
+ return PrecedingIf;
+ }
+ return nullptr;
+}
+
void MisleadingIndentationCheck::danglingElseCheck(const SourceManager &SM,
+ ASTContext *Context,
const IfStmt *If) {
SourceLocation IfLoc = If->getIfLoc();
SourceLocation ElseLoc = If->getElseLoc();
@@ -29,6 +44,11 @@
SM.getExpansionLineNumber(ElseLoc))
return;
+ // Find location of first 'if' in a 'if else if' chain.
+ for (auto PrecedingIf = getPrecedingIf(SM, Context, If); PrecedingIf;
+ PrecedingIf = getPrecedingIf(SM, Context, PrecedingIf))
+IfLoc = PrecedingIf->getIfLoc();
+
if (SM.getExpansionColumnNumber(IfLoc) !=
SM.getExpansionColumnNumber(ElseLoc))
diag(ElseLoc, "different indentation for 'if' and corresponding 'else'");
@@ -92,7 +112,7 @@
void MisleadingIndentationCheck::check(const MatchFinder::MatchResult &Result)
{
if (const auto *If = Result.Nodes.getNodeAs("if"))
-danglingElseCheck(*Result.SourceManager, If);
+danglingElseCheck(*Result.SourceManager, Result.Context, If);
if (const auto *CStmt = Result.Nodes.getNodeAs("compound"))
missingBracesCheck(*Result.SourceManager, CStmt);
Index: test/clang-tidy/readability-misleading-indentation.cpp
===
--- test/clang-tidy/readability-misleading-indentation.cpp
+++ test/clang-tidy/readability-misleading-indentation.cpp
@@ -76,5 +76,31 @@
{
}
+ if(cond1) {
+ }
+ else if (cond2) {
+ }
+ else {
+ }
+
+ if(cond1) {
+ }
+ else if (cond2) {
+ }
+ else {
+ }
+ // CHECK-MESSAGES: :[[@LINE-2]]:8: warning: different indentation for 'if' and corresponding 'else' [readability-misleading-indentation]
+
+ if (cond1) {
+if (cond1) {
+}
+else if (cond2) {
+}
+else {
+}
+ }
+ else if (cond2) {
+ }
+
BLOCK
}
Index: clang-tidy/readability/MisleadingIndentationCheck.h
===
--- clang-tidy/readability/MisleadingIndentationCheck.h
+++ clang-tidy/readability/MisleadingIndentationCheck.h
@@ -30,7 +30,8 @@
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
private:
- void danglingElseCheck(const SourceManager &SM, const IfStmt *If);
+ void danglingElseCheck(const SourceManager &SM, ASTContext *Context,
+ const IfStmt *If);
void missingBracesCheck
[PATCH] D30592: [clang-tidy] Fix diag message for catch-by-value
fgross created this revision.
Herald added a subscriber: JDevlieghere.
catch (std::exception ex)
{
}
Was flagged with "catch handler catches a pointer value".
https://reviews.llvm.org/D30592
Files:
clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
Index: clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
===
--- clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
+++ clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
@@ -131,22 +131,24 @@
void ThrowByValueCatchByReferenceCheck::diagnoseCatchLocations(
const CXXCatchStmt *catchStmt, ASTContext &context) {
- const char *diagMsgCatchReference = "catch handler catches a pointer value; "
- "should throw a non-pointer value and "
- "catch by reference instead";
if (!catchStmt)
return;
auto caughtType = catchStmt->getCaughtType();
if (caughtType.isNull())
return;
auto *varDecl = catchStmt->getExceptionDecl();
if (const auto *PT = caughtType.getCanonicalType()->getAs()) {
+const char *diagMsgCatchReference = "catch handler catches a pointer
value; "
+"should throw a non-pointer value and "
+"catch by reference instead";
// We do not diagnose when catching pointer to strings since we also allow
// throwing string literals.
if (!PT->getPointeeType()->isAnyCharacterType())
diag(varDecl->getLocStart(), diagMsgCatchReference);
} else if (!caughtType->isReferenceType()) {
-// If it's not a pointer and not a reference then it must be thrown "by
+const char *diagMsgCatchReference = "catch handler catches by value; "
+"should catch by reference instead";
+// If it's not a pointer and not a reference then it must be caught "by
// value". In this case we should emit a diagnosis message unless the type
// is trivial.
if (!caughtType.isTrivialType(context))
Index: clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
===
--- clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
+++ clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
@@ -131,22 +131,24 @@
void ThrowByValueCatchByReferenceCheck::diagnoseCatchLocations(
const CXXCatchStmt *catchStmt, ASTContext &context) {
- const char *diagMsgCatchReference = "catch handler catches a pointer value; "
- "should throw a non-pointer value and "
- "catch by reference instead";
if (!catchStmt)
return;
auto caughtType = catchStmt->getCaughtType();
if (caughtType.isNull())
return;
auto *varDecl = catchStmt->getExceptionDecl();
if (const auto *PT = caughtType.getCanonicalType()->getAs()) {
+const char *diagMsgCatchReference = "catch handler catches a pointer value; "
+"should throw a non-pointer value and "
+"catch by reference instead";
// We do not diagnose when catching pointer to strings since we also allow
// throwing string literals.
if (!PT->getPointeeType()->isAnyCharacterType())
diag(varDecl->getLocStart(), diagMsgCatchReference);
} else if (!caughtType->isReferenceType()) {
-// If it's not a pointer and not a reference then it must be thrown "by
+const char *diagMsgCatchReference = "catch handler catches by value; "
+"should catch by reference instead";
+// If it's not a pointer and not a reference then it must be caught "by
// value". In this case we should emit a diagnosis message unless the type
// is trivial.
if (!caughtType.isTrivialType(context))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30592: [clang-tidy] Fix diag message for catch-by-value
fgross updated this revision to Diff 90572.
fgross added a comment.
Updated test case.
https://reviews.llvm.org/D30592
Files:
clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
test/clang-tidy/misc-throw-by-value-catch-by-reference.cpp
Index: test/clang-tidy/misc-throw-by-value-catch-by-reference.cpp
===
--- test/clang-tidy/misc-throw-by-value-catch-by-reference.cpp
+++ test/clang-tidy/misc-throw-by-value-catch-by-reference.cpp
@@ -62,7 +62,7 @@
try {
testThrowFunc();
} catch (logic_error e) {
-// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: catch handler catches a
pointer value; should throw a non-pointer value and catch by reference instead
[misc-throw-by-value-catch-by-reference]
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: catch handler catches by
value; should catch by reference instead
[misc-throw-by-value-catch-by-reference]
}
}
Index: clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
===
--- clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
+++ clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
@@ -131,22 +131,24 @@
void ThrowByValueCatchByReferenceCheck::diagnoseCatchLocations(
const CXXCatchStmt *catchStmt, ASTContext &context) {
- const char *diagMsgCatchReference = "catch handler catches a pointer value; "
- "should throw a non-pointer value and "
- "catch by reference instead";
if (!catchStmt)
return;
auto caughtType = catchStmt->getCaughtType();
if (caughtType.isNull())
return;
auto *varDecl = catchStmt->getExceptionDecl();
if (const auto *PT = caughtType.getCanonicalType()->getAs()) {
+const char *diagMsgCatchReference = "catch handler catches a pointer
value; "
+"should throw a non-pointer value and "
+"catch by reference instead";
// We do not diagnose when catching pointer to strings since we also allow
// throwing string literals.
if (!PT->getPointeeType()->isAnyCharacterType())
diag(varDecl->getLocStart(), diagMsgCatchReference);
} else if (!caughtType->isReferenceType()) {
-// If it's not a pointer and not a reference then it must be thrown "by
+const char *diagMsgCatchReference = "catch handler catches by value; "
+"should catch by reference instead";
+// If it's not a pointer and not a reference then it must be caught "by
// value". In this case we should emit a diagnosis message unless the type
// is trivial.
if (!caughtType.isTrivialType(context))
Index: test/clang-tidy/misc-throw-by-value-catch-by-reference.cpp
===
--- test/clang-tidy/misc-throw-by-value-catch-by-reference.cpp
+++ test/clang-tidy/misc-throw-by-value-catch-by-reference.cpp
@@ -62,7 +62,7 @@
try {
testThrowFunc();
} catch (logic_error e) {
-// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: catch handler catches a pointer value; should throw a non-pointer value and catch by reference instead [misc-throw-by-value-catch-by-reference]
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: catch handler catches by value; should catch by reference instead [misc-throw-by-value-catch-by-reference]
}
}
Index: clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
===
--- clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
+++ clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
@@ -131,22 +131,24 @@
void ThrowByValueCatchByReferenceCheck::diagnoseCatchLocations(
const CXXCatchStmt *catchStmt, ASTContext &context) {
- const char *diagMsgCatchReference = "catch handler catches a pointer value; "
- "should throw a non-pointer value and "
- "catch by reference instead";
if (!catchStmt)
return;
auto caughtType = catchStmt->getCaughtType();
if (caughtType.isNull())
return;
auto *varDecl = catchStmt->getExceptionDecl();
if (const auto *PT = caughtType.getCanonicalType()->getAs()) {
+const char *diagMsgCatchReference = "catch handler catches a pointer value; "
+"should throw a non-pointer value and "
+"catch by reference instead";
// We do not diagnose when catching pointer to strings since we also allow
// throwing string literals.
if (!PT->getPointeeType()->isAnyCharacterType())
diag(varDecl->getLocStart(), diagMsgCatchReference);
} else if (!caughtType->isReferenceType()) {
-// If it's not a pointer and not a reference then it must be thrown "by
+const char *diagMsgCatchReference = "catch handler catches by value;
[PATCH] D30610: [clang-tidy] Added options to cppcoreguidelines-special-member-functions check
fgross created this revision.
Herald added subscribers: JDevlieghere, nemanjai.
Added two options to cppcoreguidelines-special-member-functions to allow a more
relaxed checking.
With 'AllowSoleDefaultDtor' set to 1, classes with explicitly defaulted
destructors but no other special members are not flagged anymore:
class foo {
// typical scenario: defaulted virtual destructor in base classes
virtual ~foo() = default;
};
With 'AllowMissingMoveFunctions' set to 1, classes with no move functions at
all are not flagged anymore:
class bar{
~bar();
bar(const bar&);
bar& operator=(const bar&);
};
https://reviews.llvm.org/D30610
Files:
clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp
test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp
test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
Index: test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
===
--- test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
+++ test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
@@ -3,7 +3,12 @@
class DefinesDestructor {
~DefinesDestructor();
};
-// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a non-default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesDefaultedDestructor {
+ ~DefinesDefaultedDestructor() = default;
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDefaultedDestructor' defines a default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
class DefinesCopyConstructor {
DefinesCopyConstructor(const DefinesCopyConstructor &);
Index: test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp
===
--- test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp
+++ test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp
@@ -0,0 +1,71 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-special-member-functions %t -- -config="{CheckOptions: [{key: cppcoreguidelines-special-member-functions.AllowMissingMoveFunctions, value: 1}, {key: cppcoreguidelines-special-member-functions.AllowSoleDefaultDtor, value: 1}]}" --
+
+class DefinesDestructor {
+ ~DefinesDestructor();
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a non-default destructor but does not define a copy constructor or a copy assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesDefaultedDestructor {
+ ~DefinesDefaultedDestructor() = default;
+};
+
+class DefinesCopyConstructor {
+ DefinesCopyConstructor(const DefinesCopyConstructor &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyConstructor' defines a copy constructor but does not define a destructor or a copy assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesCopyAssignment {
+ DefinesCopyAssignment &operator=(const DefinesCopyAssignment &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyAssignment' defines a copy assignment operator but does not define a destructor or a copy constructor [cppcoreguidelines-special-member-functions]
+
+class DefinesMoveConstructor {
+ DefinesMoveConstructor(DefinesMoveConstructor &&);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesMoveConstructor' defines a move constructor but does not define a destructor, a copy constructor, a copy assignment operator or a move assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesMoveAssignment {
+ DefinesMoveAssignment &operator=(DefinesMoveAssignment &&);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesMoveAssignment' defines a move assignment operator but does not define a destructor, a copy constructor, a copy assignment operator or a move constructor [cppcoreguidelines-special-member-functions]
+class DefinesNothing {
+};
+
+class DefinesEverything {
+ DefinesEverything(const DefinesEverything &);
+ DefinesEverything &operator=(const DefinesEverything &);
+ DefinesEverything(DefinesEverything &&);
+ DefinesEverything &operator=(DefinesEverything &&);
+ ~DefinesEverything();
+};
+
+class DeletesEverything {
+ DeletesEverything(const DeletesEverything &) = delete;
+ DeletesEverything
[PATCH] D30610: [clang-tidy] Added options to cppcoreguidelines-special-member-functions check
fgross updated this revision to Diff 90593.
fgross added a comment.
Updated documentation, got rid of code duplication.
https://reviews.llvm.org/D30610
Files:
clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp
test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp
test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
Index: test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
===
--- test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
+++ test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
@@ -3,7 +3,12 @@
class DefinesDestructor {
~DefinesDestructor();
};
-// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a non-default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesDefaultedDestructor {
+ ~DefinesDefaultedDestructor() = default;
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDefaultedDestructor' defines a default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
class DefinesCopyConstructor {
DefinesCopyConstructor(const DefinesCopyConstructor &);
Index: test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp
===
--- test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp
+++ test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp
@@ -0,0 +1,71 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-special-member-functions %t -- -config="{CheckOptions: [{key: cppcoreguidelines-special-member-functions.AllowMissingMoveFunctions, value: 1}, {key: cppcoreguidelines-special-member-functions.AllowSoleDefaultDtor, value: 1}]}" --
+
+class DefinesDestructor {
+ ~DefinesDestructor();
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a non-default destructor but does not define a copy constructor or a copy assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesDefaultedDestructor {
+ ~DefinesDefaultedDestructor() = default;
+};
+
+class DefinesCopyConstructor {
+ DefinesCopyConstructor(const DefinesCopyConstructor &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyConstructor' defines a copy constructor but does not define a destructor or a copy assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesCopyAssignment {
+ DefinesCopyAssignment &operator=(const DefinesCopyAssignment &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyAssignment' defines a copy assignment operator but does not define a destructor or a copy constructor [cppcoreguidelines-special-member-functions]
+
+class DefinesMoveConstructor {
+ DefinesMoveConstructor(DefinesMoveConstructor &&);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesMoveConstructor' defines a move constructor but does not define a destructor, a copy constructor, a copy assignment operator or a move assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesMoveAssignment {
+ DefinesMoveAssignment &operator=(DefinesMoveAssignment &&);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesMoveAssignment' defines a move assignment operator but does not define a destructor, a copy constructor, a copy assignment operator or a move constructor [cppcoreguidelines-special-member-functions]
+class DefinesNothing {
+};
+
+class DefinesEverything {
+ DefinesEverything(const DefinesEverything &);
+ DefinesEverything &operator=(const DefinesEverything &);
+ DefinesEverything(DefinesEverything &&);
+ DefinesEverything &operator=(DefinesEverything &&);
+ ~DefinesEverything();
+};
+
+class DeletesEverything {
+ DeletesEverything(const DeletesEverything &) = delete;
+ DeletesEverything &operator=(const DeletesEverything &) = delete;
+ DeletesEverything(DeletesEverything &&) = delete;
+ DeletesEverything &operator=(DeletesEverything &&) = delete;
+ ~DeletesEverything() = delete;
+};
+
+class DeletesCopyDefaultsMove {
+ DeletesCopyDefaultsMove(const DeletesCopyDefaultsMove &) = delete;
+ DeletesCopyDefaultsMove &operator=(const DeletesCopyDefaultsMove &) = delete;
+ DeletesCopyDefaultsMove(DeletesCopyDefaults
[PATCH] D30610: [clang-tidy] Added options to cppcoreguidelines-special-member-functions check
fgross marked 6 inline comments as done. fgross added inline comments. Comment at: clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp:110 +ClassWithSpecialMembers[ID]; +if (find(Members, Kind) == Members.end()) + Members.push_back(Kind); aaron.ballman wrote: > Please qualify the find with std::, here and elsewhere. It's actually llvm::; added namespace and switched to is_contained. https://reviews.llvm.org/D30610 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30610: [clang-tidy] Added options to cppcoreguidelines-special-member-functions check
fgross updated this revision to Diff 90607.
fgross marked an inline comment as done.
fgross added a comment.
reformatted
https://reviews.llvm.org/D30610
Files:
clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp
test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp
test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
Index: test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
===
--- test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
+++ test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
@@ -3,7 +3,12 @@
class DefinesDestructor {
~DefinesDestructor();
};
-// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a non-default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesDefaultedDestructor {
+ ~DefinesDefaultedDestructor() = default;
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDefaultedDestructor' defines a default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
class DefinesCopyConstructor {
DefinesCopyConstructor(const DefinesCopyConstructor &);
Index: test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp
===
--- test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp
+++ test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp
@@ -0,0 +1,71 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-special-member-functions %t -- -config="{CheckOptions: [{key: cppcoreguidelines-special-member-functions.AllowMissingMoveFunctions, value: 1}, {key: cppcoreguidelines-special-member-functions.AllowSoleDefaultDtor, value: 1}]}" --
+
+class DefinesDestructor {
+ ~DefinesDestructor();
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a non-default destructor but does not define a copy constructor or a copy assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesDefaultedDestructor {
+ ~DefinesDefaultedDestructor() = default;
+};
+
+class DefinesCopyConstructor {
+ DefinesCopyConstructor(const DefinesCopyConstructor &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyConstructor' defines a copy constructor but does not define a destructor or a copy assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesCopyAssignment {
+ DefinesCopyAssignment &operator=(const DefinesCopyAssignment &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyAssignment' defines a copy assignment operator but does not define a destructor or a copy constructor [cppcoreguidelines-special-member-functions]
+
+class DefinesMoveConstructor {
+ DefinesMoveConstructor(DefinesMoveConstructor &&);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesMoveConstructor' defines a move constructor but does not define a destructor, a copy constructor, a copy assignment operator or a move assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesMoveAssignment {
+ DefinesMoveAssignment &operator=(DefinesMoveAssignment &&);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesMoveAssignment' defines a move assignment operator but does not define a destructor, a copy constructor, a copy assignment operator or a move constructor [cppcoreguidelines-special-member-functions]
+class DefinesNothing {
+};
+
+class DefinesEverything {
+ DefinesEverything(const DefinesEverything &);
+ DefinesEverything &operator=(const DefinesEverything &);
+ DefinesEverything(DefinesEverything &&);
+ DefinesEverything &operator=(DefinesEverything &&);
+ ~DefinesEverything();
+};
+
+class DeletesEverything {
+ DeletesEverything(const DeletesEverything &) = delete;
+ DeletesEverything &operator=(const DeletesEverything &) = delete;
+ DeletesEverything(DeletesEverything &&) = delete;
+ DeletesEverything &operator=(DeletesEverything &&) = delete;
+ ~DeletesEverything() = delete;
+};
+
+class DeletesCopyDefaultsMove {
+ DeletesCopyDefaultsMove(const DeletesCopyDefaultsMove &) = delete;
+ DeletesCopyDefaultsMove &operator=(const DeletesCopyDefaultsMove &) = delete;
+ DeletesCopyDefaultsMove(DeletesCopyDefault
[PATCH] D30610: [clang-tidy] Added options to cppcoreguidelines-special-member-functions check
fgross updated this revision to Diff 91183.
fgross added a comment.
Added examples to options doc.
https://reviews.llvm.org/D30610
Files:
clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp
test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
Index: test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
===
--- test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
+++ test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
@@ -3,7 +3,12 @@
class DefinesDestructor {
~DefinesDestructor();
};
-// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a non-default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesDefaultedDestructor {
+ ~DefinesDefaultedDestructor() = default;
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDefaultedDestructor' defines a default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
class DefinesCopyConstructor {
DefinesCopyConstructor(const DefinesCopyConstructor &);
Index: test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp
===
--- test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp
+++ test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp
@@ -3,7 +3,7 @@
class DefinesDestructor {
~DefinesDestructor();
};
-// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a destructor but does not define a copy constructor or a copy assignment operator [cppcoreguidelines-special-member-functions]
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a non-default destructor but does not define a copy constructor or a copy assignment operator [cppcoreguidelines-special-member-functions]
class DefinesCopyConstructor {
DefinesCopyConstructor(const DefinesCopyConstructor &);
Index: docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
===
--- docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
+++ docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
@@ -19,3 +19,31 @@
Guidelines, corresponding to rule C.21. See
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c21-if-you-define-or-delete-any-default-operation-define-or-delete-them-all.
+
+Options
+---
+
+.. option:: AllowSoleDefaultDtor
+
+ When set to `1` (default is `0`), this check doesn't flag classes with a sole, explicitly
+ defaulted destructor. An example for such a class is:
+
+ .. code-block:: c++
+
+ struct A {
+ virtual ~A() = default;
+ };
+
+.. option:: AllowMissingMoveFunctions
+
+ When set to `1` (default is `0`), this check doesn't flag classes which define no move
+ operations at all. It still flags classes which define only one of either
+ move constructor or move assignment operator. With this option enabled, the following class won't be flagged:
+
+ .. code-block:: c++
+
+ struct A {
+ A(const A&);
+ A& operator=(const A&);
+ ~A();
+ }
Index: clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
===
--- clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
+++ clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
@@ -25,14 +25,16 @@
/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-special-member-functions.html
class SpecialMemberFunctionsCheck : public ClangTidyCheck {
public:
- SpecialMemberFunctionsCheck(StringRef Name, ClangTidyContext *Context)
- : ClangTidyCheck(Name, Context) {}
+ SpecialMemberFunctionsCheck(StringRef Name, ClangTidyContext *Context);
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
void onEndOfTranslationUnit() override;
enum class SpecialMemberFunctionKind : uint8_t {
Destructor,
+DefaultDestructor,
+NonDefaultDestructor,
CopyConstructor,
CopyAssignment
[PATCH] D30610: [clang-tidy] Added options to cppcoreguidelines-special-member-functions check
fgross updated this revision to Diff 91188.
fgross added a comment.
Diff was missing cppcoreguidelines-special-member-functions-relaxed.cpp...
https://reviews.llvm.org/D30610
Files:
clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h
docs/clang-tidy/checks/cppcoreguidelines-special-member-functions.rst
test/clang-tidy/cppcoreguidelines-special-member-functions-cxx-03.cpp
test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp
test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
Index: test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
===
--- test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
+++ test/clang-tidy/cppcoreguidelines-special-member-functions.cpp
@@ -3,7 +3,12 @@
class DefinesDestructor {
~DefinesDestructor();
};
-// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a non-default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesDefaultedDestructor {
+ ~DefinesDefaultedDestructor() = default;
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDefaultedDestructor' defines a default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
class DefinesCopyConstructor {
DefinesCopyConstructor(const DefinesCopyConstructor &);
Index: test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp
===
--- test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp
+++ test/clang-tidy/cppcoreguidelines-special-member-functions-relaxed.cpp
@@ -0,0 +1,71 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-special-member-functions %t -- -config="{CheckOptions: [{key: cppcoreguidelines-special-member-functions.AllowMissingMoveFunctions, value: 1}, {key: cppcoreguidelines-special-member-functions.AllowSoleDefaultDtor, value: 1}]}" --
+
+class DefinesDestructor {
+ ~DefinesDestructor();
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a non-default destructor but does not define a copy constructor or a copy assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesDefaultedDestructor {
+ ~DefinesDefaultedDestructor() = default;
+};
+
+class DefinesCopyConstructor {
+ DefinesCopyConstructor(const DefinesCopyConstructor &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyConstructor' defines a copy constructor but does not define a destructor or a copy assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesCopyAssignment {
+ DefinesCopyAssignment &operator=(const DefinesCopyAssignment &);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyAssignment' defines a copy assignment operator but does not define a destructor or a copy constructor [cppcoreguidelines-special-member-functions]
+
+class DefinesMoveConstructor {
+ DefinesMoveConstructor(DefinesMoveConstructor &&);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesMoveConstructor' defines a move constructor but does not define a destructor, a copy constructor, a copy assignment operator or a move assignment operator [cppcoreguidelines-special-member-functions]
+
+class DefinesMoveAssignment {
+ DefinesMoveAssignment &operator=(DefinesMoveAssignment &&);
+};
+// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesMoveAssignment' defines a move assignment operator but does not define a destructor, a copy constructor, a copy assignment operator or a move constructor [cppcoreguidelines-special-member-functions]
+class DefinesNothing {
+};
+
+class DefinesEverything {
+ DefinesEverything(const DefinesEverything &);
+ DefinesEverything &operator=(const DefinesEverything &);
+ DefinesEverything(DefinesEverything &&);
+ DefinesEverything &operator=(DefinesEverything &&);
+ ~DefinesEverything();
+};
+
+class DeletesEverything {
+ DeletesEverything(const DeletesEverything &) = delete;
+ DeletesEverything &operator=(const DeletesEverything &) = delete;
+ DeletesEverything(DeletesEverything &&) = delete;
+ DeletesEverything &operator=(DeletesEverything &&) = delete;
+ ~DeletesEverything() = delete;
+};
+
+class DeletesCopyDefaultsMove {
+ DeletesCopyDefaultsMove(const DeletesCopyDefaultsMove &) = delete;
+ DeletesCopyDefaultsMove &operator=(const DeletesCopyDefaultsMove &) = delete;
+ DeletesCopyDefaultsM
[PATCH] D30841: [clang-tidy] readability-misleading-indentation: fix chained if
fgross created this revision.
Herald added a subscriber: JDevlieghere.
Fixed erroneously flagging of chained if statements when styled like this:
if (cond) {
}
else if (cond) {
}
else {
}
https://reviews.llvm.org/D30841
Files:
clang-tidy/readability/MisleadingIndentationCheck.cpp
clang-tidy/readability/MisleadingIndentationCheck.h
test/clang-tidy/readability-misleading-indentation.cpp
Index: test/clang-tidy/readability-misleading-indentation.cpp
===
--- test/clang-tidy/readability-misleading-indentation.cpp
+++ test/clang-tidy/readability-misleading-indentation.cpp
@@ -76,5 +76,31 @@
{
}
+ if(cond1) {
+ }
+ else if (cond2) {
+ }
+ else {
+ }
+
+ if(cond1) {
+ }
+ else if (cond2) {
+ }
+ else {
+ }
+ // CHECK-MESSAGES: :[[@LINE-2]]:8: warning: different indentation for 'if'
and corresponding 'else' [readability-misleading-indentation]
+
+ if (cond1) {
+if (cond1) {
+}
+else if (cond2) {
+}
+else {
+}
+ }
+ else if (cond2) {
+ }
+
BLOCK
}
Index: clang-tidy/readability/MisleadingIndentationCheck.h
===
--- clang-tidy/readability/MisleadingIndentationCheck.h
+++ clang-tidy/readability/MisleadingIndentationCheck.h
@@ -11,6 +11,7 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_MISLEADING_INDENTATION_H
#include "../ClangTidy.h"
+#include
namespace clang {
namespace tidy {
@@ -32,6 +33,10 @@
private:
void danglingElseCheck(const SourceManager &SM, const IfStmt *If);
void missingBracesCheck(const SourceManager &SM, const CompoundStmt *CStmt);
+
+ /// Key: Chained If
+ /// Value: Preceding If
+ std::map ChainedIfs;
};
} // namespace readability
Index: clang-tidy/readability/MisleadingIndentationCheck.cpp
===
--- clang-tidy/readability/MisleadingIndentationCheck.cpp
+++ clang-tidy/readability/MisleadingIndentationCheck.cpp
@@ -25,10 +25,20 @@
if (IfLoc.isMacroID() || ElseLoc.isMacroID())
return;
+ if (const auto *ChainedIf = dyn_cast(If->getElse())) {
+if (SM.getExpansionLineNumber(ElseLoc) ==
+SM.getExpansionLineNumber(ChainedIf->getIfLoc()))
+ ChainedIfs.emplace(ChainedIf, If);
+ }
+
if (SM.getExpansionLineNumber(If->getThen()->getLocEnd()) ==
SM.getExpansionLineNumber(ElseLoc))
return;
+ for (auto Iter = ChainedIfs.find(If); Iter != ChainedIfs.end();
+ Iter = ChainedIfs.find(Iter->second))
+IfLoc = Iter->second->getIfLoc();
+
if (SM.getExpansionColumnNumber(IfLoc) !=
SM.getExpansionColumnNumber(ElseLoc))
diag(ElseLoc, "different indentation for 'if' and corresponding 'else'");
Index: test/clang-tidy/readability-misleading-indentation.cpp
===
--- test/clang-tidy/readability-misleading-indentation.cpp
+++ test/clang-tidy/readability-misleading-indentation.cpp
@@ -76,5 +76,31 @@
{
}
+ if(cond1) {
+ }
+ else if (cond2) {
+ }
+ else {
+ }
+
+ if(cond1) {
+ }
+ else if (cond2) {
+ }
+ else {
+ }
+ // CHECK-MESSAGES: :[[@LINE-2]]:8: warning: different indentation for 'if' and corresponding 'else' [readability-misleading-indentation]
+
+ if (cond1) {
+if (cond1) {
+}
+else if (cond2) {
+}
+else {
+}
+ }
+ else if (cond2) {
+ }
+
BLOCK
}
Index: clang-tidy/readability/MisleadingIndentationCheck.h
===
--- clang-tidy/readability/MisleadingIndentationCheck.h
+++ clang-tidy/readability/MisleadingIndentationCheck.h
@@ -11,6 +11,7 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_MISLEADING_INDENTATION_H
#include "../ClangTidy.h"
+#include
namespace clang {
namespace tidy {
@@ -32,6 +33,10 @@
private:
void danglingElseCheck(const SourceManager &SM, const IfStmt *If);
void missingBracesCheck(const SourceManager &SM, const CompoundStmt *CStmt);
+
+ /// Key: Chained If
+ /// Value: Preceding If
+ std::map ChainedIfs;
};
} // namespace readability
Index: clang-tidy/readability/MisleadingIndentationCheck.cpp
===
--- clang-tidy/readability/MisleadingIndentationCheck.cpp
+++ clang-tidy/readability/MisleadingIndentationCheck.cpp
@@ -25,10 +25,20 @@
if (IfLoc.isMacroID() || ElseLoc.isMacroID())
return;
+ if (const auto *ChainedIf = dyn_cast(If->getElse())) {
+if (SM.getExpansionLineNumber(ElseLoc) ==
+SM.getExpansionLineNumber(ChainedIf->getIfLoc()))
+ ChainedIfs.emplace(ChainedIf, If);
+ }
+
if (SM.getExpansionLineNumber(If->getThen()->getLocEnd()) ==
SM.getExpansionLineNumber(ElseLoc))
return;
+ for (auto Iter = ChainedIfs.find(If); Iter != ChainedIfs.end();
+ Iter = ChainedIfs.find(Iter->second))
+IfLoc = Iter->second->
[PATCH] D30841: [clang-tidy] readability-misleading-indentation: fix chained if
fgross updated this revision to Diff 91473.
fgross added a comment.
Replaced std::map with llvm::DenseMap, added comment about traversal.
I just assumed it would traverse in the "right" way, is there any documentation
about AST / matcher traversal?
https://reviews.llvm.org/D30841
Files:
clang-tidy/readability/MisleadingIndentationCheck.cpp
clang-tidy/readability/MisleadingIndentationCheck.h
test/clang-tidy/readability-misleading-indentation.cpp
Index: test/clang-tidy/readability-misleading-indentation.cpp
===
--- test/clang-tidy/readability-misleading-indentation.cpp
+++ test/clang-tidy/readability-misleading-indentation.cpp
@@ -76,5 +76,31 @@
{
}
+ if(cond1) {
+ }
+ else if (cond2) {
+ }
+ else {
+ }
+
+ if(cond1) {
+ }
+ else if (cond2) {
+ }
+ else {
+ }
+ // CHECK-MESSAGES: :[[@LINE-2]]:8: warning: different indentation for 'if'
and corresponding 'else' [readability-misleading-indentation]
+
+ if (cond1) {
+if (cond1) {
+}
+else if (cond2) {
+}
+else {
+}
+ }
+ else if (cond2) {
+ }
+
BLOCK
}
Index: clang-tidy/readability/MisleadingIndentationCheck.h
===
--- clang-tidy/readability/MisleadingIndentationCheck.h
+++ clang-tidy/readability/MisleadingIndentationCheck.h
@@ -11,6 +11,7 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_MISLEADING_INDENTATION_H
#include "../ClangTidy.h"
+#include "llvm/ADT/DenseMap.h"
namespace clang {
namespace tidy {
@@ -32,6 +33,10 @@
private:
void danglingElseCheck(const SourceManager &SM, const IfStmt *If);
void missingBracesCheck(const SourceManager &SM, const CompoundStmt *CStmt);
+
+ /// Key: Chained If
+ /// Value: Preceding If
+ llvm::DenseMap ChainedIfs;
};
} // namespace readability
Index: clang-tidy/readability/MisleadingIndentationCheck.cpp
===
--- clang-tidy/readability/MisleadingIndentationCheck.cpp
+++ clang-tidy/readability/MisleadingIndentationCheck.cpp
@@ -25,10 +25,22 @@
if (IfLoc.isMacroID() || ElseLoc.isMacroID())
return;
+ if (const auto *ChainedIf = dyn_cast(If->getElse())) {
+if (SM.getExpansionLineNumber(ElseLoc) ==
+SM.getExpansionLineNumber(ChainedIf->getIfLoc()))
+ ChainedIfs.insert({ ChainedIf, If });
+ }
+
if (SM.getExpansionLineNumber(If->getThen()->getLocEnd()) ==
SM.getExpansionLineNumber(ElseLoc))
return;
+ // Find location of first 'if' in a 'if else if' chain.
+ // Works because parent nodes will be matched before child nodes.
+ for (auto Iter = ChainedIfs.find(If); Iter != ChainedIfs.end();
+ Iter = ChainedIfs.find(Iter->second))
+IfLoc = Iter->second->getIfLoc();
+
if (SM.getExpansionColumnNumber(IfLoc) !=
SM.getExpansionColumnNumber(ElseLoc))
diag(ElseLoc, "different indentation for 'if' and corresponding 'else'");
Index: test/clang-tidy/readability-misleading-indentation.cpp
===
--- test/clang-tidy/readability-misleading-indentation.cpp
+++ test/clang-tidy/readability-misleading-indentation.cpp
@@ -76,5 +76,31 @@
{
}
+ if(cond1) {
+ }
+ else if (cond2) {
+ }
+ else {
+ }
+
+ if(cond1) {
+ }
+ else if (cond2) {
+ }
+ else {
+ }
+ // CHECK-MESSAGES: :[[@LINE-2]]:8: warning: different indentation for 'if' and corresponding 'else' [readability-misleading-indentation]
+
+ if (cond1) {
+if (cond1) {
+}
+else if (cond2) {
+}
+else {
+}
+ }
+ else if (cond2) {
+ }
+
BLOCK
}
Index: clang-tidy/readability/MisleadingIndentationCheck.h
===
--- clang-tidy/readability/MisleadingIndentationCheck.h
+++ clang-tidy/readability/MisleadingIndentationCheck.h
@@ -11,6 +11,7 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_MISLEADING_INDENTATION_H
#include "../ClangTidy.h"
+#include "llvm/ADT/DenseMap.h"
namespace clang {
namespace tidy {
@@ -32,6 +33,10 @@
private:
void danglingElseCheck(const SourceManager &SM, const IfStmt *If);
void missingBracesCheck(const SourceManager &SM, const CompoundStmt *CStmt);
+
+ /// Key: Chained If
+ /// Value: Preceding If
+ llvm::DenseMap ChainedIfs;
};
} // namespace readability
Index: clang-tidy/readability/MisleadingIndentationCheck.cpp
===
--- clang-tidy/readability/MisleadingIndentationCheck.cpp
+++ clang-tidy/readability/MisleadingIndentationCheck.cpp
@@ -25,10 +25,22 @@
if (IfLoc.isMacroID() || ElseLoc.isMacroID())
return;
+ if (const auto *ChainedIf = dyn_cast(If->getElse())) {
+if (SM.getExpansionLineNumber(ElseLoc) ==
+SM.getExpansionLineNumber(ChainedIf->getIfLoc()))
+ ChainedIfs.insert({ ChainedIf, If });
+ }
+
if (SM.getExpans
[PATCH] D30610: [clang-tidy] Added options to cppcoreguidelines-special-member-functions check
fgross added a comment. No commit access, could someone please take care of this? Thanks! https://reviews.llvm.org/D30610 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33354: [clang-tidy] readability-braces-around-statements false positive with char literals
fgross created this revision.
Herald added a subscriber: xazax.hun.
Single-line if statements cause a false positive when the last token in the
conditional statement is a char constant:
if (condition)
return 'a';
For some reason `findEndLocation` seems to skips too many (vertical)
whitespaces in this case. The same problem already occured with string literals
(https://reviews.llvm.org/D25558), and was fixed by adding a special check for
this very case. I just extended the condition to also include char constants.
No idea what really causes the issue though.
https://reviews.llvm.org/D33354
Files:
clang-tidy/readability/BracesAroundStatementsCheck.cpp
include/clang/Basic/TokenKinds.h
test/clang-tidy/readability-braces-around-statements-single-line.cpp
Index: include/clang/Basic/TokenKinds.h
===
--- include/clang/Basic/TokenKinds.h
+++ include/clang/Basic/TokenKinds.h
@@ -82,12 +82,17 @@
K == tok::utf32_string_literal;
}
+/// \brief Return true if this is a char constant token
+inline bool isCharConstant(TokenKind K) {
+ return K == tok::char_constant || K == tok::wide_char_constant ||
+ K == tok::utf8_char_constant || K == tok::utf16_char_constant ||
+ K == tok::utf32_char_constant;
+}
+
/// \brief Return true if this is a "literal" kind, like a numeric
/// constant, string, etc.
inline bool isLiteral(TokenKind K) {
- return K == tok::numeric_constant || K == tok::char_constant ||
- K == tok::wide_char_constant || K == tok::utf8_char_constant ||
- K == tok::utf16_char_constant || K == tok::utf32_char_constant ||
+ return K == tok::numeric_constant || isCharConstant(K) ||
isStringLiteral(K) || K == tok::angle_string_literal;
}
Index: test/clang-tidy/readability-braces-around-statements-single-line.cpp
===
--- test/clang-tidy/readability-braces-around-statements-single-line.cpp
+++ test/clang-tidy/readability-braces-around-statements-single-line.cpp
@@ -31,3 +31,21 @@
// CHECK-FIXES: if (cond("if4") /*comment*/) {
// CHECK-FIXES: }
}
+
+const char *test2() {
+ if (cond("if1"))
+return "string";
+
+
+}
+
+char test3() {
+ if (cond("if1"))
+return 'a';
+
+
+ if (cond("if1"))
+return (char)L'a';
+
+
+}
Index: clang-tidy/readability/BracesAroundStatementsCheck.cpp
===
--- clang-tidy/readability/BracesAroundStatementsCheck.cpp
+++ clang-tidy/readability/BracesAroundStatementsCheck.cpp
@@ -61,7 +61,8 @@
bool SkipEndWhitespaceAndComments = true;
tok::TokenKind TokKind = getTokenKind(Loc, SM, Context);
if (TokKind == tok::NUM_TOKENS || TokKind == tok::semi ||
- TokKind == tok::r_brace || isStringLiteral(TokKind)) {
+ TokKind == tok::r_brace || isCharConstant(TokKind) ||
+ isStringLiteral(TokKind)) {
// If we are at ";" or "}", we found the last token. We could use as well
// `if (isa(S))`, but it wouldn't work for nested statements.
SkipEndWhitespaceAndComments = false;
Index: include/clang/Basic/TokenKinds.h
===
--- include/clang/Basic/TokenKinds.h
+++ include/clang/Basic/TokenKinds.h
@@ -82,12 +82,17 @@
K == tok::utf32_string_literal;
}
+/// \brief Return true if this is a char constant token
+inline bool isCharConstant(TokenKind K) {
+ return K == tok::char_constant || K == tok::wide_char_constant ||
+ K == tok::utf8_char_constant || K == tok::utf16_char_constant ||
+ K == tok::utf32_char_constant;
+}
+
/// \brief Return true if this is a "literal" kind, like a numeric
/// constant, string, etc.
inline bool isLiteral(TokenKind K) {
- return K == tok::numeric_constant || K == tok::char_constant ||
- K == tok::wide_char_constant || K == tok::utf8_char_constant ||
- K == tok::utf16_char_constant || K == tok::utf32_char_constant ||
+ return K == tok::numeric_constant || isCharConstant(K) ||
isStringLiteral(K) || K == tok::angle_string_literal;
}
Index: test/clang-tidy/readability-braces-around-statements-single-line.cpp
===
--- test/clang-tidy/readability-braces-around-statements-single-line.cpp
+++ test/clang-tidy/readability-braces-around-statements-single-line.cpp
@@ -31,3 +31,21 @@
// CHECK-FIXES: if (cond("if4") /*comment*/) {
// CHECK-FIXES: }
}
+
+const char *test2() {
+ if (cond("if1"))
+return "string";
+
+
+}
+
+char test3() {
+ if (cond("if1"))
+return 'a';
+
+
+ if (cond("if1"))
+return (char)L'a';
+
+
+}
Index: clang-tidy/readability/BracesAroundStatementsCheck.cpp
===
--- clang-tidy/readability/BracesAroundStatementsCheck.cpp
+++ clang-tidy/readability/BracesAroundStatementsCheck.cpp
@@ -61,7 +6
[PATCH] D33358: [clang-tidy] readability-redundant-declaration false positive for defaulted function
fgross created this revision.
Herald added a subscriber: xazax.hun.
template
struct C {
C();
};
template
C::C() = default;
Causes a readability-redundant-declaration diagnostic. This is caused by
`isDefinition` not matching defaulted functions.
https://reviews.llvm.org/D33358
Files:
clang-tidy/readability/RedundantDeclarationCheck.cpp
test/clang-tidy/readability-redundant-declaration.cpp
Index: test/clang-tidy/readability-redundant-declaration.cpp
===
--- test/clang-tidy/readability-redundant-declaration.cpp
+++ test/clang-tidy/readability-redundant-declaration.cpp
@@ -34,3 +34,11 @@
static int I;
};
int C::I;
+
+template
+struct C2 {
+ C2();
+};
+
+template
+C2::C2() = default;
Index: clang-tidy/readability/RedundantDeclarationCheck.cpp
===
--- clang-tidy/readability/RedundantDeclarationCheck.cpp
+++ clang-tidy/readability/RedundantDeclarationCheck.cpp
@@ -19,11 +19,12 @@
namespace readability {
void RedundantDeclarationCheck::registerMatchers(MatchFinder *Finder) {
- auto UnlessDefinition = unless(isDefinition());
- Finder->addMatcher(namedDecl(anyOf(varDecl(UnlessDefinition),
- functionDecl(UnlessDefinition)))
- .bind("Decl"),
- this);
+ Finder->addMatcher(
+ namedDecl(
+ anyOf(varDecl(unless(isDefinition())),
+functionDecl(unless(anyOf(isDefinition(), isDefaulted())
+ .bind("Decl"),
+ this);
}
void RedundantDeclarationCheck::check(const MatchFinder::MatchResult &Result) {
Index: test/clang-tidy/readability-redundant-declaration.cpp
===
--- test/clang-tidy/readability-redundant-declaration.cpp
+++ test/clang-tidy/readability-redundant-declaration.cpp
@@ -34,3 +34,11 @@
static int I;
};
int C::I;
+
+template
+struct C2 {
+ C2();
+};
+
+template
+C2::C2() = default;
Index: clang-tidy/readability/RedundantDeclarationCheck.cpp
===
--- clang-tidy/readability/RedundantDeclarationCheck.cpp
+++ clang-tidy/readability/RedundantDeclarationCheck.cpp
@@ -19,11 +19,12 @@
namespace readability {
void RedundantDeclarationCheck::registerMatchers(MatchFinder *Finder) {
- auto UnlessDefinition = unless(isDefinition());
- Finder->addMatcher(namedDecl(anyOf(varDecl(UnlessDefinition),
- functionDecl(UnlessDefinition)))
- .bind("Decl"),
- this);
+ Finder->addMatcher(
+ namedDecl(
+ anyOf(varDecl(unless(isDefinition())),
+functionDecl(unless(anyOf(isDefinition(), isDefaulted())
+ .bind("Decl"),
+ this);
}
void RedundantDeclarationCheck::check(const MatchFinder::MatchResult &Result) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33354: [clang-tidy] readability-braces-around-statements false positive with char literals
fgross updated this revision to Diff 99576.
fgross added a comment.
After some digging into it, here is my uneducated guess:
The comment in `findEndLocation` states that //"Loc points to the beginning of
the last token before ';'"//. But `checkStmt` calls it with
`FileRange.getEnd().getLocWithOffset(-1)`
so in fact it points to the last char of the last token. For a string literal
this would be '"' or ''', not enough for `Lexer::getLocForEndOfToken` to query
the correct token type. It ends up moving behind the following ';' and skipping
all whitespaces to next token.
I've updated the diff, this seems to resolve the issue. But I'm sure there is a
way to pass the correct location in the first place.
https://reviews.llvm.org/D33354
Files:
clang-tidy/readability/BracesAroundStatementsCheck.cpp
Index: clang-tidy/readability/BracesAroundStatementsCheck.cpp
===
--- clang-tidy/readability/BracesAroundStatementsCheck.cpp
+++ clang-tidy/readability/BracesAroundStatementsCheck.cpp
@@ -54,14 +54,15 @@
SourceLocation findEndLocation(SourceLocation LastTokenLoc,
const SourceManager &SM,
const ASTContext *Context) {
- SourceLocation Loc = LastTokenLoc;
+ SourceLocation Loc =
+ Lexer::GetBeginningOfToken(LastTokenLoc, SM, Context->getLangOpts());
// Loc points to the beginning of the last (non-comment non-ws) token
// before end or ';'.
assert(Loc.isValid());
bool SkipEndWhitespaceAndComments = true;
tok::TokenKind TokKind = getTokenKind(Loc, SM, Context);
if (TokKind == tok::NUM_TOKENS || TokKind == tok::semi ||
- TokKind == tok::r_brace || isStringLiteral(TokKind)) {
+ TokKind == tok::r_brace) {
// If we are at ";" or "}", we found the last token. We could use as well
// `if (isa(S))`, but it wouldn't work for nested statements.
SkipEndWhitespaceAndComments = false;
Index: clang-tidy/readability/BracesAroundStatementsCheck.cpp
===
--- clang-tidy/readability/BracesAroundStatementsCheck.cpp
+++ clang-tidy/readability/BracesAroundStatementsCheck.cpp
@@ -54,14 +54,15 @@
SourceLocation findEndLocation(SourceLocation LastTokenLoc,
const SourceManager &SM,
const ASTContext *Context) {
- SourceLocation Loc = LastTokenLoc;
+ SourceLocation Loc =
+ Lexer::GetBeginningOfToken(LastTokenLoc, SM, Context->getLangOpts());
// Loc points to the beginning of the last (non-comment non-ws) token
// before end or ';'.
assert(Loc.isValid());
bool SkipEndWhitespaceAndComments = true;
tok::TokenKind TokKind = getTokenKind(Loc, SM, Context);
if (TokKind == tok::NUM_TOKENS || TokKind == tok::semi ||
- TokKind == tok::r_brace || isStringLiteral(TokKind)) {
+ TokKind == tok::r_brace) {
// If we are at ";" or "}", we found the last token. We could use as well
// `if (isa(S))`, but it wouldn't work for nested statements.
SkipEndWhitespaceAndComments = false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33354: [clang-tidy] readability-braces-around-statements false positive with char literals
fgross added a comment. Thanks a lot, will do. Repository: rL LLVM https://reviews.llvm.org/D33354 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
