Skip to content

Commit 14fa455

Browse files
committed
[clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists
1 parent d39ac3a commit 14fa455

File tree

6 files changed

+208
-0
lines changed

6 files changed

+208
-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
Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
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 "clang/ASTMatchers/ASTMatchFinder.h"
11+
#include "clang/Frontend/CompilerInstance.h"
12+
#include "clang/Lex/Lexer.h"
13+
14+
using namespace clang::ast_matchers;
15+
16+
namespace clang::tidy::modernize {
17+
18+
MinMaxUseInitializerListCheck::MinMaxUseInitializerListCheck(
19+
StringRef Name, ClangTidyContext *Context)
20+
: ClangTidyCheck(Name, Context),
21+
Inserter(Options.getLocalOrGlobal("IncludeStyle",
22+
utils::IncludeSorter::IS_LLVM),
23+
areDiagsSelfContained()) {}
24+
25+
void MinMaxUseInitializerListCheck::storeOptions(
26+
ClangTidyOptions::OptionMap &Opts) {
27+
Options.store(Opts, "IncludeStyle", Inserter.getStyle());
28+
}
29+
30+
void MinMaxUseInitializerListCheck::registerMatchers(MatchFinder *Finder) {
31+
Finder->addMatcher(
32+
callExpr(
33+
callee(functionDecl(hasName("::std::max"))),
34+
hasAnyArgument(callExpr(callee(functionDecl(hasName("::std::max"))))),
35+
unless(
36+
hasParent(callExpr(callee(functionDecl(hasName("::std::max")))))))
37+
.bind("maxCall"),
38+
this);
39+
40+
Finder->addMatcher(
41+
callExpr(
42+
callee(functionDecl(hasName("::std::min"))),
43+
hasAnyArgument(callExpr(callee(functionDecl(hasName("::std::min"))))),
44+
unless(
45+
hasParent(callExpr(callee(functionDecl(hasName("::std::min")))))))
46+
.bind("minCall"),
47+
this);
48+
}
49+
50+
void MinMaxUseInitializerListCheck::registerPPCallbacks(
51+
const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
52+
Inserter.registerPreprocessor(PP);
53+
}
54+
55+
void MinMaxUseInitializerListCheck::check(
56+
const MatchFinder::MatchResult &Result) {
57+
const auto *MaxCall = Result.Nodes.getNodeAs<CallExpr>("maxCall");
58+
const auto *MinCall = Result.Nodes.getNodeAs<CallExpr>("minCall");
59+
60+
const CallExpr *TopCall = MaxCall ? MaxCall : MinCall;
61+
if (!TopCall) {
62+
return;
63+
}
64+
const QualType ResultType =
65+
TopCall->getDirectCallee()->getReturnType().getNonReferenceType();
66+
67+
const Expr *FirstArg = nullptr;
68+
const Expr *LastArg = nullptr;
69+
std::vector<const Expr *> Args;
70+
findArgs(TopCall, &FirstArg, &LastArg, Args);
71+
72+
if (!FirstArg || !LastArg || Args.size() <= 2) {
73+
return;
74+
}
75+
76+
std::string ReplacementText = "{";
77+
for (const Expr *Arg : Args) {
78+
QualType ArgType = Arg->getType();
79+
bool CastNeeded =
80+
ArgType.getCanonicalType() != ResultType.getCanonicalType();
81+
82+
if (CastNeeded)
83+
ReplacementText += "static_cast<" + ResultType.getAsString() + ">(";
84+
85+
ReplacementText += Lexer::getSourceText(
86+
CharSourceRange::getTokenRange(Arg->getSourceRange()),
87+
*Result.SourceManager, Result.Context->getLangOpts());
88+
89+
if (CastNeeded)
90+
ReplacementText += ")";
91+
ReplacementText += ", ";
92+
}
93+
ReplacementText = ReplacementText.substr(0, ReplacementText.size() - 2) + "}";
94+
95+
diag(TopCall->getBeginLoc(),
96+
"do not use nested std::%0 calls, use %1 instead")
97+
<< TopCall->getDirectCallee()->getName() << ReplacementText
98+
<< FixItHint::CreateReplacement(
99+
CharSourceRange::getTokenRange(
100+
FirstArg->getBeginLoc(),
101+
Lexer::getLocForEndOfToken(TopCall->getEndLoc(), 0,
102+
Result.Context->getSourceManager(),
103+
Result.Context->getLangOpts())
104+
.getLocWithOffset(-2)),
105+
ReplacementText)
106+
<< Inserter.createMainFileIncludeInsertion("<algorithm>");
107+
}
108+
109+
void MinMaxUseInitializerListCheck::findArgs(const CallExpr *Call,
110+
const Expr **First,
111+
const Expr **Last,
112+
std::vector<const Expr *> &Args) {
113+
if (!Call) {
114+
return;
115+
}
116+
117+
const FunctionDecl *Callee = Call->getDirectCallee();
118+
if (!Callee) {
119+
return;
120+
}
121+
122+
for (const Expr *Arg : Call->arguments()) {
123+
if (!*First)
124+
*First = Arg;
125+
126+
const CallExpr *InnerCall = dyn_cast<CallExpr>(Arg);
127+
if (InnerCall && InnerCall->getDirectCallee() &&
128+
InnerCall->getDirectCallee()->getNameAsString() ==
129+
Call->getDirectCallee()->getNameAsString()) {
130+
findArgs(InnerCall, First, Last, Args);
131+
} else
132+
Args.push_back(Arg);
133+
134+
*Last = Arg;
135+
}
136+
}
137+
138+
} // namespace clang::tidy::modernize
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
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+
/// Transforms the repeated calls to `std::min` and `std::max` into a single
18+
/// call using initializer lists.
19+
///
20+
/// It identifies cases where `std::min` or `std::max` is used to find the
21+
/// minimum or maximum value among more than two items through repeated calls.
22+
/// The check replaces these calls with a single call to `std::min` or
23+
/// `std::max` that uses an initializer list. This makes the code slightly more
24+
/// efficient.
25+
/// \n\n
26+
/// For example:
27+
///
28+
/// \code
29+
/// int a = std::max(std::max(i, j), k);
30+
/// \endcode
31+
///
32+
/// This code is transformed to:
33+
///
34+
/// \code
35+
/// int a = std::max({i, j, k});
36+
/// \endcode
37+
class MinMaxUseInitializerListCheck : public ClangTidyCheck {
38+
public:
39+
MinMaxUseInitializerListCheck(StringRef Name, ClangTidyContext *Context);
40+
41+
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
42+
return LangOpts.CPlusPlus11;
43+
}
44+
void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
45+
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
46+
void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
47+
Preprocessor *ModuleExpanderPP) override;
48+
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
49+
50+
private:
51+
utils::IncludeInserter Inserter;
52+
void findArgs(const CallExpr *call, const Expr **first, const Expr **last,
53+
std::vector<const Expr *> &args);
54+
};
55+
56+
} // namespace clang::tidy::modernize
57+
58+
#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
@@ -110,6 +110,12 @@ New checks
110110
Detects error-prone Curiously Recurring Template Pattern usage, when the CRTP
111111
can be constructed outside itself and the derived class.
112112

113+
- New :doc:`modernize-min-max-use-initializer-list
114+
<clang-tidy/checks/modernize/min-max-use-initializer-list>` check.
115+
116+
Replaces chained ``std::min`` and ``std::max`` calls that can be written as
117+
initializer lists.
118+
113119
- New :doc:`modernize-use-designated-initializers
114120
<clang-tidy/checks/modernize/use-designated-initializers>` check.
115121

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,7 @@ Clang-Tidy Checks
274274
:doc:`modernize-macro-to-enum <modernize/macro-to-enum>`, "Yes"
275275
:doc:`modernize-make-shared <modernize/make-shared>`, "Yes"
276276
:doc:`modernize-make-unique <modernize/make-unique>`, "Yes"
277+
:doc:`modernize-min-max-use-initializer-list <modernize/min-max-use-initializer-list>`, "Yes"
277278
:doc:`modernize-pass-by-value <modernize/pass-by-value>`, "Yes"
278279
:doc:`modernize-raw-string-literal <modernize/raw-string-literal>`, "Yes"
279280
:doc:`modernize-redundant-void-arg <modernize/redundant-void-arg>`, "Yes"
@@ -324,6 +325,7 @@ Clang-Tidy Checks
324325
:doc:`performance-inefficient-algorithm <performance/inefficient-algorithm>`, "Yes"
325326
:doc:`performance-inefficient-string-concatenation <performance/inefficient-string-concatenation>`,
326327
:doc:`performance-inefficient-vector-operation <performance/inefficient-vector-operation>`, "Yes"
328+
:doc:`performance-modernize-min-max-use-initializer-list <performance/modernize-min-max-use-initializer-list>`,
327329
:doc:`performance-move-const-arg <performance/move-const-arg>`, "Yes"
328330
:doc:`performance-move-constructor-init <performance/move-constructor-init>`,
329331
:doc:`performance-no-automatic-move <performance/no-automatic-move>`,

0 commit comments

Comments
 (0)