[PATCH] D103385: [clang-tidy] bugprone-forwarding-reference-overload: support non-type template parameters
jwtowner marked 2 inline comments as done. jwtowner added a comment. > Thanks! What name and email address would you like me to use for patch > attribution? The following will do: Jesse Towner Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103385/new/ https://reviews.llvm.org/D103385 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D103385: [clang-tidy] bugprone-forwarding-reference-overload: support non-type template parameters
jwtowner marked 2 inline comments as done.
jwtowner added a comment.
It should be good to go. I do not have commit access so feel free to commit.
Thanks!
Comment at:
clang-tools-extra/docs/clang-tidy/checks/bugprone-forwarding-reference-overload.rst:34
+ template,A&&...>,int> = 0>
+ explicit Person(A&&... a) {}
aaron.ballman wrote:
>
The reason I didn't put the spaces in here was that I was trying to match the
style above for `enable_if_t,void>`. I can add a space after the
comma there as well.
Comment at:
clang-tools-extra/docs/clang-tidy/checks/bugprone-forwarding-reference-overload.rst:56
+constructor is guarded.
\ No newline at end of file
aaron.ballman wrote:
> Can you add the newline back?
Will do! Sorry about this.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103385/new/
https://reviews.llvm.org/D103385
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D103385: [clang-tidy] bugprone-forwarding-reference-overload: support non-type template parameters
jwtowner updated this revision to Diff 361289.
jwtowner added a comment.
Improved the code formatting of the samples in the documentation and added back
in the newline at the end of the bugprone-forwarding-reference-overload.rst
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103385/new/
https://reviews.llvm.org/D103385
Files:
clang-tools-extra/clang-tidy/bugprone/ForwardingReferenceOverloadCheck.cpp
clang-tools-extra/docs/clang-tidy/checks/bugprone-forwarding-reference-overload.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone-forwarding-reference-overload.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-forwarding-reference-overload.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-forwarding-reference-overload.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-forwarding-reference-overload.cpp
@@ -150,3 +150,93 @@
constexpr variant(_Arg&& __arg) {}
// CHECK-NOTES: :[[@LINE-1]]:13: warning: constructor accepting a forwarding reference can hide the copy and move constructors
};
+
+namespace std {
+template struct is_same { static constexpr bool value = false; };
+template struct is_same { static constexpr bool value = true; };
+template constexpr bool is_same_v = is_same::value;
+template struct remove_reference { using type = T; };
+template struct remove_reference { using type = T; };
+template struct remove_reference { using type = T; };
+template using remove_reference_t = typename remove_reference::type;
+template struct remove_cv { using type = T; };
+template struct remove_cv { using type = T; };
+template struct remove_cv { using type = T; };
+template struct remove_cv { using type = T; };
+template using remove_cv_t = typename remove_cv::type;
+template struct remove_cvref { using type = remove_cv_t>; };
+template using remove_cvref_t = typename remove_cvref::type;
+} // namespace std
+
+// Handle enable_if when used as a non-type template parameter.
+class Test7 {
+public:
+ // Guarded with enable_if.
+ template , int>, int>::type = 0>
+ Test7(T &&t);
+
+ // Guarded with enable_if.
+ template , Test7>
+ && !std::is_same_v, bool>, int> = true>
+ Test7(T &&t);
+
+ Test7(const Test7 &other) = default;
+ Test7(Test7 &&other) = default;
+};
+
+// Handle enable_if when used as a non-type template parameter following
+// a variadic template parameter pack.
+class Test8 {
+public:
+ // Guarded with enable_if.
+ template , Test8>
+ || (sizeof...(A) > 0)>* = nullptr>
+ Test8(T &&t, A &&... a);
+
+ Test8(const Test8 &other) = default;
+ Test8(Test8 &&other) = default;
+};
+
+// Non-type template parameter failure cases.
+class Test9 {
+public:
+ // Requires a default argument (such as a literal, implicit cast expression, etc.)
+ template , bool>, int>>
+ Test9(T &&t);
+ // CHECK-NOTES: :[[@LINE-1]]:3: warning: constructor accepting a forwarding reference can hide the copy and move constructors
+ // CHECK-NOTES: 240:3: note: copy constructor declared here
+ // CHECK-NOTES: 241:3: note: move constructor declared here
+
+ // Requires a default argument (such as a literal, implicit cast expression, etc.)
+ template , long>>*>
+ Test9(T &&t);
+ // CHECK-NOTES: :[[@LINE-1]]:3: warning: constructor accepting a forwarding reference can hide the copy and move constructors
+ // CHECK-NOTES: 240:3: note: copy constructor declared here
+ // CHECK-NOTES: 241:3: note: move constructor declared here
+
+ // Only std::enable_if or std::enable_if_t are supported
+ template ::type* = nullptr>
+ Test9(T &&t);
+ // CHECK-NOTES: :[[@LINE-1]]:3: warning: constructor accepting a forwarding reference can hide the copy and move constructors
+ // CHECK-NOTES: 240:3: note: copy constructor declared here
+ // CHECK-NOTES: 241:3: note: move constructor declared here
+
+ // Only std::enable_if or std::enable_if_t are supported
+ template ::type = 0>
+ Test9(T &&t);
+ // CHECK-NOTES: :[[@LINE-1]]:3: warning: constructor accepting a forwarding reference can hide the copy and move constructors
+ // CHECK-NOTES: 240:3: note: copy constructor declared here
+ // CHECK-NOTES: 241:3: note: move constructor declared here
+
+ Test9(const Test9 &other) = default;
+ Test9(Test9 &&other) = default;
+};
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-forwarding-reference-overload.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone-forwarding-reference-overload.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-forwarding-reference-overload.rst
@@ -26,9 +26,14 @@
explicit Person(T&& n, int x = 1) {}
// C3: perfect forwarding ctor guarded with enable_if
- template,void>>
+ template, void>>
explicit Person(T&& n) {}
+ // C4: variadic perfect forwarding ctor guarded with enable_if
+ templa
[PATCH] D103385: [clang-tidy] bugprone-forwarding-reference-overload: support non-type template parameters
jwtowner added a comment. Friendly bump in case this was missed. Without this fix, bugprone-forwarding-reference-overload is not currently usable in a lot of codebases. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103385/new/ https://reviews.llvm.org/D103385 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D103385: [clang-tidy] bugprone-forwarding-reference-overload: support non-type template parameters
jwtowner added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/ForwardingReferenceOverloadCheck.cpp:83 + hasType(isEnableIf()), + anyOf(hasDescendant(cxxBoolLiteral()), +hasDescendant(cxxNullPtrLiteralExpr()), Note that I'm using `hasDescendant` here, because sometimes the literal ends up inside of an `ImplicitCastExpr` node and sometimes its a direct child of the `NonTypeTemplateParmDecl` node. I'm not sure exactly what causes the difference. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103385/new/ https://reviews.llvm.org/D103385 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D103385: [clang-tidy] bugprone-forwarding-reference-overload: support non-type template parameters
jwtowner created this revision.
jwtowner added reviewers: alexfh, aaron.ballman, hokein.
Herald added a subscriber: xazax.hun.
jwtowner requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
Many concepts emulation libraries, such as the one found in Range v3, tend to
use non-type template parameters for the enable_if type expression, due to
their versatility in template functions and constructors containing variadic
template parameter packs.
Unfortunately the bugprone-forwarding-reference-overload check does not
handle non-type template parameters, as was first noted in this bug report:
https://bugs.llvm.org/show_bug.cgi?id=38081
This patch fixes this long standing issue and allows for the check to be
suppressed
with the use of a non-type template parameter containing enable_if or
enable_if_t in
the type expression, so long as it has a default literal value.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D103385
Files:
clang-tools-extra/clang-tidy/bugprone/ForwardingReferenceOverloadCheck.cpp
clang-tools-extra/docs/clang-tidy/checks/bugprone-forwarding-reference-overload.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone-forwarding-reference-overload.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-forwarding-reference-overload.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-forwarding-reference-overload.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-forwarding-reference-overload.cpp
@@ -150,3 +150,93 @@
constexpr variant(_Arg&& __arg) {}
// CHECK-NOTES: :[[@LINE-1]]:13: warning: constructor accepting a forwarding reference can hide the copy and move constructors
};
+
+namespace std {
+template struct is_same { static constexpr bool value = false; };
+template struct is_same { static constexpr bool value = true; };
+template constexpr bool is_same_v = is_same::value;
+template struct remove_reference { using type = T; };
+template struct remove_reference { using type = T; };
+template struct remove_reference { using type = T; };
+template using remove_reference_t = typename remove_reference::type;
+template struct remove_cv { using type = T; };
+template struct remove_cv { using type = T; };
+template struct remove_cv { using type = T; };
+template struct remove_cv { using type = T; };
+template using remove_cv_t = typename remove_cv::type;
+template struct remove_cvref { using type = remove_cv_t>; };
+template using remove_cvref_t = typename remove_cvref::type;
+} // namespace std
+
+// Handle enable_if when used as a non-type template parameter.
+class Test7 {
+public:
+ // Guarded with enable_if.
+ template , int>, int>::type = 0>
+ Test7(T &&t);
+
+ // Guarded with enable_if.
+ template , Test7>
+ && !std::is_same_v, bool>, int> = true>
+ Test7(T &&t);
+
+ Test7(const Test7 &other) = default;
+ Test7(Test7 &&other) = default;
+};
+
+// Handle enable_if when used as a non-type template parameter following
+// a variadic template parameter pack.
+class Test8 {
+public:
+ // Guarded with enable_if.
+ template , Test8>
+ || (sizeof...(A) > 0)>* = nullptr>
+ Test8(T &&t, A &&... a);
+
+ Test8(const Test8 &other) = default;
+ Test8(Test8 &&other) = default;
+};
+
+// Non-type template parameter failure cases.
+class Test9 {
+public:
+ // Requires a default argument (such as a literal, implicit cast expression, etc.)
+ template , bool>, int>>
+ Test9(T &&t);
+ // CHECK-NOTES: :[[@LINE-1]]:3: warning: constructor accepting a forwarding reference can hide the copy and move constructors
+ // CHECK-NOTES: 240:3: note: copy constructor declared here
+ // CHECK-NOTES: 241:3: note: move constructor declared here
+
+ // Requires a default argument (such as a literal, implicit cast expression, etc.)
+ template , long>>*>
+ Test9(T &&t);
+ // CHECK-NOTES: :[[@LINE-1]]:3: warning: constructor accepting a forwarding reference can hide the copy and move constructors
+ // CHECK-NOTES: 240:3: note: copy constructor declared here
+ // CHECK-NOTES: 241:3: note: move constructor declared here
+
+ // Only std::enable_if or std::enable_if_t are supported
+ template ::type* = nullptr>
+ Test9(T &&t);
+ // CHECK-NOTES: :[[@LINE-1]]:3: warning: constructor accepting a forwarding reference can hide the copy and move constructors
+ // CHECK-NOTES: 240:3: note: copy constructor declared here
+ // CHECK-NOTES: 241:3: note: move constructor declared here
+
+ // Only std::enable_if or std::enable_if_t are supported
+ template ::type = 0>
+ Test9(T &&t);
+ // CHECK-NOTES: :[[@LINE-1]]:3: warning: constructor accepting a forwarding reference can hide the copy and move constructors
+ // CHECK-NOTES: 240:3: note: copy constructor declared here
+ // CHECK-NOTES: 241:3: note: move constructor declared here
+
+ Test9(const Test9 &other) = default;
+ Test9(Test9 &&other)
