llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-analysis Author: M. Zeeshan Siddiqui (codemzs) <details> <summary>Changes</summary> ## Motivation This is a follow-up fix for bug exposed by the `warn-unreachable_crash.cpp` test introduced in #<!-- -->142897 , which caused downstream test failures. ## Technical description Teach `CheckIncorrectLogicOperator` to become cast‑agnostic so that `-Wunreachable-code` warns correctly when a target provides *native* scalar half‑precision. The diagnostic previously relied on an implicit `FloatingCast` that exists on x86‑64 (`__fp16` / `_Float16` → `float`) but is absent on AArch64 `+fullfp16`, RISC‑V `+zfh`, GPUs, etc. @<!-- -->ziqingluo-90 @<!-- -->yronglin Could you please review promptly? (feel free to also merge it on my behalf) Thanks! Fixes #<!-- -->149967 --- Full diff: https://github.com/llvm/llvm-project/pull/149972.diff 3 Files Affected: - (modified) clang/lib/AST/Expr.cpp (+9-2) - (modified) clang/lib/Analysis/CFG.cpp (+5-2) - (modified) clang/test/Sema/warn-unreachable_crash.cpp (+29-12) ``````````diff diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp index 2e1a9a3d9ad63..d9aa1ba4f10ba 100644 --- a/clang/lib/AST/Expr.cpp +++ b/clang/lib/AST/Expr.cpp @@ -4234,8 +4234,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 `````````` </details> https://github.com/llvm/llvm-project/pull/149972 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits