alexfh added inline comments.
================
Comment at: clang-tidy/readability/DeletedDefaultCheck.cpp:26
@@ +25,3 @@
+void DeletedDefaultCheck::registerMatchers(MatchFinder *Finder) {
+ const auto NotTemplate = unless(hasAncestor(
+ cxxRecordDecl(::clang::ast_matchers::isTemplateInstantiation())));
----------------
`unless(isInTemplateInstantiation())` is the canonical way to do this.
================
Comment at: clang-tidy/readability/DeletedDefaultCheck.cpp:30
@@ +29,3 @@
+ // We should actually use isExplicitlyDefaulted, but it does not exist.
+ Finder->addMatcher(
+ cxxConstructorDecl(isDefaultConstructor(), isExplicitlyDefaulted(),
----------------
I think, we need at most two matchers here: one for constructors and one for
assignment operators. We could also cram these into a single matcher. I also
suggest to combine the `diag()` calls to a single one using `%N` or
`%select{}N` format to parametrize the message. Something along the lines of:
Finder->addMatcher(cxxMethodDecl(isDefaulted(), isDeleted(),
unless(isImplicit()), unless(isInTemplateInstantiation()),
anyOf(isCopyAssignmentOperator(), isMoveAssignmentOperator(),
cxxConstructorDecl().bind("ctor"))).bind("method"));
...
const auto *Method = Result.Nodes.getNodeAs<CXXMethodDecl>("method");
assert(Method != nullptr);
diag(Method->getLocStart(),
"this %select{method|constructor}0 is marked '= default' but is
actually "
"implicitly deleted, probably because an instance variable or a base "
"class is not copyable nor movable; this definition should either be
removed "
"or explicitly marked as '= delete'") <<
(Result.Nodes.getNodeAs<Decl>("ctor") ? 1 : 0);
If you think that adding "copy", "move" or "default" makes the message any
better, this could also be accommodated in this approach.
================
Comment at: clang-tidy/readability/DeletedDefaultCheck.cpp:31
@@ +30,3 @@
+ Finder->addMatcher(
+ cxxConstructorDecl(isDefaultConstructor(), isExplicitlyDefaulted(),
+ isDeleted(), NotTemplate)
----------------
Alternatively, you can use `isDefaulted(), unless(isImplicit())`.
================
Comment at: test/clang-tidy/readability-deleted-default.cpp:13
@@ +12,3 @@
+ MissingEverything() = default;
+ // CHECK-MESSAGES: warning: this default constructor is marked '= default'
but is actually implicitly deleted
+ MissingEverything(MissingEverything&& Other) = default;
----------------
Please specify the message completely once.
http://reviews.llvm.org/D18961
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
