Re: [PATCH] D23353: [clang-tidy] Add check 'misc-use-after-move'

[Matt Kulukundis via cfe-commits]

fowles added a subscriber: fowles.


Comment at: test/clang-tidy/misc-use-after-move.cpp:158
@@ +157,3 @@
+std::move(ptr);
+ptr.get();
+  }

would this warn on:

std::unique_ptr ptr;
std::move(ptr);
ptr->Foo();

I would like it to since that is a likely segfault.


Comment at: test/clang-tidy/misc-use-after-move.cpp:279
@@ +278,3 @@
+A a;
+auto lambda = [=]() { a.foo(); };
+std::move(a);

can you add tests with reference capture?

also what about:

A a;
auto lambda = [&]() { a.foo(); };
std::move(a);
lambda();


https://reviews.llvm.org/D23353



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

Re: [PATCH] D18408: readability check for const params in declarations

[Matt Kulukundis via cfe-commits]

fowles added a comment.

ping?


http://reviews.llvm.org/D18408



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

Re: [PATCH] D18408: readability check for const params in declarations

[Matt Kulukundis via cfe-commits]

fowles updated this revision to Diff 51954.
fowles marked 2 inline comments as done.
fowles added a comment.

- review comments
- rename variables and remove MSVC compat issues


http://reviews.llvm.org/D18408

Files:
  clang-tidy/readability/AvoidConstParamsInDecls.cpp
  clang-tidy/readability/AvoidConstParamsInDecls.h
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-avoid-const-params-in-decls.rst
  test/clang-tidy/readability-avoid-const-params-in-decls.cpp

Index: test/clang-tidy/readability-avoid-const-params-in-decls.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-avoid-const-params-in-decls.cpp
@@ -0,0 +1,78 @@
+// RUN: %check_clang_tidy %s readability-avoid-const-params-in-decls %t
+
+using alias_type = bool;
+using alias_const_type = const bool;
+
+
+void F1(const int i);
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'i' is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]
+// CHECK-FIXES: void F1(int i);
+
+void F2(const int *const i);
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'i' is const-qualified
+// CHECK-FIXES: void F2(const int * i);
+
+void F3(int const i);
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'i' is const-qualified
+// CHECK-FIXES: void F3(int i);
+
+void F4(alias_type const i);
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'i' is const-qualified
+// CHECK-FIXES: void F4(alias_type i);
+
+void F5(const int);
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 1 is const-qualified
+// CHECK-FIXES: void F5(int);
+
+void F6(const int *const);
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 1 is const-qualified
+// BUG(b/27584482): void F6(const int *);  should be produced
+
+void F7(int, const int);
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: parameter 2 is const-qualified
+// CHECK-FIXES: void F7(int, int);
+
+void F8(const int, const int);
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 1 is const-qualified
+// CHECK-MESSAGES: :[[@LINE-2]]:20: warning: parameter 2 is const-qualified
+// CHECK-FIXES: void F8(int, int);
+
+void F9(const int long_name);
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'long_name'
+// CHECK-FIXES: void F9(int long_name);
+
+void F10(const int *const *const long_name);
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: parameter 'long_name'
+// CHECK-FIXES: void F10(const int *const * long_name);
+
+
+struct Foo {
+  Foo(const int i);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: parameter 'i'
+  // CHECK-FIXES: Foo(int i);
+
+  void operator()(const int i);
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: parameter 'i'
+  // CHECK-FIXES: void operator()(int i);
+};
+
+// Do not match on definitions
+void NF1(const int i) {}
+void NF2(const int *const i) {}
+void NF3(int const i) {}
+void NF4(alias_type const i) {}
+void NF5(const int) {}
+void NF6(const int *const) {}
+void NF7(int, const int) {}
+void NF8(const int, const int) {}
+
+// Do not match on other stuff
+void NF(const alias_type& i);
+void NF(const int &i);
+void NF(const int *i);
+void NF(alias_const_type i);
+void NF(const alias_type&);
+void NF(const int&);
+void NF(const int*);
+void NF(const int* const*);
+void NF(alias_const_type);
Index: docs/clang-tidy/checks/readability-avoid-const-params-in-decls.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-avoid-const-params-in-decls.rst
@@ -0,0 +1,17 @@
+.. title:: clang-tidy - readability-avoid-const-params-in-decls
+
+readability-avoid-const-params-in-decls
+===
+
+Checks whether a function declaration has parameters that are top level const.
+
+`const` values in declarations do not affect the signature of a function, so
+they should not be put there.  For example:
+
+Examples:
+
+.. code:: c++
+
+  void f(const string);   // Bad: const is top level.
+  void f(const string&);  // Good: const is not top level.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -89,8 +89,9 @@
performance-faster-string-find
performance-for-range-copy
performance-implicit-cast-in-loop
-   performance-unnecessary-value-param
performance-unnecessary-copy-initialization
+   performance-unnecessary-value-param
+   readability-avoid-const-params-in-decls
readability-braces-around-statements
readability-container-size-empty
readability-else-after-return
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readab

Re: [PATCH] D18408: readability check for const params in declarations

[Matt Kulukundis via cfe-commits]

fowles added a subscriber: fowles.
fowles added a comment.

My attempts to do this end with:

$ arc diff
Linting...
No lint engine configured for this project.
Running unit tests...
No unit test engine is configured for this project.
 SKIP STAGING  Unable to determine repository for this change.
Exception
ERR_CLOSED: This revision has already been closed.
(Run with `--trace` for a full exception trace.)


Repository:
  rL LLVM

http://reviews.llvm.org/D18408



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

Re: [PATCH] D18408: readability check for const params in declarations

[Matt Kulukundis via cfe-commits]

fowles added a comment.

that is what I was trying to do.  I can't seem to make arc play nice.


Repository:
  rL LLVM

http://reviews.llvm.org/D18408



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

[PATCH] D18608: note for top-level consts in function decls tidy

[Matt Kulukundis via cfe-commits]

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

Add missing release note

http://reviews.llvm.org/D18608

Files:
  docs/ReleaseNotes.rst

Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -150,6 +150,8 @@
 
 - New checks flagging various readability-related issues:
 
+  * `readability-avoid-const-params-in-decls
+
`_
   * `readability-identifier-naming
 
`_
   * `readability-implicit-bool-cast


Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -150,6 +150,8 @@
 
 - New checks flagging various readability-related issues:
 
+  * `readability-avoid-const-params-in-decls
+`_
   * `readability-identifier-naming
 `_
   * `readability-implicit-bool-cast
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18408: readability check for const params in declarations

[Matt Kulukundis via cfe-commits]

fowles added a comment.

Helped by sbenza@ http://reviews.llvm.org/D18608


Repository:
  rL LLVM

http://reviews.llvm.org/D18408



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

Re: [PATCH] D18608: note for top-level consts in function decls tidy

[Matt Kulukundis via cfe-commits]

fowles added inline comments.


Comment at: docs/ReleaseNotes.rst:153-154
@@ -152,2 +152,4 @@
 
+  * `readability-avoid-const-params-in-decls
+
`_
   * `readability-identifier-naming

LegalizeAdulthood wrote:
> Was this added in 3.8 or is it being added in 3.9?
> 
> That URL gives me 404 not found.
> 
> If it is new to 3.9, it should be farther up in the file.
I don't know.  It was committed yesterday.


http://reviews.llvm.org/D18608



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

Re: [PATCH] D18608: note for top-level consts in function decls tidy

[Matt Kulukundis via cfe-commits]

fowles updated this revision to Diff 52126.
fowles added a comment.

- move release note to 3.9 section


http://reviews.llvm.org/D18608

Files:
  docs/ReleaseNotes.rst

Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -68,6 +68,10 @@
   This check selectively replaces string literals containing escaped
   characters with raw string literals.
 
+- New ``readability-avoid-const-params-in-decls`` check
+
+  This check warns about top-level const parameters in function delcartions.
+
 Clang-tidy changes from 3.7 to 3.8
 ^^
 


Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -68,6 +68,10 @@
   This check selectively replaces string literals containing escaped
   characters with raw string literals.
 
+- New ``readability-avoid-const-params-in-decls`` check
+
+  This check warns about top-level const parameters in function delcartions.
+
 Clang-tidy changes from 3.7 to 3.8
 ^^
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18608: note for top-level consts in function decls tidy

[Matt Kulukundis via cfe-commits]

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

http://reviews.llvm.org/D18608



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

Re: [PATCH] D18993: [clang-tidy] fix readability-avoid-const-params-in-decls creating invalid code in fix-its

[Matt Kulukundis via cfe-commits]

fowles accepted this revision.
fowles added a comment.
This revision is now accepted and ready to land.

Thanks for doing this!  I tried a bunch of variants and just couldn't figure it 
out.


http://reviews.llvm.org/D18993



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

Re: [PATCH] D20010: [clang-tidy] UnnecessaryCopyInitialization - Extend to trigger on non-const "this" object argument if it is not modified

[Matt Kulukundis via cfe-commits]

fowles added a comment.

LGTM, but I will wait for people who know this stuff better to sign off


http://reviews.llvm.org/D20010



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

Re: [PATCH] D18149: Add check for unneeded copies of locals

[Matt Kulukundis via cfe-commits]

fowles updated this revision to Diff 50927.

http://reviews.llvm.org/D18149

Files:
  clang-tidy/performance/UnnecessaryCopyInitialization.cpp
  clang-tidy/performance/UnnecessaryCopyInitialization.h
  docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
  test/clang-tidy/performance-unnecessary-copy-initialization.cpp

Index: test/clang-tidy/performance-unnecessary-copy-initialization.cpp
===
--- test/clang-tidy/performance-unnecessary-copy-initialization.cpp
+++ test/clang-tidy/performance-unnecessary-copy-initialization.cpp
@@ -1,28 +1,33 @@
 // RUN: %check_clang_tidy %s performance-unnecessary-copy-initialization %t
 
 struct ExpensiveToCopyType {
-  ExpensiveToCopyType() {}
-  virtual ~ExpensiveToCopyType() {}
-  const ExpensiveToCopyType &reference() const { return *this; }
-  void nonConstMethod() {}
+  ExpensiveToCopyType();
+  virtual ~ExpensiveToCopyType();
+  const ExpensiveToCopyType &reference() const;
+  void nonConstMethod();
+  bool constMethod() const;
 };
 
 struct TrivialToCopyType {
-  const TrivialToCopyType &reference() const { return *this; }
+  const TrivialToCopyType &reference() const;
 };
 
-const ExpensiveToCopyType &ExpensiveTypeReference() {
-  static const ExpensiveToCopyType *Type = new ExpensiveToCopyType();
-  return *Type;
-}
+struct WeirdCopyCtorType {
+  WeirdCopyCtorType();
+  WeirdCopyCtorType(const WeirdCopyCtorType &w, bool oh_yes = true);
 
-const TrivialToCopyType &TrivialTypeReference() {
-  static const TrivialToCopyType *Type = new TrivialToCopyType();
-  return *Type;
-}
+  void nonConstMethod();
+  bool constMethod() const;
+};
+
+ExpensiveToCopyType global_expensive_to_copy_type;
+
+const ExpensiveToCopyType &ExpensiveTypeReference();
+const TrivialToCopyType &TrivialTypeReference();
 
 void mutate(ExpensiveToCopyType &);
 void mutate(ExpensiveToCopyType *);
+void useAsConstPointer(const ExpensiveToCopyType *);
 void useAsConstReference(const ExpensiveToCopyType &);
 void useByValue(ExpensiveToCopyType);
 
@@ -203,23 +208,119 @@
   ExpensiveToCopyType Obj;
 };
 
-#define UNNECESSARY_COPY_INIT_IN_MACRO_BODY(TYPE)	\
-  void functionWith##TYPE(const TYPE& T) {		\
-auto AssignedInMacro = T.reference();		\
-  }			\
+#define UNNECESSARY_COPY_INIT_IN_MACRO_BODY(TYPE)  \
+  void functionWith##TYPE(const TYPE &T) { \
+auto AssignedInMacro = T.reference();  \
+  }\
 // Ensure fix is not applied.
 // CHECK-FIXES: auto AssignedInMacro = T.reference();
 
-
 UNNECESSARY_COPY_INIT_IN_MACRO_BODY(ExpensiveToCopyType)
 // CHECK-MESSAGES: [[@LINE-1]]:1: warning: the variable 'AssignedInMacro' is copy-constructed
 
-#define UNNECESSARY_COPY_INIT_IN_MACRO_ARGUMENT(ARGUMENT)	\
-  ARGUMENT
+#define UNNECESSARY_COPY_INIT_IN_MACRO_ARGUMENT(ARGUMENT) ARGUMENT
 
 void PositiveMacroArgument(const ExpensiveToCopyType &Obj) {
   UNNECESSARY_COPY_INIT_IN_MACRO_ARGUMENT(auto CopyInMacroArg = Obj.reference());
   // CHECK-MESSAGES: [[@LINE-1]]:48: warning: the variable 'CopyInMacroArg' is copy-constructed
   // Ensure fix is not applied.
   // CHECK-FIXES: auto CopyInMacroArg = Obj.reference()
 }
+
+void PositiveLocalCopyConstMethodInvoked() {
+  ExpensiveToCopyType orig;
+  ExpensiveToCopyType copy_1 = orig;
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warning: local copy 'copy_1' of the variable 'orig' is never modified; consider avoiding the copy [performance-unnecessary-copy-initialization]
+  // CHECK-FIXES: const ExpensiveToCopyType& copy_1 = orig;
+  copy_1.constMethod();
+  orig.constMethod();
+}
+
+void PositiveLocalCopyUsingExplicitCopyCtor() {
+  ExpensiveToCopyType orig;
+  ExpensiveToCopyType copy_2(orig);
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warning: local copy 'copy_2'
+  // CHECK-FIXES: const ExpensiveToCopyType& copy_2(orig);
+  copy_2.constMethod();
+  orig.constMethod();
+}
+
+void PositiveLocalCopyCopyIsArgument(const ExpensiveToCopyType &orig) {
+  ExpensiveToCopyType copy_3 = orig;
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warning: local copy 'copy_3'
+  // CHECK-FIXES: const ExpensiveToCopyType& copy_3 = orig;
+  copy_3.constMethod();
+}
+
+void PositiveLocalCopyUsedAsConstRef() {
+  ExpensiveToCopyType orig;
+  ExpensiveToCopyType copy_4 = orig;
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warning: local copy 'copy_4'
+  // CHECK-FIXES: const ExpensiveToCopyType& copy_4 = orig;
+  useAsConstReference(orig);
+}
+
+void PositiveLocalCopyWeirdCopy() {
+  WeirdCopyCtorType orig;
+  WeirdCopyCtorType weird_1(orig);
+  // CHECK-MESSAGES: [[@LINE-1]]:21: warning: local copy 'weird_1'
+  // CHECK-FIXES: const WeirdCopyCtorType& weird_1(orig);
+  weird_1.constMethod();
+
+  WeirdCopyCtorType weird_2 = orig;
+  // CHECK-MESSAGES: [[@LINE-1]]:21: warning: local copy 'weird_2'
+  // CHECK-FIXES: const WeirdCopyCtorType& weird_2 = 

Re: [PATCH] D18149: Add check for unneeded copies of locals

[Matt Kulukundis via cfe-commits]

fowles updated this revision to Diff 50878.

http://reviews.llvm.org/D18149

Files:
  clang-tidy/performance/UnnecessaryCopyInitialization.cpp
  clang-tidy/performance/UnnecessaryCopyInitialization.h
  docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
  test/clang-tidy/performance-unnecessary-copy-initialization.cpp

Index: test/clang-tidy/performance-unnecessary-copy-initialization.cpp
===
--- test/clang-tidy/performance-unnecessary-copy-initialization.cpp
+++ test/clang-tidy/performance-unnecessary-copy-initialization.cpp
@@ -1,44 +1,53 @@
 // RUN: %check_clang_tidy %s performance-unnecessary-copy-initialization %t
 
 struct ExpensiveToCopyType {
-  ExpensiveToCopyType() {}
-  virtual ~ExpensiveToCopyType() {}
-  const ExpensiveToCopyType &reference() const { return *this; }
-  void nonConstMethod() {}
+  ExpensiveToCopyType();
+  virtual ~ExpensiveToCopyType();
+  const ExpensiveToCopyType &reference() const;
+  void nonConstMethod();
+  bool constMethod() const;
 };
 
 struct TrivialToCopyType {
-  const TrivialToCopyType &reference() const { return *this; }
+  const TrivialToCopyType &reference() const;
 };
 
-const ExpensiveToCopyType &ExpensiveTypeReference() {
-  static const ExpensiveToCopyType *Type = new ExpensiveToCopyType();
-  return *Type;
-}
+struct WeirdCopyCtorType {
+  WeirdCopyCtorType();
+  WeirdCopyCtorType(const WeirdCopyCtorType &w, bool oh_yes = true);
 
-const TrivialToCopyType &TrivialTypeReference() {
-  static const TrivialToCopyType *Type = new TrivialToCopyType();
-  return *Type;
-}
+  void nonConstMethod();
+  bool constMethod() const;
+};
+
+ExpensiveToCopyType global_expensive_to_copy_type;
+
+const ExpensiveToCopyType &ExpensiveTypeReference();
+const TrivialToCopyType &TrivialTypeReference();
 
 void mutate(ExpensiveToCopyType &);
 void mutate(ExpensiveToCopyType *);
+void useAsConstPointer(const ExpensiveToCopyType *);
 void useAsConstReference(const ExpensiveToCopyType &);
 void useByValue(ExpensiveToCopyType);
 
 void PositiveFunctionCall() {
   const auto AutoAssigned = ExpensiveTypeReference();
-  // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned' is copy-constructed from a const reference; consider making it a const reference [performance-unnecessary-copy-initialization]
+  // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable
+  // 'AutoAssigned' is copy-constructed from a const reference; consider making
+  // it a const reference [performance-unnecessary-copy-initialization]
   // CHECK-FIXES: const auto& AutoAssigned = ExpensiveTypeReference();
   const auto AutoCopyConstructed(ExpensiveTypeReference());
   // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable
   // CHECK-FIXES: const auto& AutoCopyConstructed(ExpensiveTypeReference());
   const ExpensiveToCopyType VarAssigned = ExpensiveTypeReference();
   // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable
-  // CHECK-FIXES:   const ExpensiveToCopyType& VarAssigned = ExpensiveTypeReference();
+  // CHECK-FIXES:   const ExpensiveToCopyType& VarAssigned =
+  // ExpensiveTypeReference();
   const ExpensiveToCopyType VarCopyConstructed(ExpensiveTypeReference());
   // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable
-  // CHECK-FIXES: const ExpensiveToCopyType& VarCopyConstructed(ExpensiveTypeReference());
+  // CHECK-FIXES: const ExpensiveToCopyType&
+  // VarCopyConstructed(ExpensiveTypeReference());
 }
 
 void PositiveMethodCallConstReferenceParam(const ExpensiveToCopyType &Obj) {
@@ -53,7 +62,8 @@
   // CHECK-FIXES: const ExpensiveToCopyType& VarAssigned = Obj.reference();
   const ExpensiveToCopyType VarCopyConstructed(Obj.reference());
   // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable
-  // CHECK-FIXES: const ExpensiveToCopyType& VarCopyConstructed(Obj.reference());
+  // CHECK-FIXES: const ExpensiveToCopyType&
+  // VarCopyConstructed(Obj.reference());
 }
 
 void PositiveMethodCallConstParam(const ExpensiveToCopyType Obj) {
@@ -68,7 +78,8 @@
   // CHECK-FIXES: const ExpensiveToCopyType& VarAssigned = Obj.reference();
   const ExpensiveToCopyType VarCopyConstructed(Obj.reference());
   // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable
-  // CHECK-FIXES: const ExpensiveToCopyType& VarCopyConstructed(Obj.reference());
+  // CHECK-FIXES: const ExpensiveToCopyType&
+  // VarCopyConstructed(Obj.reference());
 }
 
 void PositiveMethodCallConstPointerParam(const ExpensiveToCopyType *const Obj) {
@@ -83,7 +94,8 @@
   // CHECK-FIXES: const ExpensiveToCopyType& VarAssigned = Obj->reference();
   const ExpensiveToCopyType VarCopyConstructed(Obj->reference());
   // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable
-  // CHECK-FIXES: const ExpensiveToCopyType& VarCopyConstructed(Obj->reference());
+  // CHECK-FIXES: const ExpensiveToCopyType&
+  // Var

Re: [PATCH] D18149: Add check for unneeded copies of locals

[Matt Kulukundis via cfe-commits]

fowles updated this revision to Diff 50879.

http://reviews.llvm.org/D18149

Files:
  clang-tidy/performance/UnnecessaryCopyInitialization.cpp
  clang-tidy/performance/UnnecessaryCopyInitialization.h
  docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
  test/clang-tidy/performance-unnecessary-copy-initialization.cpp

Index: test/clang-tidy/performance-unnecessary-copy-initialization.cpp
===
--- test/clang-tidy/performance-unnecessary-copy-initialization.cpp
+++ test/clang-tidy/performance-unnecessary-copy-initialization.cpp
@@ -1,28 +1,33 @@
 // RUN: %check_clang_tidy %s performance-unnecessary-copy-initialization %t
 
 struct ExpensiveToCopyType {
-  ExpensiveToCopyType() {}
-  virtual ~ExpensiveToCopyType() {}
-  const ExpensiveToCopyType &reference() const { return *this; }
-  void nonConstMethod() {}
+  ExpensiveToCopyType();
+  virtual ~ExpensiveToCopyType();
+  const ExpensiveToCopyType &reference() const;
+  void nonConstMethod();
+  bool constMethod() const;
 };
 
 struct TrivialToCopyType {
-  const TrivialToCopyType &reference() const { return *this; }
+  const TrivialToCopyType &reference() const;
 };
 
-const ExpensiveToCopyType &ExpensiveTypeReference() {
-  static const ExpensiveToCopyType *Type = new ExpensiveToCopyType();
-  return *Type;
-}
+struct WeirdCopyCtorType {
+  WeirdCopyCtorType();
+  WeirdCopyCtorType(const WeirdCopyCtorType &w, bool oh_yes = true);
 
-const TrivialToCopyType &TrivialTypeReference() {
-  static const TrivialToCopyType *Type = new TrivialToCopyType();
-  return *Type;
-}
+  void nonConstMethod();
+  bool constMethod() const;
+};
+
+ExpensiveToCopyType global_expensive_to_copy_type;
+
+const ExpensiveToCopyType &ExpensiveTypeReference();
+const TrivialToCopyType &TrivialTypeReference();
 
 void mutate(ExpensiveToCopyType &);
 void mutate(ExpensiveToCopyType *);
+void useAsConstPointer(const ExpensiveToCopyType *);
 void useAsConstReference(const ExpensiveToCopyType &);
 void useByValue(ExpensiveToCopyType);
 
@@ -203,23 +208,119 @@
   ExpensiveToCopyType Obj;
 };
 
-#define UNNECESSARY_COPY_INIT_IN_MACRO_BODY(TYPE)	\
-  void functionWith##TYPE(const TYPE& T) {		\
-auto AssignedInMacro = T.reference();		\
-  }			\
+#define UNNECESSARY_COPY_INIT_IN_MACRO_BODY(TYPE)  \
+  void functionWith##TYPE(const TYPE &T) { \
+auto AssignedInMacro = T.reference();  \
+  }\
 // Ensure fix is not applied.
 // CHECK-FIXES: auto AssignedInMacro = T.reference();
 
-
 UNNECESSARY_COPY_INIT_IN_MACRO_BODY(ExpensiveToCopyType)
 // CHECK-MESSAGES: [[@LINE-1]]:1: warning: the variable 'AssignedInMacro' is copy-constructed
 
-#define UNNECESSARY_COPY_INIT_IN_MACRO_ARGUMENT(ARGUMENT)	\
-  ARGUMENT
+#define UNNECESSARY_COPY_INIT_IN_MACRO_ARGUMENT(ARGUMENT) ARGUMENT
 
 void PositiveMacroArgument(const ExpensiveToCopyType &Obj) {
   UNNECESSARY_COPY_INIT_IN_MACRO_ARGUMENT(auto CopyInMacroArg = Obj.reference());
   // CHECK-MESSAGES: [[@LINE-1]]:48: warning: the variable 'CopyInMacroArg' is copy-constructed
   // Ensure fix is not applied.
   // CHECK-FIXES: auto CopyInMacroArg = Obj.reference()
 }
+
+void PositiveLocalCopyConstMethodInvoked() {
+  ExpensiveToCopyType orig;
+  ExpensiveToCopyType copy_1 = orig;
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warning: local copy 'copy_1' of the variable 'orig' is never modified; consider avoiding the copy [performance-unnecessary-copy-initialization]
+  // CHECK-FIXES: const ExpensiveToCopyType& copy_1 = orig;
+  copy_1.constMethod();
+  orig.constMethod();
+}
+
+void PositiveLocalCopyUsingExplicitCopyCtor() {
+  ExpensiveToCopyType orig;
+  ExpensiveToCopyType copy_2(orig);
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warning: local copy 'copy_2'
+  // CHECK-FIXES: const ExpensiveToCopyType& copy_2(orig);
+  copy_2.constMethod();
+  orig.constMethod();
+}
+
+void PositiveLocalCopyCopyIsArgument(const ExpensiveToCopyType &orig) {
+  ExpensiveToCopyType copy_3 = orig;
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warning: local copy 'copy_3'
+  // CHECK-FIXES: const ExpensiveToCopyType& copy_3 = orig;
+  copy_3.constMethod();
+}
+
+void PositiveLocalCopyUsedAsConstRef() {
+  ExpensiveToCopyType orig;
+  ExpensiveToCopyType copy_4 = orig;
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warning: local copy 'copy_4'
+  // CHECK-FIXES: const ExpensiveToCopyType& copy_4 = orig;
+  useAsConstReference(orig);
+}
+
+void PositiveLocalCopyWeirdCopy() {
+  WeirdCopyCtorType orig;
+  WeirdCopyCtorType weird_1(orig);
+  // CHECK-MESSAGES: [[@LINE-1]]:21: warning: local copy 'weird_1'
+  // CHECK-FIXES: const WeirdCopyCtorType& weird_1(orig);
+  weird_1.constMethod();
+
+  WeirdCopyCtorType weird_2 = orig;
+  // CHECK-MESSAGES: [[@LINE-1]]:21: warning: local copy 'weird_2'
+  // CHECK-FIXES: const WeirdCopyCtorType& weird_2 = 

Re: [PATCH] D18149: Add check for unneeded copies of locals

[Matt Kulukundis via cfe-commits]

fowles marked 4 inline comments as done.
fowles added a comment.

http://reviews.llvm.org/D18149



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

Re: [PATCH] D18149: Add check for unneeded copies of locals

[Matt Kulukundis via cfe-commits]

fowles updated this revision to Diff 51215.
fowles marked 2 inline comments as done.

http://reviews.llvm.org/D18149

Files:
  clang-tidy/performance/UnnecessaryCopyInitialization.cpp
  clang-tidy/performance/UnnecessaryCopyInitialization.h
  docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
  test/clang-tidy/performance-unnecessary-copy-initialization.cpp

Index: test/clang-tidy/performance-unnecessary-copy-initialization.cpp
===
--- test/clang-tidy/performance-unnecessary-copy-initialization.cpp
+++ test/clang-tidy/performance-unnecessary-copy-initialization.cpp
@@ -1,28 +1,33 @@
 // RUN: %check_clang_tidy %s performance-unnecessary-copy-initialization %t
 
 struct ExpensiveToCopyType {
-  ExpensiveToCopyType() {}
-  virtual ~ExpensiveToCopyType() {}
-  const ExpensiveToCopyType &reference() const { return *this; }
-  void nonConstMethod() {}
+  ExpensiveToCopyType();
+  virtual ~ExpensiveToCopyType();
+  const ExpensiveToCopyType &reference() const;
+  void nonConstMethod();
+  bool constMethod() const;
 };
 
 struct TrivialToCopyType {
-  const TrivialToCopyType &reference() const { return *this; }
+  const TrivialToCopyType &reference() const;
 };
 
-const ExpensiveToCopyType &ExpensiveTypeReference() {
-  static const ExpensiveToCopyType *Type = new ExpensiveToCopyType();
-  return *Type;
-}
+struct WeirdCopyCtorType {
+  WeirdCopyCtorType();
+  WeirdCopyCtorType(const WeirdCopyCtorType &w, bool oh_yes = true);
 
-const TrivialToCopyType &TrivialTypeReference() {
-  static const TrivialToCopyType *Type = new TrivialToCopyType();
-  return *Type;
-}
+  void nonConstMethod();
+  bool constMethod() const;
+};
+
+ExpensiveToCopyType global_expensive_to_copy_type;
+
+const ExpensiveToCopyType &ExpensiveTypeReference();
+const TrivialToCopyType &TrivialTypeReference();
 
 void mutate(ExpensiveToCopyType &);
 void mutate(ExpensiveToCopyType *);
+void useAsConstPointer(const ExpensiveToCopyType *);
 void useAsConstReference(const ExpensiveToCopyType &);
 void useByValue(ExpensiveToCopyType);
 
@@ -203,23 +208,133 @@
   ExpensiveToCopyType Obj;
 };
 
-#define UNNECESSARY_COPY_INIT_IN_MACRO_BODY(TYPE)	\
-  void functionWith##TYPE(const TYPE& T) {		\
-auto AssignedInMacro = T.reference();		\
-  }			\
+#define UNNECESSARY_COPY_INIT_IN_MACRO_BODY(TYPE)  \
+  void functionWith##TYPE(const TYPE &T) { \
+auto AssignedInMacro = T.reference();  \
+  }\
 // Ensure fix is not applied.
 // CHECK-FIXES: auto AssignedInMacro = T.reference();
 
-
 UNNECESSARY_COPY_INIT_IN_MACRO_BODY(ExpensiveToCopyType)
 // CHECK-MESSAGES: [[@LINE-1]]:1: warning: the variable 'AssignedInMacro' is copy-constructed
 
-#define UNNECESSARY_COPY_INIT_IN_MACRO_ARGUMENT(ARGUMENT)	\
-  ARGUMENT
+#define UNNECESSARY_COPY_INIT_IN_MACRO_ARGUMENT(ARGUMENT) ARGUMENT
 
 void PositiveMacroArgument(const ExpensiveToCopyType &Obj) {
   UNNECESSARY_COPY_INIT_IN_MACRO_ARGUMENT(auto CopyInMacroArg = Obj.reference());
   // CHECK-MESSAGES: [[@LINE-1]]:48: warning: the variable 'CopyInMacroArg' is copy-constructed
   // Ensure fix is not applied.
   // CHECK-FIXES: auto CopyInMacroArg = Obj.reference()
 }
+
+void PositiveLocalCopyConstMethodInvoked() {
+  ExpensiveToCopyType orig;
+  ExpensiveToCopyType copy_1 = orig;
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warning: local copy 'copy_1' of the variable 'orig' is never modified; consider avoiding the copy [performance-unnecessary-copy-initialization]
+  // CHECK-FIXES: const ExpensiveToCopyType& copy_1 = orig;
+  copy_1.constMethod();
+  orig.constMethod();
+}
+
+void PositiveLocalCopyUsingExplicitCopyCtor() {
+  ExpensiveToCopyType orig;
+  ExpensiveToCopyType copy_2(orig);
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warning: local copy 'copy_2'
+  // CHECK-FIXES: const ExpensiveToCopyType& copy_2(orig);
+  copy_2.constMethod();
+  orig.constMethod();
+}
+
+void PositiveLocalCopyCopyIsArgument(const ExpensiveToCopyType &orig) {
+  ExpensiveToCopyType copy_3 = orig;
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warning: local copy 'copy_3'
+  // CHECK-FIXES: const ExpensiveToCopyType& copy_3 = orig;
+  copy_3.constMethod();
+}
+
+void PositiveLocalCopyUsedAsConstRef() {
+  ExpensiveToCopyType orig;
+  ExpensiveToCopyType copy_4 = orig;
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warning: local copy 'copy_4'
+  // CHECK-FIXES: const ExpensiveToCopyType& copy_4 = orig;
+  useAsConstReference(orig);
+}
+
+void PositiveLocalCopyTwice() {
+  ExpensiveToCopyType orig;
+  ExpensiveToCopyType copy_5 = orig;
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warning: local copy 'copy_5'
+  // CHECK-FIXES: const ExpensiveToCopyType& copy_5 = orig;
+  ExpensiveToCopyType copy_6 = copy_5;
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warning: local copy 'copy_6'
+  // CHECK-FIXES: const ExpensiveToCo

Re: [PATCH] D18149: Add check for unneeded copies of locals

[Matt Kulukundis via cfe-commits]

fowles added inline comments.


Comment at: clang-tidy/performance/UnnecessaryCopyInitialization.cpp:21
@@ +20,3 @@
+
+void recordFixes(const VarDecl &Var, ASTContext &Context,
+ DiagnosticBuilder &Diagnostic) {

I am inclined to just leave it as is for the moment, this is already isolated 
code that doesn't escape the current file.


http://reviews.llvm.org/D18149



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

Re: [PATCH] D18149: Add check for unneeded copies of locals

[Matt Kulukundis via cfe-commits]

fowles updated this revision to Diff 51289.
fowles added a comment.

rebased to latest.  Can someone submit this for me I don't have commit bits.


http://reviews.llvm.org/D18149

Files:
  clang-tidy/performance/UnnecessaryCopyInitialization.cpp
  clang-tidy/performance/UnnecessaryCopyInitialization.h
  docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
  test/clang-tidy/performance-unnecessary-copy-initialization.cpp

Index: test/clang-tidy/performance-unnecessary-copy-initialization.cpp
===
--- test/clang-tidy/performance-unnecessary-copy-initialization.cpp
+++ test/clang-tidy/performance-unnecessary-copy-initialization.cpp
@@ -1,28 +1,33 @@
 // RUN: %check_clang_tidy %s performance-unnecessary-copy-initialization %t
 
 struct ExpensiveToCopyType {
-  ExpensiveToCopyType() {}
-  virtual ~ExpensiveToCopyType() {}
-  const ExpensiveToCopyType &reference() const { return *this; }
-  void nonConstMethod() {}
+  ExpensiveToCopyType();
+  virtual ~ExpensiveToCopyType();
+  const ExpensiveToCopyType &reference() const;
+  void nonConstMethod();
+  bool constMethod() const;
 };
 
 struct TrivialToCopyType {
-  const TrivialToCopyType &reference() const { return *this; }
+  const TrivialToCopyType &reference() const;
 };
 
-const ExpensiveToCopyType &ExpensiveTypeReference() {
-  static const ExpensiveToCopyType *Type = new ExpensiveToCopyType();
-  return *Type;
-}
+struct WeirdCopyCtorType {
+  WeirdCopyCtorType();
+  WeirdCopyCtorType(const WeirdCopyCtorType &w, bool oh_yes = true);
 
-const TrivialToCopyType &TrivialTypeReference() {
-  static const TrivialToCopyType *Type = new TrivialToCopyType();
-  return *Type;
-}
+  void nonConstMethod();
+  bool constMethod() const;
+};
+
+ExpensiveToCopyType global_expensive_to_copy_type;
+
+const ExpensiveToCopyType &ExpensiveTypeReference();
+const TrivialToCopyType &TrivialTypeReference();
 
 void mutate(ExpensiveToCopyType &);
 void mutate(ExpensiveToCopyType *);
+void useAsConstPointer(const ExpensiveToCopyType *);
 void useAsConstReference(const ExpensiveToCopyType &);
 void useByValue(ExpensiveToCopyType);
 
@@ -203,23 +208,133 @@
   ExpensiveToCopyType Obj;
 };
 
-#define UNNECESSARY_COPY_INIT_IN_MACRO_BODY(TYPE)	\
-  void functionWith##TYPE(const TYPE& T) {		\
-auto AssignedInMacro = T.reference();		\
-  }			\
+#define UNNECESSARY_COPY_INIT_IN_MACRO_BODY(TYPE)  \
+  void functionWith##TYPE(const TYPE &T) { \
+auto AssignedInMacro = T.reference();  \
+  }\
 // Ensure fix is not applied.
 // CHECK-FIXES: auto AssignedInMacro = T.reference();
 
-
 UNNECESSARY_COPY_INIT_IN_MACRO_BODY(ExpensiveToCopyType)
 // CHECK-MESSAGES: [[@LINE-1]]:1: warning: the variable 'AssignedInMacro' is copy-constructed
 
-#define UNNECESSARY_COPY_INIT_IN_MACRO_ARGUMENT(ARGUMENT)	\
-  ARGUMENT
+#define UNNECESSARY_COPY_INIT_IN_MACRO_ARGUMENT(ARGUMENT) ARGUMENT
 
 void PositiveMacroArgument(const ExpensiveToCopyType &Obj) {
   UNNECESSARY_COPY_INIT_IN_MACRO_ARGUMENT(auto CopyInMacroArg = Obj.reference());
   // CHECK-MESSAGES: [[@LINE-1]]:48: warning: the variable 'CopyInMacroArg' is copy-constructed
   // Ensure fix is not applied.
   // CHECK-FIXES: auto CopyInMacroArg = Obj.reference()
 }
+
+void PositiveLocalCopyConstMethodInvoked() {
+  ExpensiveToCopyType orig;
+  ExpensiveToCopyType copy_1 = orig;
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warning: local copy 'copy_1' of the variable 'orig' is never modified; consider avoiding the copy [performance-unnecessary-copy-initialization]
+  // CHECK-FIXES: const ExpensiveToCopyType& copy_1 = orig;
+  copy_1.constMethod();
+  orig.constMethod();
+}
+
+void PositiveLocalCopyUsingExplicitCopyCtor() {
+  ExpensiveToCopyType orig;
+  ExpensiveToCopyType copy_2(orig);
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warning: local copy 'copy_2'
+  // CHECK-FIXES: const ExpensiveToCopyType& copy_2(orig);
+  copy_2.constMethod();
+  orig.constMethod();
+}
+
+void PositiveLocalCopyCopyIsArgument(const ExpensiveToCopyType &orig) {
+  ExpensiveToCopyType copy_3 = orig;
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warning: local copy 'copy_3'
+  // CHECK-FIXES: const ExpensiveToCopyType& copy_3 = orig;
+  copy_3.constMethod();
+}
+
+void PositiveLocalCopyUsedAsConstRef() {
+  ExpensiveToCopyType orig;
+  ExpensiveToCopyType copy_4 = orig;
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warning: local copy 'copy_4'
+  // CHECK-FIXES: const ExpensiveToCopyType& copy_4 = orig;
+  useAsConstReference(orig);
+}
+
+void PositiveLocalCopyTwice() {
+  ExpensiveToCopyType orig;
+  ExpensiveToCopyType copy_5 = orig;
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warning: local copy 'copy_5'
+  // CHECK-FIXES: const ExpensiveToCopyType& copy_5 = orig;
+  ExpensiveToCopyType copy_6 = copy_5;
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warni

Re: [PATCH] D17491: Add performance check to flag function parameters of expensive to copy types that can be safely converted to const references.

[Matt Kulukundis via cfe-commits]

fowles added a subscriber: fowles.


Comment at: test/clang-tidy/performance-unnecessary-value-param.cpp:8
@@ +7,3 @@
+  void nonConstMethod() {}
+  virtual ~ExpensiveToCopyType() {}
+};

you don't actually need to fill in these methods, just declare them


Comment at: test/clang-tidy/performance-unnecessary-value-param.cpp:52
@@ +51,3 @@
+void positiveUnnamedParam(const ExpensiveToCopyType) {
+  // CHECK-MESSAGES: [[@LINE-1]]:52: warning: the const qualified parameter #1
+}

no fix for this case?


http://reviews.llvm.org/D17491



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

Re: [PATCH] D18149: Add check for unneeded copies of locals

[Matt Kulukundis via cfe-commits]

fowles added a comment.

thank you!


Repository:
  rL LLVM

http://reviews.llvm.org/D18149



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

[PATCH] D18408: readability check for const params in declarations

[Matt Kulukundis via cfe-commits]

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

Adds a clang-tidy warning for top-level consts in function declarations.

http://reviews.llvm.org/D18408

Files:
  clang-tidy/readability/AvoidConstParamsInDecls.cpp
  clang-tidy/readability/AvoidConstParamsInDecls.h
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-avoid-const-params-in-decls.rst
  test/clang-tidy/readability-avoid-const-params-in-decls.cpp

Index: test/clang-tidy/readability-avoid-const-params-in-decls.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-avoid-const-params-in-decls.cpp
@@ -0,0 +1,78 @@
+// RUN: %check_clang_tidy %s readability-avoid-const-params-in-decls %t
+
+using alias_type = bool;
+using alias_const_type = const bool;
+
+
+void F1(const int i);
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'i' is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]
+// CHECK-FIXES: void F1(int i);
+
+void F2(const int *const i);
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'i' is const-qualified
+// CHECK-FIXES: void F2(const int * i);
+
+void F3(int const i);
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'i' is const-qualified
+// CHECK-FIXES: void F3(int i);
+
+void F4(alias_type const i);
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'i' is const-qualified
+// CHECK-FIXES: void F4(alias_type i);
+
+void F5(const int);
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 1 is const-qualified
+// CHECK-FIXES: void F5(int);
+
+void F6(const int *const);
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 1 is const-qualified
+// BUG(b/27584482): void F6(const int *);  should be produced
+
+void F7(int, const int);
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: parameter 2 is const-qualified
+// CHECK-FIXES: void F7(int, int);
+
+void F8(const int, const int);
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 1 is const-qualified
+// CHECK-MESSAGES: :[[@LINE-2]]:20: warning: parameter 2 is const-qualified
+// CHECK-FIXES: void F8(int, int);
+
+void F9(const int long_name);
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'long_name'
+// CHECK-FIXES: void F9(int long_name);
+
+void F10(const int *const *const long_name);
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: parameter 'long_name'
+// CHECK-FIXES: void F10(const int *const * long_name);
+
+
+struct Foo {
+  Foo(const int i);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: parameter 'i'
+  // CHECK-FIXES: Foo(int i);
+
+  void operator()(const int i);
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: parameter 'i'
+  // CHECK-FIXES: void operator()(int i);
+};
+
+// Do not match on definitions
+void NF1(const int i) {}
+void NF2(const int *const i) {}
+void NF3(int const i) {}
+void NF4(alias_type const i) {}
+void NF5(const int) {}
+void NF6(const int *const) {}
+void NF7(int, const int) {}
+void NF8(const int, const int) {}
+
+// Do not match on other stuff
+void NF(const alias_type& i);
+void NF(const int &i);
+void NF(const int *i);
+void NF(alias_const_type i);
+void NF(const alias_type&);
+void NF(const int&);
+void NF(const int*);
+void NF(const int* const*);
+void NF(alias_const_type);
Index: docs/clang-tidy/checks/readability-avoid-const-params-in-decls.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-avoid-const-params-in-decls.rst
@@ -0,0 +1,17 @@
+.. title:: clang-tidy - readability-avoid-const-params-in-decls
+
+readability-avoid-const-params-in-decls
+==
+
+Checks whether a function declaration has parameters that are top level const.
+
+`const` values in declarations do not have any affect on the signature of a
+function, so they should not be put there.  For example:
+
+Examples:
+
+.. code:: c++
+
+  void f(const string);   // bad. const is top level
+  void f(const string&);  // ok. const is not top level
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -88,6 +88,7 @@
performance-faster-string-find
performance-for-range-copy
performance-implicit-cast-in-loop
+   readability-avoid-const-params-in-decls
readability-braces-around-statements
readability-container-size-empty
readability-else-after-return
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #in

Re: [PATCH] D18408: readability check for const params in declarations

[Matt Kulukundis via cfe-commits]

fowles updated this revision to Diff 51596.
fowles marked 5 inline comments as done.
fowles added a comment.

- review comments


http://reviews.llvm.org/D18408

Files:
  clang-tidy/readability/AvoidConstParamsInDecls.cpp
  clang-tidy/readability/AvoidConstParamsInDecls.h
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-avoid-const-params-in-decls.rst
  test/clang-tidy/readability-avoid-const-params-in-decls.cpp

Index: test/clang-tidy/readability-avoid-const-params-in-decls.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-avoid-const-params-in-decls.cpp
@@ -0,0 +1,78 @@
+// RUN: %check_clang_tidy %s readability-avoid-const-params-in-decls %t
+
+using alias_type = bool;
+using alias_const_type = const bool;
+
+
+void F1(const int i);
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'i' is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]
+// CHECK-FIXES: void F1(int i);
+
+void F2(const int *const i);
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'i' is const-qualified
+// CHECK-FIXES: void F2(const int * i);
+
+void F3(int const i);
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'i' is const-qualified
+// CHECK-FIXES: void F3(int i);
+
+void F4(alias_type const i);
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'i' is const-qualified
+// CHECK-FIXES: void F4(alias_type i);
+
+void F5(const int);
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 1 is const-qualified
+// CHECK-FIXES: void F5(int);
+
+void F6(const int *const);
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 1 is const-qualified
+// BUG(b/27584482): void F6(const int *);  should be produced
+
+void F7(int, const int);
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: parameter 2 is const-qualified
+// CHECK-FIXES: void F7(int, int);
+
+void F8(const int, const int);
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 1 is const-qualified
+// CHECK-MESSAGES: :[[@LINE-2]]:20: warning: parameter 2 is const-qualified
+// CHECK-FIXES: void F8(int, int);
+
+void F9(const int long_name);
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'long_name'
+// CHECK-FIXES: void F9(int long_name);
+
+void F10(const int *const *const long_name);
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: parameter 'long_name'
+// CHECK-FIXES: void F10(const int *const * long_name);
+
+
+struct Foo {
+  Foo(const int i);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: parameter 'i'
+  // CHECK-FIXES: Foo(int i);
+
+  void operator()(const int i);
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: parameter 'i'
+  // CHECK-FIXES: void operator()(int i);
+};
+
+// Do not match on definitions
+void NF1(const int i) {}
+void NF2(const int *const i) {}
+void NF3(int const i) {}
+void NF4(alias_type const i) {}
+void NF5(const int) {}
+void NF6(const int *const) {}
+void NF7(int, const int) {}
+void NF8(const int, const int) {}
+
+// Do not match on other stuff
+void NF(const alias_type& i);
+void NF(const int &i);
+void NF(const int *i);
+void NF(alias_const_type i);
+void NF(const alias_type&);
+void NF(const int&);
+void NF(const int*);
+void NF(const int* const*);
+void NF(alias_const_type);
Index: docs/clang-tidy/checks/readability-avoid-const-params-in-decls.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-avoid-const-params-in-decls.rst
@@ -0,0 +1,17 @@
+.. title:: clang-tidy - readability-avoid-const-params-in-decls
+
+readability-avoid-const-params-in-decls
+===
+
+Checks whether a function declaration has parameters that are top level const.
+
+`const` values in declarations do not affect the signature of a function, so
+they should not be put there.  For example:
+
+Examples:
+
+.. code:: c++
+
+  void f(const string);   // Bad: const is top level.
+  void f(const string&);  // Good: const is not top level.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -88,6 +88,7 @@
performance-faster-string-find
performance-for-range-copy
performance-implicit-cast-in-loop
+   readability-avoid-const-params-in-decls
readability-braces-around-statements
readability-container-size-empty
readability-else-after-return
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "AvoidConstParam