pilki added inline comments.
================
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(),
----------------
alexfh wrote:
> 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.
I reduced to two matchers and used a common template for the error message.
Tell me if it's ok.
================
Comment at: clang-tidy/readability/DeletedDefaultCheck.cpp:31
@@ +30,3 @@
+ Finder->addMatcher(
+ cxxConstructorDecl(isDefaultConstructor(), isExplicitlyDefaulted(),
+ isDeleted(), NotTemplate)
----------------
alexfh wrote:
> Alternatively, you can use `isDefaulted(), unless(isImplicit())`.
What is the advantage? Not writing my own matcher?
http://reviews.llvm.org/D18961
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
