[clang-tools-extra] Fix the issue #139467 ([clang-tidy] false positive narrowing conversion) (PR #139474)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-tools-extra

Author: Andrey (AndreyG)

<details>
<summary>Changes</summary>

Let's consider the following code from the issue #<!-- -->139467:
```c
void test(int cond, char c) {
    char ret = cond &gt; 0 ? ':' : c;
}
```
Initializer of `ret` looks the following:
```
-ImplicitCastExpr 'char' &lt;IntegralCast&gt;
 `-ConditionalOperator 'int'
   |-BinaryOperator 'int' '&gt;'
   | |-ImplicitCastExpr 'int' &lt;LValueToRValue&gt;
   | | `-DeclRefExpr 'int' lvalue ParmVar 'cond' 'int'
   | `-IntegerLiteral 'int' 0
   |-CharacterLiteral 'int' 58
   `-ImplicitCastExpr 'int' &lt;IntegralCast&gt;
     `-ImplicitCastExpr 'char' &lt;LValueToRValue&gt;
       `-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.

---
Full diff: https://github.com/llvm/llvm-project/pull/139474.diff


3 Files Affected:

- (modified) 
clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.cpp (+12-4) 
- (modified) clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.h 
(+2) 
- (added) 
clang-tools-extra/test/clang-tidy/checkers/bugprone/narrowing-conversions-conditional-expressions.c
 (+6) 


``````````diff
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;
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/139474
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits