Skip to content

[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

Merged

Conversation

AMS21
Copy link
Contributor

@AMS21 AMS21 commented Feb 26, 2024

A 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.

godbolt

which is obviously not correct and this simple patch fixes that.

@llvmbot
Copy link
Member

llvmbot commented Feb 26, 2024

@llvm/pr-subscribers-clang-tidy

@llvm/pr-subscribers-clang-tools-extra

Author: None (AMS21)

Changes

A definition like this:

template &lt;typename T&gt;
void f(T &amp;&amp;) = delete;

would previously generate the the following output:

&lt;source&gt;:2:12: warning: forwarding reference parameter '' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
    2 | void f(T &amp;&amp;) = delete;
      |            ^
1 warning generated.

godbolt

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:

  • (modified) clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp (+2-1)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4)
  • (modified) clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp (+15)
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

@AMS21 AMS21 force-pushed the missing_std_forward_deleted_func_false_positive branch from a47b09f to dd493ba Compare February 26, 2024 20:53
…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.
@AMS21 AMS21 force-pushed the missing_std_forward_deleted_func_false_positive branch from dd493ba to 821fbb9 Compare February 27, 2024 07:42
Copy link
Member

@PiotrZSL PiotrZSL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants