diff --git a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp index 418699ffbc4d1a..9861f4681db1b4 100644 --- a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp @@ -72,7 +72,11 @@ static FindArgsResult findArgs(const CallExpr *Call) { return Result; } -static SmallVector +// Returns `true` as `first` only if a nested call to `std::min` or +// `std::max` was found. Checking if `FixItHint`s were generated is not enough, +// as the explicit casts that the check introduces may be generated without a +// nested `std::min` or `std::max` call. +static std::pair> generateReplacements(const MatchFinder::MatchResult &Match, const CallExpr *TopCall, const FindArgsResult &Result, const bool IgnoreNonTrivialTypes, @@ -91,13 +95,15 @@ generateReplacements(const MatchFinder::MatchResult &Match, const bool IsResultTypeTrivial = ResultType.isTrivialType(*Match.Context); if ((!IsResultTypeTrivial && IgnoreNonTrivialTypes)) - return FixItHints; + return {false, FixItHints}; if (IsResultTypeTrivial && static_cast( Match.Context->getTypeSizeInChars(ResultType).getQuantity()) > IgnoreTrivialTypesOfSizeAbove) - return FixItHints; + return {false, FixItHints}; + + bool FoundNestedCall = false; for (const Expr *Arg : Result.Args) { const auto *InnerCall = dyn_cast(Arg->IgnoreParenImpCasts()); @@ -146,6 +152,9 @@ generateReplacements(const MatchFinder::MatchResult &Match, *Match.Context)) continue; + // We have found a nested call + FoundNestedCall = true; + // remove the function call FixItHints.push_back( FixItHint::CreateRemoval(InnerCall->getCallee()->getSourceRange())); @@ -168,7 +177,7 @@ generateReplacements(const MatchFinder::MatchResult &Match, CharSourceRange::getTokenRange(InnerResult.First->getEndLoc()))); } - const SmallVector InnerReplacements = generateReplacements( + const auto [_, InnerReplacements] = generateReplacements( Match, InnerCall, InnerResult, IgnoreNonTrivialTypes, IgnoreTrivialTypesOfSizeAbove); @@ -189,7 +198,7 @@ generateReplacements(const MatchFinder::MatchResult &Match, } } - return FixItHints; + return {FoundNestedCall, FixItHints}; } MinMaxUseInitializerListCheck::MinMaxUseInitializerListCheck( @@ -238,11 +247,11 @@ void MinMaxUseInitializerListCheck::check( const auto *TopCall = Match.Nodes.getNodeAs("topCall"); const FindArgsResult Result = findArgs(TopCall); - const SmallVector Replacements = + const auto [FoundNestedCall, Replacements] = generateReplacements(Match, TopCall, Result, IgnoreNonTrivialTypes, IgnoreTrivialTypesOfSizeAbove); - if (Replacements.empty()) + if (!FoundNestedCall) return; const DiagnosticBuilder Diagnostic = diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index c297ed88ece0a2..a72c22187d5b4c 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -136,6 +136,11 @@ Changes in existing checks ` check to avoid false positive for C++23 deducing this. +- Improved :doc:`modernize-min-max-use-initializer-list + ` check by fixing + a false positive when only an implicit conversion happened inside an + initializer list. + - Improved :doc:`modernize-use-std-print ` check to support replacing member function calls too. diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/min-max-use-initializer-list.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/min-max-use-initializer-list.cpp index c7632fe007a4f4..f4e21316718045 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/min-max-use-initializer-list.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/min-max-use-initializer-list.cpp @@ -323,5 +323,11 @@ struct GH91982 { } }; +struct GH107594 { + int foo(int a, int b, char c) { + return std::max({a, b, c}); + } +}; + } // namespace