Skip to content

Commit 9bff6bd

Browse files
codemzsZeeshan Siddiqui
authored andcommitted
[Sema] Fix -Wunreachable-code false negative when operands differ only by implicit casts (llvm#149972)
## Motivation `-Wunreachable-code` missed—or in rare cases crashed on—tautological comparisons such as ```cpp x != 0 || x != 1.0 // always true x == 0 && x == 1.0 // always false ``` when the *same* variable appears on both sides but one operand goes through a floating‑rank promotion that is target‑dependent. On back‑ends with **native half‑precision** (`_Float16` / `__fp16`) such as AArch64 `+fullfp16`, no promotion occurs, so the cast stacks between the two operands differ and the existing heuristic bails out. ## Technical description * **Extends `Expr::isSameComparisonOperand()`** – the helper now ignores parentheses **and value‑preserving implicit casts** (`CK_LValueToRValue`, floating‑rank `CK_FloatingCast`) before comparing the underlying operands. This prevents floating‑rank promotions and lvalue‑to‑rvalue conversions from blocking the unreachable‑code diagnostic on targets with native FP16. *No change needed in `CheckIncorrectLogicOperator`; it simply benefits from the improved helper.* * **Regression test** – `warn-unreachable_crash.cpp` updated to cover both the promoted case (x86‑64) and the native‑half case (AArch64 `+fullfp16`). * **Docs** – release‑note bullet added under *Bug Fixes in This Version*. @ziqingluo-90 @yronglin Could you please review promptly? (feel free to also merge it on my behalf) Thanks! Fixes llvm#149967 Co-authored-by: Zeeshan Siddiqui <[email protected]>
1 parent 27cc35b commit 9bff6bd

File tree

3 files changed

+45
-15
lines changed

3 files changed

+45
-15
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,11 @@ Bug Fixes in This Version
134134
-------------------------
135135
- Fix a crash when marco name is empty in ``#pragma push_macro("")`` or
136136
``#pragma pop_macro("")``. (#GH149762).
137+
- `-Wunreachable-code`` now diagnoses tautological or contradictory
138+
comparisons such as ``x != 0 || x != 1.0`` and ``x == 0 && x == 1.0`` on
139+
targets that treat ``_Float16``/``__fp16`` as native scalar types. Previously
140+
the warning was silently lost because the operands differed only by an implicit
141+
cast chain. (#GH149967).
137142

138143
Bug Fixes to Compiler Builtins
139144
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

clang/lib/AST/Expr.cpp

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4233,8 +4233,15 @@ bool Expr::isSameComparisonOperand(const Expr* E1, const Expr* E2) {
42334233
// template parameters.
42344234
const auto *DRE1 = cast<DeclRefExpr>(E1);
42354235
const auto *DRE2 = cast<DeclRefExpr>(E2);
4236-
return DRE1->isPRValue() && DRE2->isPRValue() &&
4237-
DRE1->getDecl() == DRE2->getDecl();
4236+
4237+
if (DRE1->getDecl() != DRE2->getDecl())
4238+
return false;
4239+
4240+
if ((DRE1->isPRValue() && DRE2->isPRValue()) ||
4241+
(DRE1->isLValue() && DRE2->isLValue()))
4242+
return true;
4243+
4244+
return false;
42384245
}
42394246
case ImplicitCastExprClass: {
42404247
// Peel off implicit casts.
@@ -4244,7 +4251,8 @@ bool Expr::isSameComparisonOperand(const Expr* E1, const Expr* E2) {
42444251
if (!ICE1 || !ICE2)
42454252
return false;
42464253
if (ICE1->getCastKind() != ICE2->getCastKind())
4247-
return false;
4254+
return isSameComparisonOperand(ICE1->IgnoreParenImpCasts(),
4255+
ICE2->IgnoreParenImpCasts());
42484256
E1 = ICE1->getSubExpr()->IgnoreParens();
42494257
E2 = ICE2->getSubExpr()->IgnoreParens();
42504258
// The final cast must be one of these types.
Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,33 @@
1-
// RUN: %clang_cc1 -verify -Wunreachable-code %s
1+
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -verify -Wunreachable-code %s
2+
// RUN: %clang_cc1 -triple aarch64-unknown-linux-gnu -target-feature +fullfp16 -verify -Wunreachable-code %s
3+
// REQUIRES: aarch64-registered-target
24

3-
// Previously this test will crash
4-
static void test(__fp16& x) {
5-
if (x != 0 || x != 1.0) { // expected-note{{}} no-crash
6-
x = 0.9;
7-
} else
8-
x = 0.8; // expected-warning{{code will never be executed}}
5+
// ======= __fp16 version =======
6+
static void test_fp16(__fp16 &x) {
7+
if (x != 0 || x != 1.0) { // expected-note {{}} no-crash
8+
x = 0.9;
9+
} else
10+
x = 0.8; // expected-warning{{code will never be executed}}
911
}
1012

11-
static void test2(__fp16& x) {
12-
if (x != 1 && x == 1.0) { // expected-note{{}} no-crash
13-
x = 0.9; // expected-warning{{code will never be executed}}
14-
} else
15-
x = 0.8;
13+
static void test_fp16_b(__fp16 &x) {
14+
if (x != 1 && x == 1.0) { // expected-note {{}} no-crash
15+
x = 0.9; // expected-warning{{code will never be executed}}
16+
} else
17+
x = 0.8;
18+
}
19+
20+
// ======= _Float16 version =======
21+
static void test_f16(_Float16 &x) {
22+
if (x != 0 || x != 1.0) { // expected-note {{}} no-crash
23+
x = 0.9;
24+
} else
25+
x = 0.8; // expected-warning{{code will never be executed}}
26+
}
27+
28+
static void test_f16_b(_Float16 &x) {
29+
if (x != 1 && x == 1.0) { // expected-note {{}} no-crash
30+
x = 0.9; // expected-warning{{code will never be executed}}
31+
} else
32+
x = 0.8;
1633
}

0 commit comments

Comments
 (0)