Author: Chris Cotter Date: 2024-01-04T21:47:14+01:00 New Revision: 03ef103235752830da7b9ce5e825c0e3ddf7f45a
URL: https://github.com/llvm/llvm-project/commit/03ef103235752830da7b9ce5e825c0e3ddf7f45a DIFF: https://github.com/llvm/llvm-project/commit/03ef103235752830da7b9ce5e825c0e3ddf7f45a.diff LOG: [clang-tidy] Fix bug in modernize-use-emplace (#66169) emplace_back cannot construct an aggregate with arguments used to initialize the aggregate. Closes #62387 Test plan: Added test test from #62387 which contains code that should not be replaced by the check. Added: Modified: clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp index 4438f0b22063f5..07e296cc26528d 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp @@ -13,6 +13,10 @@ using namespace clang::ast_matchers; namespace clang::tidy::modernize { namespace { +AST_MATCHER_P(InitListExpr, initCountLeq, unsigned, N) { + return Node.getNumInits() <= N; +} + // Identical to hasAnyName, except it does not take template specifiers into // account. This is used to match the functions names as in // DefaultEmplacyFunctions below without caring about the template types of the @@ -205,11 +209,13 @@ void UseEmplaceCheck::registerMatchers(MatchFinder *Finder) { auto HasConstructExpr = has(ignoringImplicit(SoughtConstructExpr)); // allow for T{} to be replaced, even if no CTOR is declared - auto HasConstructInitListExpr = has(initListExpr(anyOf( - allOf(has(SoughtConstructExpr), - has(cxxConstructExpr(argumentCountIs(0)))), - has(cxxBindTemporaryExpr(has(SoughtConstructExpr), - has(cxxConstructExpr(argumentCountIs(0)))))))); + auto HasConstructInitListExpr = + has(initListExpr(initCountLeq(1), + anyOf(allOf(has(SoughtConstructExpr), + has(cxxConstructExpr(argumentCountIs(0)))), + has(cxxBindTemporaryExpr( + has(SoughtConstructExpr), + has(cxxConstructExpr(argumentCountIs(0)))))))); auto HasBracedInitListExpr = anyOf(has(cxxBindTemporaryExpr(HasConstructInitListExpr)), HasConstructInitListExpr); diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 571808a51596a8..4d25e2ebe85f5f 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -394,6 +394,10 @@ Changes in existing checks false-positives when constructing the container with ``count`` copies of elements with value ``value``. +- Improved :doc:`modernize-use-emplace + <clang-tidy/checks/modernize/use-emplace>` to not replace aggregates that + ``emplace`` cannot construct with aggregate initialization. + - 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/use-emplace.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp index fead2b6151d021..f7b1ad55f5df51 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp @@ -1183,6 +1183,11 @@ struct NonTrivialWithVector { std::vector<int> it; }; +struct NonTrivialWithIntAndVector { + int x; + std::vector<int> it; +}; + struct NonTrivialWithCtor { NonTrivialWithCtor(); NonTrivialWithCtor(std::vector<int> const&); @@ -1332,6 +1337,14 @@ void testBracedInitTemporaries() { v3.push_back(NonTrivialWithCtor{{}}); v3.push_back({{0}}); v3.push_back({{}}); + + std::vector<NonTrivialWithIntAndVector> v4; + + // These should not be noticed or fixed; after the correction, the code won't + // compile. + v4.push_back(NonTrivialWithIntAndVector{1, {}}); + v4.push_back(NonTrivialWithIntAndVector{}); + v4.push_back({}); } void testWithPointerTypes() { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits