Skip to content

Commit 6abf8a2

Browse files
sopybsopyb
committed
Adding comments and code cleanup
1 parent d506f63 commit 6abf8a2

File tree

2 files changed

+131
-106
lines changed

2 files changed

+131
-106
lines changed

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

Lines changed: 130 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ struct FindArgsResult {
2121
const Expr *First;
2222
const Expr *Last;
2323
const Expr *Compare;
24-
std::vector<const clang::Expr *> Args;
24+
SmallVector<const clang::Expr *, 2> Args;
2525
};
2626

2727
} // anonymous namespace
@@ -36,30 +36,35 @@ static FindArgsResult findArgs(const CallExpr *Call) {
3636
Result.Last = nullptr;
3737
Result.Compare = nullptr;
3838

39-
if (Call->getNumArgs() == 3) {
40-
auto ArgIterator = Call->arguments().begin();
41-
std::advance(ArgIterator, 2);
42-
Result.Compare = *ArgIterator;
43-
} else {
39+
// check if the function has initializer list argument
40+
if (Call->getNumArgs() < 3) {
4441
auto ArgIterator = Call->arguments().begin();
4542

46-
if (const auto *InitListExpr =
47-
dyn_cast<CXXStdInitializerListExpr>(*ArgIterator)) {
48-
if (const auto *InitList = dyn_cast<clang::InitListExpr>(
49-
InitListExpr->getSubExpr()->IgnoreImplicit())) {
50-
Result.Args.insert(Result.Args.begin(), InitList->inits().begin(),
51-
InitList->inits().end());
52-
53-
Result.First = *ArgIterator;
54-
Result.Last = *ArgIterator;
55-
56-
std::advance(ArgIterator, 1);
57-
if (ArgIterator != Call->arguments().end()) {
58-
Result.Compare = *ArgIterator;
59-
}
60-
return Result;
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 (InitListExpr && InitList) {
52+
Result.Args.insert(Result.Args.begin(), InitList->inits().begin(),
53+
InitList->inits().end());
54+
Result.First = *ArgIterator;
55+
Result.Last = *ArgIterator;
56+
57+
// check if there is a comparison argument
58+
std::advance(ArgIterator, 1);
59+
if (ArgIterator != Call->arguments().end()) {
60+
Result.Compare = *ArgIterator;
6161
}
62+
63+
return Result;
6264
}
65+
// if it has 3 arguments then the last will be the comparison
66+
} else {
67+
Result.Compare = *(std::next(Call->arguments().begin(), 2));
6368
}
6469

6570
for (const Expr *Arg : Call->arguments()) {
@@ -76,114 +81,133 @@ static FindArgsResult findArgs(const CallExpr *Call) {
7681
return Result;
7782
}
7883

79-
static std::vector<FixItHint>
84+
static SmallVector<FixItHint>
8085
generateReplacement(const MatchFinder::MatchResult &Match,
8186
const CallExpr *TopCall, const FindArgsResult &Result) {
82-
std::vector<FixItHint> FixItHints;
87+
SmallVector<FixItHint> FixItHints;
8388

8489
const QualType ResultType = TopCall->getDirectCallee()
8590
->getReturnType()
8691
.getNonReferenceType()
8792
.getUnqualifiedType()
8893
.getCanonicalType();
94+
const auto &SourceMngr = *Match.SourceManager;
95+
const auto LanguageOpts = Match.Context->getLangOpts();
8996
const bool IsInitializerList = Result.First == Result.Last;
9097

91-
if (!IsInitializerList)
98+
// add { and } if the top call doesn't have an initializer list arg
99+
if (!IsInitializerList) {
92100
FixItHints.push_back(
93101
FixItHint::CreateInsertion(Result.First->getBeginLoc(), "{"));
94102

103+
if (Result.Compare)
104+
FixItHints.push_back(FixItHint::CreateInsertion(
105+
Lexer::getLocForEndOfToken(Result.Last->getEndLoc(), 0, SourceMngr,
106+
LanguageOpts),
107+
"}"));
108+
else
109+
FixItHints.push_back(
110+
FixItHint::CreateInsertion(TopCall->getEndLoc(), "}"));
111+
}
112+
95113
for (const Expr *Arg : Result.Args) {
96-
if (const auto *InnerCall =
97-
dyn_cast<CallExpr>(Arg->IgnoreParenImpCasts())) {
98-
const FindArgsResult InnerResult = findArgs(InnerCall);
99-
const std::vector<FixItHint> InnerReplacements =
100-
generateReplacement(Match, InnerCall, InnerResult);
101-
if (InnerCall->getDirectCallee()->getQualifiedNameAsString() ==
102-
TopCall->getDirectCallee()->getQualifiedNameAsString() &&
103-
((!Result.Compare && !InnerResult.Compare) ||
104-
utils::areStatementsIdentical(Result.Compare, InnerResult.Compare,
105-
*Match.Context))) {
114+
const auto *InnerCall = dyn_cast<CallExpr>(Arg->IgnoreParenImpCasts());
106115

107-
FixItHints.push_back(
108-
FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
109-
InnerCall->getCallee()->getSourceRange())));
116+
// If the argument is not a nested call
117+
if (!InnerCall) {
118+
// check if typecast is required
119+
const QualType ArgType = Arg->IgnoreParenImpCasts()
120+
->getType()
121+
.getUnqualifiedType()
122+
.getCanonicalType();
110123

111-
const auto LParen = utils::lexer::findNextTokenSkippingComments(
112-
InnerCall->getCallee()->getEndLoc(), *Match.SourceManager,
113-
Match.Context->getLangOpts());
114-
if (LParen && LParen->getKind() == tok::l_paren)
115-
FixItHints.push_back(
116-
FixItHint::CreateRemoval(SourceRange(LParen->getLocation())));
124+
if (ArgType == ResultType)
125+
continue;
126+
127+
const std::string ArgText =
128+
Lexer::getSourceText(
129+
CharSourceRange::getTokenRange(Arg->getSourceRange()), SourceMngr,
130+
LanguageOpts)
131+
.str();
132+
133+
Twine Replacement = llvm::Twine("static_cast<")
134+
.concat(ResultType.getAsString(LanguageOpts))
135+
.concat(">(")
136+
.concat(ArgText)
137+
.concat(")");
138+
139+
FixItHints.push_back(FixItHint::CreateReplacement(Arg->getSourceRange(),
140+
Replacement.str()));
117141

118-
FixItHints.push_back(
119-
FixItHint::CreateRemoval(SourceRange(InnerCall->getRParenLoc())));
120-
121-
if (InnerResult.First == InnerResult.Last) {
122-
FixItHints.insert(FixItHints.end(), InnerReplacements.begin(),
123-
InnerReplacements.end());
124-
125-
FixItHints.push_back(
126-
FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
127-
InnerResult.First->getBeginLoc())));
128-
FixItHints.push_back(FixItHint::CreateRemoval(
129-
CharSourceRange::getTokenRange(InnerResult.First->getEndLoc())));
130-
} else
131-
FixItHints.insert(FixItHints.end(), InnerReplacements.begin() + 1,
132-
InnerReplacements.end() - 1);
133-
134-
if (InnerResult.Compare) {
135-
const auto Comma = utils::lexer::findNextTokenSkippingComments(
136-
InnerResult.Last->getEndLoc(), *Match.SourceManager,
137-
Match.Context->getLangOpts());
138-
if (Comma && Comma->getKind() == tok::comma)
139-
FixItHints.push_back(
140-
FixItHint::CreateRemoval(SourceRange(Comma->getLocation())));
141-
142-
if (utils::lexer::getPreviousToken(
143-
InnerResult.Compare->getExprLoc(), *Match.SourceManager,
144-
Match.Context->getLangOpts(), false)
145-
.getLocation() == Comma->getLocation())
146-
FixItHints.push_back(
147-
FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
148-
Comma->getLocation(), InnerResult.Compare->getEndLoc())));
149-
else {
150-
FixItHints.push_back(
151-
FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
152-
InnerResult.Compare->getSourceRange())));
153-
}
154-
}
155-
}
156142
continue;
157143
}
158144

