From d1b76f7649e1b1239ad328c7da9b1f9d8fb2c5f3 Mon Sep 17 00:00:00 2001 From: Congcong Cai Date: Sun, 8 Oct 2023 01:00:15 +0800 Subject: [PATCH 1/2] [clang-tidy][modernize-return-braced-init-list]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 --- .../modernize/ReturnBracedInitListCheck.cpp | 16 +++++++++++--- clang-tools-extra/docs/ReleaseNotes.rst | 6 ++++++ .../modernize/return-braced-init-list.cpp | 21 ++++++++++++++++--- 3 files changed, 37 insertions(+), 6 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/ReturnBracedInitListCheck.cpp b/clang-tools-extra/clang-tidy/modernize/ReturnBracedInitListCheck.cpp index 407de610d13a7..edb0468d29146 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 c1b926b296b05..053d6b20a6fe8 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -269,6 +269,12 @@ Changes in existing checks ` to support for-loops with iterators initialized by free functions like ``begin``, ``end``, or ``size``. +- Improved :doc:`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 ` 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 4db1d49da2ea8..b9526ce5746de 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 class vector { public: - vector(T) {} - vector(std::initializer_list) {} + vector(T); + vector(size_t, T); + vector(std::initializer_list); }; } @@ -98,12 +99,26 @@ Foo f6() { return Foo(b6, 1); } -std::vector f7() { +std::vector vectorWithOneParameter() { int i7 = 1; return std::vector(i7); // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: avoid repeating the return type } +std::vector vectorIntWithTwoParameter() { + return std::vector(1, 2); +} + +std::vector vectorDoubleWithTwoParameter() { + return std::vector(1, 2.1); +} +struct A {}; +std::vector vectorRecordWithTwoParameter() { + A a{}; + return std::vector(1, a); +} + + Bar f8() { return {}; } From 6734f61505040694d71a9c31ab5178b6b2eb3127 Mon Sep 17 00:00:00 2001 From: Congcong Cai Date: Sun, 8 Oct 2023 06:34:22 +0800 Subject: [PATCH 2/2] fix comments --- .../modernize/ReturnBracedInitListCheck.cpp | 15 +++++++++++---- clang-tools-extra/docs/ReleaseNotes.rst | 3 +-- .../modernize/return-braced-init-list.cpp | 7 +++++-- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/ReturnBracedInitListCheck.cpp b/clang-tools-extra/clang-tidy/modernize/ReturnBracedInitListCheck.cpp index edb0468d29146..d7796666d0db2 100644 --- a/clang-tools-extra/clang-tidy/modernize/ReturnBracedInitListCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ReturnBracedInitListCheck.cpp @@ -9,6 +9,7 @@ #include "ReturnBracedInitListCheck.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Lex/Lexer.h" #include "clang/Tooling/FixIt.h" @@ -18,9 +19,15 @@ namespace clang::tidy::modernize { void ReturnBracedInitListCheck::registerMatchers(MatchFinder *Finder) { auto SemanticallyDifferentContainer = allOf( - argumentCountIs(2), hasArgument(0, hasType(isInteger())), - hasType(cxxRecordDecl(hasAnyName("::std::vector", "::std::deque", - "::std::forward_list", "::std::list")))); + hasDeclaration( + // Container(size_type count, const T &value, + // const Allocator &alloc = Allocator()); + cxxConstructorDecl(parameterCountIs(3), + hasParameter(0, hasType(qualType(hasCanonicalType( + isInteger())))))), + hasType(cxxRecordDecl(hasAnyName("::std::basic_string", "::std::vector", + "::std::deque", "::std::forward_list", + "::std::list")))); auto ConstructExpr = cxxConstructExpr( @@ -30,7 +37,7 @@ void ReturnBracedInitListCheck::registerMatchers(MatchFinder *Finder) { // Skip list initialization and constructors with an initializer // list. isListInitialization(), hasDescendant(initListExpr()), - // Skip container `vector(size_type, const T&)`. + // Skip container `vector(size_type, const T&, ...)`. SemanticallyDifferentContainer))) .bind("ctor"); diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 053d6b20a6fe8..60d92ccf97149 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -272,8 +272,7 @@ Changes in existing checks - Improved :doc:`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);``). + elements with value ``value``. - Improved :doc:`modernize-use-equals-delete ` check to ignore 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 b9526ce5746de..02e95e15499dc 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 @@ -30,13 +30,16 @@ class initializer_list { }; template +struct allocator {}; + +template > class vector { public: vector(T); - vector(size_t, T); + vector(size_t, T, const Allocator &alloc = Allocator()); vector(std::initializer_list); }; -} +} // namespace std class Bar {};