Re: [PATCH] D13871: Add modernize-use-default check to clang-tidy.

2015-10-20 Thread Angel Garcia via cfe-commits 
angelgarcia updated this revision to Diff 37833.
angelgarcia added a comment.

Global variable -> static, and a few other additions.

> Will be good idea to add handling of cases where default constructor is empty 
> and only call base class(es) default constructor/members default constructors


I agree, but checking all members and base classes is the main difficulty of 
the copy-constructor/copy-assignment operator. When I implement those I will 
probably update this.


http://reviews.llvm.org/D13871

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/UseDefaultCheck.cpp
  clang-tidy/modernize/UseDefaultCheck.h
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-use-default.rst
  test/clang-tidy/modernize-use-default.cpp

Index: test/clang-tidy/modernize-use-default.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-default.cpp
@@ -0,0 +1,137 @@
+// RUN: %python %S/check_clang_tidy.py %s modernize-use-default %t
+
+class A {
+public:
+  A();
+  ~A();
+};
+
+A::A() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use '= default' to define a trivial default constructor [modernize-use-default]
+// CHECK-FIXES: A::A() = default;
+A::~A() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use '= default' to define a trivial destructor [modernize-use-default]
+// CHECK-FIXES: A::~A() = default;
+
+// Inline definitions.
+class B {
+public:
+  B() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
+  // CHECK-FIXES: B() = default;
+  ~B() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
+  // CHECK-FIXES: ~B() = default;
+};
+
+void f();
+
+class C {
+public:
+  // Non-empty constructor body.
+  C() { f(); }
+  // Non-empty destructor body.
+  ~C() { f(); }
+};
+
+class D {
+public:
+  // Constructor with initializer.
+  D() : Field(5) {}
+  // Constructor with arguments.
+  D(int Arg1, int Arg2) {}
+  int Field;
+};
+
+// Private constructor/destructor.
+class E {
+  E() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
+  // CHECK-FIXES: E() = default;
+  ~E() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
+  // CHECK-FIXES: ~E() = default;
+};
+
+// struct.
+struct F {
+  F() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
+  // CHECK-FIXES: F() = default;
+  ~F() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
+  // CHECK-FIXES: F() = default;
+};
+
+// Deleted constructor/destructor.
+class G {
+public:
+  G() = delete;
+  ~G() = delete;
+};
+
+// Do not remove other keywords.
+class H {
+public:
+  explicit H() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
+  // CHECK-FIXES: explicit H() = default;
+  virtual ~H() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
+  // CHECK-FIXES: virtual ~H() = default;
+};
+
+// Nested class.
+struct I {
+  struct II {
+II() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use '= default'
+// CHECK-FIXES: II() = default;
+~II() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use '= default'
+// CHECK-FIXES: ~II() = default;
+  };
+  int Int;
+};
+
+// Class template.
+template 
+class J {
+public:
+  J() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
+  // CHECK-FIXES: J() = default;
+  ~J() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
+  // CHECK-FIXES: ~J() = default;
+};
+
+// Non user-provided constructor/destructor.
+struct K {
+  int Int;
+};
+void g() {
+  K *PtrK = new K();
+  PtrK->~K();
+  delete PtrK;
+}
+
+// Already using default.
+struct L {
+  L() = default;
+  ~L() = default;
+};
+struct M {
+  M();
+  ~M();
+};
+M::M() = default;
+M::~M() = default;
+
+// Delegating constructor and overriden destructor.
+struct N : H {
+  N() : H() {}
+  ~N() override {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
+  // CHECK-FIXES: ~N() override = default;
+};
Index: docs/clang-tidy/checks/modernize-use-default.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-use-default.rst
@@ -0,0 +1,27 @@
+modernize-use-default
+=
+
+This check replaces default bodies of special member functions with ``=
+default;``.  The explicitly defaulted function declarations enable more
+opportunities in optimization, because the compiler might treat explicitly
+defaulted functions as trivial.
+
+.. code-block:: c++
+
+  struct A {
+A() {}
+~A();
+  };
+  A::~A() {}
+
+  // becomes
+
+  struct A {
+A() = default;
+~A();
+  };
+  A::~A() = default;
+
+.. note::
+  Copy-constructor, copy-assignment operator, move-constructor and
+  move-assignment operator are not supported yet.
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++

Re: [PATCH] D13871: Add modernize-use-default check to clang-tidy.

2015-10-20 Thread Angel Garcia via cfe-commits 
angelgarcia updated this revision to Diff 37859.
angelgarcia added a comment.

Remove the fixme.


http://reviews.llvm.org/D13871

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/UseDefaultCheck.cpp
  clang-tidy/modernize/UseDefaultCheck.h
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-use-default.rst
  test/clang-tidy/modernize-use-default.cpp

Index: test/clang-tidy/modernize-use-default.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-default.cpp
@@ -0,0 +1,137 @@
+// RUN: %python %S/check_clang_tidy.py %s modernize-use-default %t
+
+class A {
+public:
+  A();
+  ~A();
+};
+
+A::A() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use '= default' to define a trivial default constructor [modernize-use-default]
+// CHECK-FIXES: A::A() = default;
+A::~A() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use '= default' to define a trivial destructor [modernize-use-default]
+// CHECK-FIXES: A::~A() = default;
+
+// Inline definitions.
+class B {
+public:
+  B() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
+  // CHECK-FIXES: B() = default;
+  ~B() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
+  // CHECK-FIXES: ~B() = default;
+};
+
+void f();
+
+class C {
+public:
+  // Non-empty constructor body.
+  C() { f(); }
+  // Non-empty destructor body.
+  ~C() { f(); }
+};
+
+class D {
+public:
+  // Constructor with initializer.
+  D() : Field(5) {}
+  // Constructor with arguments.
+  D(int Arg1, int Arg2) {}
+  int Field;
+};
+
+// Private constructor/destructor.
+class E {
+  E() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
+  // CHECK-FIXES: E() = default;
+  ~E() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
+  // CHECK-FIXES: ~E() = default;
+};
+
+// struct.
+struct F {
+  F() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
+  // CHECK-FIXES: F() = default;
+  ~F() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
+  // CHECK-FIXES: F() = default;
+};
+
+// Deleted constructor/destructor.
+class G {
+public:
+  G() = delete;
+  ~G() = delete;
+};
+
+// Do not remove other keywords.
+class H {
+public:
+  explicit H() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
+  // CHECK-FIXES: explicit H() = default;
+  virtual ~H() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
+  // CHECK-FIXES: virtual ~H() = default;
+};
+
+// Nested class.
+struct I {
+  struct II {
+II() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use '= default'
+// CHECK-FIXES: II() = default;
+~II() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use '= default'
+// CHECK-FIXES: ~II() = default;
+  };
+  int Int;
+};
+
+// Class template.
+template 
+class J {
+public:
+  J() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
+  // CHECK-FIXES: J() = default;
+  ~J() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
+  // CHECK-FIXES: ~J() = default;
+};
+
+// Non user-provided constructor/destructor.
+struct K {
+  int Int;
+};
+void g() {
+  K *PtrK = new K();
+  PtrK->~K();
+  delete PtrK;
+}
+
+// Already using default.
+struct L {
+  L() = default;
+  ~L() = default;
+};
+struct M {
+  M();
+  ~M();
+};
+M::M() = default;
+M::~M() = default;
+
+// Delegating constructor and overriden destructor.
+struct N : H {
+  N() : H() {}
+  ~N() override {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
+  // CHECK-FIXES: ~N() override = default;
+};
Index: docs/clang-tidy/checks/modernize-use-default.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-use-default.rst
@@ -0,0 +1,27 @@
+modernize-use-default
+=
+
+This check replaces default bodies of special member functions with ``=
+default;``.  The explicitly defaulted function declarations enable more
+opportunities in optimization, because the compiler might treat explicitly
+defaulted functions as trivial.
+
+.. code-block:: c++
+
+  struct A {
+A() {}
+~A();
+  };
+  A::~A() {}
+
+  // becomes
+
+  struct A {
+A() = default;
+~A();
+  };
+  A::~A() = default;
+
+.. note::
+  Copy-constructor, copy-assignment operator, move-constructor and
+  move-assignment operator are not supported yet.
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -54,6 +54,7 @@
modernize-replace-auto-ptr
modernize-shrink-to-fit
modernize-use-auto
+   modernize-use-default
modernize-use-nullptr
modernize-use-override
readability-braces-around-statements
Index: clang-tidy/modernize/UseDefaultCheck.h
===
--- /dev/null

Re: [PATCH] D13871: Add modernize-use-default check to clang-tidy.

2015-10-20 Thread Manuel Klimek via cfe-commits 
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.

LG. Yay!



Comment at: clang-tidy/modernize/UseDefaultCheck.cpp:59-60
@@ +58,4 @@
+  "= default;");
+  // FIXME: this can generate a -Wpedantic warning if there is a semicolon 
after
+  // the body: "A() {};" is converted to "A() = default;;"
+}

Delete the fixme. There already is an extra semicolon in the first case.


http://reviews.llvm.org/D13871



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

Re: [PATCH] D13871: Add modernize-use-default check to clang-tidy.

2015-10-19 Thread Eugene Zelenko via cfe-commits 
Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Eugene.Zelenko added a comment.

Will be good idea to add handling of cases where default constructor is empty 
and only call base class(es) default constructor/members default constructors 
(see http://en.cppreference.com/w/cpp/language/default_constructor).

In test check should suggest to use default for N() : H() {}.

Of course, if it will be easy to implement. Otherwise, such cases may be left 
for future development.


http://reviews.llvm.org/D13871



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

Re: [PATCH] D13871: Add modernize-use-default check to clang-tidy.

2015-10-19 Thread George Burgess IV via cfe-commits 
george.burgess.iv added a subscriber: george.burgess.iv.
george.burgess.iv added a comment.

This looks like a good check -- thanks for adding it! Just one small nit for 
you.



Comment at: clang-tidy/modernize/UseDefaultCheck.cpp:19
@@ +18,3 @@
+
+const char CtorDtor[] = "CtorDtorDecl";
+

I don't see where this is used outside of UseDefaultCheck.cpp. Can we make it 
static?


http://reviews.llvm.org/D13871



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