159-
const QualType ArgType = Arg->IgnoreParenImpCasts()
160-
->getType()
161-
.getUnqualifiedType()
162-
.getCanonicalType();
145+
const auto InnerResult = findArgs(InnerCall);
146+
const auto InnerReplacements =
147+
generateReplacement(Match, InnerCall, InnerResult);
148+
const bool IsInnerInitializerList = InnerResult.First == InnerResult.Last;
163149

164-
if (ArgType != ResultType) {
165-
const std::string ArgText =
166-
Lexer::getSourceText(
167-
CharSourceRange::getTokenRange(Arg->getSourceRange()),
168-
*Match.SourceManager, Match.Context->getLangOpts())
169-
.str();
150+
// if the nested call is not the same as the top call
151+
if (InnerCall->getDirectCallee()->getQualifiedNameAsString() !=
152+
TopCall->getDirectCallee()->getQualifiedNameAsString())
153+
continue;
154+
155+
// if the nested call doesn't have the same compare function
156+
if ((Result.Compare || InnerResult.Compare) &&
157+
!utils::areStatementsIdentical(Result.Compare, InnerResult.Compare,
158+
*Match.Context))
159+
continue;
160+
161+
// remove the function call
162+
FixItHints.push_back(
163+
FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
164+
InnerCall->getCallee()->getSourceRange())));
170165

