Skip to content

[clang-tidy] hicpp-ignored-remove-result ignore functions with same prefixes as the target #87587

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
merged 1 commit into from
Apr 5, 2024

Conversation

HerrCai0907
Copy link
Contributor

Fixes: #87359
#82952 introduces regexes matching and causes false positives.
This patch fixes it by adding postfix $

…refixes as the target

Fixes: llvm#87359
llvm#82952 introduces regexes matching and causes false postives.
This patch fixes it by adding $
@llvmbot
Copy link
Member

llvmbot commented Apr 4, 2024

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

@llvm/pr-subscribers-clang-tidy

Author: Congcong Cai (HerrCai0907)

Changes

Fixes: #87359
#82952 introduces regexes matching and causes false positives.
This patch fixes it by adding postfix $


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

3 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/hicpp/IgnoredRemoveResultCheck.cpp (+3-3)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4)
  • (modified) clang-tools-extra/test/clang-tidy/checkers/hicpp/ignored-remove-result.cpp (+6)
diff --git a/clang-tools-extra/clang-tidy/hicpp/IgnoredRemoveResultCheck.cpp b/clang-tools-extra/clang-tidy/hicpp/IgnoredRemoveResultCheck.cpp
index 8020f8cd062510..b1a18485ce168d 100644
--- a/clang-tools-extra/clang-tidy/hicpp/IgnoredRemoveResultCheck.cpp
+++ b/clang-tools-extra/clang-tidy/hicpp/IgnoredRemoveResultCheck.cpp
@@ -14,9 +14,9 @@ IgnoredRemoveResultCheck::IgnoredRemoveResultCheck(llvm::StringRef Name,
                                                    ClangTidyContext *Context)
     : UnusedReturnValueCheck(Name, Context,
                              {
-                                 "::std::remove",
-                                 "::std::remove_if",
-                                 "::std::unique",
+                                 "::std::remove$",
+                                 "::std::remove_if$",
+                                 "::std::unique$",
                              }) {
   // The constructor for ClangTidyCheck needs to have been called
   // before we can access options via Options.get().
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 309b844615a121..456e09204fa2f9 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -210,6 +210,10 @@ Changes in existing checks
 - Improved :doc:`google-runtime-int <clang-tidy/checks/google/runtime-int>`
   check performance through optimizations.
 
+- Improved :doc:`hicpp-ignored-remove-result <clang-tidy/checks/hicpp/ignored-remove-result>`
+  check by ignoring other functions with same prefixes as the target specific
+  functions.
+
 - Improved :doc:`llvm-header-guard
   <clang-tidy/checks/llvm/header-guard>` check by replacing the local
   option `HeaderFileExtensions` by the global option of the same name.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/hicpp/ignored-remove-result.cpp b/clang-tools-extra/test/clang-tidy/checkers/hicpp/ignored-remove-result.cpp
index b068f085909893..fc431024303ab2 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/hicpp/ignored-remove-result.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/hicpp/ignored-remove-result.cpp
@@ -15,6 +15,10 @@ ForwardIt unique(ForwardIt, ForwardIt);
 template <class InputIt, class T>
 InputIt find(InputIt, InputIt, const T&);
 
+struct unique_disposable {
+  void* release();
+};
+
 class error_code {
 };
 
@@ -63,4 +67,6 @@ void noWarning() {
   //   bugprone-unused-return-value's checked return types.
   errorFunc();
   (void) errorFunc();
+
+  std::unique_disposable{}.release();
 }

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

Copy link
Contributor

@felix642 felix642 left a comment

Choose a reason for hiding this comment

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

LGTM

@HerrCai0907 HerrCai0907 merged commit ff1e72d into llvm:main Apr 5, 2024
@HerrCai0907 HerrCai0907 deleted the fix/hicpp-ignore-remove-result branch April 8, 2024 03:50
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.

[clang-tidy] hicpp-ignored-remove-result matches std::unique*::release()-like calls even though it is only supposed to match 3 specific functions
4 participants