Author: Andrey Date: 2025-08-03T13:39:39+03:00 New Revision: c3902e46b748cd808fbffeea25489bb41b15a6b2
URL: https://github.com/llvm/llvm-project/commit/c3902e46b748cd808fbffeea25489bb41b15a6b2 DIFF: https://github.com/llvm/llvm-project/commit/c3902e46b748cd808fbffeea25489bb41b15a6b2.diff LOG: [clang-tidy] fix `bugprone-narrowing-conversions` false positive for conditional expression (#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. Fixes #139467 Added: clang-tools-extra/test/clang-tidy/checkers/bugprone/narrowing-conversions-conditional-expressions.c clang-tools-extra/test/clang-tidy/checkers/bugprone/narrowing-conversions-conditional-expressions.cpp Modified: clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.cpp clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.h clang-tools-extra/docs/ReleaseNotes.rst Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.cpp index 53782231b6421..249c77ca0c432 100644 --- a/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.cpp @@ -555,15 +555,22 @@ 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..116a8cba8d321 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 ef0e804962927..b9261f61c8789 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -134,6 +134,10 @@ Changes in existing checks <clang-tidy/checks/bugprone/infinite-loop>` check by adding detection for variables introduced by structured bindings. +- 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-reserved-identifier <clang-tidy/checks/bugprone/reserved-identifier>` check by ignoring declarations in system headers. 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..0ea83bc8559af --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/narrowing-conversions-conditional-expressions.c @@ -0,0 +1,21 @@ +// 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..0ea83bc8559af --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/narrowing-conversions-conditional-expressions.cpp @@ -0,0 +1,21 @@ +// 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] +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
