Skip to content

Commit 2d4bac8

Browse files
authored
Reland "[clang-tidy] fix bugprone-narrowing-conversions false positive for conditional expression" (#151874)
This is another attempt to merge previously [reverted](#139474 (comment)) PR #139474. The added tests `narrowing-conversions-conditional-expressions.c[pp]` failed on [different (non x86_64) platforms](#139474 (comment)) because the expected warning is implementation-defined. That's why the test must explicitly specify target (the line `// RUN: -- -target x86_64-unknown-linux`).
1 parent 2422972 commit 2d4bac8

File tree

5 files changed

+61
-4
lines changed

5 files changed

+61
-4
lines changed

clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.cpp

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -555,15 +555,22 @@ bool NarrowingConversionsCheck::handleConditionalOperator(
555555
// We have an expression like so: `output = cond ? lhs : rhs`
556556
// From the point of view of narrowing conversion we treat it as two
557557
// expressions `output = lhs` and `output = rhs`.
558-
handleBinaryOperator(Context, CO->getLHS()->getExprLoc(), Lhs,
559-
*CO->getLHS());
560-
handleBinaryOperator(Context, CO->getRHS()->getExprLoc(), Lhs,
561-
*CO->getRHS());
558+
handleConditionalOperatorArgument(Context, Lhs, CO->getLHS());
559+
handleConditionalOperatorArgument(Context, Lhs, CO->getRHS());
562560
return true;
563561
}
564562
return false;
565563
}
566564

565+
void NarrowingConversionsCheck::handleConditionalOperatorArgument(
566+
const ASTContext &Context, const Expr &Lhs, const Expr *Arg) {
567+
if (const auto *ICE = llvm::dyn_cast<ImplicitCastExpr>(Arg))
568+
if (!Arg->getIntegerConstantExpr(Context))
569+
Arg = ICE->getSubExpr();
570+
571+
handleBinaryOperator(Context, Arg->getExprLoc(), Lhs, *Arg);
572+
}
573+
567574
void NarrowingConversionsCheck::handleImplicitCast(
568575
const ASTContext &Context, const ImplicitCastExpr &Cast) {
569576
if (Cast.getExprLoc().isMacroID())

clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,8 @@ class NarrowingConversionsCheck : public ClangTidyCheck {
8585
bool handleConditionalOperator(const ASTContext &Context, const Expr &Lhs,
8686
const Expr &Rhs);
8787

88+
void handleConditionalOperatorArgument(const ASTContext &Context,
89+
const Expr &Lhs, const Expr *Arg);
8890
void handleImplicitCast(const ASTContext &Context,
8991
const ImplicitCastExpr &Cast);
9092

clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,10 @@ Changes in existing checks
134134
<clang-tidy/checks/bugprone/infinite-loop>` check by adding detection for
135135
variables introduced by structured bindings.
136136

137+
- Improved :doc:`bugprone-narrowing-conversions
138+
<clang-tidy/checks/bugprone/narrowing-conversions>` check by fixing
139+
false positive from analysis of a conditional expression in C.
140+
137141
- Improved :doc:`bugprone-reserved-identifier
138142
<clang-tidy/checks/bugprone/reserved-identifier>` check by ignoring
139143
declarations in system headers.
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// RUN: %check_clang_tidy %s bugprone-narrowing-conversions %t -- \
2+
// RUN: -- -target x86_64-unknown-linux
3+
4+
char test_char(int cond, char c) {
5+
char ret = cond > 0 ? ':' : c;
6+
return ret;
7+
}
8+
9+
short test_short(int cond, short s) {
10+
short ret = cond > 0 ? ':' : s;
11+
return ret;
12+
}
13+
14+
int test_int(int cond, int i) {
15+
int ret = cond > 0 ? ':' : i;
16+
return ret;
17+
}
18+
19+
void test(int cond, int i) {
20+
char x = cond > 0 ? ':' : i;
21+
// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: narrowing conversion from 'int' to signed type 'char' is implementation-defined [bugprone-narrowing-conversions]
22+
}
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// RUN: %check_clang_tidy %s bugprone-narrowing-conversions %t -- \
2+
// RUN: -- -target x86_64-unknown-linux
3+
4+
char test_char(int cond, char c) {
5+
char ret = cond > 0 ? ':' : c;
6+
return ret;
7+
}
8+
9+
short test_short(int cond, short s) {
10+
short ret = cond > 0 ? ':' : s;
11+
return ret;
12+
}
13+
14+
int test_int(int cond, int i) {
15+
int ret = cond > 0 ? ':' : i;
16+
return ret;
17+
}
18+
19+
void test(int cond, int i) {
20+
char x = cond > 0 ? ':' : i;
21+
// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: narrowing conversion from 'int' to signed type 'char' is implementation-defined [bugprone-narrowing-conversions]
22+
}

0 commit comments

Comments
 (0)