sbenza added inline comments.
================
Comment at: clang-tidy/misc/AssignOperatorCheck.cpp:63
@@ +62,3 @@
+
+ Finder->addMatcher(returnStmt(IsBadReturnStatement,
hasAncestor(IsGoodAssign))
+ .bind("returnStmt"),
----------------
I dislike these uses of hasAnscestor. They are kind of slow.
But more importantly, they break with nested functions/types.
This particular example is not checking that the return statement is from the
assignment operator, only that it is within it. For example, it would match a
lambda.
I think this would trip the check:
F& operator=(const F& o) {
std::copy_if(o.begin(), o.end(), begin(), [](V v) { return v > 0; });
return *this;
}
================
Comment at: clang-tidy/misc/AssignOperatorCheck.cpp:69
@@ +68,3 @@
+void AssignOperatorCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *Method = Result.Nodes.getNodeAs<CXXMethodDecl>("method");
+ const auto *RetStmt = Result.Nodes.getNodeAs<ReturnStmt>("returnStmt");
----------------
Move this closer to where it is used.
================
Comment at: clang-tidy/misc/AssignOperatorCheck.cpp:70
@@ +69,3 @@
+ const auto *Method = Result.Nodes.getNodeAs<CXXMethodDecl>("method");
+ const auto *RetStmt = Result.Nodes.getNodeAs<ReturnStmt>("returnStmt");
+ if (RetStmt) {
----------------
Move this into the if () statement.
================
Comment at: clang-tidy/misc/AssignOperatorCheck.h:19
@@ +18,3 @@
+
+/// Finds declarations of assignment operators with the wrong return and/or
+/// argument types.
----------------
This does not talk about the return statement, only the return type.
================
Comment at: test/clang-tidy/misc-assign-operator.cpp:16
@@ +15,3 @@
+ AlsoGood& operator=(AlsoGood);
+};
+
----------------
This is a very common C++98 way of implementing copy-and-swap with copy elision
support.
You do: `T& operator=(T t) { swap(t); return *this; }`
And it will avoid the copy if the argument is already a temporary due to copy
elision on the caller.
================
Comment at: test/clang-tidy/misc-assign-operator.cpp:69
@@ +68,3 @@
+ n = rhs.n;
+ return *this;
+ }
----------------
This case is not a bad return statement, so it should not be in this class.
http://reviews.llvm.org/D18265
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
