[clang-tidy] fix false positive in modernize-min-max-use-initializer-list (#107649)
Previously, whenever a replacement was generated by the analysis, a
diagnostic was generated. This became an issue when a call to
`std::min` or `std::max` consisted only of an initializer list with at
least one argument to the list requiring a type cast.
In this case, a single replacement that added a `static_cast` was
created,
that resulted in a diagnostic being issued but with no nested call
to `std::min` or `std::max`.
Instead, explicitly track if a nested call was detected and only emit a
diagnostic if this is the case.
Fixes #107594
diff --git a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
index 418699f..9861f46 100644
--- a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
@@ -72,7 +72,11 @@
return Result;
}
-static SmallVector<FixItHint>
+// 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<bool, SmallVector<FixItHint>>
generateReplacements(const MatchFinder::MatchResult &Match,
const CallExpr *TopCall, const FindArgsResult &Result,
const bool IgnoreNonTrivialTypes,
@@ -91,13 +95,15 @@
const bool IsResultTypeTrivial = ResultType.isTrivialType(*Match.Context);
if ((!IsResultTypeTrivial && IgnoreNonTrivialTypes))
- return FixItHints;
+ return {false, FixItHints};
if (IsResultTypeTrivial &&
static_cast<std::uint64_t>(
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<CallExpr>(Arg->IgnoreParenImpCasts());
@@ -146,6 +152,9 @@
*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 @@
CharSourceRange::getTokenRange(InnerResult.First->getEndLoc())));
}
- const SmallVector<FixItHint> InnerReplacements = generateReplacements(
+ const auto [_, InnerReplacements] = generateReplacements(
Match, InnerCall, InnerResult, IgnoreNonTrivialTypes,
IgnoreTrivialTypesOfSizeAbove);
@@ -189,7 +198,7 @@
}
}
- return FixItHints;
+ return {FoundNestedCall, FixItHints};
}
MinMaxUseInitializerListCheck::MinMaxUseInitializerListCheck(
@@ -238,11 +247,11 @@
const auto *TopCall = Match.Nodes.getNodeAs<CallExpr>("topCall");
const FindArgsResult Result = findArgs(TopCall);
- const SmallVector<FixItHint> 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 c297ed8..a72c2218 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -136,6 +136,11 @@
<clang-tidy/checks/misc/unconventional-assign-operator>` check to avoid
false positive for C++23 deducing this.
+- Improved :doc:`modernize-min-max-use-initializer-list
+ <clang-tidy/checks/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
<clang-tidy/checks/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 c7632fe..f4e2131 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 GH107594 {
+ int foo(int a, int b, char c) {
+ return std::max<int>({a, b, c});
+ }
+};
+
} // namespace
