alexfh added inline comments.
================
Comment at: clang-tidy/misc/RuleOfFiveCheck.cpp:30
@@ +29,3 @@
+ .bind("copy-ctor")),
+ unless(hasDescendant(cxxMethodDecl(isCopyAssignmentOperator())))),
+ this);
----------------
Why is `hasDescendant` needed? Doesn't just `has(...)` work? I think, methods
are direct children of the CXXRecordDecl they are declared in.
Same applies to other matchers.
================
Comment at: clang-tidy/misc/RuleOfFiveCheck.cpp:80
@@ +79,3 @@
+ case RuleOfFiveCheck::SpecialFunctionKind::MoveAssignment: {
+ SourceLocation DeclBegin = MatchedDecl.getLocStart();
+ return DeclBegin.getLocWithOffset(-1);
----------------
The variable doesn't make the code shorter or easier to read. Please remove it.
================
Comment at: clang-tidy/misc/RuleOfFiveCheck.cpp:110
@@ +109,3 @@
+ return (llvm::Twine("\n") + ClassName + " &operator=(const " + ClassName +
+ " &) = " + deleteOrDefault(MatchedDecl) + ";")
+ .str();
----------------
How about removing the `deleteOrDefault` function and inserting this before the
switch:
StringRef deleteOrDefault = MethodDecl.isDefaulted() ? " = default;" : " =
delete;";
?
================
Comment at: clang-tidy/misc/RuleOfFiveCheck.cpp:117
@@ +116,3 @@
+ case RuleOfFiveCheck::SpecialFunctionKind::MoveConstructor:
+ return (llvm::Twine("\n") + ClassName + " &operator=(" + ClassName +
+ " &&) = " + deleteOrDefault(MatchedDecl) + ";")
----------------
We usually rely on clang-format to clean up formatting after applying fixes,
but in some cases clang-format looks into possible preferences already present
in the code. Placement of `*` or `&` on pointers and references is one of such
things. I think, clang-tidy should keep neutrality in this regard, e.g. by
adding spaces on both sides of the `*` an `&` in its fixes.
================
Comment at: test/clang-tidy/misc-rule-of-five.cpp:1
@@ +1,2 @@
+// RUN: %check_clang_tidy %s misc-rule-of-five %t
+
----------------
Please add tests with macros and templates with multiple instantiations.
http://reviews.llvm.org/D16376
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
