[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5902bb9584d6: [clang-tidy] Implement cppcoreguidelines F.19
(authored by ccotter, committed by PiotrZSL).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146921/new/
https://reviews.llvm.org/D146921
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/missing-std-forward.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
@@ -0,0 +1,150 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-missing-std-forward %t -- -- -fno-delayed-template-parsing
+
+// NOLINTBEGIN
+namespace std {
+
+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 constexpr T &(remove_reference_t ) noexcept;
+template constexpr T &(remove_reference_t &) noexcept;
+template constexpr remove_reference_t &(T &);
+
+} // namespace std
+// NOLINTEND
+
+struct S {
+ S();
+ S(const S&);
+ S(S&&) noexcept;
+ S& operator=(const S&);
+ S& operator=(S&&) noexcept;
+};
+
+template
+void consumes_all(Ts&&...);
+
+namespace positive_cases {
+
+template
+void does_not_forward(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ T other = t;
+}
+
+template
+void does_not_forward_invoked(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ T other = t();
+}
+
+template
+void forwards_pairwise(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ auto first = std::forward(t.first);
+ auto second = std::forward(t.second);
+}
+
+template
+void does_not_forward_pack(Ts&&... ts) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: forwarding reference parameter 'ts' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ consumes_all(ts...);
+}
+
+template
+class AClass {
+
+ template
+ AClass(U&& u) : data(u) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+
+ template
+ AClass& operator=(U&& u) { }
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+
+ template
+ void mixed_params(T&& t, U&& u) {
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+T other1 = std::move(t);
+U other2 = std::move(u);
+ }
+
+ T data;
+};
+
+template
+void does_not_forward_in_evaluated_code(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:45: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ using result_t = decltype(std::forward(t));
+ unsigned len = sizeof(std::forward(t));
+ T other = t;
+}
+
+template
+void lambda_value_capture(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ [=]() { T other = std::forward(t); };
+}
+
+template
+void lambda_value_reference(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ [&]() { T other = std::forward(t); };
+}
+
+} // namespace positive_cases
+
+namespace negative_cases {
+
+template
+void just_a_decl(T&);
+
+template
+void does_forward(T&& t) {
+ T other = std::forward(t);
+}
+
+template
+void does_forward_pack(Ts&&... ts) {
+ consumes_all(std::forward(ts)...);
+}
+
+void pass_by_value(S s) {
+ S other = std::move(s);
+}
+
+void lvalue_ref(S& s) {
+ S other = std::move(s);
+}
+
[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19
PiotrZSL updated this revision to Diff 520043.
PiotrZSL added a comment.
Add delayed template parsing in tests (to fix windows tests)
Reorder release notes
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146921/new/
https://reviews.llvm.org/D146921
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/missing-std-forward.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
@@ -0,0 +1,150 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-missing-std-forward %t -- -- -fno-delayed-template-parsing
+
+// NOLINTBEGIN
+namespace std {
+
+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 constexpr T &(remove_reference_t ) noexcept;
+template constexpr T &(remove_reference_t &) noexcept;
+template constexpr remove_reference_t &(T &);
+
+} // namespace std
+// NOLINTEND
+
+struct S {
+ S();
+ S(const S&);
+ S(S&&) noexcept;
+ S& operator=(const S&);
+ S& operator=(S&&) noexcept;
+};
+
+template
+void consumes_all(Ts&&...);
+
+namespace positive_cases {
+
+template
+void does_not_forward(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ T other = t;
+}
+
+template
+void does_not_forward_invoked(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ T other = t();
+}
+
+template
+void forwards_pairwise(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ auto first = std::forward(t.first);
+ auto second = std::forward(t.second);
+}
+
+template
+void does_not_forward_pack(Ts&&... ts) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: forwarding reference parameter 'ts' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ consumes_all(ts...);
+}
+
+template
+class AClass {
+
+ template
+ AClass(U&& u) : data(u) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+
+ template
+ AClass& operator=(U&& u) { }
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+
+ template
+ void mixed_params(T&& t, U&& u) {
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+T other1 = std::move(t);
+U other2 = std::move(u);
+ }
+
+ T data;
+};
+
+template
+void does_not_forward_in_evaluated_code(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:45: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ using result_t = decltype(std::forward(t));
+ unsigned len = sizeof(std::forward(t));
+ T other = t;
+}
+
+template
+void lambda_value_capture(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ [=]() { T other = std::forward(t); };
+}
+
+template
+void lambda_value_reference(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ [&]() { T other = std::forward(t); };
+}
+
+} // namespace positive_cases
+
+namespace negative_cases {
+
+template
+void just_a_decl(T&);
+
+template
+void does_forward(T&& t) {
+ T other = std::forward(t);
+}
+
+template
+void does_forward_pack(Ts&&... ts) {
+ consumes_all(std::forward(ts)...);
+}
+
+void pass_by_value(S s) {
+ S other = std::move(s);
+}
+
+void lvalue_ref(S& s) {
+ S other = std::move(s);
+}
+
+void rvalue_ref(S&& s) {
+ S other =
[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19
PiotrZSL added a comment. `forwarding reference parameter '' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]` consider ignoring unnamed parameters, they unused, so nothing to forward. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146921/new/ https://reviews.llvm.org/D146921 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:136 +- New :doc:`cppcoreguidelines-missing-std-forward + ` check. Should be after `cppcoreguidelines-misleading-capture-default-by-value`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146921/new/ https://reviews.llvm.org/D146921 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19
ccotter updated this revision to Diff 520032.
ccotter added a comment.
- fix merge
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146921/new/
https://reviews.llvm.org/D146921
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/missing-std-forward.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
@@ -0,0 +1,150 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-missing-std-forward %t
+
+// NOLINTBEGIN
+namespace std {
+
+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 constexpr T &(remove_reference_t ) noexcept;
+template constexpr T &(remove_reference_t &) noexcept;
+template constexpr remove_reference_t &(T &);
+
+} // namespace std
+// NOLINTEND
+
+struct S {
+ S();
+ S(const S&);
+ S(S&&) noexcept;
+ S& operator=(const S&);
+ S& operator=(S&&) noexcept;
+};
+
+template
+void consumes_all(Ts&&...);
+
+namespace positive_cases {
+
+template
+void does_not_forward(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ T other = t;
+}
+
+template
+void does_not_forward_invoked(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ T other = t();
+}
+
+template
+void forwards_pairwise(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ auto first = std::forward(t.first);
+ auto second = std::forward(t.second);
+}
+
+template
+void does_not_forward_pack(Ts&&... ts) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: forwarding reference parameter 'ts' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ consumes_all(ts...);
+}
+
+template
+class AClass {
+
+ template
+ AClass(U&& u) : data(u) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+
+ template
+ AClass& operator=(U&& u) { }
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+
+ template
+ void mixed_params(T&& t, U&& u) {
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+T other1 = std::move(t);
+U other2 = std::move(u);
+ }
+
+ T data;
+};
+
+template
+void does_not_forward_in_evaluated_code(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:45: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ using result_t = decltype(std::forward(t));
+ unsigned len = sizeof(std::forward(t));
+ T other = t;
+}
+
+template
+void lambda_value_capture(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ [=]() { T other = std::forward(t); };
+}
+
+template
+void lambda_value_reference(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ [&]() { T other = std::forward(t); };
+}
+
+} // namespace positive_cases
+
+namespace negative_cases {
+
+template
+void just_a_decl(T&);
+
+template
+void does_forward(T&& t) {
+ T other = std::forward(t);
+}
+
+template
+void does_forward_pack(Ts&&... ts) {
+ consumes_all(std::forward(ts)...);
+}
+
+void pass_by_value(S s) {
+ S other = std::move(s);
+}
+
+void lvalue_ref(S& s) {
+ S other = std::move(s);
+}
+
+void rvalue_ref(S&& s) {
+ S other = std::move(s);
+}
+
+template
+void templated_rvalue_ref(std::remove_reference_t&& t) {
+ T other = std::move(t);
[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19
ccotter added a comment. Note, @PiotrZSL I don't have commit access, so if you're happy with the updates, could you please land this for me? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146921/new/ https://reviews.llvm.org/D146921 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19
ccotter updated this revision to Diff 520027.
ccotter added a comment.
rebase
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146921/new/
https://reviews.llvm.org/D146921
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/missing-std-forward.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
@@ -0,0 +1,150 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-missing-std-forward %t
+
+// NOLINTBEGIN
+namespace std {
+
+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 constexpr T &(remove_reference_t ) noexcept;
+template constexpr T &(remove_reference_t &) noexcept;
+template constexpr remove_reference_t &(T &);
+
+} // namespace std
+// NOLINTEND
+
+struct S {
+ S();
+ S(const S&);
+ S(S&&) noexcept;
+ S& operator=(const S&);
+ S& operator=(S&&) noexcept;
+};
+
+template
+void consumes_all(Ts&&...);
+
+namespace positive_cases {
+
+template
+void does_not_forward(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ T other = t;
+}
+
+template
+void does_not_forward_invoked(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ T other = t();
+}
+
+template
+void forwards_pairwise(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ auto first = std::forward(t.first);
+ auto second = std::forward(t.second);
+}
+
+template
+void does_not_forward_pack(Ts&&... ts) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: forwarding reference parameter 'ts' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ consumes_all(ts...);
+}
+
+template
+class AClass {
+
+ template
+ AClass(U&& u) : data(u) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+
+ template
+ AClass& operator=(U&& u) { }
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+
+ template
+ void mixed_params(T&& t, U&& u) {
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+T other1 = std::move(t);
+U other2 = std::move(u);
+ }
+
+ T data;
+};
+
+template
+void does_not_forward_in_evaluated_code(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:45: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ using result_t = decltype(std::forward(t));
+ unsigned len = sizeof(std::forward(t));
+ T other = t;
+}
+
+template
+void lambda_value_capture(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ [=]() { T other = std::forward(t); };
+}
+
+template
+void lambda_value_reference(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ [&]() { T other = std::forward(t); };
+}
+
+} // namespace positive_cases
+
+namespace negative_cases {
+
+template
+void just_a_decl(T&);
+
+template
+void does_forward(T&& t) {
+ T other = std::forward(t);
+}
+
+template
+void does_forward_pack(Ts&&... ts) {
+ consumes_all(std::forward(ts)...);
+}
+
+void pass_by_value(S s) {
+ S other = std::move(s);
+}
+
+void lvalue_ref(S& s) {
+ S other = std::move(s);
+}
+
+void rvalue_ref(S&& s) {
+ S other = std::move(s);
+}
+
+template
+void templated_rvalue_ref(std::remove_reference_t&& t) {
+ T other = std::move(t);
+}
+
[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19
ccotter added inline comments.
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp:70
+ Finder->addMatcher(
+ parmVarDecl(
+ parmVarDecl().bind("param"), isTemplateTypeOfFunction(),
PiotrZSL wrote:
> maybe think like: "functionDecl(forEachDescendant(parmVarDecl" could work.
Not sure if I see any advantages of `forEachDescendant` over what I have here -
I think I'll leave it as is.
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp:71
+ parmVarDecl(
+ parmVarDecl().bind("param"), isTemplateTypeOfFunction(),
+ hasAncestor(functionDecl(isDefinition(), ToParam,
PiotrZSL wrote:
> PiotrZSL wrote:
> > probably this could be named like isTemplateTypeParameter, current name
> > suggest more that it's a FunctionDecl matcher, not ParmVarDecl.
> consider excluding system code, or code inside std namespace.
Why specifically do this in this check, but not others (very few seem to do
this)?
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp:72
+ parmVarDecl().bind("param"), isTemplateTypeOfFunction(),
+ hasAncestor(functionDecl(isDefinition(), ToParam,
+
unless(hasDescendant(ForwardCallMatcher),
PiotrZSL wrote:
> maybe we should skip also template instances.
Not sure I follow. Can you give an example?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146921/new/
https://reviews.llvm.org/D146921
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19
ccotter updated this revision to Diff 520026.
ccotter marked 6 inline comments as done.
ccotter added a comment.
- feedback
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146921/new/
https://reviews.llvm.org/D146921
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/missing-std-forward.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
@@ -0,0 +1,150 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-missing-std-forward %t
+
+// NOLINTBEGIN
+namespace std {
+
+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 constexpr T &(remove_reference_t ) noexcept;
+template constexpr T &(remove_reference_t &) noexcept;
+template constexpr remove_reference_t &(T &);
+
+} // namespace std
+// NOLINTEND
+
+struct S {
+ S();
+ S(const S&);
+ S(S&&) noexcept;
+ S& operator=(const S&);
+ S& operator=(S&&) noexcept;
+};
+
+template
+void consumes_all(Ts&&...);
+
+namespace positive_cases {
+
+template
+void does_not_forward(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ T other = t;
+}
+
+template
+void does_not_forward_invoked(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ T other = t();
+}
+
+template
+void forwards_pairwise(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ auto first = std::forward(t.first);
+ auto second = std::forward(t.second);
+}
+
+template
+void does_not_forward_pack(Ts&&... ts) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: forwarding reference parameter 'ts' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ consumes_all(ts...);
+}
+
+template
+class AClass {
+
+ template
+ AClass(U&& u) : data(u) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+
+ template
+ AClass& operator=(U&& u) { }
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+
+ template
+ void mixed_params(T&& t, U&& u) {
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+T other1 = std::move(t);
+U other2 = std::move(u);
+ }
+
+ T data;
+};
+
+template
+void does_not_forward_in_evaluated_code(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:45: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ using result_t = decltype(std::forward(t));
+ unsigned len = sizeof(std::forward(t));
+ T other = t;
+}
+
+template
+void lambda_value_capture(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ [=]() { T other = std::forward(t); };
+}
+
+template
+void lambda_value_reference(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ [&]() { T other = std::forward(t); };
+}
+
+} // namespace positive_cases
+
+namespace negative_cases {
+
+template
+void just_a_decl(T&);
+
+template
+void does_forward(T&& t) {
+ T other = std::forward(t);
+}
+
+template
+void does_forward_pack(Ts&&... ts) {
+ consumes_all(std::forward(ts)...);
+}
+
+void pass_by_value(S s) {
+ S other = std::move(s);
+}
+
+void lvalue_ref(S& s) {
+ S other = std::move(s);
+}
+
+void rvalue_ref(S&& s) {
+ S other = std::move(s);
+}
+
+template
+void
[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19
PiotrZSL accepted this revision.
PiotrZSL added a comment.
This revision is now accepted and ready to land.
Overall looks fine. My main concern are lambdas (and maybe functions/classes in
functions, but that should only hit performance).
Please close all comments before pushing this.
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp:61
+
+ StatementMatcher ForwardCallMatcher = callExpr(
+ argumentCountIs(1),
probably `auto` would be sufficient.
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp:70
+ Finder->addMatcher(
+ parmVarDecl(
+ parmVarDecl().bind("param"), isTemplateTypeOfFunction(),
maybe think like: "functionDecl(forEachDescendant(parmVarDecl" could work.
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp:71
+ parmVarDecl(
+ parmVarDecl().bind("param"), isTemplateTypeOfFunction(),
+ hasAncestor(functionDecl(isDefinition(), ToParam,
probably this could be named like isTemplateTypeParameter, current name suggest
more that it's a FunctionDecl matcher, not ParmVarDecl.
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp:71
+ parmVarDecl(
+ parmVarDecl().bind("param"), isTemplateTypeOfFunction(),
+ hasAncestor(functionDecl(isDefinition(), ToParam,
PiotrZSL wrote:
> probably this could be named like isTemplateTypeParameter, current name
> suggest more that it's a FunctionDecl matcher, not ParmVarDecl.
consider excluding system code, or code inside std namespace.
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp:72
+ parmVarDecl().bind("param"), isTemplateTypeOfFunction(),
+ hasAncestor(functionDecl(isDefinition(), ToParam,
+
unless(hasDescendant(ForwardCallMatcher),
maybe we should skip also template instances.
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp:73
+ hasAncestor(functionDecl(isDefinition(), ToParam,
+
unless(hasDescendant(ForwardCallMatcher),
+ this);
consider `std::move` those matchers, always check construction could be
faster...
Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:192
`cppcoreguidelines-macro-usage `_,
+ `cppcoreguidelines-missing-std-forward
`_, "Yes"
`cppcoreguidelines-narrowing-conversions
`_,
no fixes provided, remove
Comment at:
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp:137
+};
+
+} // namespace negative_cases
Add test with std::forward happen in lambda, like:
```
template
void does_something(T&& t) {
[=] { call_other(std::forward(t)); }
}
```
this may not be detected.
I usually solve those things like this:
```
auto ForwardCallMatcher = callExpr(forCallable(equalsBoundNode("first")));
...
hasAncestor(functionDecl().bind("first")),
hasAncestor(functionDecl(isDefinition(), equalsBoundNode("first"), ToParam,
unless(hasDescendant(ForwardCallMatcher),
```
Problem with hasAncestor, is that if first ancestor does not match then it's
trying parent one. equalsBoundNode can be used to limit this only to first
hasAncestor of specific type (unless there is other matcher for that).
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146921/new/
https://reviews.llvm.org/D146921
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19
ccotter added a comment. bump? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146921/new/ https://reviews.llvm.org/D146921 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19
ccotter updated this revision to Diff 512455.
ccotter marked an inline comment as done.
ccotter added a comment.
Fix tests
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146921/new/
https://reviews.llvm.org/D146921
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/missing-std-forward.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-missing-std-forward %t -- -- -fno-delayed-template-parsing
+
+// NOLINTBEGIN
+namespace std {
+
+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 constexpr T &(remove_reference_t ) noexcept;
+template constexpr T &(remove_reference_t &) noexcept;
+template constexpr remove_reference_t &(T &);
+
+} // namespace std
+// NOLINTEND
+
+struct S {
+ S();
+ S(const S&);
+ S(S&&) noexcept;
+ S& operator=(const S&);
+ S& operator=(S&&) noexcept;
+};
+
+template
+void consumes_all(Ts&&...);
+
+namespace positive_cases {
+
+template
+void does_not_forward(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ T other = t;
+}
+
+template
+void does_not_forward_invoked(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ T other = t();
+}
+
+template
+void forwards_pairwise(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ auto first = std::forward(t.first);
+ auto second = std::forward(t.second);
+}
+
+template
+void does_not_forward_pack(Ts&&... ts) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: forwarding reference parameter 'ts' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ consumes_all(ts...);
+}
+
+template
+class AClass {
+
+ template
+ AClass(U&& u) : data(u) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+
+ template
+ AClass& operator=(U&& u) { }
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+
+ template
+ void mixed_params(T&& t, U&& u) {
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+T other1 = std::move(t);
+U other2 = std::move(u);
+ }
+
+ T data;
+};
+
+template
+void does_not_forward_in_evaluated_code(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:45: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ using result_t = decltype(std::forward(t));
+ unsigned len = sizeof(std::forward(t));
+ T other = t;
+}
+
+} // namespace positive_cases
+
+namespace negative_cases {
+
+template
+void just_a_decl(T&);
+
+template
+void does_forward(T&& t) {
+ T other = std::forward(t);
+}
+
+template
+void does_forward_pack(Ts&&... ts) {
+ consumes_all(std::forward(ts)...);
+}
+
+void pass_by_value(S s) {
+ S other = std::move(s);
+}
+
+void lvalue_ref(S& s) {
+ S other = std::move(s);
+}
+
+void rvalue_ref(S&& s) {
+ S other = std::move(s);
+}
+
+template
+void templated_rvalue_ref(std::remove_reference_t&& t) {
+ T other = std::move(t);
+}
+
+template
+class AClass {
+
+ template
+ AClass(U&& u) : data(std::forward(u)) {}
+
+ template
+ AClass& operator=(U&& u) {
+data = std::forward(u);
+ }
+
+ void rvalue_ref(T&& t) {
+T other = std::move(t);
+ }
+
+ T data;
+};
+
+} // namespace negative_cases
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++
[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19
ccotter marked 2 inline comments as done.
ccotter added inline comments.
Comment at:
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp:138
+
+} // namespace negative_cases
PiotrZSL wrote:
> ccotter wrote:
> > ccotter wrote:
> > > PiotrZSL wrote:
> > > > what about when someone uses std::move instead of std::format ?
> > > > maybe some "note" for such issue ?
> > > Are you suggesting to have the tool add a special note in something like
> > >
> > > ```
> > > template
> > > void foo(T&& t) { T other = std::move(t); }
> > > ```
> > >
> > > I'm not sure I completely followed what you were saying. Or perhaps a
> > > fixit for this specific case of using move on a forwarding reference
> > > (fixit to replace `move` with `forward`).
> > @PiotrZSL - I wasn't sure what you meant here.
> Yes, I meant that situation. But only because "forwarding reference parameter
> 't' is never forwarded inside the function body" could be confused in such
> case, where there is std::move instead of std::forward.
> But that could be ignored, or covered by other check that would simply detect
> that you doing move when you should do forward.
Ya,
https://clang.llvm.org/extra/clang-tidy/checks/bugprone/move-forwarding-reference.html
should cover this. This new check missing-std-forward only knows how to find
code that is missing `forward`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146921/new/
https://reviews.llvm.org/D146921
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19
PiotrZSL added inline comments.
Comment at:
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp:138
+
+} // namespace negative_cases
ccotter wrote:
> ccotter wrote:
> > PiotrZSL wrote:
> > > what about when someone uses std::move instead of std::format ?
> > > maybe some "note" for such issue ?
> > Are you suggesting to have the tool add a special note in something like
> >
> > ```
> > template
> > void foo(T&& t) { T other = std::move(t); }
> > ```
> >
> > I'm not sure I completely followed what you were saying. Or perhaps a fixit
> > for this specific case of using move on a forwarding reference (fixit to
> > replace `move` with `forward`).
> @PiotrZSL - I wasn't sure what you meant here.
Yes, I meant that situation. But only because "forwarding reference parameter
't' is never forwarded inside the function body" could be confused in such
case, where there is std::move instead of std::forward.
But that could be ignored, or covered by other check that would simply detect
that you doing move when you should do forward.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146921/new/
https://reviews.llvm.org/D146921
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19
ccotter updated this revision to Diff 512318.
ccotter added a comment.
- format
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146921/new/
https://reviews.llvm.org/D146921
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/missing-std-forward.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-missing-std-forward %t
+
+// NOLINTBEGIN
+namespace std {
+
+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 constexpr T &(remove_reference_t ) noexcept;
+template constexpr T &(remove_reference_t &) noexcept;
+template constexpr remove_reference_t &(T &);
+
+} // namespace std
+// NOLINTEND
+
+struct S {
+ S();
+ S(const S&);
+ S(S&&) noexcept;
+ S& operator=(const S&);
+ S& operator=(S&&) noexcept;
+};
+
+template
+void consumes_all(Ts&&...);
+
+namespace positive_cases {
+
+template
+void does_not_forward(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ T other = t;
+}
+
+template
+void does_not_forward_invoked(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ T other = t();
+}
+
+template
+void forwards_pairwise(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ auto first = std::forward(t.first);
+ auto second = std::forward(t.second);
+}
+
+template
+void does_not_forward_pack(Ts&&... ts) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: forwarding reference parameter 'ts' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ consumes_all(ts...);
+}
+
+template
+class AClass {
+
+ template
+ AClass(U&& u) : data(u) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+
+ template
+ AClass& operator=(U&& u) { }
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+
+ template
+ void mixed_params(T&& t, U&& u) {
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+T other1 = std::move(t);
+U other2 = std::move(u);
+ }
+
+ T data;
+};
+
+template
+void does_not_forward_in_evaluated_code(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:45: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ using result_t = decltype(std::forward(t));
+ unsigned len = sizeof(std::forward(t));
+ T other = t;
+}
+
+} // namespace positive_cases
+
+namespace negative_cases {
+
+template
+void just_a_decl(T&);
+
+template
+void does_forward(T&& t) {
+ T other = std::forward(t);
+}
+
+template
+void does_forward_pack(Ts&&... ts) {
+ consumes_all(std::forward(ts)...);
+}
+
+void pass_by_value(S s) {
+ S other = std::move(s);
+}
+
+void lvalue_ref(S& s) {
+ S other = std::move(s);
+}
+
+void rvalue_ref(S&& s) {
+ S other = std::move(s);
+}
+
+template
+void templated_rvalue_ref(std::remove_reference_t&& t) {
+ T other = std::move(t);
+}
+
+template
+class AClass {
+
+ template
+ AClass(U&& u) : data(std::forward(u)) {}
+
+ template
+ AClass& operator=(U&& u) {
+data = std::forward(u);
+ }
+
+ void rvalue_ref(T&& t) {
+T other = std::move(t);
+ }
+
+ T data;
+};
+
+} // namespace negative_cases
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@
[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19
ccotter added inline comments.
Comment at:
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp:138
+
+} // namespace negative_cases
ccotter wrote:
> PiotrZSL wrote:
> > what about when someone uses std::move instead of std::format ?
> > maybe some "note" for such issue ?
> Are you suggesting to have the tool add a special note in something like
>
> ```
> template
> void foo(T&& t) { T other = std::move(t); }
> ```
>
> I'm not sure I completely followed what you were saying. Or perhaps a fixit
> for this specific case of using move on a forwarding reference (fixit to
> replace `move` with `forward`).
@PiotrZSL - I wasn't sure what you meant here.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146921/new/
https://reviews.llvm.org/D146921
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19
ccotter updated this revision to Diff 511847.
ccotter added a comment.
- Rename
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146921/new/
https://reviews.llvm.org/D146921
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/missing-std-forward.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-missing-std-forward %t
+
+// NOLINTBEGIN
+namespace std {
+
+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 constexpr T &(remove_reference_t ) noexcept;
+template constexpr T &(remove_reference_t &) noexcept;
+template constexpr remove_reference_t &(T &);
+
+} // namespace std
+// NOLINTEND
+
+struct S {
+ S();
+ S(const S&);
+ S(S&&) noexcept;
+ S& operator=(const S&);
+ S& operator=(S&&) noexcept;
+};
+
+template
+void consumes_all(Ts&&...);
+
+namespace positive_cases {
+
+template
+void does_not_forward(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ T other = t;
+}
+
+template
+void does_not_forward_invoked(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ T other = t();
+}
+
+template
+void forwards_pairwise(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ auto first = std::forward(t.first);
+ auto second = std::forward(t.second);
+}
+
+template
+void does_not_forward_pack(Ts&&... ts) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: forwarding reference parameter 'ts' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ consumes_all(ts...);
+}
+
+template
+class AClass {
+
+ template
+ AClass(U&& u) : data(u) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+
+ template
+ AClass& operator=(U&& u) { }
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+
+ template
+ void mixed_params(T&& t, U&& u) {
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+T other1 = std::move(t);
+U other2 = std::move(u);
+ }
+
+ T data;
+};
+
+template
+void does_not_forward_in_evaluated_code(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:45: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+ using result_t = decltype(std::forward(t));
+ unsigned len = sizeof(std::forward(t));
+ T other = t;
+}
+
+} // namespace positive_cases
+
+namespace negative_cases {
+
+template
+void just_a_decl(T&);
+
+template
+void does_forward(T&& t) {
+ T other = std::forward(t);
+}
+
+template
+void does_forward_pack(Ts&&... ts) {
+ consumes_all(std::forward(ts)...);
+}
+
+void pass_by_value(S s) {
+ S other = std::move(s);
+}
+
+void lvalue_ref(S& s) {
+ S other = std::move(s);
+}
+
+void rvalue_ref(S&& s) {
+ S other = std::move(s);
+}
+
+template
+void templated_rvalue_ref(std::remove_reference_t&& t) {
+ T other = std::move(t);
+}
+
+template
+class AClass {
+
+ template
+ AClass(U&& u) : data(std::forward(u)) {}
+
+ template
+ AClass& operator=(U&& u) {
+data = std::forward(u);
+ }
+
+ void rvalue_ref(T&& t) {
+T other = std::move(t);
+ }
+
+ T data;
+};
+
+} // namespace negative_cases
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@
[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19
ccotter updated this revision to Diff 509539.
ccotter added a comment.
- Use common function
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146921/new/
https://reviews.llvm.org/D146921
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/forwarding-reference-param-not-forwarded.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-forwarding-reference-param-not-forwarded %t -- -- -fno-delayed-template-parsing
+
+// NOLINTBEGIN
+namespace std {
+
+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 constexpr T &(remove_reference_t ) noexcept;
+template constexpr T &(remove_reference_t &) noexcept;
+template constexpr remove_reference_t &(T &);
+
+} // namespace std
+// NOLINTEND
+
+struct S {
+ S();
+ S(const S&);
+ S(S&&) noexcept;
+ S& operator=(const S&);
+ S& operator=(S&&) noexcept;
+};
+
+template
+void consumes_all(Ts&&...);
+
+namespace positive_cases {
+
+template
+void does_not_forward(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+ T other = t;
+}
+
+template
+void does_not_forward_invoked(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+ T other = t();
+}
+
+template
+void forwards_pairwise(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+ auto first = std::forward(t.first);
+ auto second = std::forward(t.second);
+}
+
+template
+void does_not_forward_pack(Ts&&... ts) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: forwarding reference parameter 'ts' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+ consumes_all(ts...);
+}
+
+template
+class AClass {
+
+ template
+ AClass(U&& u) : data(u) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+
+ template
+ AClass& operator=(U&& u) { }
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+
+ template
+ void mixed_params(T&& t, U&& u) {
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+T other1 = std::move(t);
+U other2 = std::move(u);
+ }
+
+ T data;
+};
+
+template
+void does_not_forward_in_evaluated_code(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:45: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+ using result_t = decltype(std::forward(t));
+ unsigned len = sizeof(std::forward(t));
+ T other = t;
+}
+
+} // namespace positive_cases
+
+namespace negative_cases {
+
+template
+void just_a_decl(T&);
+
+template
+void does_forward(T&& t) {
+ T other = std::forward(t);
+}
+
+template
+void does_forward_pack(Ts&&... ts) {
+ consumes_all(std::forward(ts)...);
+}
+
+void pass_by_value(S s) {
+ S other = std::move(s);
+}
+
+void lvalue_ref(S& s) {
+ S other = std::move(s);
+}
+
+void rvalue_ref(S&& s) {
+ S other = std::move(s);
+}
+
+template
+void templated_rvalue_ref(std::remove_reference_t&& t) {
+ T other = std::move(t);
+}
+
+template
+class AClass {
+
+ template
+ AClass(U&& u) : data(std::forward(u)) {}
+
+ template
+ AClass& operator=(U&& u) {
+data =
[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19
ccotter updated this revision to Diff 508869.
ccotter marked 2 inline comments as done.
ccotter added a comment.
- fix docs, fix test
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146921/new/
https://reviews.llvm.org/D146921
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/forwarding-reference-param-not-forwarded.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-forwarding-reference-param-not-forwarded %t -- -- -fno-delayed-template-parsing
+
+// NOLINTBEGIN
+namespace std {
+
+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 constexpr T &(remove_reference_t ) noexcept;
+template constexpr T &(remove_reference_t &) noexcept;
+template constexpr remove_reference_t &(T &);
+
+} // namespace std
+// NOLINTEND
+
+struct S {
+ S();
+ S(const S&);
+ S(S&&) noexcept;
+ S& operator=(const S&);
+ S& operator=(S&&) noexcept;
+};
+
+template
+void consumes_all(Ts&&...);
+
+namespace positive_cases {
+
+template
+void does_not_forward(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+ T other = t;
+}
+
+template
+void does_not_forward_invoked(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+ T other = t();
+}
+
+template
+void forwards_pairwise(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+ auto first = std::forward(t.first);
+ auto second = std::forward(t.second);
+}
+
+template
+void does_not_forward_pack(Ts&&... ts) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: forwarding reference parameter 'ts' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+ consumes_all(ts...);
+}
+
+template
+class AClass {
+
+ template
+ AClass(U&& u) : data(u) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+
+ template
+ AClass& operator=(U&& u) { }
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+
+ template
+ void mixed_params(T&& t, U&& u) {
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+T other1 = std::move(t);
+U other2 = std::move(u);
+ }
+
+ T data;
+};
+
+template
+void does_not_forward_in_evaluated_code(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:45: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+ using result_t = decltype(std::forward(t));
+ unsigned len = sizeof(std::forward(t));
+ T other = t;
+}
+
+} // namespace positive_cases
+
+namespace negative_cases {
+
+template
+void just_a_decl(T&);
+
+template
+void does_forward(T&& t) {
+ T other = std::forward(t);
+}
+
+template
+void does_forward_pack(Ts&&... ts) {
+ consumes_all(std::forward(ts)...);
+}
+
+void pass_by_value(S s) {
+ S other = std::move(s);
+}
+
+void lvalue_ref(S& s) {
+ S other = std::move(s);
+}
+
+void rvalue_ref(S&& s) {
+ S other = std::move(s);
+}
+
+template
+void templated_rvalue_ref(std::remove_reference_t&& t) {
+ T other = std::move(t);
+}
+
+template
+class AClass {
+
+ template
+ AClass(U&& u) : data(std::forward(u)) {}
+
+ template
+ AClass&
[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19
ccotter added inline comments.
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.cpp:20
+
+AST_MATCHER(Expr, hasUnevaluatedContext) {
+ if (isa(Node) || isa(Node))
PiotrZSL wrote:
> move this matcher to some utils/...
> It may be needed by other checks.
>
Will be done in https://reviews.llvm.org/D146929
Comment at:
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp:138
+
+} // namespace negative_cases
PiotrZSL wrote:
> what about when someone uses std::move instead of std::format ?
> maybe some "note" for such issue ?
Are you suggesting to have the tool add a special note in something like
```
template
void foo(T&& t) { T other = std::move(t); }
```
I'm not sure I completely followed what you were saying. Or perhaps a fixit for
this specific case of using move on a forwarding reference (fixit to replace
`move` with `forward`).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146921/new/
https://reviews.llvm.org/D146921
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19
PiotrZSL added inline comments.
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.cpp:20
+
+AST_MATCHER(Expr, hasUnevaluatedContext) {
+ if (isa(Node) || isa(Node))
move this matcher to some utils/...
It may be needed by other checks.
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.h:32
+ }
+};
+
ccotter wrote:
> PiotrZSL wrote:
> > add here that thing about UnlessSpelledInSource
> Sorry, I think I'm missing some context here. Would you mind clarifying your
> comment?
I mean:
```std::optional getCheckTraversalKind() const override {
return TK_IgnoreUnlessSpelledInSource;
}```
just to specify this explicitly..
Comment at:
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/forwarding-reference-param-not-forwarded.rst:7
+Warns when a forwarding reference parameter is not forwarded inside the
+function body.
+
Add link to cpp guidelines, mention rule name like in other checks.
Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:189
`cppcoreguidelines-avoid-reference-coroutine-parameters
`_,
+ `cppcoreguidelines-forwarding-reference-param-not-forwarded
`_, "Yes"
`cppcoreguidelines-init-variables
`_, "Yes"
no fixes
Comment at:
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp:138
+
+} // namespace negative_cases
what about when someone uses std::move instead of std::format ?
maybe some "note" for such issue ?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146921/new/
https://reviews.llvm.org/D146921
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19
ccotter updated this revision to Diff 508450.
ccotter added a comment.
- add tests, simplify expr, handle unevaluated exprs
- formatting
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146921/new/
https://reviews.llvm.org/D146921
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/forwarding-reference-param-not-forwarded.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-forwarding-reference-param-not-forwarded %t
+
+// NOLINTBEGIN
+namespace std {
+
+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 constexpr T &(remove_reference_t ) noexcept;
+template constexpr T &(remove_reference_t &) noexcept;
+template constexpr remove_reference_t &(T &);
+
+} // namespace std
+// NOLINTEND
+
+struct S {
+ S();
+ S(const S&);
+ S(S&&) noexcept;
+ S& operator=(const S&);
+ S& operator=(S&&) noexcept;
+};
+
+template
+void consumes_all(Ts&&...);
+
+namespace positive_cases {
+
+template
+void does_not_forward(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+ T other = t;
+}
+
+template
+void does_not_forward_invoked(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+ T other = t();
+}
+
+template
+void forwards_pairwise(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+ auto first = std::forward(t.first);
+ auto second = std::forward(t.second);
+}
+
+template
+void does_not_forward_pack(Ts&&... ts) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: forwarding reference parameter 'ts' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+ consumes_all(ts...);
+}
+
+template
+class AClass {
+
+ template
+ AClass(U&& u) : data(u) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+
+ template
+ AClass& operator=(U&& u) { }
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+
+ template
+ void mixed_params(T&& t, U&& u) {
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+T other1 = std::move(t);
+U other2 = std::move(u);
+ }
+
+ T data;
+};
+
+template
+void does_not_forward_in_evaluated_code(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:45: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+ using result_t = decltype(std::forward(t));
+ unsigned len = sizeof(std::forward(t));
+ T other = t;
+}
+
+} // namespace positive_cases
+
+namespace negative_cases {
+
+template
+void just_a_decl(T&);
+
+template
+void does_forward(T&& t) {
+ T other = std::forward(t);
+}
+
+template
+void does_forward_pack(Ts&&... ts) {
+ consumes_all(std::forward(ts)...);
+}
+
+void pass_by_value(S s) {
+ S other = std::move(s);
+}
+
+void lvalue_ref(S& s) {
+ S other = std::move(s);
+}
+
+void rvalue_ref(S&& s) {
+ S other = std::move(s);
+}
+
+template
+void templated_rvalue_ref(std::remove_reference_t&& t) {
+ T other = std::move(t);
+}
+
+template
+class AClass {
+
+ template
+ AClass(U&& u) : data(std::forward(u)) {}
+
+ template
+ AClass& operator=(U&& u) {
+data =
[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:127 + + Warns when a forwarding reference parameter is not forwarded within the function body. + Please follow 80 characters limit for text. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146921/new/ https://reviews.llvm.org/D146921 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19
ccotter added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.cpp:83-84 + + if (!Param) +return; + PiotrZSL wrote: > I thing this can never happen Another review suggested I check the matched node for nullness, even though it was the only possible match (https://reviews.llvm.org/D140793). I'll keep it as is since in the spirit of defensive programming and to assist future developers who may add more matches that might invalidate the invariant that `Param` is always non-null. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.h:32 + } +}; + PiotrZSL wrote: > add here that thing about UnlessSpelledInSource Sorry, I think I'm missing some context here. Would you mind clarifying your comment? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146921/new/ https://reviews.llvm.org/D146921 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19
ccotter updated this revision to Diff 508441.
ccotter marked 4 inline comments as done.
ccotter retitled this revision from "[clang-tidy] Implement cppcoreguidelines
F.19 " to "[clang-tidy] Implement cppcoreguidelines F.19".
ccotter added a comment.
- add tests, simplify expr, handle unevaluated exprs
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146921/new/
https://reviews.llvm.org/D146921
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/forwarding-reference-param-not-forwarded.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/forwarding-reference-param-not-forwarded.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-forwarding-reference-param-not-forwarded %t
+
+// NOLINTBEGIN
+namespace std {
+
+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 constexpr T &(remove_reference_t ) noexcept;
+template constexpr T &(remove_reference_t &) noexcept;
+template constexpr remove_reference_t &(T &);
+
+} // namespace std
+// NOLINTEND
+
+struct S {
+ S();
+ S(const S&);
+ S(S&&) noexcept;
+ S& operator=(const S&);
+ S& operator=(S&&) noexcept;
+};
+
+template
+void consumes_all(Ts&&...);
+
+namespace positive_cases {
+
+template
+void does_not_forward(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+ T other = t;
+}
+
+template
+void does_not_forward_invoked(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+ T other = t();
+}
+
+template
+void forwards_pairwise(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+ auto first = std::forward(t.first);
+ auto second = std::forward(t.second);
+}
+
+template
+void does_not_forward_pack(Ts&&... ts) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: forwarding reference parameter 'ts' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+ consumes_all(ts...);
+}
+
+template
+class AClass {
+
+ template
+ AClass(U&& u) : data(u) {}
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+
+ template
+ AClass& operator=(U&& u) { }
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+
+ template
+ void mixed_params(T&& t, U&& u) {
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: forwarding reference parameter 'u' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+T other1 = std::move(t);
+U other2 = std::move(u);
+ }
+
+ T data;
+};
+
+template
+void does_not_forward_in_evaluated_code(T&& t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:45: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-forwarding-reference-param-not-forwarded]
+ using result_t = decltype(std::forward(t));
+ unsigned len = sizeof(std::forward(t));
+ T other = t;
+}
+
+} // namespace positive_cases
+
+namespace negative_cases {
+
+template
+void just_a_decl(T&);
+
+template
+void does_forward(T&& t) {
+ T other = std::forward(t);
+}
+
+template
+void does_forward_pack(Ts&&... ts) {
+ consumes_all(std::forward(ts)...);
+}
+
+void pass_by_value(S s) {
+ S other = std::move(s);
+}
+
+void lvalue_ref(S& s) {
+ S other = std::move(s);
+}
+
+void rvalue_ref(S&& s) {
+ S other = std::move(s);
+}
+
+template
+void templated_rvalue_ref(std::remove_reference_t&& t) {
+ T other =
[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19
PiotrZSL added a comment.
maybe some other name for check, like missing-std-forward.
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.cpp:62
+ namedDecl(hasUnderlyingDecl(hasName("::std::forward")),
+ argumentCountIs(1),
+ hasArgument(0,
declRefExpr(to(equalsBoundNode("param"))).bind("ref")))
move this up to be checkered first
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.cpp:67
+ Finder->addMatcher(
+ parmVarDecl(
+ parmVarDecl().bind("param"), isTemplateTypeOfFunction(),
anyway invert this logic.
would be nice to invert this to start matching with functionDecl, but we don't
have forEachParameter
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.cpp:69-72
+ anyOf(hasAncestor(cxxConstructorDecl(
+ToParam, isDefinition(),
+optionally(hasDescendant(ForwardCallMatcher,
+hasAncestor(functionDecl(
use isDefinition also for functionDecl,
anyway maybe
```
hasAncestor(functionnDecl(isDefinition(),
ToParam,
unless(hasDescendant(ForwardCallMatcher)))
```
I don't see reason for eplicit handling of cxxConstructorDecl, hasDescendant
will match also initialization list.
Exclude code that is used in decltype, like:
```
template
void test(T&& t) {
using Type = decltype(callSomething(std::forward(t)));
callOther(t);
}
```
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.cpp:83-84
+
+ if (!Param)
+return;
+
I thing this can never happen
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.h:32
+ }
+};
+
add here that thing about UnlessSpelledInSource
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146921/new/
https://reviews.llvm.org/D146921
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
