https://github.com/5chmidti created
https://github.com/llvm/llvm-project/pull/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 was created that resulted in an
issued diagnostic.
Instead, explicitly track if a nested call was detected and only emit a
diagnostic if this is the case.
Fixes #107594
>From 097a679f33bdded29fa67833b7fcd4707057cbf3 Mon Sep 17 00:00:00 2001
From: Julian Schmidt <git.julian.schm...@gmail.com>
Date: Sat, 7 Sep 2024 00:10:08 +0200
Subject: [PATCH] [clang-tidy] fix false positive in
modernize-min-max-use-initializer-list
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 was created that resulted in an
issued diagnostic.
Instead, explicitly track if a nested call was detected and only emit a
diagnostic if this is the case.
Fixes #107594
---
.../MinMaxUseInitializerListCheck.cpp | 30 +++++++++++--------
clang-tools-extra/docs/ReleaseNotes.rst | 5 ++++
.../min-max-use-initializer-list.cpp | 6 ++++
3 files changed, 28 insertions(+), 13 deletions(-)
diff --git
a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
index 418699ffbc4d1a..f4afae2fe6e79e 100644
--- a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
@@ -72,11 +72,10 @@ static FindArgsResult findArgs(const CallExpr *Call) {
return Result;
}
-static SmallVector<FixItHint>
-generateReplacements(const MatchFinder::MatchResult &Match,
- const CallExpr *TopCall, const FindArgsResult &Result,
- const bool IgnoreNonTrivialTypes,
- const std::uint64_t IgnoreTrivialTypesOfSizeAbove) {
+static std::pair<bool, SmallVector<FixItHint>> generateReplacements(
+ const MatchFinder::MatchResult &Match, const CallExpr *TopCall,
+ const FindArgsResult &Result, const bool IgnoreNonTrivialTypes,
+ const std::uint64_t IgnoreTrivialTypesOfSizeAbove, bool FoundNestedCall) {
SmallVector<FixItHint> FixItHints;
const SourceManager &SourceMngr = *Match.SourceManager;
const LangOptions &LanguageOpts = Match.Context->getLangOpts();
@@ -91,13 +90,13 @@ generateReplacements(const MatchFinder::MatchResult &Match,
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};
for (const Expr *Arg : Result.Args) {
const auto *InnerCall = dyn_cast<CallExpr>(Arg->IgnoreParenImpCasts());
@@ -146,6 +145,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,9 +170,11 @@ generateReplacements(const MatchFinder::MatchResult &Match,
CharSourceRange::getTokenRange(InnerResult.First->getEndLoc())));
}
- const SmallVector<FixItHint> InnerReplacements = generateReplacements(
+ const auto [FoundNestedCallInner, InnerReplacements] =
generateReplacements(
Match, InnerCall, InnerResult, IgnoreNonTrivialTypes,
- IgnoreTrivialTypesOfSizeAbove);
+ IgnoreTrivialTypesOfSizeAbove, false);
+
+ FoundNestedCall |= FoundNestedCallInner;
FixItHints.append(InnerReplacements);
@@ -189,7 +193,7 @@ generateReplacements(const MatchFinder::MatchResult &Match,
}
}
- return FixItHints;
+ return {FoundNestedCall, FixItHints};
}
MinMaxUseInitializerListCheck::MinMaxUseInitializerListCheck(
@@ -238,11 +242,11 @@ void MinMaxUseInitializerListCheck::check(
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);
+ IgnoreTrivialTypesOfSizeAbove, false);
- 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 8d028f8863cb7a..dcfe11e58f4710 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -116,6 +116,11 @@ Changes in existing checks
<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 removing
+ 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 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<int>({a, b, c});
+ }
+};
+
} // namespace
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
