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