[clang] [Sema] Fix -Wunreachable-code false negative when operands differ only by implicit casts (PR #149972)
M. Zeeshan Siddiqui via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 29 12:05:11 PDT 2025
https://github.com/codemzs updated https://github.com/llvm/llvm-project/pull/149972
>From 20ad1ba3b919a37a86f39ba7d30249a0b9b9d2c6 Mon Sep 17 00:00:00 2001
From: Zeeshan Siddiqui <mzs at ntdev.microsoft.com>
Date: Tue, 22 Jul 2025 06:37:54 +0000
Subject: [PATCH 1/2] [Sema] Make incorrect-logic operator check cast-agnostic
The check for tautological comparisons (e.g. `x != 0 || x != 1`)
failed on targets that support native scalar half precision because the
operands differed only by an implicit `FloatingCast`.
Teach CheckIncorrectLogicOperator to retry the operand-equality test
after stripping implicit casts and parentheses. This preserves the
original, cast-sensitive fast-path and fixes Issue#149967.
Updates existing test exercising both `__fp16` (promoted) and `_Float16`
(native with +fullfp16) on x86-64 and AArch64.
---
clang/lib/AST/Expr.cpp | 11 ++++--
clang/lib/Analysis/CFG.cpp | 7 ++--
clang/test/Sema/warn-unreachable_crash.cpp | 41 +++++++++++++++-------
3 files changed, 43 insertions(+), 16 deletions(-)
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index d85655b1596f4..784e5c9bdc6ca 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -4233,8 +4233,15 @@ bool Expr::isSameComparisonOperand(const Expr* E1, const Expr* E2) {
// template parameters.
const auto *DRE1 = cast<DeclRefExpr>(E1);
const auto *DRE2 = cast<DeclRefExpr>(E2);
- return DRE1->isPRValue() && DRE2->isPRValue() &&
- DRE1->getDecl() == DRE2->getDecl();
+
+ if (DRE1->getDecl() != DRE2->getDecl())
+ return false;
+
+ if ((DRE1->isPRValue() && DRE2->isPRValue()) ||
+ (DRE1->isLValue() && DRE2->isLValue()))
+ return true;
+
+ return false;
}
case ImplicitCastExprClass: {
// Peel off implicit casts.
diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index d960d5130332b..3ddb1370a3145 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -1160,8 +1160,11 @@ class CFGBuilder {
return {};
// Check that it is the same variable on both sides.
- if (!Expr::isSameComparisonOperand(DeclExpr1, DeclExpr2))
- return {};
+ if (!Expr::isSameComparisonOperand(DeclExpr1, DeclExpr2)) {
+ if (!Expr::isSameComparisonOperand(DeclExpr1->IgnoreParenImpCasts(),
+ DeclExpr2->IgnoreParenImpCasts()))
+ return {};
+ }
// Make sure the user's intent is clear (e.g. they're comparing against two
// int literals, or two things from the same enum)
diff --git a/clang/test/Sema/warn-unreachable_crash.cpp b/clang/test/Sema/warn-unreachable_crash.cpp
index 628abcc83f810..dd3995ad4d912 100644
--- a/clang/test/Sema/warn-unreachable_crash.cpp
+++ b/clang/test/Sema/warn-unreachable_crash.cpp
@@ -1,16 +1,33 @@
-// RUN: %clang_cc1 -verify -Wunreachable-code %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -verify -Wunreachable-code %s
+// RUN: %clang_cc1 -triple aarch64-unknown-linux-gnu -target-feature +fullfp16 -verify -Wunreachable-code %s
+// REQUIRES: aarch64-registered-target
-// Previously this test will crash
-static void test(__fp16& x) {
- if (x != 0 || x != 1.0) { // expected-note{{}} no-crash
- x = 0.9;
- } else
- x = 0.8; // expected-warning{{code will never be executed}}
+// ======= __fp16 version =======
+static void test_fp16(__fp16 &x) {
+ if (x != 0 || x != 1.0) { // expected-note {{}} no-crash
+ x = 0.9;
+ } else
+ x = 0.8; // expected-warning{{code will never be executed}}
}
-static void test2(__fp16& x) {
- if (x != 1 && x == 1.0) { // expected-note{{}} no-crash
- x = 0.9; // expected-warning{{code will never be executed}}
- } else
- x = 0.8;
+static void test_fp16_b(__fp16 &x) {
+ if (x != 1 && x == 1.0) { // expected-note {{}} no-crash
+ x = 0.9; // expected-warning{{code will never be executed}}
+ } else
+ x = 0.8;
}
+
+// ======= _Float16 version =======
+static void test_f16(_Float16 &x) {
+ if (x != 0 || x != 1.0) { // expected-note {{}} no-crash
+ x = 0.9;
+ } else
+ x = 0.8; // expected-warning{{code will never be executed}}
+}
+
+static void test_f16_b(_Float16 &x) {
+ if (x != 1 && x == 1.0) { // expected-note {{}} no-crash
+ x = 0.9; // expected-warning{{code will never be executed}}
+ } else
+ x = 0.8;
+}
\ No newline at end of file
>From 849731769d97374c23a213fbddcba66e2649a4f4 Mon Sep 17 00:00:00 2001
From: Zeeshan Siddiqui <mzs at ntdev.microsoft.com>
Date: Tue, 29 Jul 2025 19:03:12 +0000
Subject: [PATCH 2/2] PR feedback.
---
clang/lib/AST/Expr.cpp | 3 ++-
clang/lib/Analysis/CFG.cpp | 7 ++-----
clang/test/Sema/warn-unreachable_crash.cpp | 2 +-
3 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 784e5c9bdc6ca..cd9672d32a013 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -4251,7 +4251,8 @@ bool Expr::isSameComparisonOperand(const Expr* E1, const Expr* E2) {
if (!ICE1 || !ICE2)
return false;
if (ICE1->getCastKind() != ICE2->getCastKind())
- return false;
+ return isSameComparisonOperand(ICE1->IgnoreParenImpCasts(),
+ ICE2->IgnoreParenImpCasts());
E1 = ICE1->getSubExpr()->IgnoreParens();
E2 = ICE2->getSubExpr()->IgnoreParens();
// The final cast must be one of these types.
diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index 3ddb1370a3145..d960d5130332b 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -1160,11 +1160,8 @@ class CFGBuilder {
return {};
// Check that it is the same variable on both sides.
- if (!Expr::isSameComparisonOperand(DeclExpr1, DeclExpr2)) {
- if (!Expr::isSameComparisonOperand(DeclExpr1->IgnoreParenImpCasts(),
- DeclExpr2->IgnoreParenImpCasts()))
- return {};
- }
+ if (!Expr::isSameComparisonOperand(DeclExpr1, DeclExpr2))
+ return {};
// Make sure the user's intent is clear (e.g. they're comparing against two
// int literals, or two things from the same enum)
diff --git a/clang/test/Sema/warn-unreachable_crash.cpp b/clang/test/Sema/warn-unreachable_crash.cpp
index dd3995ad4d912..1955c2c2547f5 100644
--- a/clang/test/Sema/warn-unreachable_crash.cpp
+++ b/clang/test/Sema/warn-unreachable_crash.cpp
@@ -30,4 +30,4 @@ static void test_f16_b(_Float16 &x) {
x = 0.9; // expected-warning{{code will never be executed}}
} else
x = 0.8;
-}
\ No newline at end of file
+}
More information about the cfe-commits
mailing list