[PATCH] D18961: Add a readability-deleted-default clang-tidy check.

[Alex Pilkiewicz via cfe-commits]

pilki created this revision.
pilki added a reviewer: alexfh.
pilki added a subscriber: cfe-commits.

Checks if constructors and assignment operators that are marked '= default' are 
actually deleted by the compiler.

http://reviews.llvm.org/D18961

Files:
   clang-tidy/readability/CMakeLists.txt
   clang-tidy/readability/DeletedDefaultCheck.cpp
   clang-tidy/readability/DeletedDefaultCheck.h
   clang-tidy/readability/ReadabilityTidyModule.cpp
   docs/clang-tidy/checks/list.rst
   docs/clang-tidy/checks/readability-deleted-default.rst
   test/clang-tidy/readability-deleted-default.cpp

Index:  test/clang-tidy/readability-deleted-default.cpp
===
---  test/clang-tidy/readability-deleted-default.cpp
+++  test/clang-tidy/readability-deleted-default.cpp
@@ -0,0 +1,103 @@
+// RUN: %check_clang_tidy %s readability-deleted-default %t
+
+class NoDefault {
+public:
+  NoDefault() = delete;
+  NoDefault(NoDefault&& Other) = delete;
+  NoDefault(const NoDefault& Other) = delete;
+};
+
+class MissingEverything {
+public:
+  MissingEverything() = default;
+  // CHECK-MESSAGES: warning: this default constructor is marked '= default' but is actually implicitly deleted
+  MissingEverything(MissingEverything&& Other) = default;
+  // CHECK-MESSAGES: warning: this move constructor is marked '= default' but is actually implicitly deleted
+  MissingEverything(const MissingEverything& Other) = default;
+  // CHECK-MESSAGES: warning: this copy constructor is marked '= default' but is actually implicitly deleted
+  MissingEverything& operator=(MissingEverything&& Other) = default;
+  // CHECK-MESSAGES: warning: this move assignment operator is marked '= default' but is actually implicitly deleted
+  MissingEverything& operator=(const MissingEverything& Other) = default;
+  // CHECK-MESSAGES: warning: this copy assignment operator is marked '= default' but is actually implicitly deleted
+
+private:
+  NoDefault ND;
+};
+
+class NotAssignable {
+public:
+  NotAssignable(NotAssignable&& Other) = default;
+  NotAssignable(const NotAssignable& Other) = default;
+  NotAssignable& operator=(NotAssignable&& Other) = default;
+  // CHECK-MESSAGES: warning: this move assignment operator is marked '= default' but is actually implicitly deleted
+  NotAssignable& operator=(const NotAssignable& Other) = default;
+  // CHECK-MESSAGES: warning: this copy assignment operator is marked '= default' but is actually implicitly deleted
+
+private:
+  const int I = 0;
+};
+
+class Movable {
+public:
+  Movable() = default;
+  Movable(Movable&& Other) = default;
+  Movable(const Movable& Other) = delete;
+  Movable& operator=(Movable&& Other) = default;
+  Movable& operator=(const Movable& Other) = delete;
+};
+
+class NotCopyable {
+public:
+  NotCopyable(NotCopyable&& Other) = default;
+  NotCopyable(const NotCopyable& Other) = default;
+  // CHECK-MESSAGES: warning: this copy constructor is marked '= default' but is actually implicitly deleted
+  NotCopyable& operator=(NotCopyable&& Other) = default;
+  NotCopyable& operator=(const NotCopyable& Other) = default;
+  // CHECK-MESSAGES: warning: this copy assignment operator is marked '= default' but is actually implicitly deleted
+private:
+  Movable M;
+};
+
+template 
+class Templated {
+public:
+  // No warning here, it is a templated class.
+  Templated() = default;
+  Templated(Templated&& Other) = default;
+  Templated(const Templated& Other) = default;
+  Templated& operator=(Templated&& Other) = default;
+  Templated& operator=(const Templated& Other) = default;
+
+  class InnerTemplated {
+  public:
+// This class is not in itself templated, but we still don't have warning.
+InnerTemplated() = default;
+InnerTemplated(InnerTemplated&& Other) = default;
+InnerTemplated(const InnerTemplated& Other) = default;
+InnerTemplated& operator=(InnerTemplated&& Other) = default;
+InnerTemplated& operator=(const InnerTemplated& Other) = default;
+  private:
+T TVar;
+  };
+
+  class InnerNotTemplated {
+  public:
+// This one could technically have warnings, but currently doesn't.
+InnerNotTemplated() = default;
+InnerNotTemplated(InnerNotTemplated&& Other) = default;
+InnerNotTemplated(const InnerNotTemplated& Other) = default;
+InnerNotTemplated& operator=(InnerNotTemplated&& Other) = default;
+InnerNotTemplated& operator=(const InnerNotTemplated& Other) = default;
+  private:
+int I;
+  };
+
+private:
+  const T TVar{};
+};
+
+void Foo() {
+ Templated V1;
+ Templated::InnerTemplated V2;
+ Templated::InnerNotTemplated V3;
+}
Index:  docs/clang-tidy/checks/readability-deleted-default.rst
===
---  docs/clang-tidy/checks/readability-deleted-default.rst
+++  docs/clang-tidy/checks/readability-deleted-default.rst
@@ -0,0 +1,21 @@
+.. title:: clang-tidy - readability-deleted-default
+
+readability-deleted-default
+===

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