171-
FixItHints.push_back(FixItHint::CreateReplacement(
172-
Arg->getSourceRange(),
173-
"static_cast<" + ResultType.getAsString() + ">(" + ArgText + ")"));
166+
// remove the parentheses
167+
const auto LParen = utils::lexer::findNextTokenSkippingComments(
168+
InnerCall->getCallee()->getEndLoc(), SourceMngr, LanguageOpts);
169+
FixItHints.push_back(
170+
FixItHint::CreateRemoval(SourceRange(LParen->getLocation())));
171+
FixItHints.push_back(
172+
FixItHint::CreateRemoval(SourceRange(InnerCall->getRParenLoc())));
173+
174+
// if the inner call has an initializer list arg
175+
if (IsInnerInitializerList) {
176+
// remove the initializer list braces
177+
FixItHints.push_back(FixItHint::CreateRemoval(
178+
CharSourceRange::getTokenRange(InnerResult.First->getBeginLoc())));
179+
FixItHints.push_back(FixItHint::CreateRemoval(
180+
CharSourceRange::getTokenRange(InnerResult.First->getEndLoc())));
174181
}
175-
}
176182

177-
if (!IsInitializerList) {
178-
if (Result.Compare)
179-
FixItHints.push_back(FixItHint::CreateInsertion(
180-
Lexer::getLocForEndOfToken(Result.Last->getEndLoc(), 0,
181-
*Match.SourceManager,
182-
Match.Context->getLangOpts()),
183-
"}"));
184-
else
185-
FixItHints.push_back(
186-
FixItHint::CreateInsertion(TopCall->getEndLoc(), "}"));
183+
FixItHints.insert(FixItHints.end(),
184+
// ignore { and } insertions for the inner call if it does
185+
// not have an initializer list arg
186+
InnerReplacements.begin() + (!IsInnerInitializerList) * 2,
187+
InnerReplacements.end());
188+
189+
if (InnerResult.Compare) {
190+
// find the comma after the value arguments
191+
const auto Comma = utils::lexer::findNextTokenSkippingComments(
192+
InnerResult.Last->getEndLoc(), SourceMngr, LanguageOpts);
193+
194+
// if there are comments between the comma and the comparison
195+
if (utils::lexer::getPreviousToken(InnerResult.Compare->getExprLoc(),
196+
SourceMngr, LanguageOpts, false)
197+
.getLocation() != Comma->getLocation()) {
198+
// remove the comma and the comparison
199+
FixItHints.push_back(
200+
FixItHint::CreateRemoval(SourceRange(Comma->getLocation())));
201+
202+
FixItHints.push_back(
203+
FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
204+
InnerResult.Compare->getSourceRange())));
205+
} else
206+
// remove everything after the last argument
207+
FixItHints.push_back(
208+
FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
209+
Comma->getLocation(), InnerResult.Compare->getEndLoc())));
210+
}
187211
}
188212

189213
return FixItHints;
@@ -229,7 +253,7 @@ void MinMaxUseInitializerListCheck::check(
229253
const auto *TopCall = Match.Nodes.getNodeAs<CallExpr>("topCall");
230254

231255
const FindArgsResult Result = findArgs(TopCall);
232-
const std::vector<FixItHint> Replacement =
256+
const SmallVector<FixItHint> Replacement =
233257
generateReplacement(Match, TopCall, Result);
234258

235259
if (Replacement.size() <= 2) {

‎clang-tools-extra/test/clang-tidy/checkers/modernize/min-max-use-initializer-list.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,7 @@ int max2d = std::max(std::max({1, 2, 3}), 4);
159159
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use nested 'std::max' calls, use an initializer list instead [modernize-min-max-use-initializer-list]
160160
// CHECK-FIXES: int max2d = std::max({1, 2, 3, 4});
161161

162+
162163
int max2e = std::max(1, max(2, max(3, 4)));
163164
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use nested 'std::max' calls, use an initializer list instead [modernize-min-max-use-initializer-list]
164165
// CHECK-FIXES: int max2e = std::max({1, 2, 3, 4});

0 commit comments

Comments
 (0)