Skip to content
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] Ignore unused parameters in rvalue-reference-param-not-moved check #69045

Merged
merged 1 commit into from
Oct 14, 2023

Conversation

AMS21
Copy link
Contributor

@AMS21 AMS21 commented Oct 14, 2023

With this patch we no longer issue a warning for unused parameters which are marked as such.

This fixes #68209

@llvmbot
Copy link
Collaborator

llvmbot commented Oct 14, 2023

@llvm/pr-subscribers-clang-tidy

Author: None (AMS21)

Changes

With this patch we no longer issue a warning for unused parameters which are marked as such.

This fixes #68209


Full diff: https://github.com/llvm/llvm-project/pull/69045.diff

3 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp (+3)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4)
  • (modified) clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp (+19)
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp
index efcaffb45d9ad8a..88b00dc17470f32 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp
@@ -84,6 +84,9 @@ void RvalueReferenceParamNotMovedCheck::check(
   if (IgnoreUnnamedParams && Param->getName().empty())
     return;
 
+  if (!Param->isUsed() && Param->hasAttr<UnusedAttr>())
+    return;
+
   const auto *Function = dyn_cast<FunctionDecl>(Param->getDeclContext());
   if (!Function)
     return;
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 03e5dc6f164af2a..c732d4904df13fa 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -237,6 +237,10 @@ Changes in existing checks
   <clang-tidy/checks/cppcoreguidelines/pro-type-vararg>` check to ignore
   false-positives in unevaluated context (e.g., ``decltype``, ``sizeof``, ...).
 
+- Improved :doc:`cppcoreguidelines-rvalue-reference-param-not-moved
+  <clang-tidy/checks/cppcoreguidelines/rvalue-reference-param-not-moved>` check
+  to ignore unused parameters when they are marked as unused.
+
 - Improved :doc:`llvm-namespace-comment
   <clang-tidy/checks/llvm/namespace-comment>` check to provide fixes for
   ``inline`` namespaces in the same format as :program:`clang-format`.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp
index 8f8e272e1e8a90d..c6a70aef810ac90 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp
@@ -333,3 +333,22 @@ void instantiate_a_class_template() {
   AClassTemplate<Obj&> withObjRef(o);
   withObjRef.never_moves(o);
 }
+
+namespace gh68209
+{
+  void f1([[maybe_unused]] int&& x) {}
+
+  void f2(__attribute__((unused)) int&& x) {}
+
+  void f3(int&& x) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: rvalue reference parameter 'x' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+
+  template <typename T>
+  void f4([[maybe_unused]] T&& x) {}
+
+  template <typename T>
+  void f5(__attribute((unused)) T&& x) {}
+
+  template<typename T>
+  void f6(T&& x) {}
+} // namespace gh68209

@AMS21
Copy link
Contributor Author

AMS21 commented Oct 14, 2023

I'm not sure if we should rather have this as an option like IgnoreUnnamedParams

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.

Looks fine, I don't thing that this needs to be configurable.
If someone put explicitly attribute, then there is assumption that such param can be unused.

…moved check`

With this patch we no longer issue a warning for unused parameters which
are marked as such.

This fixes llvm#68209
@AMS21
Copy link
Contributor Author

AMS21 commented Oct 14, 2023

Added the suggested change and also added another test case

@AMS21
Copy link
Contributor Author

AMS21 commented Oct 14, 2023

If there are no more problems I would kindly ask for someone to push this on my behalf :)

@PiotrZSL PiotrZSL merged commit bb6a98c into llvm:main Oct 14, 2023
3 checks passed
@AMS21 AMS21 deleted the gh68209 branch October 14, 2023 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants