Re: [PATCH] D19194: fix for clang-tidy modernize-pass-by-value on copy constructor

[Kamal Essoufi via cfe-commits]

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

Done


http://reviews.llvm.org/D19194



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

Re: [PATCH] D19194: fix for clang-tidy modernize-pass-by-value on copy constructor

[Kamal Essoufi via cfe-commits]

Kessoufi updated this revision to Diff 54524.
Kessoufi added a comment.

removed the redundant matcher


http://reviews.llvm.org/D19194

Files:
  clang-tidy/modernize/PassByValueCheck.cpp

Index: clang-tidy/modernize/PassByValueCheck.cpp
===
--- clang-tidy/modernize/PassByValueCheck.cpp
+++ clang-tidy/modernize/PassByValueCheck.cpp
@@ -148,7 +148,7 @@
   nonConstValueType()
 .bind("Param",
 hasDeclaration(cxxConstructorDecl(
-isCopyConstructor(), unless(isDeleted()),
+unless(isCopyConstructor()),
 hasDeclContext(
 cxxRecordDecl(isMoveConstructible(
 .bind("Initializer")))


Index: clang-tidy/modernize/PassByValueCheck.cpp
===
--- clang-tidy/modernize/PassByValueCheck.cpp
+++ clang-tidy/modernize/PassByValueCheck.cpp
@@ -148,7 +148,7 @@
   nonConstValueType()
 .bind("Param",
 hasDeclaration(cxxConstructorDecl(
-isCopyConstructor(), unless(isDeleted()),
+unless(isCopyConstructor()),
 hasDeclContext(
 cxxRecordDecl(isMoveConstructible(
 .bind("Initializer")))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D19194: fix for clang-tidy modernize-pass-by-value on copy constructor

[Kamal Essoufi via cfe-commits]

Kessoufi updated this revision to Diff 54762.
Kessoufi added a comment.

- my previous fix was applied on the wrong location and accidentally worked.

now applied on the right location

- previous tests still pass
- added a specific test to highlight my issue


http://reviews.llvm.org/D19194

Files:
  clang-tidy/modernize/PassByValueCheck.cpp
  test/clang-tidy/modernize-pass-by-value.cpp

Index: test/clang-tidy/modernize-pass-by-value.cpp
===
--- test/clang-tidy/modernize-pass-by-value.cpp
+++ test/clang-tidy/modernize-pass-by-value.cpp
@@ -194,3 +194,12 @@
   Movable M;
 };
 
+// Test exclusion of copy constructors
+// allowing modernize-pass-by-value on copy constructors causes:
+//  - compiler error: copy constructor must pass its first argument by
+//reference
+//  - conflict when applying replacement for both modernize-pass-by-value and
+//modernize-use-default
+struct T {
+T(const T&) {}
+};
Index: clang-tidy/modernize/PassByValueCheck.cpp
===
--- clang-tidy/modernize/PassByValueCheck.cpp
+++ clang-tidy/modernize/PassByValueCheck.cpp
@@ -151,7 +151,8 @@
 isCopyConstructor(), unless(isDeleted()),
 hasDeclContext(
 cxxRecordDecl(isMoveConstructible(
-.bind("Initializer")))
+.bind("Initializer")),
+ unless(isCopyConstructor()))
 .bind("Ctor"),
 this);
   }


Index: test/clang-tidy/modernize-pass-by-value.cpp
===
--- test/clang-tidy/modernize-pass-by-value.cpp
+++ test/clang-tidy/modernize-pass-by-value.cpp
@@ -194,3 +194,12 @@
   Movable M;
 };
 
+// Test exclusion of copy constructors
+// allowing modernize-pass-by-value on copy constructors causes:
+//  - compiler error: copy constructor must pass its first argument by
+//reference
+//  - conflict when applying replacement for both modernize-pass-by-value and
+//modernize-use-default
+struct T {
+T(const T&) {}
+};
Index: clang-tidy/modernize/PassByValueCheck.cpp
===
--- clang-tidy/modernize/PassByValueCheck.cpp
+++ clang-tidy/modernize/PassByValueCheck.cpp
@@ -151,7 +151,8 @@
 isCopyConstructor(), unless(isDeleted()),
 hasDeclContext(
 cxxRecordDecl(isMoveConstructible(
-.bind("Initializer")))
+.bind("Initializer")),
+ unless(isCopyConstructor()))
 .bind("Ctor"),
 this);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits