Skip to content

[clang-tidy] fix false negatives for performance-inefficient-vector-operation #95667

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

Conversation

HerrCai0907
Copy link
Contributor

@HerrCai0907 HerrCai0907 commented Jun 15, 2024

Fixes: #95596
Avoid false negatives caused by different variable definition type and variable initial value type in loop initialization expression.

…peration

Fixes: llvm#95596
Check will warn if the loop var type is not same as var init expr type
@llvmbot
Copy link
Member

llvmbot commented Jun 15, 2024

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

@llvm/pr-subscribers-clang-tidy

Author: Congcong Cai (HerrCai0907)

Changes

Fixes: #95596
Check will warn if the loop var type is not same as var init expr type


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

3 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.cpp (+3-3)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5)
  • (modified) clang-tools-extra/test/clang-tidy/checkers/performance/inefficient-vector-operation.cpp (+35)
diff --git a/clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.cpp b/clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.cpp
index 20aea5a79fe9a..dc6e0cf9c7d12 100644
--- a/clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.cpp
@@ -105,9 +105,9 @@ void InefficientVectorOperationCheck::addMatcher(
           onImplicitObjectArgument(declRefExpr(to(TargetVarDecl))))
           .bind(AppendCallName);
   const auto AppendCall = expr(ignoringImplicit(AppendCallExpr));
-  const auto LoopVarInit =
-      declStmt(hasSingleDecl(varDecl(hasInitializer(integerLiteral(equals(0))))
-                                 .bind(LoopInitVarName)));
+  const auto LoopVarInit = declStmt(hasSingleDecl(
+      varDecl(hasInitializer(ignoringParenImpCasts(integerLiteral(equals(0)))))
+          .bind(LoopInitVarName)));
   const auto RefersToLoopVar = ignoringParenImpCasts(
       declRefExpr(to(varDecl(equalsBoundNode(LoopInitVarName)))));
 
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 7092d0b6fdb02..3bdd735f7dcf7 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -387,6 +387,11 @@ Changes in existing checks
 - Improved :doc:`modernize-use-using <clang-tidy/checks/modernize/use-using>`
   check by adding support for detection of typedefs declared on function level.
 
+- Improved :doc:`performance-inefficient-vector-operation
+  <clang-tidy/checks/performance/inefficient-vector-operation>` fixing false
+  negatives caused by different variable definition type and variable initial
+  value type in loop initialization expression.
+
 - Improved :doc:`performance-move-const-arg
   <clang-tidy/checks/performance/move-const-arg>` check by ignoring
   ``std::move()`` calls when their target is used as an rvalue.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/inefficient-vector-operation.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/inefficient-vector-operation.cpp
index c28592f4d6368..50424920d72b7 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/inefficient-vector-operation.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/inefficient-vector-operation.cpp
@@ -387,3 +387,38 @@ void foo(const StructWithFieldContainer &Src) {
     B.push_back(Number);
   }
 }
+
+namespace gh95596 {
+
+void f(std::vector<int>& t) {
+  {
+    std::vector<int> gh95596_0;
+    // CHECK-FIXES: gh95596_0.reserve(10);
+    for (unsigned i = 0; i < 10; ++i)
+      gh95596_0.push_back(i);
+      // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'push_back' is called inside a loop; consider pre-allocating the container capacity before the loop
+  }
+  {
+    std::vector<int> gh95596_1;
+    // CHECK-FIXES: gh95596_1.reserve(10);
+    for (int i = 0U; i < 10; ++i)
+      gh95596_1.push_back(i);
+      // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'push_back' is called inside a loop; consider pre-allocating the container capacity before the loop
+  }
+  {
+    std::vector<int> gh95596_2;
+    // CHECK-FIXES: gh95596_2.reserve(10);
+    for (unsigned i = 0U; i < 10; ++i)
+      gh95596_2.push_back(i);
+      // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'push_back' is called inside a loop; consider pre-allocating the container capacity before the loop
+  }
+  {
+    std::vector<int> gh95596_3;
+    // CHECK-FIXES: gh95596_3.reserve(10U);
+    for (int i = 0; i < 10U; ++i)
+      gh95596_3.push_back(i);
+      // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'push_back' is called inside a loop; consider pre-allocating the container capacity before the loop
+  }
+}
+
+} // namespace gh95596
\ No newline at end of file

@HerrCai0907 HerrCai0907 requested review from PiotrZSL, 5chmidti and SimplyDanny and removed request for PiotrZSL June 15, 2024 16:38
Copy link
Contributor

@5chmidti 5chmidti 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 dcbe0d4 into llvm:main Jun 16, 2024
8 checks passed
@HerrCai0907 HerrCai0907 deleted the 95596-clang-tidy-performance-inefficient-vector-operation-doesnt-warn-if-loop-has-unsigned-index branch June 16, 2024 04:01
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] performance-inefficient-vector-operation doesn't warn if loop has unsigned index
4 participants