From d8bd3cfeb6b24a17b3f9f4bf9feb97fcfd012557 Mon Sep 17 00:00:00 2001 From: MARCROCK22 Date: Tue, 22 Sep 2026 02:27:17 -0400 Subject: [PATCH 1/3] checker: reject ordered comparisons of operands without a common order `<`, `>`, `<=` and `>=` had their operands checked only when both were arrays, fn values or structs. Anything else went to the C backend as it was: a thread handle, map, sum type or struct compared with a number failed in the C compiler, and a bool, enum, channel or fn value compared with a number compiled into a comparison that means nothing. They now get the compatibility check of `==`, with the messages V1 gives for them: numbers compare with any number, a pointer with an integer, a `voidptr` with what C can order it with, and a struct with its own `<` only with its own type. V1 let three more kinds of operands reach the C compiler, and they are rejected too: maps, interfaces, and an option against `none`. `unsafe { x == 0 }` is accepted only when `x` is a struct reached through a pointer, such as a `mut` parameter, and only there does the checker suggest `unsafe`: a struct value compared with zero failed in the C compiler. The conditions of `match` branches, as in `match true { a < b {} }`, now get the checks an `if` condition gets; none of them ran before. Finding the operator of a comparison for a diagnostic no longer slices past the end of the V file that template code is attributed to, whose offsets can run beyond it. --- .../ordered_comparison_operands_test.v | 936 ++++++++++++++++++ vlib/v/types/checker.v | 1 + vlib/v/types/checker_comptime.v | 158 ++- vlib/v/types/checker_tail.v | 2 + vlib/v/types/checker_tail_stmt.v | 5 + 5 files changed, 1095 insertions(+), 7 deletions(-) create mode 100644 vlib/v/compiler_tests/ordered_comparison_operands_test.v diff --git a/vlib/v/compiler_tests/ordered_comparison_operands_test.v b/vlib/v/compiler_tests/ordered_comparison_operands_test.v new file mode 100644 index 00000000000000..24cef688162b47 --- /dev/null +++ b/vlib/v/compiler_tests/ordered_comparison_operands_test.v @@ -0,0 +1,936 @@ +// Tests for the operands of `<`, `>`, `<=` and `>=`. They need an order in +// common: without one, the C backend emits a comparison that the C compiler +// rejects (structs, maps, sum types, thread handles...) or one that compiles +// and means nothing (bools, enums, fn values or channels against numbers). +// The expected messages are the ones V1 reports for the same code, except for +// the comparisons marked below that V1 let through to the C compiler. +import os +import rand + +const vexe = @VEXE +const tests_dir = os.dir(@FILE) +const v3_dir = os.dir(tests_dir) +const vlib_dir = os.dir(v3_dir) +const v3_src = os.join_path(v3_dir, 'v.v') +const ordered_v3_bin = os.join_path(os.temp_dir(), 'v3_ordered_comparison_test_${os.getpid()}') + +const ordered_ops = ['<', '>', '<=', '>='] + +const prelude = "module main + +import time + +struct Foo { + x int +} + +struct Op { + x int +} + +fn (a Op) < (b Op) bool { + return a.x < b.x +} + +type OpAlias = Op + +type Sum = int | string + +interface Speaker { + speak() string +} + +struct Dog {} + +fn (d Dog) speak() string { + return 'woof' +} + +enum Color { + red + green +} + +@[flag] +enum Perm { + read + write +} + +type MyInt = int +type MyStr = string +type FooAlias = Foo + +type Money = int + +fn (a Money) < (b Money) bool { + return int(a) < int(b) +} + +fn epoch() time.Time { + return time.unix(0) +} +" + +// Operand is one side of a generated comparison: a parameter of type `decl`, +// or the literal `text` when `decl` is empty. Two operands of the same +// non-empty `group` can be ordered. An `integer` also orders with a pointer, +// as pointer arithmetic, and a `c_scalar` with a `voidptr`, which the C +// compiler compares with pointers and scalars; any other pair cannot. +struct Operand { + key string + decl string + text string + group string + integer bool + c_scalar bool +} + +fn (o Operand) expr(name string) string { + return if o.decl == '' { o.text } else { name } +} + +fn (o Operand) is_pointer() bool { + return o.key in ['int_pointer', 'voidptr'] +} + +fn comparison_is_valid(lhs Operand, rhs Operand) bool { + if lhs.group != '' && lhs.group == rhs.group { + return true + } + if lhs.key == 'voidptr' { + return rhs.is_pointer() || rhs.c_scalar + } + if rhs.key == 'voidptr' { + // `bool < x` is rejected for any `x`. + return lhs.is_pointer() || (lhs.c_scalar && lhs.key !in ['bool', 'bool_literal']) + } + return (lhs.is_pointer() && rhs.integer) || (rhs.is_pointer() && lhs.integer) +} + +const operands = [ + Operand{ + key: 'int' + decl: 'int' + group: 'number' + integer: true + c_scalar: true + }, + Operand{ + key: 'i64' + decl: 'i64' + group: 'number' + integer: true + c_scalar: true + }, + Operand{ + key: 'u8' + decl: 'u8' + group: 'number' + integer: true + c_scalar: true + }, + Operand{ + key: 'f64' + decl: 'f64' + group: 'number' + }, + Operand{ + key: 'rune' + decl: 'rune' + group: 'number' + integer: true + c_scalar: true + }, + Operand{ + key: 'int_alias' + decl: 'MyInt' + group: 'number' + integer: true + c_scalar: true + }, + Operand{ + key: 'int_literal' + text: '1' + group: 'number' + integer: true + c_scalar: true + }, + Operand{ + key: 'zero_literal' + text: '0' + group: 'number' + integer: true + c_scalar: true + }, + Operand{ + key: 'float_literal' + text: '1.5' + group: 'number' + }, + Operand{ + key: 'rune_literal' + text: '`a`' + group: 'number' + integer: true + c_scalar: true + }, + Operand{ + key: 'char' + decl: 'char' + group: 'number' + integer: true + c_scalar: true + }, + Operand{ + key: 'int_pointer' + decl: '&int' + group: 'int_pointer' + }, + Operand{ + key: 'voidptr' + decl: 'voidptr' + group: 'voidptr' + }, + Operand{ + key: 'string' + decl: 'string' + group: 'string' + }, + Operand{ + key: 'string_alias' + decl: 'MyStr' + group: 'string' + }, + Operand{ + key: 'string_literal' + text: "'a'" + group: 'string' + }, + Operand{ + key: 'struct_with_lt' + decl: 'Op' + group: 'op' + }, + Operand{ + key: 'struct_with_lt_alias' + decl: 'OpAlias' + group: 'op' + }, + Operand{ + key: 'time' + decl: 'time.Time' + group: 'time' + }, + Operand{ + key: 'chan' + decl: 'chan int' + group: 'chan' + c_scalar: true + }, + Operand{ + key: 'bool' + decl: 'bool' + c_scalar: true + }, + Operand{ + key: 'bool_literal' + text: 'true' + c_scalar: true + }, + Operand{ + key: 'thread' + decl: 'thread int' + }, + Operand{ + key: 'struct' + decl: 'Foo' + }, + Operand{ + key: 'struct_alias' + decl: 'FooAlias' + }, + Operand{ + key: 'map' + decl: 'map[string]int' + }, + Operand{ + key: 'array' + decl: '[]int' + }, + Operand{ + key: 'fixed_array' + decl: '[3]int' + }, + Operand{ + key: 'sum' + decl: 'Sum' + }, + Operand{ + key: 'interface' + decl: 'Speaker' + }, + Operand{ + key: 'enum' + decl: 'Color' + c_scalar: true + }, + Operand{ + key: 'flag_enum' + decl: 'Perm' + c_scalar: true + }, + Operand{ + key: 'fn' + decl: 'fn () int' + }, +] + +struct MatrixCase { + line int + name string + valid bool +} + +// ErrorAt is an error the checker reports: its file, line, column and message. +struct ErrorAt { + file string + line int + col int + msg string +} + +fn setup_v3_cache() { + cache_dir := os.join_path(os.temp_dir(), 'v3_ordered_comparison_cache_${os.getpid()}') + if os.getenv('V3CACHE') == cache_dir { + return + } + os.rmdir_all(cache_dir) or {} + os.rm(ordered_v3_bin) or {} + os.setenv('V3CACHE', cache_dir, true) +} + +// build_v3 builds the compiler under test once per test process. +fn build_v3() string { + setup_v3_cache() + if os.is_executable(ordered_v3_bin) { + return ordered_v3_bin + } + build := + os.execute('${vexe} -gc none -path "${vlib_dir}|@vlib|@vmodules" -o ${ordered_v3_bin} ${v3_src}') + assert build.exit_code == 0, build.output + return ordered_v3_bin +} + +fn unique_temp_path(name string) string { + return os.join_path(os.temp_dir(), 'v3_ordered_${name}_${os.getpid()}_${rand.ulid()}') +} + +fn check_errors(name string, src string) []ErrorAt { + path := unique_temp_path(name) + '.v' + os.write_file(path, src) or { panic(err) } + defer { + os.rm(path) or {} + } + errors := check_file_errors(path) + for err in errors { + assert err.file == path, 'error in another file: ${err}' + } + return errors +} + +// check_file_errors runs the checker alone over the file at `path` and returns +// every error it reports, uncapped, in whichever file it is. +fn check_file_errors(path string) []ErrorAt { + v3_bin := build_v3() + result := os.execute('${v3_bin} -nocache -check -nocolor -checker-fixture ${path}') + mut errors := []ErrorAt{} + for line in result.output.split_into_lines() { + if !line.contains(': error: ') { + continue + } + location := line.all_before(': error: ').split(':') + if location.len < 3 { + continue + } + errors << ErrorAt{ + file: location[..location.len - 2].join(':') + line: location[location.len - 2].int() + col: location[location.len - 1].int() + msg: line.all_after(': error: ') + } + } + assert errors.len > 0 || result.exit_code == 0, 'no error lines in a failed check:\n${result.output}' + return errors +} + +fn run_good(name string, src string) string { + v3_bin := build_v3() + out := unique_temp_path(name) + good_src := out + '.v' + os.write_file(good_src, src) or { panic(err) } + defer { + os.rm(good_src) or {} + os.rm(out) or {} + } + compile := os.execute('${v3_bin} -nocache ${good_src} -b c -o ${out}') + assert compile.exit_code == 0, '${name}: compile failed: ${compile.output}' + assert !compile.output.contains('C compilation failed'), '${name}: C compilation failed: ${compile.output}' + run := os.execute(out) + assert run.exit_code == 0, '${name}: run failed: ${run.output}' + return run.output.trim_space() +} + +// comparison_matrix returns a program with one `return lhs op rhs` per pair of +// operands and operator, and where each of them is. +fn comparison_matrix(ops []string) (string, []MatrixCase) { + mut lines := prelude.split_into_lines() + mut cases := []MatrixCase{} + for li, lhs in operands { + for ri, rhs in operands { + for oi, op in ops { + mut params := []string{} + if lhs.decl != '' { + params << 'a ${lhs.decl}' + } + if rhs.decl != '' { + params << 'b ${rhs.decl}' + } + lines << 'fn case_${li}_${ri}_${oi}(${params.join(', ')}) bool {' + lines << '\treturn ${lhs.expr('a')} ${op} ${rhs.expr('b')}' + cases << MatrixCase{ + line: lines.len + name: '${lhs.key} ${op} ${rhs.key}' + valid: comparison_is_valid(lhs, rhs) + } + lines << '}' + lines << '' + } + } + } + lines << 'fn main() {}' + return lines.join('\n') + '\n', cases +} + +fn test_ordered_comparisons_need_operands_with_a_common_order() { + src, cases := comparison_matrix(ordered_ops) + errors := check_errors('matrix', src) + mut lines_with_errors := map[int][]string{} + for err in errors { + mut msgs := lines_with_errors[err.line] or { []string{} } + msgs << err.msg + lines_with_errors[err.line] = msgs + } + mut case_lines := map[int]bool{} + mut wrong := []string{} + for c in cases { + case_lines[c.line] = true + msgs := lines_with_errors[c.line] or { []string{} } + if c.valid && msgs.len > 0 { + wrong << '${c.name}: rejected: ${msgs}' + } else if !c.valid && msgs.len == 0 { + wrong << '${c.name}: accepted' + } + } + for err in errors { + assert err.line in case_lines, 'error outside the generated comparisons: ${err}' + } + assert wrong.len == 0, '${wrong.len} of ${cases.len} comparisons got the wrong verdict:\n${wrong.join('\n')}' +} + +// MessageCase is a function whose body is `return `, and the errors the +// checker has to report on that line, as `col: message`. +struct MessageCase { + params string + expr string + expected []string +} + +const infix_bool_msg = 'bool types only have the following operators defined: `==`, `!=`, `||`, and `&&`' + +const message_cases = [ + MessageCase{'t thread int', 't > 0', [ + '9: infix expr: cannot use `int literal` (right expression) as `thread int`', + ]}, + MessageCase{'t thread int', '0 < t', [ + '9: infix expr: cannot use `thread int` (right expression) as `int literal`', + ]}, + MessageCase{'t thread int, n int', 't <= n', [ + '9: infix expr: cannot use `int` (right expression) as `thread int`', + ]}, + MessageCase{'c chan int', 'c >= 1', [ + '9: infix expr: cannot use `int literal` (right expression) as `chan int`', + ]}, + MessageCase{'a Foo', 'a <= 0', [ + '9: infix expr: cannot use `int literal` (right expression) as `Foo`', + ]}, + // An alias without an ordering of its own is named after the type it stands for. + MessageCase{'a FooAlias', 'a > 0', [ + '9: infix expr: cannot use `int literal` (right expression) as `Foo`', + ]}, + MessageCase{'n int, s MyStr', 'n < s', [ + '9: infix expr: cannot use `string` (right expression) as `int`', + ]}, + MessageCase{'o OpAlias', 'o >= 1', [ + '9: infix expr: cannot use `int literal` (right expression) as `Op`', + ]}, + MessageCase{'m Money, s string', 'm < s', [ + '9: infix expr: cannot use `string` (right expression) as `Money`', + ]}, + MessageCase{'m map[string]int', 'm < 1', [ + '9: infix expr: cannot use `int literal` (right expression) as `map[string]int`', + ]}, + MessageCase{'a []int', 'a > 0', [ + '9: infix expr: cannot use `int literal` (right expression) as `[]int`', + ]}, + MessageCase{'a [3]int', 'a > 0', [ + '9: infix expr: cannot use `int literal` (right expression) as `[3]int`', + ]}, + MessageCase{'s Sum', 's > 0', [ + '11: cannot use operator `>` with `Sum`', + '9: infix expr: cannot use `int literal` (right expression) as `Sum`', + ]}, + MessageCase{'n int, s Sum', 'n < s', [ + '11: cannot use operator `<` with `Sum`', + '9: infix expr: cannot use `Sum` (right expression) as `int`', + ]}, + MessageCase{'s Sum, t Sum', 's < t', [ + '11: cannot use operator `<` with `Sum`', + ]}, + MessageCase{'i Speaker', 'i < 1', [ + '9: infix expr: cannot use `int literal` (right expression) as `Speaker`', + ]}, + MessageCase{'f fn () int', 'f > 0', [ + '9: mismatched types `fn () int` and `int literal`', + '9: infix expr: cannot use `int literal` (right expression) as `fn () int`', + ]}, + MessageCase{'b bool', 'b > false', [ + '11: ${infix_bool_msg}', + ]}, + MessageCase{'b bool', 'b < 1', [ + '11: ${infix_bool_msg}', + '9: infix expr: cannot use `int literal` (right expression) as `bool`', + ]}, + MessageCase{'n int, b bool', 'n >= b', [ + '9: infix expr: cannot use `bool` (right expression) as `int`', + ]}, + MessageCase{'a int, b int, c int', 'a < b < c', [ + '15: ${infix_bool_msg}', + '9: infix expr: cannot use `int` (right expression) as `bool`', + ]}, + MessageCase{'c Color', 'c > 0', [ + '9: infix expr: cannot use `int literal` (right expression) as `Color`', + ]}, + MessageCase{'s string', 's > 1', [ + '9: infix expr: cannot use `int literal` (right expression) as `string`', + ]}, + MessageCase{'r rune', "r > 'a'", [ + '9: infix expr: cannot use `string` (right expression) as `rune`', + ]}, + MessageCase{'o Op', 'o >= 0', [ + '9: infix expr: cannot use `int literal` (right expression) as `Op`', + ]}, + MessageCase{'t time.Time', 't < 0', [ + '9: infix expr: cannot use `int literal` (right expression) as `time.Time`', + ]}, + // A struct with its own `<` still takes an operand of its own type. + MessageCase{'o Op, f Foo', 'o < f', [ + '9: mismatched types `Op` and `Foo`', + '9: infix expr: cannot use `Foo` (right expression) as `Op`', + ]}, + MessageCase{'o Op, f Foo', 'o > f', [ + '9: infix expr: cannot use `Foo` (right expression) as `Op`', + ]}, + MessageCase{'o Op, f FooAlias', 'o < f', [ + '9: mismatched types `Op` and `FooAlias`', + '9: infix expr: cannot use `Foo` (right expression) as `Op`', + ]}, + MessageCase{'o Op, t thread int', 'o < t', [ + '9: infix expr: cannot use `thread int` (right expression) as `Op`', + ]}, + MessageCase{'o Op, t time.Time', 'o < t', [ + '9: infix expr: cannot use `time.Time` (right expression) as `Op`', + ]}, + MessageCase{'t time.Time, o Op', 't >= o', [ + '9: infix expr: cannot use `Op` (right expression) as `time.Time`', + ]}, + MessageCase{'p Perm', 'p < 1', [ + '9: infix expr: cannot use `int literal` (right expression) as `Perm`', + ]}, + MessageCase{'n int, p Perm', 'n < p', [ + '9: infix expr: cannot use `Perm` (right expression) as `int`', + ]}, + MessageCase{'r rune, f f64', 'r < f', []string{}}, + MessageCase{'n int, c char', 'n > c', []string{}}, + MessageCase{'c char', 'c <= 1.5', []string{}}, + MessageCase{'p voidptr, c chan int', 'p < c', []string{}}, + MessageCase{'p voidptr, b bool', 'p >= b', []string{}}, + // V1 took a `voidptr` for anything; the C compiler orders it only against + // pointers and scalars. + MessageCase{'p voidptr, f Foo', 'p < f', [ + '9: infix expr: cannot use `Foo` (right expression) as `voidptr`', + ]}, + MessageCase{'p voidptr', 'p > 1.5', [ + '9: infix expr: cannot use `float literal` (right expression) as `voidptr`', + ]}, + MessageCase{'s string, p voidptr', 's <= p', [ + '9: infix expr: cannot use `voidptr` (right expression) as `string`', + ]}, + MessageCase{'p &int, n i8', 'p < n', []string{}}, + MessageCase{'o Op, p OpAlias', 'o <= p', []string{}}, + MessageCase{'a [3]int, b [3]int', 'a < b', [ + '11: only `==` and `!=` are defined on arrays', + ]}, + MessageCase{'a []int, b [3]int', 'a <= b', [ + '11: only `==` and `!=` are defined on arrays', + '9: infix expr: cannot use `[3]int` (right expression) as `[]int`', + ]}, + // V1 let the next three through, and the C compiler rejected them. + MessageCase{'a map[string]int, b map[string]int', 'a < b', [ + '11: only `==` and `!=` are defined on maps', + ]}, + MessageCase{'a Speaker, b Speaker', 'a > b', [ + '9: undefined operation `Speaker` > `Speaker`', + ]}, + MessageCase{'a ?int', 'a < none', [ + '9: invalid operator `<` to `?int` and `none`', + ]}, + // Only a pointer compares with zero; `unsafe` does not change a struct value. + MessageCase{'a Foo', 'a == 0', [ + '9: infix expr: cannot use `int literal` (right expression) as `Foo`', + ]}, + MessageCase{'a Foo', 'unsafe { a == 0 }', [ + '18: infix expr: cannot use `int literal` (right expression) as `Foo`', + ]}, + MessageCase{'a Foo', 'unsafe { 0 != a }', [ + '18: infix expr: cannot use `Foo` (right expression) as `int literal`', + ]}, + MessageCase{'t thread int', 'unsafe { t == 0 }', [ + '18: infix expr: cannot use `int literal` (right expression) as `thread int`', + ]}, + MessageCase{'mut node Foo', 'node == 0', [ + '9: infix expr: cannot use `int literal` (right expression) as `Foo` (you can use it inside an `unsafe` block)', + ]}, + MessageCase{'mut node Foo', 'unsafe { node == 0 }', []string{}}, + MessageCase{'mut node Foo', 'unsafe { 0 != node }', []string{}}, +] + +fn test_ordered_comparison_messages_match_v1() { + mut lines := prelude.split_into_lines() + mut expr_lines := []int{} + for i, c in message_cases { + lines << 'fn message_case_${i}(${c.params}) bool {' + lines << '\treturn ${c.expr}' + expr_lines << lines.len + lines << '}' + lines << '' + } + lines << 'fn main() {}' + errors := check_errors('messages', lines.join('\n') + '\n') + mut wrong := []string{} + for i, c in message_cases { + mut got := []string{} + for err in errors { + if err.line == expr_lines[i] { + got << '${err.col}: ${err.msg}' + } + } + mut want := c.expected.clone() + got.sort() + want.sort() + if got != want { + wrong << '`${c.expr}` with (${c.params}):\n want ${want}\n got ${got}' + } + } + for err in errors { + assert err.line in expr_lines, 'error outside the checked expressions: ${err}' + } + assert wrong.len == 0, wrong.join('\n') +} + +fn test_ordered_comparisons_are_checked_in_every_context() { + // Each line marked `// bad` compares a thread handle with a number. + src := prelude + ' +fn check_threads(ts []thread int, t thread int) { + _ := ts.filter(fn (x thread int) bool { + return x > 0 // bad + }) + _ := ts.filter(it < 0) // bad + _ := ts.any(it <= 0) // bad + if t >= 0 { // bad + } + for t < 0 { // bad + } + assert t > 0 // bad + ok := t < 1 // bad + _ = ok + f := fn [t] () bool { + return t > 0 // bad + } + _ = f + _ := match true { + t > 1 { 1 } // bad + else { 0 } + } +} + +fn (f Foo) above(limit int) bool { + return f > limit // bad +} + +fn main() {} +' + mut bad_lines := []int{} + for i, line in src.split_into_lines() { + if line.ends_with('// bad') { + bad_lines << i + 1 + } + } + errors := check_errors('contexts', src) + mut lines_with_errors := map[int]bool{} + for err in errors { + assert err.line in bad_lines, 'unexpected error: ${err}' + lines_with_errors[err.line] = true + } + for line in bad_lines { + assert line in lines_with_errors, 'no error on line ${line}:\n${src.split_into_lines()[line - 1]}' + } +} + +fn test_ordered_comparisons_between_ordered_operands_compile_and_run() { + src := prelude + " +struct Point { + x int + y int +} + +fn (p Point) far(limit int) bool { + return p.x > limit || p.y >= limit +} + +fn lt[T](a T, b T) bool { + return a < b +} + +fn positive[T](a T) bool { + \$if T is \$int { + return a > 0 + } \$else \$if T is \$float { + return a > 0.0 + } \$else { + return false + } +} + +fn grow_below(mut o Op, limit Op) bool { + o = Op{o.x + 1} + return o < limit +} + +fn is_null(mut node Foo) bool { + return unsafe { node == 0 } +} + +fn (mut f Foo) is_null_receiver() bool { + return unsafe { 0 == f } +} + +fn main() { + println(lt(1, 2)) + println(lt(2.5, 1.5)) + println(lt('a', 'b')) + println(lt(Op{1}, Op{2})) + println(lt(true, false)) + println(positive(3)) + println(positive(-1.5)) + println(positive('x')) + mut nums := [3, 1, 2] + nums.sort(a < b) + println(nums) + nums.sort(a > b) + println(nums) + mut points := [Point{2, 1}, Point{1, 2}] + points.sort(a.x < b.x) + println(points.map(it.x)) + println(nums.filter(it > 1)) + println(nums.any(it >= 3)) + println(nums.all(it <= 3)) + s := Sum(5) + if s is int { + println(s > 4) + } + o := ?int(7) + if v := o { + println(v >= 7) + } + println((o or { 0 }) < 8) + mut op := Op{1} + println(grow_below(mut op, Op{3})) + mut foo := Foo{} + println(is_null(mut foo)) + println(foo.is_null_receiver()) + x := 3 + p := &x + println(p > 0) + c1 := chan int{} + c2 := c1 + println(c1 <= c2) + println(Point{3, 4}.far(3)) + println(MyInt(3) > 2) + println(`a` < `b`) + println(i64(-1) < 0) + println(u8(200) >= 100) + println(1.5 > 1) + println(f32(1.5) < 2.5) + println('abc' < 'abd') + println(MyStr('a') < 'b') + println(epoch() < time.unix(2)) + println(Op{3} >= Op{2}) + println(OpAlias(Op{1}) <= Op{1}) + m := {'a': 1} + println(m['a'] > 0) + println(nums.len > 2) + \$for f in Point.fields { + \$if f.typ is int { + println(Point{5, 6}.\$(f.name) > 5) + } + } + a, b := 1, 2 + println(a < b && b < 3) + println(match a { + 1 { 10 } + else { 20 } + } > 5) + unsafe { + q := &x + println(p <= q) + } +} +" + assert run_good('valid', src).split_into_lines() == [ + 'true', + 'false', + 'true', + 'true', + 'false', + 'true', + 'false', + 'false', + '[1, 2, 3]', + '[3, 2, 1]', + '[1, 2]', + '[3, 2]', + 'true', + 'true', + 'true', + 'true', + 'true', + 'true', + 'false', + 'false', + 'true', + 'true', + 'true', + 'true', + 'true', + 'true', + 'true', + 'true', + 'true', + 'true', + 'true', + 'true', + 'true', + 'true', + 'true', + 'true', + 'false', + 'true', + 'true', + 'true', + 'true', + ] +} + +fn test_sort_comparators_report_each_mismatch_once() { + src := prelude + " +struct Named { + x int + name string +} + +fn sort_threads(mut lines []string) { + lines.sort((go a.split('/').last()) < b.split('/').last()) +} + +fn sort_named(mut items []Named) { + items.sort(a.x < b.name) +} + +fn main() {} +" + lines := src.split_into_lines() + threads_line := lines.index("\tlines.sort((go a.split('/').last()) < b.split('/').last())") + 1 + named_line := lines.index('\titems.sort(a.x < b.name)') + 1 + errors := check_errors('sort', src) + // `.sort()` reports a thread handle on the left of its comparison itself. + thread_mismatches := errors.filter(it.line == threads_line && it.msg.starts_with('infix expr')) + assert thread_mismatches.len == 1, errors.str() + assert thread_mismatches[0].msg == 'infix expr: cannot use `string` (right expression) as `thread string`' + assert errors.any(it.line == named_line + && it.msg == 'infix expr: cannot use `string` (right expression) as `int`'), errors.str() +} + +fn test_ordered_comparisons_in_templates() { + // The code of a template is checked as part of the function that renders + // it, with positions that belong to the template rather than to the V file. + dir := unique_temp_path('templates') + os.mkdir_all(dir) or { panic(err) } + defer { + os.rmdir_all(dir) or {} + } + os.write_file(os.join_path(dir, 'good.html'), "@if n > 0\npositive\n@end\n@if name < 'm'\nearly\n@end\n") or { + panic(err) + } + good_src := os.join_path(dir, 'good.v') + os.write_file(good_src, "module main\n\nfn page(n int, name string) string {\n\treturn \$tmpl('good.html')\n}\n\nfn main() {\n\tprint(page(1, 'a'))\n}\n") or { + panic(err) + } + good_bin := os.join_path(dir, 'good') + compile := os.execute('${build_v3()} -nocache ${good_src} -b c -o ${good_bin}') + assert compile.exit_code == 0, compile.output + run := os.execute(good_bin) + assert run.exit_code == 0, run.output + assert run.output.split_into_lines().map(it.trim_space()).filter(it != '') == [ + 'positive', + 'early', + ] + + os.write_file(os.join_path(dir, 'bad.html'), '@if name > 1\nlate\n@end\n') or { panic(err) } + bad_src := os.join_path(dir, 'bad.v') + os.write_file(bad_src, "module main\n\nfn page(name string) string {\n\treturn \$tmpl('bad.html')\n}\n\nfn main() {\n\tprint(page('a'))\n}\n") or { + panic(err) + } + errors := check_file_errors(bad_src) + assert errors.len == 1, errors.str() + assert errors[0].file.ends_with('bad.html') && errors[0].line == 1 && errors[0].col == 5, errors.str() + assert errors[0].msg.starts_with('infix expr: cannot use `int literal` (right expression) as `string`'), errors.str() +} + +fn test_translated_files_keep_c_comparisons() { + errors := check_errors('translated', '@[translated] +module main + +enum Level { + low + high +} + +fn above(l Level, n int) bool { + return l > n +} + +fn main() {} +') + assert errors.len == 0, errors.str() +} diff --git a/vlib/v/types/checker.v b/vlib/v/types/checker.v index 4fc5ceeddbd1fa..0fab2320091331 100644 --- a/vlib/v/types/checker.v +++ b/vlib/v/types/checker.v @@ -799,6 +799,7 @@ pub mut: cur_module string cur_file string unsafe_depth int + sort_comparator_depth int lock_depth int comptime_static_depth int errors []TypeError diff --git a/vlib/v/types/checker_comptime.v b/vlib/v/types/checker_comptime.v index ec79a54f550da3..6ed1cd84cea7de 100644 --- a/vlib/v/types/checker_comptime.v +++ b/vlib/v/types/checker_comptime.v @@ -7456,15 +7456,17 @@ fn (mut tc TypeChecker) check_infix(id flat.NodeId, node flat.Node) { compatible := if lhs_is_sum != rhs_is_sum { false } else { - tc.type_compatible(lhs_type, rhs_type) || tc.type_compatible(rhs_type, lhs_type) - || tc.expr_compatible(lhs_id, lhs_type, rhs_type) - || tc.expr_compatible(rhs_id, rhs_type, lhs_type) + tc.infix_operands_compatible(lhs_id, lhs_type, rhs_id, rhs_type) || pointer_value_comparison_allowed || pointer_integer_zero_comparison || c_literal_scalar_comparison } + // Only a struct reached through a pointer, such as a `mut` parameter, + // compares with zero: `unsafe` does not turn a struct value into one. + lhs_is_struct_reference := lhs_clean is Struct && tc.infix_operand_is_auto_deref(lhs_id) + rhs_is_struct_reference := rhs_clean is Struct && tc.infix_operand_is_auto_deref(rhs_id) unsafe_zero_struct_comparison := - ((lhs_clean is Struct && tc.zero_literal_expr_id(rhs_id) != none) - || (rhs_clean is Struct && tc.zero_literal_expr_id(lhs_id) != none)) + ((lhs_is_struct_reference && tc.zero_literal_expr_id(rhs_id) != none) + || (rhs_is_struct_reference && tc.zero_literal_expr_id(lhs_id) != none)) && (tc.unsafe_depth > 0 || tc.expr_is_inside_unsafe_block(id)) if !compatible && !unsafe_zero_struct_comparison && lhs_node.kind !in [.none_expr, .nil_literal] @@ -7476,7 +7478,7 @@ fn (mut tc TypeChecker) check_infix(id flat.NodeId, node flat.Node) { tc.diagnostic_expr_type_name(lhs_id, lhs_type) } rhs_name := tc.diagnostic_expr_type_name(rhs_id, rhs_type) - suffix := if tc.unsafe_depth == 0 && lhs_clean is Struct + suffix := if tc.unsafe_depth == 0 && lhs_is_struct_reference && tc.zero_literal_expr_id(rhs_id) != none { ' (you can use it inside an `unsafe` block)' } else if tc.unsafe_depth == 0 && pointer_value_comparison { @@ -7550,9 +7552,24 @@ fn (mut tc TypeChecker) check_infix(id flat.NodeId, node flat.Node) { tc.record_error_at(.condition_mismatch, 'mismatched types `${lhs_name}` and `${rhs_name}`', id, diagnostic_pos) tc.record_error_at(.condition_mismatch, 'infix expr: cannot use `${rhs_name}` (right expression) as `${lhs_name}`', id, diagnostic_pos) } + return + } + // The `<` of the left struct takes an operand of its own type. V1 also + // names the pair for `<` when the right struct has no `<` of its own. + if tc.type_name(lhs_order_type) != tc.type_name(rhs_order_type) + && !tc.translated_files[tc.cur_file] { + if node.op == .lt && !tc.type_has_infix_operator_method(rhs_order_type, required_op) + && tc.thread_wait_return_type(rhs_type) == none { + tc.record_error_at(.condition_mismatch, 'mismatched types `${tc.diagnostic_expr_type_name(lhs_id, lhs_type)}` and `${tc.diagnostic_expr_type_name(rhs_id, rhs_type)}`', id, node.pos) + } + tc.record_ordered_operands_mismatch(id, node, lhs_id, lhs_type, rhs_id, rhs_type) } return } + if node.op in [.lt, .gt, .le, .ge] { + tc.check_ordered_comparison(id, node, lhs_id, lhs_type, rhs_id, rhs_type) + return + } if node.op == .arrow { channel_type := unalias_and_unwrap_pointer_type(lhs_type) if channel_type !is Channel { @@ -7787,6 +7804,131 @@ fn (mut tc TypeChecker) check_infix(id flat.NodeId, node flat.Node) { tc.record_error(.assignment_mismatch, 'infix expr: cannot use `${rhs_name}` (right expression) as `${lhs_name}`', id) } +// infix_operands_compatible reports whether either operand of a comparison +// accepts the type of the other one. +fn (tc &TypeChecker) infix_operands_compatible(lhs_id flat.NodeId, lhs_type Type, rhs_id flat.NodeId, rhs_type Type) bool { + return tc.type_compatible(lhs_type, rhs_type) || tc.type_compatible(rhs_type, lhs_type) + || tc.expr_compatible(lhs_id, lhs_type, rhs_type) + || tc.expr_compatible(rhs_id, rhs_type, lhs_type) +} + +// infix_operand_is_auto_deref reports whether the operand `id` is a `mut` +// parameter, or another binding that the generated C reaches through a +// pointer: comparing it compares that pointer. +fn (tc &TypeChecker) infix_operand_is_auto_deref(id flat.NodeId) bool { + node := tc.a.node(id) + return node.kind == .ident + && tc.mut_param_base_for_current_ident(node.value, tc.resolve_type(id)) != none +} + +// ordered_operand_name names an operand of `<`, `>`, `<=` or `>=` as V1 does: +// an alias without an ordering of its own is named after the type it stands for. +fn (tc &TypeChecker) ordered_operand_name(id flat.NodeId, typ Type) string { + if typ is Alias && !tc.type_has_infix_operator_method(typ, .lt) { + return tc.diagnostic_expr_type_name(id, unalias_type(typ)) + } + return tc.diagnostic_expr_type_name(id, typ) +} + +// check_ordered_comparison reports `<`, `>`, `<=` and `>=` between operands +// without an order in common. The C backend compares them as they are, so a +// struct, map or sum type would only fail in the C compiler, and a bool, enum, +// channel or fn value would compile into a comparison that means nothing. +fn (mut tc TypeChecker) check_ordered_comparison(id flat.NodeId, node flat.Node, lhs_id flat.NodeId, lhs_type Type, rhs_id flat.NodeId, rhs_type Type) { + if lhs_type is Unknown || rhs_type is Unknown || tc.translated_files[tc.cur_file] { + return + } + lhs_clean := unalias_type(lhs_type) + rhs_clean := unalias_type(rhs_type) + // Numbers compare across their types, as V1 has it: most comparisons end here. + if ordered_number(lhs_clean) && ordered_number(rhs_clean) { + return + } + // `.sort()` reports a thread handle on the left of its comparison itself. + if tc.sort_comparator_depth > 0 && tc.thread_wait_return_type(lhs_type) != none { + return + } + op := infix_operator_name(node.op) or { return } + lhs_is_none := tc.a.node(lhs_id).kind == .none_expr + rhs_is_none := tc.a.node(rhs_id).kind == .none_expr + if lhs_is_none || rhs_is_none { + lhs_name := if lhs_is_none { 'none' } else { tc.ordered_operand_name(lhs_id, lhs_type) } + rhs_name := if rhs_is_none { 'none' } else { tc.ordered_operand_name(rhs_id, rhs_type) } + tc.record_error_at(.condition_mismatch, 'invalid operator `${op}` to `${lhs_name}` and `${rhs_name}`', id, node.pos) + return + } + lhs_is_array := array_type_from_receiver(lhs_type) != none || lhs_clean is ArrayFixed + rhs_is_array := array_type_from_receiver(rhs_type) != none || rhs_clean is ArrayFixed + if lhs_is_array && rhs_is_array { + tc.record_error_at(.assignment_mismatch, 'only `==` and `!=` are defined on arrays', id, tc.infix_operator_pos(node, op)) + if tc.type_name(lhs_clean) != tc.type_name(rhs_clean) { + tc.record_ordered_operands_mismatch(id, node, lhs_id, lhs_type, rhs_id, rhs_type) + } + return + } else if lhs_clean is Map && rhs_clean is Map { + tc.record_error_at(.assignment_mismatch, 'only `==` and `!=` are defined on maps', id, tc.infix_operator_pos(node, op)) + } + if (fn_type_from_type(lhs_clean) != none) != (fn_type_from_type(rhs_clean) != none) { + tc.record_error_at(.condition_mismatch, 'mismatched types `${tc.diagnostic_expr_type_name(lhs_id, lhs_type)}` and `${tc.diagnostic_expr_type_name(rhs_id, rhs_type)}`', id, node.pos) + } + if tc.type_name(lhs_type) == 'bool' { + tc.record_error_at(.condition_mismatch, 'bool types only have the following operators defined: `==`, `!=`, `||`, and `&&`', id, tc.infix_operator_pos(node, op)) + } + if lhs_clean is SumType { + tc.record_error_at(.condition_mismatch, 'cannot use operator `${op}` with `${tc.ordered_operand_name(lhs_id, lhs_type)}`', id, tc.infix_operator_pos(node, op)) + } else if rhs_clean is SumType { + tc.record_error_at(.condition_mismatch, 'cannot use operator `${op}` with `${tc.ordered_operand_name(rhs_id, rhs_type)}`', id, tc.infix_operator_pos(node, op)) + } + // A `voidptr` compares with whatever the C compiler can order it with. An + // enum compares only with an enum, which the enum check before this one handles. + compatible := if (is_voidptr_type(lhs_clean) && voidptr_orders_with(rhs_clean)) + || (is_voidptr_type(rhs_clean) && voidptr_orders_with(lhs_clean)) { + true + } else if (lhs_clean is Enum) != (rhs_clean is Enum) || (lhs_clean is SumType) != (rhs_clean is SumType) { + false + } else { + tc.infix_operands_compatible(lhs_id, lhs_type, rhs_id, rhs_type) + } + // A pointer against an integer is pointer arithmetic, as V1 has it. + pointer_and_integer := (lhs_clean is Pointer && ordered_integer(rhs_clean)) + || (rhs_clean is Pointer && ordered_integer(lhs_clean)) + if !compatible && !pointer_and_integer { + tc.record_ordered_operands_mismatch(id, node, lhs_id, lhs_type, rhs_id, rhs_type) + } else if compatible && (lhs_clean is Interface || rhs_clean is Interface) { + tc.record_error_at(.condition_mismatch, 'undefined operation `${tc.ordered_operand_name(lhs_id, lhs_type)}` ${op} `${tc.ordered_operand_name(rhs_id, rhs_type)}`', id, node.pos) + } +} + +// record_ordered_operands_mismatch reports the operands of `<`, `>`, `<=` or +// `>=` that do not accept each other, in the words of V1. +fn (mut tc TypeChecker) record_ordered_operands_mismatch(id flat.NodeId, node flat.Node, lhs_id flat.NodeId, lhs_type Type, rhs_id flat.NodeId, rhs_type Type) { + tc.record_error_at(.assignment_mismatch, 'infix expr: cannot use `${tc.ordered_operand_name(rhs_id, rhs_type)}` (right expression) as `${tc.ordered_operand_name(lhs_id, lhs_type)}`', id, node.pos) +} + +// ordered_integer reports whether the unaliased type `t` is an integer in the +// generated C, `char` included. +fn ordered_integer(t Type) bool { + return t.is_integer() || t is Char +} + +// ordered_number reports whether the unaliased type `t` is a number that `<` +// compares with any other number. +fn ordered_number(t Type) bool { + return ordered_integer(t) || t.is_float() +} + +fn is_voidptr_type(t Type) bool { + return t is Pointer && t.base_type is Void +} + +// voidptr_orders_with reports whether the C compiler orders a `voidptr` against +// the unaliased type `t`: a pointer, channel, integer, bool or enum, and not an +// aggregate or a float. +fn voidptr_orders_with(t Type) bool { + return t is Pointer || t is Channel || t is Enum || ordered_integer(t) + || (t is Primitive && t.props.has(.boolean)) +} + fn (tc &TypeChecker) integer_shift_bit_size(typ Type) int { clean := unalias_type(typ) if clean is Primitive { @@ -8305,7 +8447,9 @@ fn (tc &TypeChecker) infix_operator_pos(node flat.Node, op string) token.Pos { source := tc.source_texts_by_file[file.name] or { return node.pos } start := int_max(lhs.pos.end, node.pos.offset) end := int_min(rhs.pos.offset, node.pos.end) - if start < end { + // Template code can carry offsets past the end of the V file it is + // attributed to. + if start >= 0 && start < end && end <= source.len { if relative := source[start..end].index(op) { op_start := start + relative return token.new_span(node.pos.id, op_start, op_start + op.len) diff --git a/vlib/v/types/checker_tail.v b/vlib/v/types/checker_tail.v index c0c62d03fcf5b7..50a357146446d5 100644 --- a/vlib/v/types/checker_tail.v +++ b/vlib/v/types/checker_tail.v @@ -15317,7 +15317,9 @@ fn (mut tc TypeChecker) check_array_sort_call(id flat.NodeId, node flat.Node, ca arg_id, method_pos) } tc.push_array_dsl_scope(node, 'array.sort') + tc.sort_comparator_depth++ tc.check_node(arg_id) + tc.sort_comparator_depth-- if invalid_id := tc.sort_first_invalid_ident(arg_id) { invalid := tc.a.node(invalid_id) mut error_index := tc.errors.len diff --git a/vlib/v/types/checker_tail_stmt.v b/vlib/v/types/checker_tail_stmt.v index 42d32b9e52b0ee..755eb2eb819456 100644 --- a/vlib/v/types/checker_tail_stmt.v +++ b/vlib/v/types/checker_tail_stmt.v @@ -1315,6 +1315,11 @@ fn (mut tc TypeChecker) check_match_stmt(id flat.NodeId, node flat.Node) { } for j in 0 .. n_conds { cond_id := tc.a.child(branch, j) + // A condition that is an expression, as in `match true { a < b {} }`, + // gets the checks an `if` condition gets. + if tc.a.node(cond_id).kind == .infix { + tc.check_node(cond_id) + } tc.check_match_range_types(subject_id, subject_type, cond_id) tc.check_match_condition_type(subject_type, cond_id) tc.check_match_alias_condition(subject_declared_type, cond_id) From e11c223dd25747167ff6fa37a27afd83b23050d6 Mon Sep 17 00:00:00 2001 From: MARCROCK22 Date: Tue, 22 Sep 2026 11:39:44 -0400 Subject: [PATCH 2/3] checker: check match conditions that wrap a comparison, quote test paths A match condition such as `!(t > 1)`, `(t > 1)` or `is_true(t > 1)` has a prefix, parenthesis or call node at its root, so the comparison inside it was still left unchecked. Every condition that is an expression now gets the checks of an `if` condition; names, which can stand for types, type patterns, ranges, enum shorthands and `none` keep the checks of their own. The test helpers quote the paths they pass to the shell, so a checkout or a temporary directory whose path has spaces works. --- .../ordered_comparison_operands_test.v | 33 +++++++++++++++---- vlib/v/types/checker_tail_stmt.v | 17 ++++++++-- 2 files changed, 41 insertions(+), 9 deletions(-) diff --git a/vlib/v/compiler_tests/ordered_comparison_operands_test.v b/vlib/v/compiler_tests/ordered_comparison_operands_test.v index 24cef688162b47..fcae6ce5ff2a9a 100644 --- a/vlib/v/compiler_tests/ordered_comparison_operands_test.v +++ b/vlib/v/compiler_tests/ordered_comparison_operands_test.v @@ -317,7 +317,7 @@ fn build_v3() string { return ordered_v3_bin } build := - os.execute('${vexe} -gc none -path "${vlib_dir}|@vlib|@vmodules" -o ${ordered_v3_bin} ${v3_src}') + os.execute('${os.quoted_path(vexe)} -gc none -path ${os.quoted_path('${vlib_dir}|@vlib|@vmodules')} -o ${os.quoted_path(ordered_v3_bin)} ${os.quoted_path(v3_src)}') assert build.exit_code == 0, build.output return ordered_v3_bin } @@ -343,7 +343,8 @@ fn check_errors(name string, src string) []ErrorAt { // every error it reports, uncapped, in whichever file it is. fn check_file_errors(path string) []ErrorAt { v3_bin := build_v3() - result := os.execute('${v3_bin} -nocache -check -nocolor -checker-fixture ${path}') + result := + os.execute('${os.quoted_path(v3_bin)} -nocache -check -nocolor -checker-fixture ${os.quoted_path(path)}') mut errors := []ErrorAt{} for line in result.output.split_into_lines() { if !line.contains(': error: ') { @@ -373,10 +374,11 @@ fn run_good(name string, src string) string { os.rm(good_src) or {} os.rm(out) or {} } - compile := os.execute('${v3_bin} -nocache ${good_src} -b c -o ${out}') + compile := + os.execute('${os.quoted_path(v3_bin)} -nocache ${os.quoted_path(good_src)} -b c -o ${os.quoted_path(out)}') assert compile.exit_code == 0, '${name}: compile failed: ${compile.output}' assert !compile.output.contains('C compilation failed'), '${name}: C compilation failed: ${compile.output}' - run := os.execute(out) + run := os.execute(os.quoted_path(out)) assert run.exit_code == 0, '${name}: run failed: ${run.output}' return run.output.trim_space() } @@ -670,10 +672,17 @@ fn check_threads(ts []thread int, t thread int) { _ = f _ := match true { t > 1 { 1 } // bad + !(t > 1) { 2 } // bad + (t < 0) { 3 } // bad + is_true(t >= 0) { 4 } // bad else { 0 } } } +fn is_true(b bool) bool { + return b +} + fn (f Foo) above(limit int) bool { return f > limit // bad } @@ -805,6 +814,15 @@ fn main() { q := &x println(p <= q) } + n := 5 + println(match true { + !(n > 10) { 'small' } + else { 'big' } + }) + println(match true { + (n >= 5) { 'five or more' } + else { 'less' } + }) } " assert run_good('valid', src).split_into_lines() == [ @@ -849,6 +867,8 @@ fn main() { 'true', 'true', 'true', + 'small', + 'five or more', ] } @@ -897,9 +917,10 @@ fn test_ordered_comparisons_in_templates() { panic(err) } good_bin := os.join_path(dir, 'good') - compile := os.execute('${build_v3()} -nocache ${good_src} -b c -o ${good_bin}') + compile := + os.execute('${os.quoted_path(build_v3())} -nocache ${os.quoted_path(good_src)} -b c -o ${os.quoted_path(good_bin)}') assert compile.exit_code == 0, compile.output - run := os.execute(good_bin) + run := os.execute(os.quoted_path(good_bin)) assert run.exit_code == 0, run.output assert run.output.split_into_lines().map(it.trim_space()).filter(it != '') == [ 'positive', diff --git a/vlib/v/types/checker_tail_stmt.v b/vlib/v/types/checker_tail_stmt.v index 755eb2eb819456..9fcad7cc6156bc 100644 --- a/vlib/v/types/checker_tail_stmt.v +++ b/vlib/v/types/checker_tail_stmt.v @@ -1315,9 +1315,9 @@ fn (mut tc TypeChecker) check_match_stmt(id flat.NodeId, node flat.Node) { } for j in 0 .. n_conds { cond_id := tc.a.child(branch, j) - // A condition that is an expression, as in `match true { a < b {} }`, - // gets the checks an `if` condition gets. - if tc.a.node(cond_id).kind == .infix { + // A condition that is an expression, as in `match true { a < b {} }` + // or `!(a < b)`, gets the checks an `if` condition gets. + if tc.match_condition_is_expression(cond_id) { tc.check_node(cond_id) } tc.check_match_range_types(subject_id, subject_type, cond_id) @@ -1543,6 +1543,17 @@ fn (tc &TypeChecker) match_trailing_or_parent(id flat.NodeId) ?flat.NodeId { return parent_id } +// match_condition_is_expression reports whether the match condition `id` is an +// expression, such as a comparison, a call or a literal. A name can stand for a +// type, and ranges, enum shorthands and `none` have checks of their own. +fn (tc &TypeChecker) match_condition_is_expression(id flat.NodeId) bool { + node := tc.a.node(id) + if node.kind in [.ident, .selector, .range, .enum_val, .none_expr] { + return false + } + return tc.match_type_pattern(node) == none +} + fn (mut tc TypeChecker) check_match_branch_structure(id flat.NodeId, node flat.Node) bool { mut else_ids := []flat.NodeId{} mut non_else_count := 0 From 377ac1fdf3eb8e24e77c2ebc8bf4066d12d1b557 Mon Sep 17 00:00:00 2001 From: MARCROCK22 Date: Tue, 22 Sep 2026 17:44:13 -0400 Subject: [PATCH 3/3] checker: check match conditions that select a field of an expression A match condition such as `predicate(t > 1).value` has a selector at its root, and every selector was left unchecked because `mod.Type` is one too. Only a chain of names, which can be a qualified type, is left out now; a selector on a call, a struct initializer, an index or any other expression gets the checks of an `if` condition, so a comparison in its receiver is reported. The contexts test also checks that each error is a mismatch of the operands, reported at the comparison. --- .../ordered_comparison_operands_test.v | 76 ++++++++++++++++++- vlib/v/types/checker_tail_stmt.v | 10 ++- 2 files changed, 80 insertions(+), 6 deletions(-) diff --git a/vlib/v/compiler_tests/ordered_comparison_operands_test.v b/vlib/v/compiler_tests/ordered_comparison_operands_test.v index fcae6ce5ff2a9a..726e73daf4c0d3 100644 --- a/vlib/v/compiler_tests/ordered_comparison_operands_test.v +++ b/vlib/v/compiler_tests/ordered_comparison_operands_test.v @@ -70,6 +70,26 @@ fn (a Money) < (b Money) bool { fn epoch() time.Time { return time.unix(0) } + +struct Predicate { + value bool +} + +struct Wrapper { + inner Predicate +} + +fn predicate(value bool) Predicate { + return Predicate{ + value: value + } +} + +fn wrap(value bool) Wrapper { + return Wrapper{ + inner: predicate(value) + } +} " // Operand is one side of a generated comparison: a parameter of type `decl`, @@ -653,7 +673,7 @@ fn test_ordered_comparison_messages_match_v1() { fn test_ordered_comparisons_are_checked_in_every_context() { // Each line marked `// bad` compares a thread handle with a number. src := prelude + ' -fn check_threads(ts []thread int, t thread int) { +fn check_threads(ts []thread int, t thread int, preds []Predicate) { _ := ts.filter(fn (x thread int) bool { return x > 0 // bad }) @@ -675,6 +695,10 @@ fn check_threads(ts []thread int, t thread int) { !(t > 1) { 2 } // bad (t < 0) { 3 } // bad is_true(t >= 0) { 4 } // bad + predicate(t > 1).value { 5 } // bad + (Predicate{value: t > 1}).value { 6 } // bad + wrap(t > 1).inner.value { 7 } // bad + preds[if t > 1 { 0 } else { 1 }].value { 8 } // bad else { 0 } } } @@ -689,8 +713,9 @@ fn (f Foo) above(limit int) bool { fn main() {} ' + lines := src.split_into_lines() mut bad_lines := []int{} - for i, line in src.split_into_lines() { + for i, line in lines { if line.ends_with('// bad') { bad_lines << i + 1 } @@ -699,13 +724,30 @@ fn main() {} mut lines_with_errors := map[int]bool{} for err in errors { assert err.line in bad_lines, 'unexpected error: ${err}' + // The comparison itself is reported, as a mismatch of its operands. + assert err.msg.starts_with('infix expr: cannot use '), 'not an operand mismatch: ${err}' + assert err.col == comparison_column(lines[err.line - 1]), 'not at the comparison: ${err}' lines_with_errors[err.line] = true } for line in bad_lines { - assert line in lines_with_errors, 'no error on line ${line}:\n${src.split_into_lines()[line - 1]}' + assert line in lines_with_errors, 'no error on line ${line}:\n${lines[line - 1]}' } } +// comparison_column returns the column of the left operand of the ordered +// comparison on `line`, where the checker reports a mismatch of the operands. +fn comparison_column(line string) int { + for op in [' < ', ' > ', ' <= ', ' >= '] { + idx := line.index(op) or { continue } + mut start := idx + for start > 0 && (line[start - 1].is_alnum() || line[start - 1] == `_`) { + start-- + } + return start + 1 + } + return 0 +} + fn test_ordered_comparisons_between_ordered_operands_compile_and_run() { src := prelude + " struct Point { @@ -713,6 +755,8 @@ struct Point { y int } +type Stamp = int | time.Time + fn (p Point) far(limit int) bool { return p.x > limit || p.y >= limit } @@ -823,6 +867,28 @@ fn main() { (n >= 5) { 'five or more' } else { 'less' } }) + pred := predicate(n > 1) + println(match true { + pred.value { 'field' } + else { 'no field' } + }) + println(match true { + predicate(n > 10).value { 'call' } + (Predicate{value: n < 1}).value { 'struct' } + wrap(n >= 5).inner.value { 'wrapped' } + else { 'none' } + }) + preds := [pred, predicate(n < 1)] + println(match true { + preds[if n > 1 { 1 } else { 0 }].value { 'second' } + preds[0].value { 'first' } + else { 'neither' } + }) + stamp := Stamp(epoch()) + println(match stamp { + time.Time { 'time' } + int { 'int' } + }) } " assert run_good('valid', src).split_into_lines() == [ @@ -869,6 +935,10 @@ fn main() { 'true', 'small', 'five or more', + 'field', + 'wrapped', + 'first', + 'time', ] } diff --git a/vlib/v/types/checker_tail_stmt.v b/vlib/v/types/checker_tail_stmt.v index 9fcad7cc6156bc..3887c7ae01d61e 100644 --- a/vlib/v/types/checker_tail_stmt.v +++ b/vlib/v/types/checker_tail_stmt.v @@ -1544,10 +1544,14 @@ fn (tc &TypeChecker) match_trailing_or_parent(id flat.NodeId) ?flat.NodeId { } // match_condition_is_expression reports whether the match condition `id` is an -// expression, such as a comparison, a call or a literal. A name can stand for a -// type, and ranges, enum shorthands and `none` have checks of their own. +// expression, such as a comparison, a call or a literal. A name, qualified or +// not, can stand for a type, and ranges, enum shorthands and `none` have checks +// of their own. A field of an expression, as in `f(a < b).x`, is an expression. fn (tc &TypeChecker) match_condition_is_expression(id flat.NodeId) bool { - node := tc.a.node(id) + mut node := tc.a.node(id) + for node.kind == .selector && node.children_count > 0 { + node = tc.a.child_node(node, 0) + } if node.kind in [.ident, .selector, .range, .enum_val, .none_expr] { return false }