[PATCH] D30547: [clang-tidy] Forwarding reference overload in constructors

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

added my thoughts. i like the check, seems really thought out :)



================
Comment at: docs/clang-tidy/checks/misc-forwarding-reference-overload.rst:36
+    
+The check warns for constructors C1 and C2, because those can act like
+described above. We suppress warnings if the copy and move constructor is
----------------
maybe "can act like described above" should be different.
i think being more explicit with the hiding/unexpected call to those is clearer.


================
Comment at: test/clang-tidy/misc-forwarding-reference-overload.cpp:21
+  Person(T &&n);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'Person' can hide copy and move constructors [misc-forwarding-reference-overload]
+
----------------
could the check output a note when there was a userdefined constructor and point to that one if it gets hidden? that would make it clearer, what and how the problem occurs. 


================
Comment at: test/clang-tidy/misc-forwarding-reference-overload.cpp:28
+
+template <typename U>
+class Person2 {
----------------
maybe be more explicit with whats Ok with a comment // Ok or sth like that on all following constructors?


https://reviews.llvm.org/D30547