[Alex Pilkiewicz via cfe-commits]

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("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("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
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

[Alex Pilkiewicz via cfe-commits]

pilki marked an inline comment as done.
pilki added a comment.

http://reviews.llvm.org/D18961



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

[Alex Pilkiewicz via cfe-commits]

pilki updated this revision to Diff 53257.
pilki marked 3 inline comments as done.

http://reviews.llvm.org/D18961

Files:
   clang-tidy/readability/CMakeLists.txt
   clang-tidy/readability/DeletedDefaultCheck.cpp
   clang-tidy/readability/DeletedDefaultCheck.h
   clang-tidy/readability/ReadabilityTidyModule.cpp
   docs/clang-tidy/checks/list.rst
   docs/clang-tidy/checks/readability-deleted-default.rst
   test/clang-tidy/readability-deleted-default.cpp

Index:  test/clang-tidy/readability-deleted-default.cpp
===
---  test/clang-tidy/readability-deleted-default.cpp
+++  test/clang-tidy/readability-deleted-default.cpp
@@ -0,0 +1,104 @@
+// RUN: %check_clang_tidy %s readability-deleted-default %t
+
+class NoDefault {
+public:
+  NoDefault() = delete;
+  NoDefault(NoDefault &&Other) = delete;
+  NoDefault(const NoDefault &Other) = delete;
+};
+
+class MissingEverything {
+public:
+  MissingEverything() = default;
+  // CHECK-MESSAGES: warning: this default constructor is marked '= default' but is actually implicitly deleted, probably because an instance variable or a base class is lacking a default constructor. You should either remove this definition or explicitly mark it as '= delete' [readability-deleted-default]
+  MissingEverything(MissingEverything &&Other) = default;
+  // CHECK-MESSAGES: warning: this move constructor is marked '= default' but is actually implicitly deleted, probably because an instance variable or a base class is not copyable nor movable. You should either remove this definition or explicitly mark it as '= delete' [readability-deleted-default]
+  MissingEverything(const MissingEverything &Other) = default;
+  // CHECK-MESSAGES: warning: this copy constructor is marked '= default' but is actually implicitly deleted, probably because an instance variable or a base class is not copyable. You should either remove this definition or explicitly mark it as '= delete' [readability-deleted-default]
+  MissingEverything &operator=(MissingEverything &&Other) = default;
+  // CHECK-MESSAGES: warning: this move assignment operator is marked '= default' but is actually implicitly deleted, probably because a base class or an instance variable is not assignable, e.g. because the latter is marked as const. You should either remove this definition or explicitly mark it as '= delete' [readability-deleted-default]
+  MissingEverything &operator=(const MissingEverything &Other) = default;
+  // CHECK-MESSAGES: warning: this copy assignment operator is marked '= default' but is actually implicitly deleted, probably because a base class or an instance variable is not assignable, e.g. because the latter is marked as const. You should either remove this definition or explicitly mark it as '= delete' [readability-deleted-default]
+
+private:
+  NoDefault ND;
+};
+
+class NotAssignable {
+public:
+  NotAssignable(NotAssignable &&Other) = default;
+  NotAssignable(const NotAssignable &Other) = default;
+  NotAssignable &operator=(NotAssignable &&Other) = default;
+  // CHECK-MESSAGES: warning: this move assignment operator is marked '= default' but is actually implicitly deleted
+  NotAssignable &operator=(const NotAssignable &Other) = default;
+  // CHECK-MESSAGES: warning: this copy assignment operator is marked '= default' but is actually implicitly deleted
+
+private:
+  const int I = 0;
+};
+
+class Movable {
+public:
+  Movable() = default;
+  Movable(Movable &&Other) = default;
+  Movable(const Movable &Other) = delete;
+  Movable &operator=(Movable &&Other) = default;
+  Movable &operator=(const Movable &Other) = delete;
+};
+
+class NotCopyable {
+public:
+  NotCopyable(NotCopyable &&Other) = default;
+  NotCopyable(const NotCopyable &Other) = default;
+  // CHECK-MESSAGES: warning: this copy constructor is marked '= default' but is actually implicitly deleted
+  NotCopyable &operator=(NotCopyable &&Other) = default;
+  NotCopyable &operator=(const NotCopyable &Other) = default;
+  // CHECK-MESSAGES: warning: this copy assignment operator is marked '= default' but is actually implicitly deleted
+private:
+  Movable M;
+};
+
+template  class Templated {
+public:
+  // No warning here, it is a templated class.
+  Templated() = default;
+  Templated(Templated &&Other) = default;
+  Templated(const Templated &Other) = default;
+  Templated &operator=(Templated &&Other) = default;
+  Templated &operator=(const Templated &Other) = default;
+
+  class InnerTemplated {
+  public:
+// This class is not in itself templated, but we still don't have warning.
+InnerTemplated() = default;
+InnerTemplated(InnerTemplated &&Other) = default;
+InnerTemplated(const InnerTemplated &Other) = default;
+InnerTemplated &operator=(InnerTemplated &&Other) = default;
+InnerTemplated &operator=(const InnerTemplated &Other) = default;
+
+  private:
+T TVar;
+  };
+
+  class InnerNotTemplated {
+  public:
+// This one could technically h

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

[Alex Pilkiewicz via cfe-commits]

pilki added a comment.

In http://reviews.llvm.org/D18961#397163, @dblaikie wrote:

> I'd consider just making this a compiler warning, perhaps?


I honestly don't feel competent right now to write compiler warnings: 
clang-tidy has a nice, well defined interface. A compiler warning would force 
me to dig in the internals of clang. But if you feel strongly about it and give 
me a couple of pointers (like a diff that adds a similar warning, and maybe a 
pointer to the general area in the code where I would add it), I can maybe give 
it a try in the next few days/weeks.


http://reviews.llvm.org/D18961



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

[Alex Pilkiewicz via cfe-commits]

pilki updated this revision to Diff 53289.
pilki marked 5 inline comments as done.

http://reviews.llvm.org/D18961

Files:
   clang-tidy/readability/CMakeLists.txt
   clang-tidy/readability/DeletedDefaultCheck.cpp
   clang-tidy/readability/DeletedDefaultCheck.h
   clang-tidy/readability/ReadabilityTidyModule.cpp
   docs/ReleaseNotes.rst
   docs/clang-tidy/checks/list.rst
   docs/clang-tidy/checks/readability-deleted-default.rst
   test/clang-tidy/readability-deleted-default.cpp

Index:  test/clang-tidy/readability-deleted-default.cpp
===
---  test/clang-tidy/readability-deleted-default.cpp
+++  test/clang-tidy/readability-deleted-default.cpp
@@ -0,0 +1,127 @@
+// RUN: %check_clang_tidy %s readability-deleted-default %t
+
+class NoDefault {
+public:
+  NoDefault() = delete;
+  NoDefault(NoDefault &&Other) = delete;
+  NoDefault(const NoDefault &Other) = delete;
+};
+
+class MissingEverything {
+public:
+  MissingEverything() = default;
+  // CHECK-MESSAGES: warning: default constructor is marked '= default' but is actually implicitly deleted, probably because an instance variable or a base class is lacking a default constructor; this definition should either be removed or explicitly marked as '= delete' [readability-deleted-default]
+  MissingEverything(MissingEverything &&Other) = default;
+  // CHECK-MESSAGES: warning: move constructor 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' [readability-deleted-default]
+  MissingEverything(const MissingEverything &Other) = default;
+  // CHECK-MESSAGES: warning: copy constructor is marked '= default' but is actually implicitly deleted, probably because an instance variable or a base class is not copyable; this definition should either be removed or explicitly marked as '= delete' [readability-deleted-default]
+  MissingEverything &operator=(MissingEverything &&Other) = default;
+  // CHECK-MESSAGES: warning: move assignment operator is marked '= default' but is actually implicitly deleted, probably because a base class or an instance variable is not assignable, e.g. because the latter is marked 'const'; this definition should either be removed or explicitly marked as '= delete' [readability-deleted-default]
+  MissingEverything &operator=(const MissingEverything &Other) = default;
+  // CHECK-MESSAGES: warning: copy assignment operator is marked '= default' but is actually implicitly deleted, probably because a base class or an instance variable is not assignable, e.g. because the latter is marked 'const'; this definition should either be removed or explicitly marked as '= delete' [readability-deleted-default]
+
+private:
+  NoDefault ND;
+};
+
+class NotAssignable {
+public:
+  NotAssignable(NotAssignable &&Other) = default;
+  NotAssignable(const NotAssignable &Other) = default;
+  NotAssignable &operator=(NotAssignable &&Other) = default;
+  // CHECK-MESSAGES: warning: move assignment operator is marked '= default' but is actually implicitly deleted
+  NotAssignable &operator=(const NotAssignable &Other) = default;
+  // CHECK-MESSAGES: warning: copy assignment operator is marked '= default' but is actually implicitly deleted
+
+private:
+  const int I = 0;
+};
+
+class Movable {
+public:
+  Movable() = default;
+  Movable(Movable &&Other) = default;
+  Movable(const Movable &Other) = delete;
+  Movable &operator=(Movable &&Other) = default;
+  Movable &operator=(const Movable &Other) = delete;
+};
+
+class NotCopyable {
+public:
+  NotCopyable(NotCopyable &&Other) = default;
+  NotCopyable(const NotCopyable &Other) = default;
+  // CHECK-MESSAGES: warning: copy constructor is marked '= default' but is actually implicitly deleted
+  NotCopyable &operator=(NotCopyable &&Other) = default;
+  NotCopyable &operator=(const NotCopyable &Other) = default;
+  // CHECK-MESSAGES: warning: copy assignment operator is marked '= default' but is actually implicitly deleted
+private:
+  Movable M;
+};
+
+template  class Templated {
+public:
+  // No warning here, it is a templated class.
+  Templated() = default;
+  Templated(Templated &&Other) = default;
+  Templated(const Templated &Other) = default;
+  Templated &operator=(Templated &&Other) = default;
+  Templated &operator=(const Templated &Other) = default;
+
+  class InnerTemplated {
+  public:
+// This class is not in itself templated, but we still don't have warning.
+InnerTemplated() = default;
+InnerTemplated(InnerTemplated &&Other) = default;
+InnerTemplated(const InnerTemplated &Other) = default;
+InnerTemplated &operator=(InnerTemplated &&Other) = default;
+InnerTemplated &operator=(const InnerTemplated &Other) = default;
+
+  private:
+T TVar;
+  };
+
+  class InnerNotTemplated {
+  public:
+// This one could technically have warnings, but currently

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

[Alex Pilkiewicz via cfe-commits]

pilki added a comment.

I hope I answered all comments (sorry if I missed some, I'm not yet used to 
this UI).

I have an open question about isInTemplateInstantiation, and added a test 
(since I missed templated function)



Comment at:  clang-tidy/readability/DeletedDefaultCheck.cpp:27
@@ +26,3 @@
+  const auto NotTemplate = unless(hasAncestor(
+  cxxRecordDecl(::clang::ast_matchers::isTemplateInstantiation(;
+

alexfh wrote:
> What about this comment?
isInTemplateInstantiation() is a matcher on Stmt only (when hasAncestor is a 
polymorphic matcher). When using it, it does not compile. So I copied the body 
here. I don't know how bad that is.


http://reviews.llvm.org/D18961



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

[Alex Pilkiewicz via cfe-commits]

pilki updated this revision to Diff 53371.
pilki marked an inline comment as done.

http://reviews.llvm.org/D18961

Files:
   clang-tidy/readability/CMakeLists.txt
   clang-tidy/readability/DeletedDefaultCheck.cpp
   clang-tidy/readability/DeletedDefaultCheck.h
   clang-tidy/readability/ReadabilityTidyModule.cpp
   docs/ReleaseNotes.rst
   docs/clang-tidy/checks/list.rst
   docs/clang-tidy/checks/readability-deleted-default.rst
   test/clang-tidy/readability-deleted-default.cpp

Index:  test/clang-tidy/readability-deleted-default.cpp
===
---  test/clang-tidy/readability-deleted-default.cpp
+++  test/clang-tidy/readability-deleted-default.cpp
@@ -0,0 +1,127 @@
+// RUN: %check_clang_tidy %s readability-deleted-default %t
+
+class NoDefault {
+public:
+  NoDefault() = delete;
+  NoDefault(NoDefault &&Other) = delete;
+  NoDefault(const NoDefault &Other) = delete;
+};
+
+class MissingEverything {
+public:
+  MissingEverything() = default;
+  // CHECK-MESSAGES: warning: default constructor is marked '= default' but is actually implicitly deleted, probably because an instance variable or a base class is lacking a default constructor; this definition should either be removed or explicitly marked as '= delete' [readability-deleted-default]
+  MissingEverything(MissingEverything &&Other) = default;
+  // CHECK-MESSAGES: warning: move constructor 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' [readability-deleted-default]
+  MissingEverything(const MissingEverything &Other) = default;
+  // CHECK-MESSAGES: warning: copy constructor is marked '= default' but is actually implicitly deleted, probably because an instance variable or a base class is not copyable; this definition should either be removed or explicitly marked as '= delete' [readability-deleted-default]
+  MissingEverything &operator=(MissingEverything &&Other) = default;
+  // CHECK-MESSAGES: warning: move assignment operator is marked '= default' but is actually implicitly deleted, probably because a base class or an instance variable is not assignable, e.g. because the latter is marked 'const'; this definition should either be removed or explicitly marked as '= delete' [readability-deleted-default]
+  MissingEverything &operator=(const MissingEverything &Other) = default;
+  // CHECK-MESSAGES: warning: copy assignment operator is marked '= default' but is actually implicitly deleted, probably because a base class or an instance variable is not assignable, e.g. because the latter is marked 'const'; this definition should either be removed or explicitly marked as '= delete' [readability-deleted-default]
+
+private:
+  NoDefault ND;
+};
+
+class NotAssignable {
+public:
+  NotAssignable(NotAssignable &&Other) = default;
+  NotAssignable(const NotAssignable &Other) = default;
+  NotAssignable &operator=(NotAssignable &&Other) = default;
+  // CHECK-MESSAGES: warning: move assignment operator is marked '= default' but is actually implicitly deleted
+  NotAssignable &operator=(const NotAssignable &Other) = default;
+  // CHECK-MESSAGES: warning: copy assignment operator is marked '= default' but is actually implicitly deleted
+
+private:
+  const int I = 0;
+};
+
+class Movable {
+public:
+  Movable() = default;
+  Movable(Movable &&Other) = default;
+  Movable(const Movable &Other) = delete;
+  Movable &operator=(Movable &&Other) = default;
+  Movable &operator=(const Movable &Other) = delete;
+};
+
+class NotCopyable {
+public:
+  NotCopyable(NotCopyable &&Other) = default;
+  NotCopyable(const NotCopyable &Other) = default;
+  // CHECK-MESSAGES: warning: copy constructor is marked '= default' but is actually implicitly deleted
+  NotCopyable &operator=(NotCopyable &&Other) = default;
+  NotCopyable &operator=(const NotCopyable &Other) = default;
+  // CHECK-MESSAGES: warning: copy assignment operator is marked '= default' but is actually implicitly deleted
+private:
+  Movable M;
+};
+
+template  class Templated {
+public:
+  // No warning here, it is a templated class.
+  Templated() = default;
+  Templated(Templated &&Other) = default;
+  Templated(const Templated &Other) = default;
+  Templated &operator=(Templated &&Other) = default;
+  Templated &operator=(const Templated &Other) = default;
+
+  class InnerTemplated {
+  public:
+// This class is not in itself templated, but we still don't have warning.
+InnerTemplated() = default;
+InnerTemplated(InnerTemplated &&Other) = default;
+InnerTemplated(const InnerTemplated &Other) = default;
+InnerTemplated &operator=(InnerTemplated &&Other) = default;
+InnerTemplated &operator=(const InnerTemplated &Other) = default;
+
+  private:
+T TVar;
+  };
+
+  class InnerNotTemplated {
+  public:
+// This one could technically have warnings, but currently

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

[Alex Pilkiewicz via cfe-commits]

pilki added inline comments.


Comment at:  clang-tidy/readability/DeletedDefaultCheck.cpp:35
@@ +34,3 @@
+ this);
+  Finder->addMatcher(cxxMethodDecl(anyOf(isCopyAssignmentOperator(),
+ isMoveAssignmentOperator()),

alexfh wrote:
> It's not overly important, but you can further reduce the code by folding the 
> first matcher into the second one (move `cxxConstructorDecl()` inside `anyOf` 
> here, since `CXXConstructorDecl` is a `CXXMethodDecl`). You can also use just 
> a single id to bind the node and distinguish the constructor using 
> `dyn_cast`. Then you'll be able to use a single `diag()` call below and 
> remove the unneeded variables `Message` and `isBadlyDefaulted`.
Mmh, I'm not convinced it really helps with readability here. I can do it if 
you insist though.


http://reviews.llvm.org/D18961



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

[Alex Pilkiewicz via cfe-commits]

pilki updated this revision to Diff 53381.
pilki marked an inline comment as done.

http://reviews.llvm.org/D18961

Files:
   clang-tidy/readability/CMakeLists.txt
   clang-tidy/readability/DeletedDefaultCheck.cpp
   clang-tidy/readability/DeletedDefaultCheck.h
   clang-tidy/readability/ReadabilityTidyModule.cpp
   docs/ReleaseNotes.rst
   docs/clang-tidy/checks/list.rst
   docs/clang-tidy/checks/readability-deleted-default.rst
   test/clang-tidy/readability-deleted-default.cpp

Index:  test/clang-tidy/readability-deleted-default.cpp
===
---  test/clang-tidy/readability-deleted-default.cpp
+++  test/clang-tidy/readability-deleted-default.cpp
@@ -0,0 +1,127 @@
+// RUN: %check_clang_tidy %s readability-deleted-default %t
+
+class NoDefault {
+public:
+  NoDefault() = delete;
+  NoDefault(NoDefault &&Other) = delete;
+  NoDefault(const NoDefault &Other) = delete;
+};
+
+class MissingEverything {
+public:
+  MissingEverything() = default;
+  // CHECK-MESSAGES: warning: default constructor is marked '= default' but is actually implicitly deleted, probably because an instance variable or a base class is lacking a default constructor; this definition should either be removed or explicitly marked as '= delete' [readability-deleted-default]
+  MissingEverything(MissingEverything &&Other) = default;
+  // CHECK-MESSAGES: warning: move constructor 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' [readability-deleted-default]
+  MissingEverything(const MissingEverything &Other) = default;
+  // CHECK-MESSAGES: warning: copy constructor is marked '= default' but is actually implicitly deleted, probably because an instance variable or a base class is not copyable; this definition should either be removed or explicitly marked as '= delete' [readability-deleted-default]
+  MissingEverything &operator=(MissingEverything &&Other) = default;
+  // CHECK-MESSAGES: warning: move assignment operator is marked '= default' but is actually implicitly deleted, probably because a base class or an instance variable is not assignable, e.g. because the latter is marked 'const'; this definition should either be removed or explicitly marked as '= delete' [readability-deleted-default]
+  MissingEverything &operator=(const MissingEverything &Other) = default;
+  // CHECK-MESSAGES: warning: copy assignment operator is marked '= default' but is actually implicitly deleted, probably because a base class or an instance variable is not assignable, e.g. because the latter is marked 'const'; this definition should either be removed or explicitly marked as '= delete' [readability-deleted-default]
+
+private:
+  NoDefault ND;
+};
+
+class NotAssignable {
+public:
+  NotAssignable(NotAssignable &&Other) = default;
+  NotAssignable(const NotAssignable &Other) = default;
+  NotAssignable &operator=(NotAssignable &&Other) = default;
+  // CHECK-MESSAGES: warning: move assignment operator is marked '= default' but is actually implicitly deleted
+  NotAssignable &operator=(const NotAssignable &Other) = default;
+  // CHECK-MESSAGES: warning: copy assignment operator is marked '= default' but is actually implicitly deleted
+
+private:
+  const int I = 0;
+};
+
+class Movable {
+public:
+  Movable() = default;
+  Movable(Movable &&Other) = default;
+  Movable(const Movable &Other) = delete;
+  Movable &operator=(Movable &&Other) = default;
+  Movable &operator=(const Movable &Other) = delete;
+};
+
+class NotCopyable {
+public:
+  NotCopyable(NotCopyable &&Other) = default;
+  NotCopyable(const NotCopyable &Other) = default;
+  // CHECK-MESSAGES: warning: copy constructor is marked '= default' but is actually implicitly deleted
+  NotCopyable &operator=(NotCopyable &&Other) = default;
+  NotCopyable &operator=(const NotCopyable &Other) = default;
+  // CHECK-MESSAGES: warning: copy assignment operator is marked '= default' but is actually implicitly deleted
+private:
+  Movable M;
+};
+
+template  class Templated {
+public:
+  // No warning here, it is a templated class.
+  Templated() = default;
+  Templated(Templated &&Other) = default;
+  Templated(const Templated &Other) = default;
+  Templated &operator=(Templated &&Other) = default;
+  Templated &operator=(const Templated &Other) = default;
+
+  class InnerTemplated {
+  public:
+// This class is not in itself templated, but we still don't have warning.
+InnerTemplated() = default;
+InnerTemplated(InnerTemplated &&Other) = default;
+InnerTemplated(const InnerTemplated &Other) = default;
+InnerTemplated &operator=(InnerTemplated &&Other) = default;
+InnerTemplated &operator=(const InnerTemplated &Other) = default;
+
+  private:
+T TVar;
+  };
+
+  class InnerNotTemplated {
+  public:
+// This one could technically have warnings, but currently

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

[Alex Pilkiewicz via cfe-commits]

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

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

[Alex Pilkiewicz via cfe-commits]

pilki added a comment.

Fixed all the comment but the one about merging the two matchers.


http://reviews.llvm.org/D18961



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

[Alex Pilkiewicz via cfe-commits]

pilki updated this revision to Diff 53404.
pilki marked 3 inline comments as done.

http://reviews.llvm.org/D18961

Files:
   clang-tidy/readability/CMakeLists.txt
   clang-tidy/readability/DeletedDefaultCheck.cpp
   clang-tidy/readability/DeletedDefaultCheck.h
   clang-tidy/readability/ReadabilityTidyModule.cpp
   docs/ReleaseNotes.rst
   docs/clang-tidy/checks/list.rst
   docs/clang-tidy/checks/readability-deleted-default.rst
   test/clang-tidy/readability-deleted-default.cpp

Index:  test/clang-tidy/readability-deleted-default.cpp
===
---  test/clang-tidy/readability-deleted-default.cpp
+++  test/clang-tidy/readability-deleted-default.cpp
@@ -0,0 +1,127 @@
+// RUN: %check_clang_tidy %s readability-deleted-default %t
+
+class NoDefault {
+public:
+  NoDefault() = delete;
+  NoDefault(NoDefault &&Other) = delete;
+  NoDefault(const NoDefault &Other) = delete;
+};
+
+class MissingEverything {
+public:
+  MissingEverything() = default;
+  // CHECK-MESSAGES: warning: default constructor is explicitly defaulted but implicitly deleted, probably because a non-static data member or a base class is lacking a default constructor; definition can either be removed or explicitly deleted [readability-deleted-default]
+  MissingEverything(MissingEverything &&Other) = default;
+  // CHECK-MESSAGES: warning: move constructor is explicitly defaulted but implicitly deleted, probably because a non-static data member or a base class is neither copyable nor movable; definition can either be removed or explicitly deleted [readability-deleted-default]
+  MissingEverything(const MissingEverything &Other) = default;
+  // CHECK-MESSAGES: warning: copy constructor is explicitly defaulted but implicitly deleted, probably because a non-static data member or a base class is not copyable; definition can either be removed or explicitly deleted [readability-deleted-default]
+  MissingEverything &operator=(MissingEverything &&Other) = default;
+  // CHECK-MESSAGES: warning: move assignment operator is explicitly defaulted but implicitly deleted, probably because a base class or a non-static data member is not assignable, e.g. because the latter is marked 'const'; definition can either be removed or explicitly deleted [readability-deleted-default]
+  MissingEverything &operator=(const MissingEverything &Other) = default;
+  // CHECK-MESSAGES: warning: copy assignment operator is explicitly defaulted but implicitly deleted, probably because a base class or a non-static data member is not assignable, e.g. because the latter is marked 'const'; definition can either be removed or explicitly deleted [readability-deleted-default]
+
+private:
+  NoDefault ND;
+};
+
+class NotAssignable {
+public:
+  NotAssignable(NotAssignable &&Other) = default;
+  NotAssignable(const NotAssignable &Other) = default;
+  NotAssignable &operator=(NotAssignable &&Other) = default;
+  // CHECK-MESSAGES: warning: move assignment operator is explicitly defaulted but implicitly deleted
+  NotAssignable &operator=(const NotAssignable &Other) = default;
+  // CHECK-MESSAGES: warning: copy assignment operator is explicitly defaulted but implicitly deleted
+
+private:
+  const int I = 0;
+};
+
+class Movable {
+public:
+  Movable() = default;
+  Movable(Movable &&Other) = default;
+  Movable(const Movable &Other) = delete;
+  Movable &operator=(Movable &&Other) = default;
+  Movable &operator=(const Movable &Other) = delete;
+};
+
+class NotCopyable {
+public:
+  NotCopyable(NotCopyable &&Other) = default;
+  NotCopyable(const NotCopyable &Other) = default;
+  // CHECK-MESSAGES: warning: copy constructor is explicitly defaulted but implicitly deleted
+  NotCopyable &operator=(NotCopyable &&Other) = default;
+  NotCopyable &operator=(const NotCopyable &Other) = default;
+  // CHECK-MESSAGES: warning: copy assignment operator is explicitly defaulted but implicitly deleted
+private:
+  Movable M;
+};
+
+template  class Templated {
+public:
+  // No warning here, it is a templated class.
+  Templated() = default;
+  Templated(Templated &&Other) = default;
+  Templated(const Templated &Other) = default;
+  Templated &operator=(Templated &&Other) = default;
+  Templated &operator=(const Templated &Other) = default;
+
+  class InnerTemplated {
+  public:
+// This class is not in itself templated, but we still don't have warning.
+InnerTemplated() = default;
+InnerTemplated(InnerTemplated &&Other) = default;
+InnerTemplated(const InnerTemplated &Other) = default;
+InnerTemplated &operator=(InnerTemplated &&Other) = default;
+InnerTemplated &operator=(const InnerTemplated &Other) = default;
+
+  private:
+T TVar;
+  };
+
+  class InnerNotTemplated {
+  public:
+// This one could technically have warnings, but currently doesn't.
+InnerNotTemplated() = default;
+InnerNotTemplated(InnerNotTemplated &&Other) = default;
+InnerNotTemplated(const InnerNotTemplated &Other) = default

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

[Alex Pilkiewicz via cfe-commits]

pilki added inline comments.


Comment at:  clang-tidy/readability/DeletedDefaultCheck.cpp:37
@@ +36,3 @@
+
+void DeletedDefaultCheck::check(const MatchFinder::MatchResult &Result) {
+  const StringRef Message = "%0 is explicitly defaulted but implicitly "

alexfh wrote:
> Will it be less confusing to you, if you change "assignment" to something 
> more generic, e.g. "method" or "decl"?
> 
> Also, if you find it less readable that way, we can leave it as is for now.
I implemented the requested changes


http://reviews.llvm.org/D18961



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

[Alex Pilkiewicz via cfe-commits]

pilki updated this revision to Diff 53525.
pilki marked 2 inline comments as done.

http://reviews.llvm.org/D18961

Files:
   clang-tidy/readability/CMakeLists.txt
   clang-tidy/readability/DeletedDefaultCheck.cpp
   clang-tidy/readability/DeletedDefaultCheck.h
   clang-tidy/readability/ReadabilityTidyModule.cpp
   docs/ReleaseNotes.rst
   docs/clang-tidy/checks/list.rst
   docs/clang-tidy/checks/readability-deleted-default.rst
   test/clang-tidy/readability-deleted-default.cpp

Index:  test/clang-tidy/readability-deleted-default.cpp
===
---  test/clang-tidy/readability-deleted-default.cpp
+++  test/clang-tidy/readability-deleted-default.cpp
@@ -0,0 +1,127 @@
+// RUN: %check_clang_tidy %s readability-deleted-default %t
+
+class NoDefault {
+public:
+  NoDefault() = delete;
+  NoDefault(NoDefault &&Other) = delete;
+  NoDefault(const NoDefault &Other) = delete;
+};
+
+class MissingEverything {
+public:
+  MissingEverything() = default;
+  // CHECK-MESSAGES: warning: default constructor is explicitly defaulted but implicitly deleted, probably because a non-static data member or a base class is lacking a default constructor; definition can either be removed or explicitly deleted [readability-deleted-default]
+  MissingEverything(MissingEverything &&Other) = default;
+  // CHECK-MESSAGES: warning: move constructor is explicitly defaulted but implicitly deleted, probably because a non-static data member or a base class is neither copyable nor movable; definition can either be removed or explicitly deleted [readability-deleted-default]
+  MissingEverything(const MissingEverything &Other) = default;
+  // CHECK-MESSAGES: warning: copy constructor is explicitly defaulted but implicitly deleted, probably because a non-static data member or a base class is not copyable; definition can either be removed or explicitly deleted [readability-deleted-default]
+  MissingEverything &operator=(MissingEverything &&Other) = default;
+  // CHECK-MESSAGES: warning: move assignment operator is explicitly defaulted but implicitly deleted, probably because a base class or a non-static data member is not assignable, e.g. because the latter is marked 'const'; definition can either be removed or explicitly deleted [readability-deleted-default]
+  MissingEverything &operator=(const MissingEverything &Other) = default;
+  // CHECK-MESSAGES: warning: copy assignment operator is explicitly defaulted but implicitly deleted, probably because a base class or a non-static data member is not assignable, e.g. because the latter is marked 'const'; definition can either be removed or explicitly deleted [readability-deleted-default]
+
+private:
+  NoDefault ND;
+};
+
+class NotAssignable {
+public:
+  NotAssignable(NotAssignable &&Other) = default;
+  NotAssignable(const NotAssignable &Other) = default;
+  NotAssignable &operator=(NotAssignable &&Other) = default;
+  // CHECK-MESSAGES: warning: move assignment operator is explicitly defaulted but implicitly deleted
+  NotAssignable &operator=(const NotAssignable &Other) = default;
+  // CHECK-MESSAGES: warning: copy assignment operator is explicitly defaulted but implicitly deleted
+
+private:
+  const int I = 0;
+};
+
+class Movable {
+public:
+  Movable() = default;
+  Movable(Movable &&Other) = default;
+  Movable(const Movable &Other) = delete;
+  Movable &operator=(Movable &&Other) = default;
+  Movable &operator=(const Movable &Other) = delete;
+};
+
+class NotCopyable {
+public:
+  NotCopyable(NotCopyable &&Other) = default;
+  NotCopyable(const NotCopyable &Other) = default;
+  // CHECK-MESSAGES: warning: copy constructor is explicitly defaulted but implicitly deleted
+  NotCopyable &operator=(NotCopyable &&Other) = default;
+  NotCopyable &operator=(const NotCopyable &Other) = default;
+  // CHECK-MESSAGES: warning: copy assignment operator is explicitly defaulted but implicitly deleted
+private:
+  Movable M;
+};
+
+template  class Templated {
+public:
+  // No warning here, it is a templated class.
+  Templated() = default;
+  Templated(Templated &&Other) = default;
+  Templated(const Templated &Other) = default;
+  Templated &operator=(Templated &&Other) = default;
+  Templated &operator=(const Templated &Other) = default;
+
+  class InnerTemplated {
+  public:
+// This class is not in itself templated, but we still don't have warning.
+InnerTemplated() = default;
+InnerTemplated(InnerTemplated &&Other) = default;
+InnerTemplated(const InnerTemplated &Other) = default;
+InnerTemplated &operator=(InnerTemplated &&Other) = default;
+InnerTemplated &operator=(const InnerTemplated &Other) = default;
+
+  private:
+T TVar;
+  };
+
+  class InnerNotTemplated {
+  public:
+// This one could technically have warnings, but currently doesn't.
+InnerNotTemplated() = default;
+InnerNotTemplated(InnerNotTemplated &&Other) = default;
+InnerNotTemplated(const InnerNotTemplated &Other) = default

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

[Alex Pilkiewicz via cfe-commits]

pilki updated this revision to Diff 53529.

http://reviews.llvm.org/D18961

Files:
   clang-tidy/readability/CMakeLists.txt
   clang-tidy/readability/DeletedDefaultCheck.cpp
   clang-tidy/readability/DeletedDefaultCheck.h
   clang-tidy/readability/ReadabilityTidyModule.cpp
   docs/ReleaseNotes.rst
   docs/clang-tidy/checks/list.rst
   docs/clang-tidy/checks/readability-deleted-default.rst
   test/clang-tidy/readability-deleted-default.cpp

Index:  test/clang-tidy/readability-deleted-default.cpp
===
---  test/clang-tidy/readability-deleted-default.cpp
+++  test/clang-tidy/readability-deleted-default.cpp
@@ -0,0 +1,127 @@
+// RUN: %check_clang_tidy %s readability-deleted-default %t
+
+class NoDefault {
+public:
+  NoDefault() = delete;
+  NoDefault(NoDefault &&Other) = delete;
+  NoDefault(const NoDefault &Other) = delete;
+};
+
+class MissingEverything {
+public:
+  MissingEverything() = default;
+  // CHECK-MESSAGES: warning: default constructor is explicitly defaulted but implicitly deleted, probably because a non-static data member or a base class is lacking a default constructor; definition can either be removed or explicitly deleted [readability-deleted-default]
+  MissingEverything(MissingEverything &&Other) = default;
+  // CHECK-MESSAGES: warning: move constructor is explicitly defaulted but implicitly deleted, probably because a non-static data member or a base class is neither copyable nor movable; definition can either be removed or explicitly deleted [readability-deleted-default]
+  MissingEverything(const MissingEverything &Other) = default;
+  // CHECK-MESSAGES: warning: copy constructor is explicitly defaulted but implicitly deleted, probably because a non-static data member or a base class is not copyable; definition can either be removed or explicitly deleted [readability-deleted-default]
+  MissingEverything &operator=(MissingEverything &&Other) = default;
+  // CHECK-MESSAGES: warning: move assignment operator is explicitly defaulted but implicitly deleted, probably because a base class or a non-static data member is not assignable, e.g. because the latter is marked 'const'; definition can either be removed or explicitly deleted [readability-deleted-default]
+  MissingEverything &operator=(const MissingEverything &Other) = default;
+  // CHECK-MESSAGES: warning: copy assignment operator is explicitly defaulted but implicitly deleted, probably because a base class or a non-static data member is not assignable, e.g. because the latter is marked 'const'; definition can either be removed or explicitly deleted [readability-deleted-default]
+
+private:
+  NoDefault ND;
+};
+
+class NotAssignable {
+public:
+  NotAssignable(NotAssignable &&Other) = default;
+  NotAssignable(const NotAssignable &Other) = default;
+  NotAssignable &operator=(NotAssignable &&Other) = default;
+  // CHECK-MESSAGES: warning: move assignment operator is explicitly defaulted but implicitly deleted
+  NotAssignable &operator=(const NotAssignable &Other) = default;
+  // CHECK-MESSAGES: warning: copy assignment operator is explicitly defaulted but implicitly deleted
+
+private:
+  const int I = 0;
+};
+
+class Movable {
+public:
+  Movable() = default;
+  Movable(Movable &&Other) = default;
+  Movable(const Movable &Other) = delete;
+  Movable &operator=(Movable &&Other) = default;
+  Movable &operator=(const Movable &Other) = delete;
+};
+
+class NotCopyable {
+public:
+  NotCopyable(NotCopyable &&Other) = default;
+  NotCopyable(const NotCopyable &Other) = default;
+  // CHECK-MESSAGES: warning: copy constructor is explicitly defaulted but implicitly deleted
+  NotCopyable &operator=(NotCopyable &&Other) = default;
+  NotCopyable &operator=(const NotCopyable &Other) = default;
+  // CHECK-MESSAGES: warning: copy assignment operator is explicitly defaulted but implicitly deleted
+private:
+  Movable M;
+};
+
+template  class Templated {
+public:
+  // No warning here, it is a templated class.
+  Templated() = default;
+  Templated(Templated &&Other) = default;
+  Templated(const Templated &Other) = default;
+  Templated &operator=(Templated &&Other) = default;
+  Templated &operator=(const Templated &Other) = default;
+
+  class InnerTemplated {
+  public:
+// This class is not in itself templated, but we still don't have warning.
+InnerTemplated() = default;
+InnerTemplated(InnerTemplated &&Other) = default;
+InnerTemplated(const InnerTemplated &Other) = default;
+InnerTemplated &operator=(InnerTemplated &&Other) = default;
+InnerTemplated &operator=(const InnerTemplated &Other) = default;
+
+  private:
+T TVar;
+  };
+
+  class InnerNotTemplated {
+  public:
+// This one could technically have warnings, but currently doesn't.
+InnerNotTemplated() = default;
+InnerNotTemplated(InnerNotTemplated &&Other) = default;
+InnerNotTemplated(const InnerNotTemplated &Other) = default;
+InnerNotTemplated &operator=(Inne

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

[Alex Pilkiewicz via cfe-commits]

pilki added a comment.

I did svn up and reuploaded the diff. I did not see any changes on files I 
touched, so I'm not sure it worked.


http://reviews.llvm.org/D18961



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

[Alex Pilkiewicz via cfe-commits]

pilki updated this revision to Diff 53532.

http://reviews.llvm.org/D18961

Files:
   clang-tidy/readability/CMakeLists.txt
   clang-tidy/readability/DeletedDefaultCheck.cpp
   clang-tidy/readability/DeletedDefaultCheck.h
   clang-tidy/readability/ReadabilityTidyModule.cpp
   docs/ReleaseNotes.rst
   docs/clang-tidy/checks/list.rst
   docs/clang-tidy/checks/readability-deleted-default.rst
   test/clang-tidy/readability-deleted-default.cpp

Index:  test/clang-tidy/readability-deleted-default.cpp
===
---  test/clang-tidy/readability-deleted-default.cpp
+++  test/clang-tidy/readability-deleted-default.cpp
@@ -0,0 +1,127 @@
+// RUN: %check_clang_tidy %s readability-deleted-default %t
+
+class NoDefault {
+public:
+  NoDefault() = delete;
+  NoDefault(NoDefault &&Other) = delete;
+  NoDefault(const NoDefault &Other) = delete;
+};
+
+class MissingEverything {
+public:
+  MissingEverything() = default;
+  // CHECK-MESSAGES: warning: default constructor is explicitly defaulted but implicitly deleted, probably because a non-static data member or a base class is lacking a default constructor; definition can either be removed or explicitly deleted [readability-deleted-default]
+  MissingEverything(MissingEverything &&Other) = default;
+  // CHECK-MESSAGES: warning: move constructor is explicitly defaulted but implicitly deleted, probably because a non-static data member or a base class is neither copyable nor movable; definition can either be removed or explicitly deleted [readability-deleted-default]
+  MissingEverything(const MissingEverything &Other) = default;
+  // CHECK-MESSAGES: warning: copy constructor is explicitly defaulted but implicitly deleted, probably because a non-static data member or a base class is not copyable; definition can either be removed or explicitly deleted [readability-deleted-default]
+  MissingEverything &operator=(MissingEverything &&Other) = default;
+  // CHECK-MESSAGES: warning: move assignment operator is explicitly defaulted but implicitly deleted, probably because a base class or a non-static data member is not assignable, e.g. because the latter is marked 'const'; definition can either be removed or explicitly deleted [readability-deleted-default]
+  MissingEverything &operator=(const MissingEverything &Other) = default;
+  // CHECK-MESSAGES: warning: copy assignment operator is explicitly defaulted but implicitly deleted, probably because a base class or a non-static data member is not assignable, e.g. because the latter is marked 'const'; definition can either be removed or explicitly deleted [readability-deleted-default]
+
+private:
+  NoDefault ND;
+};
+
+class NotAssignable {
+public:
+  NotAssignable(NotAssignable &&Other) = default;
+  NotAssignable(const NotAssignable &Other) = default;
+  NotAssignable &operator=(NotAssignable &&Other) = default;
+  // CHECK-MESSAGES: warning: move assignment operator is explicitly defaulted but implicitly deleted
+  NotAssignable &operator=(const NotAssignable &Other) = default;
+  // CHECK-MESSAGES: warning: copy assignment operator is explicitly defaulted but implicitly deleted
+
+private:
+  const int I = 0;
+};
+
+class Movable {
+public:
+  Movable() = default;
+  Movable(Movable &&Other) = default;
+  Movable(const Movable &Other) = delete;
+  Movable &operator=(Movable &&Other) = default;
+  Movable &operator=(const Movable &Other) = delete;
+};
+
+class NotCopyable {
+public:
+  NotCopyable(NotCopyable &&Other) = default;
+  NotCopyable(const NotCopyable &Other) = default;
+  // CHECK-MESSAGES: warning: copy constructor is explicitly defaulted but implicitly deleted
+  NotCopyable &operator=(NotCopyable &&Other) = default;
+  NotCopyable &operator=(const NotCopyable &Other) = default;
+  // CHECK-MESSAGES: warning: copy assignment operator is explicitly defaulted but implicitly deleted
+private:
+  Movable M;
+};
+
+template  class Templated {
+public:
+  // No warning here, it is a templated class.
+  Templated() = default;
+  Templated(Templated &&Other) = default;
+  Templated(const Templated &Other) = default;
+  Templated &operator=(Templated &&Other) = default;
+  Templated &operator=(const Templated &Other) = default;
+
+  class InnerTemplated {
+  public:
+// This class is not in itself templated, but we still don't have warning.
+InnerTemplated() = default;
+InnerTemplated(InnerTemplated &&Other) = default;
+InnerTemplated(const InnerTemplated &Other) = default;
+InnerTemplated &operator=(InnerTemplated &&Other) = default;
+InnerTemplated &operator=(const InnerTemplated &Other) = default;
+
+  private:
+T TVar;
+  };
+
+  class InnerNotTemplated {
+  public:
+// This one could technically have warnings, but currently doesn't.
+InnerNotTemplated() = default;
+InnerNotTemplated(InnerNotTemplated &&Other) = default;
+InnerNotTemplated(const InnerNotTemplated &Other) = default;
+InnerNotTemplated &operator=(Inne

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

[Alex Pilkiewicz via cfe-commits]

pilki updated this revision to Diff 53533.

http://reviews.llvm.org/D18961

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/DeletedDefaultCheck.cpp
  clang-tidy/readability/DeletedDefaultCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-deleted-default.rst
  test/clang-tidy/readability-deleted-default.cpp

Index: test/clang-tidy/readability-deleted-default.cpp
===
--- test/clang-tidy/readability-deleted-default.cpp
+++ test/clang-tidy/readability-deleted-default.cpp
@@ -0,0 +1,127 @@
+// RUN: %check_clang_tidy %s readability-deleted-default %t
+
+class NoDefault {
+public:
+  NoDefault() = delete;
+  NoDefault(NoDefault &&Other) = delete;
+  NoDefault(const NoDefault &Other) = delete;
+};
+
+class MissingEverything {
+public:
+  MissingEverything() = default;
+  // CHECK-MESSAGES: warning: default constructor is explicitly defaulted but implicitly deleted, probably because a non-static data member or a base class is lacking a default constructor; definition can either be removed or explicitly deleted [readability-deleted-default]
+  MissingEverything(MissingEverything &&Other) = default;
+  // CHECK-MESSAGES: warning: move constructor is explicitly defaulted but implicitly deleted, probably because a non-static data member or a base class is neither copyable nor movable; definition can either be removed or explicitly deleted [readability-deleted-default]
+  MissingEverything(const MissingEverything &Other) = default;
+  // CHECK-MESSAGES: warning: copy constructor is explicitly defaulted but implicitly deleted, probably because a non-static data member or a base class is not copyable; definition can either be removed or explicitly deleted [readability-deleted-default]
+  MissingEverything &operator=(MissingEverything &&Other) = default;
+  // CHECK-MESSAGES: warning: move assignment operator is explicitly defaulted but implicitly deleted, probably because a base class or a non-static data member is not assignable, e.g. because the latter is marked 'const'; definition can either be removed or explicitly deleted [readability-deleted-default]
+  MissingEverything &operator=(const MissingEverything &Other) = default;
+  // CHECK-MESSAGES: warning: copy assignment operator is explicitly defaulted but implicitly deleted, probably because a base class or a non-static data member is not assignable, e.g. because the latter is marked 'const'; definition can either be removed or explicitly deleted [readability-deleted-default]
+
+private:
+  NoDefault ND;
+};
+
+class NotAssignable {
+public:
+  NotAssignable(NotAssignable &&Other) = default;
+  NotAssignable(const NotAssignable &Other) = default;
+  NotAssignable &operator=(NotAssignable &&Other) = default;
+  // CHECK-MESSAGES: warning: move assignment operator is explicitly defaulted but implicitly deleted
+  NotAssignable &operator=(const NotAssignable &Other) = default;
+  // CHECK-MESSAGES: warning: copy assignment operator is explicitly defaulted but implicitly deleted
+
+private:
+  const int I = 0;
+};
+
+class Movable {
+public:
+  Movable() = default;
+  Movable(Movable &&Other) = default;
+  Movable(const Movable &Other) = delete;
+  Movable &operator=(Movable &&Other) = default;
+  Movable &operator=(const Movable &Other) = delete;
+};
+
+class NotCopyable {
+public:
+  NotCopyable(NotCopyable &&Other) = default;
+  NotCopyable(const NotCopyable &Other) = default;
+  // CHECK-MESSAGES: warning: copy constructor is explicitly defaulted but implicitly deleted
+  NotCopyable &operator=(NotCopyable &&Other) = default;
+  NotCopyable &operator=(const NotCopyable &Other) = default;
+  // CHECK-MESSAGES: warning: copy assignment operator is explicitly defaulted but implicitly deleted
+private:
+  Movable M;
+};
+
+template  class Templated {
+public:
+  // No warning here, it is a templated class.
+  Templated() = default;
+  Templated(Templated &&Other) = default;
+  Templated(const Templated &Other) = default;
+  Templated &operator=(Templated &&Other) = default;
+  Templated &operator=(const Templated &Other) = default;
+
+  class InnerTemplated {
+  public:
+// This class is not in itself templated, but we still don't have warning.
+InnerTemplated() = default;
+InnerTemplated(InnerTemplated &&Other) = default;
+InnerTemplated(const InnerTemplated &Other) = default;
+InnerTemplated &operator=(InnerTemplated &&Other) = default;
+InnerTemplated &operator=(const InnerTemplated &Other) = default;
+
+  private:
+T TVar;
+  };
+
+  class InnerNotTemplated {
+  public:
+// This one could technically have warnings, but currently doesn't.
+InnerNotTemplated() = default;
+InnerNotTemplated(InnerNotTemplated &&Other) = default;
+InnerNotTemplated(const InnerNotTemplated &Other) = default;
+InnerNotTemplated &operator=(InnerNotTemplat

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

[Alex Pilkiewicz via cfe-commits]

pilki added a comment.

Sorry, both my 'diff' and my 'svn' command added some magical coloring... It 
should be good now.


http://reviews.llvm.org/D18961



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits