From e8fae788cedcd2bf098f4bc78f294bcf544f7864 Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Thu, 26 Feb 2026 08:38:31 +0300 Subject: [PATCH] checker: add self-comparison and always true/false branch warnings (fixes #23404) --- vlib/v/checker/if.v | 48 +++++++++++++++++++ vlib/v/checker/match.v | 14 +++++- .../tests/always_true_false_branch.out | 14 ++++++ 3 files changed, 75 insertions(+), 1 deletion(-) diff --git a/vlib/v/checker/if.v b/vlib/v/checker/if.v index d9471773c..c38d6a567 100644 --- a/vlib/v/checker/if.v +++ b/vlib/v/checker/if.v @@ -7,6 +7,48 @@ import v.token import v.util import strings +fn (c &Checker) simple_expr_name_for_dead_branch_check(expr ast.Expr) string { + mut current_expr := expr + current_expr = current_expr.remove_par() + if current_expr !in [ast.Ident, ast.SelectorExpr] { + return '' + } + return current_expr.str() +} + +fn (mut c Checker) is_always_true_self_comparison(cond ast.Expr) bool { + mut cond_expr := cond + cond_expr = cond_expr.remove_par() + if mut cond_expr is ast.InfixExpr { + if cond_expr.op != .eq { + return false + } + left_name := c.simple_expr_name_for_dead_branch_check(cond_expr.left) + right_name := c.simple_expr_name_for_dead_branch_check(cond_expr.right) + if left_name == '' || left_name != right_name { + return false + } + mut left_type := if cond_expr.left_type == ast.no_type { + c.get_expr_type(cond_expr.left) + } else { + cond_expr.left_type + } + mut right_type := if cond_expr.right_type == ast.no_type { + c.get_expr_type(cond_expr.right) + } else { + cond_expr.right_type + } + left_type = c.table.unaliased_type(left_type) + right_type = c.table.unaliased_type(right_type) + if left_type.is_float() || right_type.is_float() { + // `NaN == NaN` is always false. + return false + } + return true + } + return false +} + // gen_branch_context_string generate current branches context string. // context include generic types, `$for`. fn (mut c Checker) gen_branch_context_string() string { @@ -98,6 +140,7 @@ fn (mut c Checker) if_expr(mut node ast.IfExpr) ast.Type { comptime_branch_context_str := if node.is_comptime { c.gen_branch_context_string() } else { '' } for i, mut branch in node.branches { + orig_branch_cond := branch.cond mut comptime_remove_curr_branch_stmts := false if branch.cond is ast.ParExpr && !c.pref.translated && !c.file.is_translated { c.warn('unnecessary `()` in `${if_kind}` condition, use `${if_kind} expr {` instead of `${if_kind} (expr) {`.', @@ -171,6 +214,11 @@ fn (mut c Checker) if_expr(mut node ast.IfExpr) ast.Type { } } } + if !c.pref.translated && !c.file.is_translated && !c.inside_unsafe + && c.is_always_true_self_comparison(orig_branch_cond) { + c.warn('self-comparison in `if` condition is always true; following branches may be unreachable', + branch.cond.pos()) + } } } else { // else branch diff --git a/vlib/v/checker/match.v b/vlib/v/checker/match.v index 623c7b2f5..ac10c58fe 100644 --- a/vlib/v/checker/match.v +++ b/vlib/v/checker/match.v @@ -134,7 +134,7 @@ fn (mut c Checker) match_expr(mut node ast.MatchExpr) ast.Type { } } } - for mut branch in node.branches { + for i, mut branch in node.branches { if node.is_comptime { // `idx_str` is composed of two parts: // The first part represents the current context of the branch statement, `comptime_branch_context_str`, formatted like `T=int,X=string,method.name=json` @@ -252,6 +252,7 @@ fn (mut c Checker) match_expr(mut node ast.MatchExpr) ast.Type { } if !c.pref.translated && !c.file.is_translated { + mut did_warn_self_comparison_branch := false // check for always true/false match branch for mut expr in branch.exprs { mut check_expr := ast.InfixExpr{ @@ -263,6 +264,17 @@ fn (mut c Checker) match_expr(mut node ast.MatchExpr) ast.Type { if t_expr is ast.BoolLiteral { if t_expr.val { c.note('match is always true', expr.pos()) + if c.is_always_true_self_comparison(ast.Expr(ast.InfixExpr{ + op: .eq + left: node.cond + right: expr + left_type: node.cond_type + right_type: node.cond_type + })) && i < node.branches.len - 1 && !did_warn_self_comparison_branch { + c.warn('self-comparison match branch is always true; following branches may be unreachable', + expr.pos()) + did_warn_self_comparison_branch = true + } } else { c.note('match is always false', expr.pos()) } diff --git a/vlib/v/checker/tests/always_true_false_branch.out b/vlib/v/checker/tests/always_true_false_branch.out index 05e0fa7b5..f60c88b9e 100644 --- a/vlib/v/checker/tests/always_true_false_branch.out +++ b/vlib/v/checker/tests/always_true_false_branch.out @@ -467,3 +467,17 @@ vlib/v/checker/tests/always_true_false_branch.vv:196:3: notice: match is always | ~~~ 197 | 'cc' 198 | } +vlib/v/checker/tests/always_true_false_branch.vv:5:23: warning: self-comparison in `if` condition is always true; following branches may be unreachable + 3 | fn main() { + 4 | x := 1 + 5 | if_always_true := if x == x { + | ~~~~~~ + 6 | 1 + 7 | } else if true { +vlib/v/checker/tests/always_true_false_branch.vv:134:3: warning: self-comparison match branch is always true; following branches may be unreachable + 132 | + 133 | match_always_true_var := match x { + 134 | x { + | ^ + 135 | 'haha' + 136 | } -- 2.39.5