-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[clang-tidy] Fix cppcoreguidelines-missing-std-forward
false positive for deleted functions
#83055
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[clang-tidy] Fix cppcoreguidelines-missing-std-forward
false positive for deleted functions
#83055
Conversation
@llvm/pr-subscribers-clang-tidy @llvm/pr-subscribers-clang-tools-extra Author: None (AMS21) ChangesA definition like this: template <typename T>
void f(T &&) = delete; would previously generate the the following output: <source>:2:12: warning: forwarding reference parameter '' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
2 | void f(T &&) = delete;
| ^
1 warning generated. which is obviously not correct and this simple patch fixes that. Full diff: https://github.com/llvm/llvm-project/pull/83055.diff 3 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
index 370de12999aceb..c633683570f748 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
@@ -127,7 +127,8 @@ void MissingStdForwardCheck::registerMatchers(MatchFinder *Finder) {
hasAncestor(functionDecl().bind("func")),
hasAncestor(functionDecl(
isDefinition(), equalsBoundNode("func"), ToParam,
- unless(hasDescendant(std::move(ForwardCallMatcher)))))),
+ unless(anyOf(isDeleted(), hasDescendant(std::move(
+ ForwardCallMatcher))))))),
this);
}
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 69537964f9bce0..d1b841336f35c3 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -134,6 +134,10 @@ Changes in existing checks
<clang-tidy/checks/bugprone/unused-local-non-trivial-variable>` check by
ignoring local variable with ``[maybe_unused]`` attribute.
+- Fixed :doc:`cppcoreguidelines-missing-std-forward
+ <clang-tidy/checks/cppcoreguidelines/missing-std-foward>` check giving false
+ positives for deleted functions.
+
- Cleaned up :doc:`cppcoreguidelines-prefer-member-initializer
<clang-tidy/checks/cppcoreguidelines/prefer-member-initializer>`
by removing enforcement of rule `C.48
diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
index 443f338ba2046a..20e43f04180ff3 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
@@ -173,3 +173,18 @@ void lambda_value_reference_auxiliary_var(T&& t) {
}
} // namespace negative_cases
+
+namespace deleted_functions {
+
+template <typename T>
+void f(T &&) = delete;
+
+struct S {
+ template <typename T>
+ S(T &&) = delete;
+
+ template <typename T>
+ void operator&(T &&) = delete;
+};
+
+} // namespace deleted_functions
|
a47b09f
to
dd493ba
Compare
…ve for deleted functions A definition like this: ```cpp template <typename T> void f(T &&) = delete; ``` would previously generate the the following output: ```sh <source>:2:12: warning: forwarding reference parameter '' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward] 2 | void f(T &&) = delete; | ^ 1 warning generated. ``` [godbolt](https://godbolt.org/z/9d4Y9qeWW) which is obviously not correct and this simple patch fixes that.
dd493ba
to
821fbb9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
A definition like this:
would previously generate the the following output:
godbolt
which is obviously not correct and this simple patch fixes that.