Skip to content

Commit d3f92e3

Browse files
sopybsopyb
authored
[clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (#85572)
Identifies cases where `std::min` or `std::max` is used to find the minimum or maximum value among more than two items through repeated calls. The check replaces these calls with a single call to `std::min` or `std::max` that uses an initializer list. This makes the code slightly more efficient. Closes #25340
1 parent 0f329e0 commit d3f92e3

File tree

8 files changed

+693
-0
lines changed

8 files changed

+693
-0
lines changed

‎clang-tools-extra/clang-tidy/modernize/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ add_clang_library(clangTidyModernizeModule
1616
MakeSharedCheck.cpp
1717
MakeSmartPtrCheck.cpp
1818
MakeUniqueCheck.cpp
19+
MinMaxUseInitializerListCheck.cpp
1920
ModernizeTidyModule.cpp
2021
PassByValueCheck.cpp
2122
RawStringLiteralCheck.cpp

‎clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp

Lines changed: 271 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,271 @@
1+
//===--- MinMaxUseInitializerListCheck.cpp - clang-tidy -------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include "MinMaxUseInitializerListCheck.h"
10+
#include "../utils/ASTUtils.h"
11+
#include "../utils/LexerUtils.h"
12+
#include "clang/ASTMatchers/ASTMatchFinder.h"
13+
#include "clang/Frontend/CompilerInstance.h"
14+
#include "clang/Lex/Lexer.h"
15+
16+
using namespace clang;
17+
18+
namespace {
19+
20+
struct FindArgsResult {
21+
const Expr *First;
22+
const Expr *Last;
23+
const Expr *Compare;
24+
SmallVector<const clang::Expr *, 2> Args;
25+
};
26+
27+
} // anonymous namespace
28+
29+
using namespace clang::ast_matchers;
30+
31+
namespace clang::tidy::modernize {
32+
33+
static FindArgsResult findArgs(const CallExpr *Call) {
34+
FindArgsResult Result;
35+
Result.First = nullptr;
36+
Result.Last = nullptr;
37+
Result.Compare = nullptr;
38+
39+
// check if the function has initializer list argument
40+
if (Call->getNumArgs() < 3) {
41+
auto ArgIterator = Call->arguments().begin();
42+
43+
const auto *InitListExpr =
44+
dyn_cast<CXXStdInitializerListExpr>(*ArgIterator);
45+
const auto *InitList =
46+
InitListExpr != nullptr
47+
? dyn_cast<clang::InitListExpr>(
48+
InitListExpr->getSubExpr()->IgnoreImplicit())
49+
: nullptr;
50+
51+
if (InitList) {
52+
Result.Args.append(InitList->inits().begin(), InitList->inits().end());
53+
Result.First = *ArgIterator;
54+
Result.Last = *ArgIterator;
55+
56+
// check if there is a comparison argument
57+
std::advance(ArgIterator, 1);
58+
if (ArgIterator != Call->arguments().end())
59+
Result.Compare = *ArgIterator;
60+
61+
return Result;
62+
}
63+
Result.Args = SmallVector<const Expr *>(Call->arguments());
64+
} else {
65+
// if it has 3 arguments then the last will be the comparison
66+
Result.Compare = *(std::next(Call->arguments().begin(), 2));
67+
Result.Args = SmallVector<const Expr *>(llvm::drop_end(Call->arguments()));
68+
}
69+
Result.First = Result.Args.front();
70+
Result.Last = Result.Args.back();
71+
72+
return Result;
73+
}
74+
75+
static SmallVector<FixItHint>
76+
generateReplacements(const MatchFinder::MatchResult &Match,
77+
const CallExpr *TopCall, const FindArgsResult &Result,
78+
const bool IgnoreNonTrivialTypes,
79+
const std::uint64_t IgnoreTrivialTypesOfSizeAbove) {
80+
SmallVector<FixItHint> FixItHints;
81+
const SourceManager &SourceMngr = *Match.SourceManager;
82+
const LangOptions &LanguageOpts = Match.Context->getLangOpts();
83+
84+
const QualType ResultType = TopCall->getDirectCallee()
85+
->getReturnType()
86+
.getCanonicalType()
87+
.getNonReferenceType()
88+
.getUnqualifiedType();
89+
90+
// check if the type is trivial
91+
const bool IsResultTypeTrivial = ResultType.isTrivialType(*Match.Context);
92+
93+
if ((!IsResultTypeTrivial && IgnoreNonTrivialTypes))
94+
return FixItHints;
95+
96+
if (IsResultTypeTrivial &&
97+
static_cast<std::uint64_t>(
98+
Match.Context->getTypeSizeInChars(ResultType).getQuantity()) >
99+
IgnoreTrivialTypesOfSizeAbove)
100+
return FixItHints;
101+
102+
for (const Expr *Arg : Result.Args) {
103+
const auto *InnerCall = dyn_cast<CallExpr>(Arg->IgnoreParenImpCasts());
104+
105+
// If the argument is not a nested call
106+
if (!InnerCall) {
107+
// check if typecast is required
108+
const QualType ArgType = Arg->IgnoreParenImpCasts()
109+
->getType()
110+
.getCanonicalType()
111+
.getUnqualifiedType();
112+
113+
if (ArgType == ResultType)
114+
continue;
115+
116+
const StringRef ArgText = Lexer::getSourceText(
117+
CharSourceRange::getTokenRange(Arg->getSourceRange()), SourceMngr,
118+
LanguageOpts);
119+
120+
const auto Replacement = Twine("static_cast<")
121+
.concat(ResultType.getAsString(LanguageOpts))
122+
.concat(">(")
123+
.concat(ArgText)
124+
.concat(")")
125+
.str();
126+
127+
FixItHints.push_back(
128+
FixItHint::CreateReplacement(Arg->getSourceRange(), Replacement));
129+
continue;
130+
}
131+
132+
const FindArgsResult InnerResult = findArgs(InnerCall);
133+
134+
// if the nested call doesn't have arguments skip it
135+
if (!InnerResult.First || !InnerResult.Last)
136+
continue;
137+
138+
// if the nested call is not the same as the top call
139+
if (InnerCall->getDirectCallee()->getQualifiedNameAsString() !=
140+
TopCall->getDirectCallee()->getQualifiedNameAsString())
141+
continue;
142+
143+
// if the nested call doesn't have the same compare function
144+
if ((Result.Compare || InnerResult.Compare) &&
145+
!utils::areStatementsIdentical(Result.Compare, InnerResult.Compare,
146+
*Match.Context))
147+
continue;
148+
149+
// remove the function call
150+
FixItHints.push_back(
151+
FixItHint::CreateRemoval(InnerCall->getCallee()->getSourceRange()));
152+
153+
// remove the parentheses
154+
const auto LParen = utils::lexer::findNextTokenSkippingComments(
155+
InnerCall->getCallee()->getEndLoc(), SourceMngr, LanguageOpts);
156+
if (LParen.has_value() && LParen->is(tok::l_paren))
157+
FixItHints.push_back(
158+
FixItHint::CreateRemoval(SourceRange(LParen->getLocation())));
159+
FixItHints.push_back(
160+
FixItHint::CreateRemoval(SourceRange(InnerCall->getRParenLoc())));
161+
162+
// if the inner call has an initializer list arg
163+
if (InnerResult.First == InnerResult.Last) {
164+
// remove the initializer list braces
165+
FixItHints.push_back(FixItHint::CreateRemoval(
166+
CharSourceRange::getTokenRange(InnerResult.First->getBeginLoc())));
167+
FixItHints.push_back(FixItHint::CreateRemoval(
168+
CharSourceRange::getTokenRange(InnerResult.First->getEndLoc())));
169+
}
170+
171+
const SmallVector<FixItHint> InnerReplacements = generateReplacements(
172+
Match, InnerCall, InnerResult, IgnoreNonTrivialTypes,
173+
IgnoreTrivialTypesOfSizeAbove);
174+
175+
FixItHints.append(InnerReplacements);
176+
177+
if (InnerResult.Compare) {
178+
// find the comma after the value arguments
179+
const auto Comma = utils::lexer::findNextTokenSkippingComments(
180+
InnerResult.Last->getEndLoc(), SourceMngr, LanguageOpts);
181+
182+
// remove the comma and the comparison
183+
if (Comma.has_value() && Comma->is(tok::comma))
184+
FixItHints.push_back(
185+
FixItHint::CreateRemoval(SourceRange(Comma->getLocation())));
186+
187+
FixItHints.push_back(
188+
FixItHint::CreateRemoval(InnerResult.Compare->getSourceRange()));
189+
}
190+
}
191+
192+
return FixItHints;
193+
}
194+
195+
MinMaxUseInitializerListCheck::MinMaxUseInitializerListCheck(
196+
StringRef Name, ClangTidyContext *Context)
197+
: ClangTidyCheck(Name, Context),
198+
IgnoreNonTrivialTypes(Options.get("IgnoreNonTrivialTypes", true)),
199+
IgnoreTrivialTypesOfSizeAbove(
200+
Options.get("IgnoreTrivialTypesOfSizeAbove", 32L)),
201+
Inserter(Options.getLocalOrGlobal("IncludeStyle",
202+
utils::IncludeSorter::IS_LLVM),
203+
areDiagsSelfContained()) {}
204+
205+
void MinMaxUseInitializerListCheck::storeOptions(
206+
ClangTidyOptions::OptionMap &Opts) {
207+
Options.store(Opts, "IgnoreNonTrivialTypes", IgnoreNonTrivialTypes);
208+
Options.store(Opts, "IgnoreTrivialTypesOfSizeAbove",
209+
IgnoreTrivialTypesOfSizeAbove);
210+
Options.store(Opts, "IncludeStyle", Inserter.getStyle());
211+
}
212+
213+
void MinMaxUseInitializerListCheck::registerMatchers(MatchFinder *Finder) {
214+
auto CreateMatcher = [](const StringRef FunctionName) {
215+
auto FuncDecl = functionDecl(hasName(FunctionName));
216+
auto Expression = callExpr(callee(FuncDecl));
217+
218+
return callExpr(callee(FuncDecl),
219+
anyOf(hasArgument(0, Expression),
220+
hasArgument(1, Expression),
221+
hasArgument(0, cxxStdInitializerListExpr())),
222+
unless(hasParent(Expression)))
223+
.bind("topCall");
224+
};
225+
226+
Finder->addMatcher(CreateMatcher("::std::max"), this);
227+
Finder->addMatcher(CreateMatcher("::std::min"), this);
228+
}
229+
230+
void MinMaxUseInitializerListCheck::registerPPCallbacks(
231+
const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
232+
Inserter.registerPreprocessor(PP);
233+
}
234+
235+
void MinMaxUseInitializerListCheck::check(
236+
const MatchFinder::MatchResult &Match) {
237+
238+
const auto *TopCall = Match.Nodes.getNodeAs<CallExpr>("topCall");
239+
240+
const FindArgsResult Result = findArgs(TopCall);
241+
const SmallVector<FixItHint> Replacements =
242+
generateReplacements(Match, TopCall, Result, IgnoreNonTrivialTypes,
243+
IgnoreTrivialTypesOfSizeAbove);
244+
245+
if (Replacements.empty())
246+
return;
247+
248+
const DiagnosticBuilder Diagnostic =
249+
diag(TopCall->getBeginLoc(),
250+
"do not use nested 'std::%0' calls, use an initializer list instead")
251+
<< TopCall->getDirectCallee()->getName()
252+
<< Inserter.createIncludeInsertion(
253+
Match.SourceManager->getFileID(TopCall->getBeginLoc()),
254+
"<algorithm>");
255+
256+
// if the top call doesn't have an initializer list argument
257+
if (Result.First != Result.Last) {
258+
// add { and } insertions
259+
Diagnostic << FixItHint::CreateInsertion(Result.First->getBeginLoc(), "{");
260+
261+
Diagnostic << FixItHint::CreateInsertion(
262+
Lexer::getLocForEndOfToken(Result.Last->getEndLoc(), 0,
263+
*Match.SourceManager,
264+
Match.Context->getLangOpts()),
265+
"}");
266+
}
267+
268+
Diagnostic << Replacements;
269+
}
270+
271+
} // namespace clang::tidy::modernize

‎clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.h

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
//===--- MinMaxUseInitializerListCheck.h - clang-tidy -----------*- C++ -*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_MINMAXUSEINITIALIZERLISTCHECK_H
10+
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_MINMAXUSEINITIALIZERLISTCHECK_H
11+
12+
#include "../ClangTidyCheck.h"
13+
#include "../utils/IncludeInserter.h"
14+
15+
namespace clang::tidy::modernize {
16+
17+
/// Replaces nested ``std::min`` and ``std::max`` calls with an initializer list
18+
/// where applicable.
19+
///
20+
/// For example:
21+
///
22+
/// \code
23+
/// int a = std::max(std::max(i, j), k);
24+
/// \endcode
25+
///
26+
/// This code is transformed to:
27+
///
28+
/// \code
29+
/// int a = std::max({i, j, k});
30+
/// \endcode
31+
class MinMaxUseInitializerListCheck : public ClangTidyCheck {
32+
public:
33+
MinMaxUseInitializerListCheck(StringRef Name, ClangTidyContext *Context);
34+
35+
void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
36+
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
37+
void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
38+
Preprocessor *ModuleExpanderPP) override;
39+
void check(const ast_matchers::MatchFinder::MatchResult &Match) override;
40+
41+
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
42+
return LangOpts.CPlusPlus11;
43+
}
44+
std::optional<TraversalKind> getCheckTraversalKind() const override {
45+
return TK_IgnoreUnlessSpelledInSource;
46+
}
47+
48+
private:
49+
bool IgnoreNonTrivialTypes;
50+
std::uint64_t IgnoreTrivialTypesOfSizeAbove;
51+
utils::IncludeInserter Inserter;
52+
};
53+
54+
} // namespace clang::tidy::modernize
55+
56+
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_MINMAXUSEINITIALIZERLISTCHECK_H

‎clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "MacroToEnumCheck.h"
1919
#include "MakeSharedCheck.h"
2020
#include "MakeUniqueCheck.h"
21+
#include "MinMaxUseInitializerListCheck.h"
2122
#include "PassByValueCheck.h"
2223
#include "RawStringLiteralCheck.h"
2324
#include "RedundantVoidArgCheck.h"
@@ -68,6 +69,8 @@ class ModernizeModule : public ClangTidyModule {
6869
CheckFactories.registerCheck<MacroToEnumCheck>("modernize-macro-to-enum");
6970
CheckFactories.registerCheck<MakeSharedCheck>("modernize-make-shared");
7071
CheckFactories.registerCheck<MakeUniqueCheck>("modernize-make-unique");
72+
CheckFactories.registerCheck<MinMaxUseInitializerListCheck>(
73+
"modernize-min-max-use-initializer-list");
7174
CheckFactories.registerCheck<PassByValueCheck>("modernize-pass-by-value");
7275
CheckFactories.registerCheck<UseDesignatedInitializersCheck>(
7376
"modernize-use-designated-initializers");

‎clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,12 @@ New checks
131131
to reading out-of-bounds data due to inadequate or incorrect string null
132132
termination.
133133

134+
- New :doc:`modernize-min-max-use-initializer-list
135+
<clang-tidy/checks/modernize/min-max-use-initializer-list>` check.
136+
137+
Replaces nested ``std::min`` and ``std::max`` calls with an initializer list
138+
where applicable.
139+
134140
- New :doc:`modernize-use-designated-initializers
135141
<clang-tidy/checks/modernize/use-designated-initializers>` check.
136142

‎clang-tools-extra/docs/clang-tidy/checks/list.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,7 @@ Clang-Tidy Checks
276276
:doc:`modernize-macro-to-enum <modernize/macro-to-enum>`, "Yes"
277277
:doc:`modernize-make-shared <modernize/make-shared>`, "Yes"
278278
:doc:`modernize-make-unique <modernize/make-unique>`, "Yes"
279+
:doc:`modernize-min-max-use-initializer-list <modernize/min-max-use-initializer-list>`, "Yes"
279280
:doc:`modernize-pass-by-value <modernize/pass-by-value>`, "Yes"
280281
:doc:`modernize-raw-string-literal <modernize/raw-string-literal>`, "Yes"
281282
:doc:`modernize-redundant-void-arg <modernize/redundant-void-arg>`, "Yes"

0 commit comments

Comments
 (0)