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][modernize-return-braced-init-list]fix false-positives #68491

Merged
merged 2 commits into from
Oct 8, 2023
Merged

[clang-tidy][modernize-return-braced-init-list]fix false-positives #68491

merged 2 commits into from
Oct 8, 2023

Conversation

HerrCai0907
Copy link
Contributor

@HerrCai0907 HerrCai0907 commented Oct 7, 2023

fix false-positives when constructing the container with count copies of elements with value value
(e.g., vector(size_type count, const T& value);).
Fixes: #68159

when constructing the container with ``count`` copies of
elements with value ``value``
(e.g., ``vector(size_type count, const T& value);``).
Fixes: #68159
@llvmbot
Copy link
Member

llvmbot commented Oct 7, 2023

@llvm/pr-subscribers-clang-tidy

Changes

fix false-positives when constructing the container with count copies of elements with value value
(e.g., vector(size_type count, const T& value);).
Fixes: #68159


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

3 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/modernize/ReturnBracedInitListCheck.cpp (+13-3)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+6)
  • (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/return-braced-init-list.cpp (+18-3)
diff --git a/clang-tools-extra/clang-tidy/modernize/ReturnBracedInitListCheck.cpp b/clang-tools-extra/clang-tidy/modernize/ReturnBracedInitListCheck.cpp
index 407de610d13a79f..edb0468d2914600 100644
--- a/clang-tools-extra/clang-tidy/modernize/ReturnBracedInitListCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/ReturnBracedInitListCheck.cpp
@@ -17,11 +17,21 @@ using namespace clang::ast_matchers;
 namespace clang::tidy::modernize {
 
 void ReturnBracedInitListCheck::registerMatchers(MatchFinder *Finder) {
-  // Skip list initialization and constructors with an initializer list.
+  auto SemanticallyDifferentContainer = allOf(
+      argumentCountIs(2), hasArgument(0, hasType(isInteger())),
+      hasType(cxxRecordDecl(hasAnyName("::std::vector", "::std::deque",
+                                       "::std::forward_list", "::std::list"))));
+
   auto ConstructExpr =
       cxxConstructExpr(
-          unless(anyOf(hasDeclaration(cxxConstructorDecl(isExplicit())),
-                       isListInitialization(), hasDescendant(initListExpr()))))
+          unless(anyOf(
+              // Skip explicit constructor.
+              hasDeclaration(cxxConstructorDecl(isExplicit())),
+              // Skip list initialization and constructors with an initializer
+              // list.
+              isListInitialization(), hasDescendant(initListExpr()),
+              // Skip container `vector(size_type, const T&)`.
+              SemanticallyDifferentContainer)))
           .bind("ctor");
 
   Finder->addMatcher(
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index c1b926b296b055a..053d6b20a6fe875 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -269,6 +269,12 @@ Changes in existing checks
   <clang-tidy/checks/modernize/loop-convert>` to support for-loops with
   iterators initialized by free functions like ``begin``, ``end``, or ``size``.
 
+- Improved :doc:`modernize-return-braced-init-list
+  <clang-tidy/checks/modernize/return-braced-init-list>` check to ignore
+  false-positives when constructing the container with ``count`` copies of
+  elements with value ``value``
+  (e.g., ``vector(size_type count, const T& value);``).
+
 - Improved :doc:`modernize-use-equals-delete
   <clang-tidy/checks/modernize/use-equals-delete>` check to ignore
   false-positives when special member function is actually used or implicit.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/return-braced-init-list.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/return-braced-init-list.cpp
index 4db1d49da2ea8b9..b9526ce5746dec2 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/return-braced-init-list.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/return-braced-init-list.cpp
@@ -32,8 +32,9 @@ class initializer_list {
 template <typename T>
 class vector {
 public:
-  vector(T) {}
-  vector(std::initializer_list<T>) {}
+  vector(T);
+  vector(size_t, T);
+  vector(std::initializer_list<T>);
 };
 }
 
@@ -98,12 +99,26 @@ Foo f6() {
   return Foo(b6, 1);
 }
 
-std::vector<int> f7() {
+std::vector<int> vectorWithOneParameter() {
   int i7 = 1;
   return std::vector<int>(i7);
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: avoid repeating the return type
 }
 
+std::vector<int> vectorIntWithTwoParameter() {
+  return std::vector<int>(1, 2);
+}
+
+std::vector<double> vectorDoubleWithTwoParameter() {
+  return std::vector<double>(1, 2.1);
+}
+struct A {};
+std::vector<A> vectorRecordWithTwoParameter() {
+  A a{};
+  return std::vector<A>(1, a);
+}
+
+
 Bar f8() {
   return {};
 }

@HerrCai0907 HerrCai0907 requested a review from PiotrZSL October 7, 2023 17:09
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.

Add entry to check documentation that such constructors for such types are excluded.
Consider fixing pointed out issues.
Except those, looks fine.

@HerrCai0907 HerrCai0907 merged commit 7cc1bfa into llvm:main Oct 8, 2023
@HerrCai0907 HerrCai0907 deleted the 68159-clang-tidy-bug-stdvectorsize-value-in-modernize-return-braced-init-list branch October 8, 2023 23:20
@ax3l
Copy link

ax3l commented Oct 8, 2023

Thank you! 🎉

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] 🐛 std::vector(size, value) in modernize-return-braced-init-list
4 participants