https://github.com/AndreyG created https://github.com/llvm/llvm-project/pull/139474
Let's consider the following code from the issue #139467: ```c 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. >From 19c3713883e79220e6c30bf76c2d95cfaf4dcaa3 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] [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 ++ ...rrowing-conversions-conditional-expressions.c | 6 ++++++ 3 files changed, 20 insertions(+), 4 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/narrowing-conversions-conditional-expressions.c diff --git a/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.cpp index bafcd402ca851..9e53bfe83e03e 100644 --- a/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.cpp @@ -554,15 +554,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/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..754d6425b07cd --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/narrowing-conversions-conditional-expressions.c @@ -0,0 +1,6 @@ +// RUN: %check_clang_tidy %s bugprone-narrowing-conversions %t -- -- + +char test(int cond, char c) { + char ret = cond > 0 ? ':' : c; + return ret; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits