https://github.com/AndreyG updated
https://github.com/llvm/llvm-project/pull/139474
>From b4814ba942266c5d4f4f070cbdef7b721738f33e Mon Sep 17 00:00:00 2001
From: Andrey Davydov <andrey.davy...@jetbrains.com>
Date: Sun, 11 May 2025 22:23:38 +0200
Subject: [PATCH 1/2] [clang-tidy] false positive narrowing conversion
Let's consider the following code from the issue #139467:
void test(int cond, char c) {
char ret = cond > 0 ? ':' : c;
}
Initializer of 'ret' looks the following:
-ImplicitCastExpr 'char' <IntegralCast>
`-ConditionalOperator 'int'
|-BinaryOperator 'int' '>'
| |-ImplicitCastExpr 'int' <LValueToRValue>
| | `-DeclRefExpr 'int' lvalue ParmVar 'cond' 'int'
| `-IntegerLiteral 'int' 0
|-CharacterLiteral 'int' 58
`-ImplicitCastExpr 'int' <IntegralCast>
`-ImplicitCastExpr 'char' <LValueToRValue>
`-DeclRefExpr 'char' lvalue ParmVar 'c' 'char'
So it could be seen that 'RHS' of the conditional operator is
DeclRefExpr 'c' which is casted to 'int' and then the whole conditional
expression is casted to 'char'.
But this last conversion is not narrowing, because 'RHS' was 'char' _initially_.
We should just remove the cast from 'char' to 'int' before the narrowing
conversion check.
---
.../bugprone/NarrowingConversionsCheck.cpp | 16 ++++++++++----
.../bugprone/NarrowingConversionsCheck.h | 2 ++
clang-tools-extra/docs/ReleaseNotes.rst | 4 ++++
...wing-conversions-conditional-expressions.c | 22 +++++++++++++++++++
...ng-conversions-conditional-expressions.cpp | 22 +++++++++++++++++++
5 files changed, 62 insertions(+), 4 deletions(-)
create mode 100644
clang-tools-extra/test/clang-tidy/checkers/bugprone/narrowing-conversions-conditional-expressions.c
create mode 100644
clang-tools-extra/test/clang-tidy/checkers/bugprone/narrowing-conversions-conditional-expressions.cpp
diff --git
a/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.cpp
index 8b2ca6968ea75..6b9e24b3e2d26 100644
--- a/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.cpp
@@ -553,15 +553,23 @@ bool NarrowingConversionsCheck::handleConditionalOperator(
// We have an expression like so: `output = cond ? lhs : rhs`
// From the point of view of narrowing conversion we treat it as two
// expressions `output = lhs` and `output = rhs`.
- handleBinaryOperator(Context, CO->getLHS()->getExprLoc(), Lhs,
- *CO->getLHS());
- handleBinaryOperator(Context, CO->getRHS()->getExprLoc(), Lhs,
- *CO->getRHS());
+ handleConditionalOperatorArgument(Context, Lhs, CO->getLHS());
+ handleConditionalOperatorArgument(Context, Lhs, CO->getRHS());
return true;
}
return false;
}
+void NarrowingConversionsCheck::handleConditionalOperatorArgument(
+ const ASTContext &Context, const Expr &Lhs, const Expr *Arg) {
+ if (const auto *ICE = llvm::dyn_cast<ImplicitCastExpr>(Arg)) {
+ if (!Arg->getIntegerConstantExpr(Context)) {
+ Arg = ICE->getSubExpr();
+ }
+ }
+ handleBinaryOperator(Context, Arg->getExprLoc(), Lhs, *Arg);
+}
+
void NarrowingConversionsCheck::handleImplicitCast(
const ASTContext &Context, const ImplicitCastExpr &Cast) {
if (Cast.getExprLoc().isMacroID())
diff --git a/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.h
b/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.h
index 20403f920b925..ebddbc2869675 100644
--- a/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.h
@@ -85,6 +85,8 @@ class NarrowingConversionsCheck : public ClangTidyCheck {
bool handleConditionalOperator(const ASTContext &Context, const Expr &Lhs,
const Expr &Rhs);
+ void handleConditionalOperatorArgument(const ASTContext &Context, const Expr
&Lhs,
+ const Expr *Arg);
void handleImplicitCast(const ASTContext &Context,
const ImplicitCastExpr &Cast);
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index 36a41b4bdf42d..43282bb52d9c1 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -182,6 +182,10 @@ New check aliases
Changes in existing checks
^^^^^^^^^^^^^^^^^^^^^^^^^^
+- Improved :doc:`bugprone-narrowing-conversions
+ <clang-tidy/checks/bugprone/narrowing-conversions>` check by fixing
+ false positive from analysis of a conditional expression in C.
+
- Improved :doc:`bugprone-crtp-constructor-accessibility
<clang-tidy/checks/bugprone/crtp-constructor-accessibility>` check by fixing
false positives on deleted constructors that cannot be used to construct
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/narrowing-conversions-conditional-expressions.c
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/narrowing-conversions-conditional-expressions.c
new file mode 100644
index 0000000000000..474842b7b905e
--- /dev/null
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/narrowing-conversions-conditional-expressions.c
@@ -0,0 +1,22 @@
+// RUN: %check_clang_tidy %s bugprone-narrowing-conversions %t -- --
+
+char test_char(int cond, char c) {
+ char ret = cond > 0 ? ':' : c;
+ return ret;
+}
+
+short test_short(int cond, short s) {
+ short ret = cond > 0 ? ':' : s;
+ return ret;
+}
+
+int test_int(int cond, int i) {
+ int ret = cond > 0 ? ':' : i;
+ return ret;
+}
+
+void test(int cond, int i) {
+ char x = cond > 0 ? ':' : i;
+ // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: narrowing conversion from 'int'
to signed type 'char' is implementation-defined [bugprone-narrowing-conversions]
+}
+
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/narrowing-conversions-conditional-expressions.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/narrowing-conversions-conditional-expressions.cpp
new file mode 100644
index 0000000000000..474842b7b905e
--- /dev/null
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/narrowing-conversions-conditional-expressions.cpp
@@ -0,0 +1,22 @@
+// RUN: %check_clang_tidy %s bugprone-narrowing-conversions %t -- --
+
+char test_char(int cond, char c) {
+ char ret = cond > 0 ? ':' : c;
+ return ret;
+}
+
+short test_short(int cond, short s) {
+ short ret = cond > 0 ? ':' : s;
+ return ret;
+}
+
+int test_int(int cond, int i) {
+ int ret = cond > 0 ? ':' : i;
+ return ret;
+}
+
+void test(int cond, int i) {
+ char x = cond > 0 ? ':' : i;
+ // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: narrowing conversion from 'int'
to signed type 'char' is implementation-defined [bugprone-narrowing-conversions]
+}
+
>From d145cfe66eb0e029161abc464fee19347df9158f Mon Sep 17 00:00:00 2001
From: Andrey Davydov <andrey.davy...@jetbrains.com>
Date: Mon, 14 Jul 2025 10:28:25 +0200
Subject: [PATCH 2/2] fix formatting
---
.../clang-tidy/bugprone/NarrowingConversionsCheck.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.h
b/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.h
index ebddbc2869675..116a8cba8d321 100644
--- a/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.h
@@ -85,8 +85,8 @@ class NarrowingConversionsCheck : public ClangTidyCheck {
bool handleConditionalOperator(const ASTContext &Context, const Expr &Lhs,
const Expr &Rhs);
- void handleConditionalOperatorArgument(const ASTContext &Context, const Expr
&Lhs,
- const Expr *Arg);
+ void handleConditionalOperatorArgument(const ASTContext &Context,
+ const Expr &Lhs, const Expr *Arg);
void handleImplicitCast(const ASTContext &Context,
const ImplicitCastExpr &Cast);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
