pilki added inline comments.
================
Comment at: clang-tidy/readability/DeletedDefaultCheck.cpp:36
@@ +35,3 @@
+}
+
+void DeletedDefaultCheck::check(const MatchFinder::MatchResult &Result) {
----------------
alexfh wrote:
> I would at least join matchers, since, I believe, it might be more effective
> that way.
>
> cxxMethodDecl(anyOf(cxxConstructorDecl().bind("constructor"),
> isCopyAssignmentOperator(),
> isMoveAssignmentOperator()),
> isBadlyDefaulted)
> .bind("assignment")
>
> Optionally, you might want to inline the `isBadlyDefaulted` matcher (without
> the external `allOf` matcher).
>
> As for restructuring the `check()` method, I don't insist.
>
I'm a bit confused by this suggestion. It will bind to "assignment" even when
it is a constructor. It works because we get "constructor" first in check(),
but I find the resulting contract between the check() and registerMatchers
awkward.
================
Comment at: clang-tidy/readability/DeletedDefaultCheck.cpp:49
@@ +48,3 @@
+ "constructor";
+ return;
+ }
----------------
alexfh wrote:
> If you don't feel strongly, I'd mildly suggest to turn `if { ... return }` to
> a chain of `if/else` (also the top-level ones). The code will become denser,
> but it will be easier to see the whole logic in a glance.
I thought that in the clang codebase, return were preferred to 'else'. Done.
http://reviews.llvm.org/D18961
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
