From fa2514901c5251049cb50b84d37cbc42b4d0de96 Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Sun, 19 Jul 2026 16:00:43 +0300 Subject: [PATCH 01/16] checker: fix or-unwrapping an option map field/var/index (fix #27867) An `or {}` block unwraps an option/result into a fresh value, so the unwrapped expression must not be treated as an lvalue that aliases the original map. Previously `x := c.some_map or { ... }` (and the variable and index-expression forms) wrongly triggered the `cannot copy map: call `move` or `clone` method` error, even though the equivalent `get() or {}` call and option array field cases already compiled. --- vlib/v/checker/assign.v | 12 +++- .../options/option_map_field_or_unwrap_test.v | 55 +++++++++++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 vlib/v/tests/options/option_map_field_or_unwrap_test.v diff --git a/vlib/v/checker/assign.v b/vlib/v/checker/assign.v index 94561a954fa5c5..4aed166e47ac26 100644 --- a/vlib/v/checker/assign.v +++ b/vlib/v/checker/assign.v @@ -881,7 +881,17 @@ or use an explicit `unsafe{ a[..] }`, if you do not want a copy of the slice.', } } } - right_is_lvalue := if right is ast.ComptimeSelector { + // An `or {}` block unwraps an option/result into a fresh value, so the + // unwrapped expression is not an lvalue that aliases the original map + // (see vlang/v issue #27867). This mirrors the already allowed `get() or {}` + // call and option array field cases. + right_has_or_block := match right { + ast.Ident { right.or_expr.kind != .absent } + ast.IndexExpr { right.or_expr.kind != .absent } + ast.SelectorExpr { right.or_block.kind != .absent } + else { false } + } + right_is_lvalue := !right_has_or_block && if right is ast.ComptimeSelector { right.left.is_lvalue() } else { right.is_lvalue() diff --git a/vlib/v/tests/options/option_map_field_or_unwrap_test.v b/vlib/v/tests/options/option_map_field_or_unwrap_test.v new file mode 100644 index 00000000000000..3806b8d67c0d79 --- /dev/null +++ b/vlib/v/tests/options/option_map_field_or_unwrap_test.v @@ -0,0 +1,55 @@ +// Regression test for https://github.com/vlang/v/issues/27867 +// or-unwrapping an option map (struct field, variable, index) used to fail +// with `cannot copy map: call move or clone method (or use a reference)`. +struct Translation { + title string +} + +struct Category { + title string + translations ?map[string]Translation +} + +fn test_option_map_field_or_unwrap() { + c := Category{ + translations: { + 'en': Translation{ + title: 'Hello' + } + } + } + translations := c.translations or { panic('expected') } + assert translations['en'].title == 'Hello' +} + +fn test_option_map_field_or_unwrap_none() { + c := Category{} + translations := c.translations or { + map[string]Translation{} + } + + assert translations.len == 0 +} + +fn get_option_map() ?map[string]int { + return { + 'a': 1 + } +} + +fn test_option_map_var_or_unwrap() { + x := get_option_map() + y := x or { panic('expected') } + assert y['a'] == 1 +} + +fn test_option_map_index_or_unwrap() { + inner_map := { + 'b': 2 + } + m := { + 'a': inner_map + } + inner := m['a'] or { panic('expected') } + assert inner['b'] == 2 +} From 756eced9a84de4958132c26b6334c9d8b90b5704 Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Sun, 19 Jul 2026 23:51:04 +0300 Subject: [PATCH 02/16] =?UTF-8?q?checker:=20address=20review=20=E2=80=94?= =?UTF-8?q?=20only=20relax=20the=20map-copy=20guard=20for=20immutable=20or?= =?UTF-8?q?-unwrap?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the review on #27870. The previous fix exempted every checked `or {}` unwrap of a map from the copy guard, which let a *mutable* destination silently alias container/field storage, e.g. `mut y := xs[0] or { panic('') }; y['x'] = 2` mutated `xs[0]`. Mirror V's array guard, which is mutability-aware: relax the map guard only for an immutable declaration (`x := opt_map or { ... }`), where `x` can never become a mutable alias. Mutable destinations (`mut x := m[k] or { ... }`, `x = m[k] or { ... }`) keep requiring `clone`/`move`. Parenthesized unwraps are handled via `remove_par()`, so `(c.m) or { ... }` compiles too. Adds a mutation-based checker error test for the mutable index/selector forms, and extends the runtime test with the parenthesized case. --- vlib/v/checker/assign.v | 31 ++++++++++++------- .../option_map_or_unwrap_mut_alias_err.out | 14 +++++++++ .../option_map_or_unwrap_mut_alias_err.vv | 29 +++++++++++++++++ .../options/option_map_field_or_unwrap_test.v | 19 ++++++++++-- 4 files changed, 79 insertions(+), 14 deletions(-) create mode 100644 vlib/v/checker/tests/option_map_or_unwrap_mut_alias_err.out create mode 100644 vlib/v/checker/tests/option_map_or_unwrap_mut_alias_err.vv diff --git a/vlib/v/checker/assign.v b/vlib/v/checker/assign.v index 4aed166e47ac26..4bc5535d8ab395 100644 --- a/vlib/v/checker/assign.v +++ b/vlib/v/checker/assign.v @@ -881,23 +881,30 @@ or use an explicit `unsafe{ a[..] }`, if you do not want a copy of the slice.', } } } - // An `or {}` block unwraps an option/result into a fresh value, so the - // unwrapped expression is not an lvalue that aliases the original map - // (see vlang/v issue #27867). This mirrors the already allowed `get() or {}` - // call and option array field cases. - right_has_or_block := match right { - ast.Ident { right.or_expr.kind != .absent } - ast.IndexExpr { right.or_expr.kind != .absent } - ast.SelectorExpr { right.or_block.kind != .absent } - else { false } - } - right_is_lvalue := !right_has_or_block && if right is ast.ComptimeSelector { + right_is_lvalue := if right is ast.ComptimeSelector { right.left.is_lvalue() } else { right.is_lvalue() } + // `x := opt_map or { ... }` unwraps an option/result into a new immutable + // variable, so `x` can never become a mutable alias of the underlying map + // and the shallow copy is safe. This mirrors how V already accepts the + // equivalent immutable `x := opt_array_field or { ... }`. A mutable + // destination (`mut x := m[k] or { ... }`, `x = m[k] or { ... }`) keeps + // the guard, so aliasing container/field storage still requires a + // `clone`/`move`. See vlang/v issue #27867. + mut right_is_immutable_or_unwrap := false + if node.op == .decl_assign && left is ast.Ident && !left.is_mut { + unwrapped_right := right.remove_par() + right_is_immutable_or_unwrap = match unwrapped_right { + ast.Ident { unwrapped_right.or_expr.kind != .absent } + ast.IndexExpr { unwrapped_right.or_expr.kind != .absent } + ast.SelectorExpr { unwrapped_right.or_block.kind != .absent } + else { false } + } + } if left_sym.kind == .map && is_assign && right_sym.kind == .map && !c.inside_unsafe - && !left.is_blank_ident() && right_is_lvalue + && !left.is_blank_ident() && right_is_lvalue && !right_is_immutable_or_unwrap && (!right_type.is_ptr() || (right is ast.Ident && assign_expr_is_auto_deref(right))) { // Do not allow `a = b` c.error('cannot copy map: call `move` or `clone` method (or use a reference)', diff --git a/vlib/v/checker/tests/option_map_or_unwrap_mut_alias_err.out b/vlib/v/checker/tests/option_map_or_unwrap_mut_alias_err.out new file mode 100644 index 00000000000000..25a48ee338dfbd --- /dev/null +++ b/vlib/v/checker/tests/option_map_or_unwrap_mut_alias_err.out @@ -0,0 +1,14 @@ +vlib/v/checker/tests/option_map_or_unwrap_mut_alias_err.vv:14:17: error: cannot copy map: call `move` or `clone` method (or use a reference) + 12 | 'x': 1 + 13 | }] + 14 | mut a := xs[0] or { panic('missing') } + | ~~~~~~~~~~~~~~~~~~~~~~~ + 15 | a['x'] = 2 + 16 | +vlib/v/checker/tests/option_map_or_unwrap_mut_alias_err.vv:22:13: error: cannot copy map: call `move` or `clone` method (or use a reference) + 20 | } + 21 | } + 22 | mut b := c.translations or { panic('missing') } + | ~~~~~~~~~~~~ + 23 | b['x'] = 2 + 24 | diff --git a/vlib/v/checker/tests/option_map_or_unwrap_mut_alias_err.vv b/vlib/v/checker/tests/option_map_or_unwrap_mut_alias_err.vv new file mode 100644 index 00000000000000..8a08ef9e77742f --- /dev/null +++ b/vlib/v/checker/tests/option_map_or_unwrap_mut_alias_err.vv @@ -0,0 +1,29 @@ +// Or-unwrapping an option map into a *mutable* destination would let the new +// variable alias the container/field storage, so it must still require an +// explicit `clone`/`move`. See vlang/v issue #27867 (the immutable +// `x := opt_map or { ... }` form is allowed, the mutable form is not). +struct Category { +mut: + translations ?map[string]int +} + +fn main() { + mut xs := [{ + 'x': 1 + }] + mut a := xs[0] or { panic('missing') } + a['x'] = 2 + + mut c := Category{ + translations: { + 'x': 1 + } + } + mut b := c.translations or { panic('missing') } + b['x'] = 2 + + println(xs) + println(c) + println(a) + println(b) +} diff --git a/vlib/v/tests/options/option_map_field_or_unwrap_test.v b/vlib/v/tests/options/option_map_field_or_unwrap_test.v index 3806b8d67c0d79..58339f15c0180f 100644 --- a/vlib/v/tests/options/option_map_field_or_unwrap_test.v +++ b/vlib/v/tests/options/option_map_field_or_unwrap_test.v @@ -1,6 +1,9 @@ // Regression test for https://github.com/vlang/v/issues/27867 -// or-unwrapping an option map (struct field, variable, index) used to fail -// with `cannot copy map: call move or clone method (or use a reference)`. +// or-unwrapping an option map (struct field, variable, index, parenthesized) +// into an immutable variable used to fail with +// `cannot copy map: call move or clone method (or use a reference)`. +// The mutable form (`mut x := m or { ... }`) still errors on purpose, see +// vlib/v/checker/tests/option_map_or_unwrap_mut_alias_err.vv. struct Translation { title string } @@ -53,3 +56,15 @@ fn test_option_map_index_or_unwrap() { inner := m['a'] or { panic('expected') } assert inner['b'] == 2 } + +fn test_option_map_field_paren_or_unwrap() { + c := Category{ + translations: { + 'en': Translation{ + title: 'Hello' + } + } + } + translations := (c.translations or { panic('expected') }) + assert translations['en'].title == 'Hello' +} From 2a6f7dd221055d5f5586c31d2de3b64fa89724b4 Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Sun, 19 Jul 2026 23:57:05 +0300 Subject: [PATCH 03/16] =?UTF-8?q?checker:=20address=20review=20=E2=80=94?= =?UTF-8?q?=20only=20exempt=20or-unwrap=20that=20clears=20the=20option?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the review on #27870. For `map[K]?map[...]`, an index `or` block that returns `none` (or another `?map`) does not unwrap the option: the result keeps the option-map type, so `v := m[k] or { none }` copies an option handle that still aliases the map stored in `m`. Gate the immutable or-unwrap exemption on the result type actually clearing option/result (`!right_type.has_flag(.option) && !right_type.has_flag(.result)`). The option-clearing form `m[k] or { panic(...) }` stays exempt (issue #27867); the option-preserving form is kept under the map-copy guard. Adds a checker error test for the option-preserving index `or`. --- vlib/v/checker/assign.v | 8 ++++++-- ...option_map_or_unwrap_preserves_option_err.out | 7 +++++++ .../option_map_or_unwrap_preserves_option_err.vv | 16 ++++++++++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) create mode 100644 vlib/v/checker/tests/option_map_or_unwrap_preserves_option_err.out create mode 100644 vlib/v/checker/tests/option_map_or_unwrap_preserves_option_err.vv diff --git a/vlib/v/checker/assign.v b/vlib/v/checker/assign.v index 4bc5535d8ab395..8ea48d2cd455bf 100644 --- a/vlib/v/checker/assign.v +++ b/vlib/v/checker/assign.v @@ -892,9 +892,13 @@ or use an explicit `unsafe{ a[..] }`, if you do not want a copy of the slice.', // equivalent immutable `x := opt_array_field or { ... }`. A mutable // destination (`mut x := m[k] or { ... }`, `x = m[k] or { ... }`) keeps // the guard, so aliasing container/field storage still requires a - // `clone`/`move`. See vlang/v issue #27867. + // `clone`/`move`. The `or` must actually clear the option/result: for + // `map[K]?map[...]`, `v := m[k] or { none }` keeps the option-map type, + // so `v` is still an option handle aliasing the map in `m` and the guard + // must stay. See vlang/v issue #27867. mut right_is_immutable_or_unwrap := false - if node.op == .decl_assign && left is ast.Ident && !left.is_mut { + if node.op == .decl_assign && left is ast.Ident && !left.is_mut + && !right_type.has_flag(.option) && !right_type.has_flag(.result) { unwrapped_right := right.remove_par() right_is_immutable_or_unwrap = match unwrapped_right { ast.Ident { unwrapped_right.or_expr.kind != .absent } diff --git a/vlib/v/checker/tests/option_map_or_unwrap_preserves_option_err.out b/vlib/v/checker/tests/option_map_or_unwrap_preserves_option_err.out new file mode 100644 index 00000000000000..b612b338015b2a --- /dev/null +++ b/vlib/v/checker/tests/option_map_or_unwrap_preserves_option_err.out @@ -0,0 +1,7 @@ +vlib/v/checker/tests/option_map_or_unwrap_preserves_option_err.vv:14:14: error: cannot copy map: call `move` or `clone` method (or use a reference) + 12 | 'a': ?map[string]int(inner) + 13 | } + 14 | v := m['a'] or { none } + | ~~~~~~~~~~~ + 15 | println(v) + 16 | } diff --git a/vlib/v/checker/tests/option_map_or_unwrap_preserves_option_err.vv b/vlib/v/checker/tests/option_map_or_unwrap_preserves_option_err.vv new file mode 100644 index 00000000000000..cb30dbc54de2d0 --- /dev/null +++ b/vlib/v/checker/tests/option_map_or_unwrap_preserves_option_err.vv @@ -0,0 +1,16 @@ +// For `map[K]?map[...]`, an `or` block that returns `none` (or another `?map`) +// does NOT clear the option: `v := m[k] or { none }` keeps the option-map type, +// so `v` is still an option handle that aliases the map stored in `m`. The +// map-copy guard must stay in that case, even though the destination is an +// immutable declaration. See vlang/v issue #27867 (only the option-clearing +// form `m[k] or { panic(...) }` is exempted). +fn main() { + inner := { + 'x': 1 + } + m := { + 'a': ?map[string]int(inner) + } + v := m['a'] or { none } + println(v) +} From 8c4713c4d22faa257b5114374886b8e816387342 Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Mon, 20 Jul 2026 00:03:18 +0300 Subject: [PATCH 04/16] =?UTF-8?q?checker:=20address=20review=20=E2=80=94?= =?UTF-8?q?=20keep=20map-copy=20guard=20for=20mutable=20option=20sources?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the review on #27870. The immutable or-unwrap exemption only checked that the destination local was immutable, but if the *source* option map is mutable it can be mutated later and the immutable copy observes it, e.g. `mut opt := ...; x := opt or { panic('') }; opt?['x'] = 2` left `x` aliasing `opt`'s storage. Gate the exemption on the unwrapped source itself being immutable via a new conservative `assign_or_unwrap_source_is_immutable` helper (walks Ident / SelectorExpr / IndexExpr / ParExpr roots; anything unknown keeps the guard). Immutable-source unwraps (`c := Category{}; x := c.f or { ... }`) stay exempt per issue #27867; mutable variables/fields/indexes keep requiring clone/move. Adds a checker error test covering the mutable variable, field and index sources. --- vlib/v/checker/assign.v | 38 +++++++++++++++---- .../option_map_or_unwrap_mut_source_err.out | 21 ++++++++++ .../option_map_or_unwrap_mut_source_err.vv | 37 ++++++++++++++++++ 3 files changed, 88 insertions(+), 8 deletions(-) create mode 100644 vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.out create mode 100644 vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.vv diff --git a/vlib/v/checker/assign.v b/vlib/v/checker/assign.v index 8ea48d2cd455bf..73fe40de6e9565 100644 --- a/vlib/v/checker/assign.v +++ b/vlib/v/checker/assign.v @@ -28,6 +28,20 @@ fn assign_expr_is_auto_deref(expr ast.Expr) bool { return expr.is_auto_deref_var() } +// assign_or_unwrap_source_is_immutable conservatively reports whether the source +// of an `or {}` unwrap can never be mutated later (so a copy of its map cannot +// observe a later mutation of the original). Only the plainly-immutable roots are +// recognised; anything unknown returns false, keeping the map-copy guard. +fn assign_or_unwrap_source_is_immutable(expr ast.Expr) bool { + return match expr { + ast.Ident { !expr.is_mut() } + ast.SelectorExpr { assign_or_unwrap_source_is_immutable(expr.expr) } + ast.IndexExpr { assign_or_unwrap_source_is_immutable(expr.left) } + ast.ParExpr { assign_or_unwrap_source_is_immutable(expr.expr) } + else { false } + } +} + fn (c &Checker) auto_deref_source_type_is_pointer(expr ast.Expr) bool { if expr !is ast.Ident || c.table.cur_fn == unsafe { nil } || !expr.is_auto_deref_var() { return false @@ -889,23 +903,31 @@ or use an explicit `unsafe{ a[..] }`, if you do not want a copy of the slice.', // `x := opt_map or { ... }` unwraps an option/result into a new immutable // variable, so `x` can never become a mutable alias of the underlying map // and the shallow copy is safe. This mirrors how V already accepts the - // equivalent immutable `x := opt_array_field or { ... }`. A mutable - // destination (`mut x := m[k] or { ... }`, `x = m[k] or { ... }`) keeps - // the guard, so aliasing container/field storage still requires a - // `clone`/`move`. The `or` must actually clear the option/result: for - // `map[K]?map[...]`, `v := m[k] or { none }` keeps the option-map type, - // so `v` is still an option handle aliasing the map in `m` and the guard - // must stay. See vlang/v issue #27867. + // equivalent immutable `x := opt_array_field or { ... }`. Several things + // must hold for the copy to be safe, otherwise the guard is kept so + // aliasing still requires a `clone`/`move`: + // - the destination is a new immutable variable (a mutable `mut x := ...` + // or a reassignment `x = ...` could still mutate through `x`); + // - the `or` actually clears the option/result — for `map[K]?map[...]`, + // `v := m[k] or { none }` keeps the option-map type, so `v` is still an + // option handle aliasing the map in `m`; + // - the unwrapped source itself is immutable — otherwise a later mutation + // of the source (e.g. `mut opt := ...; x := opt or { ... }; opt?[k] = v`) + // would be observed through `x`. + // See vlang/v issue #27867. mut right_is_immutable_or_unwrap := false if node.op == .decl_assign && left is ast.Ident && !left.is_mut && !right_type.has_flag(.option) && !right_type.has_flag(.result) { unwrapped_right := right.remove_par() - right_is_immutable_or_unwrap = match unwrapped_right { + has_or_block := match unwrapped_right { ast.Ident { unwrapped_right.or_expr.kind != .absent } ast.IndexExpr { unwrapped_right.or_expr.kind != .absent } ast.SelectorExpr { unwrapped_right.or_block.kind != .absent } else { false } } + + right_is_immutable_or_unwrap = has_or_block + && assign_or_unwrap_source_is_immutable(unwrapped_right) } if left_sym.kind == .map && is_assign && right_sym.kind == .map && !c.inside_unsafe && !left.is_blank_ident() && right_is_lvalue && !right_is_immutable_or_unwrap diff --git a/vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.out b/vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.out new file mode 100644 index 00000000000000..1e2e0505597496 --- /dev/null +++ b/vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.out @@ -0,0 +1,21 @@ +vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.vv:17:7: error: cannot copy map: call `move` or `clone` method (or use a reference) + 15 | 'x': 1 + 16 | } + 17 | a := opt or { panic('missing') } + | ~~~ + 18 | + 19 | mut c := Category{ +vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.vv:24:9: error: cannot copy map: call `move` or `clone` method (or use a reference) + 22 | } + 23 | } + 24 | b := c.translations or { panic('missing') } + | ~~~~~~~~~~~~ + 25 | + 26 | inner := { +vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.vv:32:14: error: cannot copy map: call `move` or `clone` method (or use a reference) + 30 | 'a': ?map[string]int(inner) + 31 | } + 32 | d := m['a'] or { panic('missing') } + | ~~~~~~~~~~~~~~~~~~~~~~~ + 33 | + 34 | println(a) diff --git a/vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.vv b/vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.vv new file mode 100644 index 00000000000000..94fed1d5348200 --- /dev/null +++ b/vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.vv @@ -0,0 +1,37 @@ +// Or-unwrapping an option map into an immutable local is only safe when the +// *source* can never be mutated later. If the source (variable, field, or index) +// is mutable, the immutable local still aliases the same map storage and would +// observe a later mutation, so the map-copy guard must stay. See vlang/v issue +// #27867 (only immutable-source unwraps like `c := Category{}; x := c.f or {...}` +// are exempted). +struct Category { +mut: + translations ?map[string]int +} + +fn main() { + mut opt := ?map[string]int(none) + opt = { + 'x': 1 + } + a := opt or { panic('missing') } + + mut c := Category{ + translations: { + 'x': 1 + } + } + b := c.translations or { panic('missing') } + + inner := { + 'x': 1 + } + mut m := { + 'a': ?map[string]int(inner) + } + d := m['a'] or { panic('missing') } + + println(a) + println(b) + println(d) +} From 66afd396ed9ffafd8b7c361c90e99364c6691f47 Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Mon, 20 Jul 2026 00:07:34 +0300 Subject: [PATCH 05/16] tests: use the review's exact mutable-source form in the alias regression test The map-copy guard for mutable option sources was already implemented in the previous commit; this only strengthens the regression coverage. Use the exact `mut opt := ?map[string]int({'x': 1})` form from the review thread and keep the mutable variable/field/index source cases, so the guarded aliasing is explicit. No checker changes. --- .../option_map_or_unwrap_mut_source_err.out | 36 +++++++++---------- .../option_map_or_unwrap_mut_source_err.vv | 7 ++-- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.out b/vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.out index 1e2e0505597496..2bb3814227de76 100644 --- a/vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.out +++ b/vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.out @@ -1,21 +1,21 @@ -vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.vv:17:7: error: cannot copy map: call `move` or `clone` method (or use a reference) - 15 | 'x': 1 - 16 | } - 17 | a := opt or { panic('missing') } +vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.vv:18:7: error: cannot copy map: call `move` or `clone` method (or use a reference) + 16 | 'x': 1 + 17 | }) + 18 | a := opt or { panic('missing') } | ~~~ - 18 | - 19 | mut c := Category{ -vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.vv:24:9: error: cannot copy map: call `move` or `clone` method (or use a reference) - 22 | } - 23 | } - 24 | b := c.translations or { panic('missing') } + 19 | + 20 | mut c := Category{ +vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.vv:25:9: error: cannot copy map: call `move` or `clone` method (or use a reference) + 23 | } + 24 | } + 25 | b := c.translations or { panic('missing') } | ~~~~~~~~~~~~ - 25 | - 26 | inner := { -vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.vv:32:14: error: cannot copy map: call `move` or `clone` method (or use a reference) - 30 | 'a': ?map[string]int(inner) - 31 | } - 32 | d := m['a'] or { panic('missing') } + 26 | + 27 | inner := { +vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.vv:33:14: error: cannot copy map: call `move` or `clone` method (or use a reference) + 31 | 'a': ?map[string]int(inner) + 32 | } + 33 | d := m['a'] or { panic('missing') } | ~~~~~~~~~~~~~~~~~~~~~~~ - 33 | - 34 | println(a) + 34 | + 35 | println(a) diff --git a/vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.vv b/vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.vv index 94fed1d5348200..c1727429fa74d1 100644 --- a/vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.vv +++ b/vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.vv @@ -10,10 +10,11 @@ mut: } fn main() { - mut opt := ?map[string]int(none) - opt = { + // mutable variable source (the exact case from the review): `a` aliases the + // map in `opt`, which can still be mutated later via `opt?['x'] = ...`. + mut opt := ?map[string]int({ 'x': 1 - } + }) a := opt or { panic('missing') } mut c := Category{ From b870fb739a98ff8d71a11b98f04fc5082f155a2b Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Mon, 20 Jul 2026 00:13:56 +0300 Subject: [PATCH 06/16] checker: reject or-unwrap through immutable pointers to mutable data Follow-up to the review on #27870. assign_or_unwrap_source_is_immutable treated an immutable pointer root as immutable, but an immutable pointer can still alias storage its owner mutates later, e.g. `mut c := Category{...}; p := &c; x := p.translations or { panic('') }; c.translations?['x'] = 2` left `x` observing the mutation. Treat a pointer/reference anywhere in the chain as mutable/unknown: reject when the root variable's type is a pointer, and when a selector receiver (`expr_type`) or index container (`left_type`) is a pointer, keeping the `clone`/`move` diagnostic. Adds a checker error test covering the pointer variable root, immutable reference parameter, and pointer array-element receiver. --- vlib/v/checker/assign.v | 27 +++++++++++--- .../option_map_or_unwrap_ptr_source_err.out | 21 +++++++++++ .../option_map_or_unwrap_ptr_source_err.vv | 36 +++++++++++++++++++ 3 files changed, 79 insertions(+), 5 deletions(-) create mode 100644 vlib/v/checker/tests/option_map_or_unwrap_ptr_source_err.out create mode 100644 vlib/v/checker/tests/option_map_or_unwrap_ptr_source_err.vv diff --git a/vlib/v/checker/assign.v b/vlib/v/checker/assign.v index 73fe40de6e9565..af17e702d670fb 100644 --- a/vlib/v/checker/assign.v +++ b/vlib/v/checker/assign.v @@ -32,13 +32,30 @@ fn assign_expr_is_auto_deref(expr ast.Expr) bool { // of an `or {}` unwrap can never be mutated later (so a copy of its map cannot // observe a later mutation of the original). Only the plainly-immutable roots are // recognised; anything unknown returns false, keeping the map-copy guard. +// A pointer/reference anywhere in the chain is treated as mutable/unknown, since +// an immutable pointer can still alias storage that its owner mutates later +// (e.g. `mut c := ...; p := &c; x := p.f or { ... }; c.f?[k] = v`). fn assign_or_unwrap_source_is_immutable(expr ast.Expr) bool { return match expr { - ast.Ident { !expr.is_mut() } - ast.SelectorExpr { assign_or_unwrap_source_is_immutable(expr.expr) } - ast.IndexExpr { assign_or_unwrap_source_is_immutable(expr.left) } - ast.ParExpr { assign_or_unwrap_source_is_immutable(expr.expr) } - else { false } + ast.Ident { + if expr.obj is ast.Var { + !expr.is_mut() && !expr.obj.typ.is_ptr() + } else { + !expr.is_mut() + } + } + ast.SelectorExpr { + !expr.expr_type.is_ptr() && assign_or_unwrap_source_is_immutable(expr.expr) + } + ast.IndexExpr { + !expr.left_type.is_ptr() && assign_or_unwrap_source_is_immutable(expr.left) + } + ast.ParExpr { + assign_or_unwrap_source_is_immutable(expr.expr) + } + else { + false + } } } diff --git a/vlib/v/checker/tests/option_map_or_unwrap_ptr_source_err.out b/vlib/v/checker/tests/option_map_or_unwrap_ptr_source_err.out new file mode 100644 index 00000000000000..85eb1fcaed32c7 --- /dev/null +++ b/vlib/v/checker/tests/option_map_or_unwrap_ptr_source_err.out @@ -0,0 +1,21 @@ +vlib/v/checker/tests/option_map_or_unwrap_ptr_source_err.vv:13:9: error: cannot copy map: call `move` or `clone` method (or use a reference) + 11 | // immutable reference parameter: `x` aliases the caller's map, which the + 12 | // caller can still mutate. + 13 | x := p.translations or { panic('missing') } + | ~~~~~~~~~~~~ + 14 | return x + 15 | } +vlib/v/checker/tests/option_map_or_unwrap_ptr_source_err.vv:26:9: error: cannot copy map: call `move` or `clone` method (or use a reference) + 24 | // aliases `c.translations`, which is mutated afterwards through `c`. + 25 | p := &c + 26 | a := p.translations or { panic('missing') } + | ~~~~~~~~~~~~ + 27 | c.translations?['x'] = 2 + 28 | +vlib/v/checker/tests/option_map_or_unwrap_ptr_source_err.vv:31:14: error: cannot copy map: call `move` or `clone` method (or use a reference) + 29 | // pointer as an array element receiver. + 30 | arr := [&c] + 31 | b := arr[0].translations or { panic('missing') } + | ~~~~~~~~~~~~ + 32 | + 33 | println(a) diff --git a/vlib/v/checker/tests/option_map_or_unwrap_ptr_source_err.vv b/vlib/v/checker/tests/option_map_or_unwrap_ptr_source_err.vv new file mode 100644 index 00000000000000..0c6b635471d073 --- /dev/null +++ b/vlib/v/checker/tests/option_map_or_unwrap_ptr_source_err.vv @@ -0,0 +1,36 @@ +// An immutable pointer/reference can still alias storage that its owner mutates +// later, so or-unwrapping an option map through a pointer root (or a selector +// receiver whose type is a pointer) must keep the map-copy guard, even though the +// pointer binding itself is immutable. See vlang/v issue #27867. +struct Category { +mut: + translations ?map[string]int +} + +fn through_ref_param(p &Category) map[string]int { + // immutable reference parameter: `x` aliases the caller's map, which the + // caller can still mutate. + x := p.translations or { panic('missing') } + return x +} + +fn main() { + mut c := Category{ + translations: { + 'x': 1 + } + } + // immutable pointer to mutable data (the exact case from the review): `a` + // aliases `c.translations`, which is mutated afterwards through `c`. + p := &c + a := p.translations or { panic('missing') } + c.translations?['x'] = 2 + + // pointer as an array element receiver. + arr := [&c] + b := arr[0].translations or { panic('missing') } + + println(a) + println(b) + println(through_ref_param(&c)) +} From 6448418396db88e454470a616132dc3ab43d5c3f Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Mon, 20 Jul 2026 00:27:38 +0300 Subject: [PATCH 07/16] checker: never exempt shared/atomic or-unwrap destinations from map-copy guard Follow-up to the review on #27870. A `shared b := c.translations or { ... }` destination is mutable under `lock b { b['x'] = 2 }`, so the immutable-destination exemption must not apply. The parser already sets `is_mut` for shared/atomic declarations (so `!left.is_mut` currently excludes them), but check `left.info.share` explicitly so this safety bypass no longer depends on that incidental detail. Adds a checker error test for the shared destination. --- vlib/v/checker/assign.v | 8 ++++++- .../option_map_or_unwrap_shared_dest_err.out | 7 ++++++ .../option_map_or_unwrap_shared_dest_err.vv | 23 +++++++++++++++++++ 3 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 vlib/v/checker/tests/option_map_or_unwrap_shared_dest_err.out create mode 100644 vlib/v/checker/tests/option_map_or_unwrap_shared_dest_err.vv diff --git a/vlib/v/checker/assign.v b/vlib/v/checker/assign.v index af17e702d670fb..b09b026ec25179 100644 --- a/vlib/v/checker/assign.v +++ b/vlib/v/checker/assign.v @@ -932,8 +932,14 @@ or use an explicit `unsafe{ a[..] }`, if you do not want a copy of the slice.', // of the source (e.g. `mut opt := ...; x := opt or { ... }; opt?[k] = v`) // would be observed through `x`. // See vlang/v issue #27867. + // A `shared`/`atomic` destination is mutable under `lock`, so it must never + // be exempted even though it is a `:=` declaration. Its `is_mut` flag is + // already set by the parser, but check `share` explicitly so this safety + // bypass does not depend on that incidental detail. + left_is_lockable_dest := left is ast.Ident && left.info is ast.IdentVar + && left.info.share in [.shared_t, .atomic_t] mut right_is_immutable_or_unwrap := false - if node.op == .decl_assign && left is ast.Ident && !left.is_mut + if node.op == .decl_assign && left is ast.Ident && !left.is_mut && !left_is_lockable_dest && !right_type.has_flag(.option) && !right_type.has_flag(.result) { unwrapped_right := right.remove_par() has_or_block := match unwrapped_right { diff --git a/vlib/v/checker/tests/option_map_or_unwrap_shared_dest_err.out b/vlib/v/checker/tests/option_map_or_unwrap_shared_dest_err.out new file mode 100644 index 00000000000000..0a384dbe76bea4 --- /dev/null +++ b/vlib/v/checker/tests/option_map_or_unwrap_shared_dest_err.out @@ -0,0 +1,7 @@ +vlib/v/checker/tests/option_map_or_unwrap_shared_dest_err.vv:16:16: error: cannot copy map: call `move` or `clone` method (or use a reference) + 14 | } + 15 | } + 16 | shared b := c.translations or { panic('missing') } + | ~~~~~~~~~~~~ + 17 | lock b { + 18 | b['x'] = 2 diff --git a/vlib/v/checker/tests/option_map_or_unwrap_shared_dest_err.vv b/vlib/v/checker/tests/option_map_or_unwrap_shared_dest_err.vv new file mode 100644 index 00000000000000..4c7455173e8da6 --- /dev/null +++ b/vlib/v/checker/tests/option_map_or_unwrap_shared_dest_err.vv @@ -0,0 +1,23 @@ +// A `shared` (or `atomic`) destination is mutable under `lock`, so or-unwrapping +// an option map into it must keep the map-copy guard: the shallow copy would +// alias the map still stored in `c`, and `lock b { b['x'] = 2 }` could mutate it. +// The immutable-destination exemption from issue #27867 must not apply here. +struct Category { +mut: + translations ?map[string]int +} + +fn main() { + c := Category{ + translations: { + 'x': 1 + } + } + shared b := c.translations or { panic('missing') } + lock b { + b['x'] = 2 + } + println(rlock b { + b.clone() + }) +} From e65b20ebe48c6c4a9d2fc1bd8c797ce58deee476 Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Mon, 20 Jul 2026 00:39:54 +0300 Subject: [PATCH 08/16] checker: keep map-copy guard when or-block default returns a mutable lvalue Follow-up to the review on #27870. `x := opt or { fallback }` makes `x` the block's value when the option is empty, so a mutable map lvalue default (e.g. `or { fallback }`) leaves `x` aliasing that storage. The exemption checked only the option source's immutability, not the default path. Also require the `or` block default to be non-aliasing: a fresh value (map literal / by-value call), an immutable lvalue, or a block that yields no value (diverges or propagates). Otherwise keep the clone/move diagnostic. Adds a checker error test for the mutable-lvalue or-block default. --- vlib/v/checker/assign.v | 48 +++++++++++++++---- .../option_map_or_unwrap_mut_default_err.out | 7 +++ .../option_map_or_unwrap_mut_default_err.vv | 18 +++++++ 3 files changed, 65 insertions(+), 8 deletions(-) create mode 100644 vlib/v/checker/tests/option_map_or_unwrap_mut_default_err.out create mode 100644 vlib/v/checker/tests/option_map_or_unwrap_mut_default_err.vv diff --git a/vlib/v/checker/assign.v b/vlib/v/checker/assign.v index b09b026ec25179..149d947f7105d8 100644 --- a/vlib/v/checker/assign.v +++ b/vlib/v/checker/assign.v @@ -59,6 +59,43 @@ fn assign_or_unwrap_source_is_immutable(expr ast.Expr) bool { } } +// assign_expr_or_block returns the `or {}` block attached to `expr`, if any. +fn assign_expr_or_block(expr ast.Expr) ast.OrExpr { + return match expr { + ast.Ident { expr.or_expr } + ast.IndexExpr { expr.or_expr } + ast.SelectorExpr { expr.or_block } + else { ast.OrExpr{} } + } +} + +// assign_or_block_default_is_safe conservatively reports whether the value an +// `or {}` block falls back to cannot alias mutable storage. `x := opt or { d }` +// makes `x` the block's value `d` when the option is empty, so if `d` is a +// mutable map lvalue (e.g. `or { fallback }`) then `x` aliases it. Only a fresh +// value (map literal / by-value call), an immutable lvalue, or a block that +// yields no value (diverges via `return`/`panic`/... or just propagates) is safe. +fn assign_or_block_default_is_safe(or_expr ast.OrExpr) bool { + if or_expr.kind != .block || or_expr.stmts.len == 0 { + return true + } + last := or_expr.stmts.last() + if last !is ast.ExprStmt { + return true + } + return match (last as ast.ExprStmt).expr { + ast.MapInit, ast.CallExpr { + true + } + ast.Ident, ast.SelectorExpr, ast.IndexExpr, ast.ParExpr { + assign_or_unwrap_source_is_immutable((last as ast.ExprStmt).expr) + } + else { + false + } + } +} + fn (c &Checker) auto_deref_source_type_is_pointer(expr ast.Expr) bool { if expr !is ast.Ident || c.table.cur_fn == unsafe { nil } || !expr.is_auto_deref_var() { return false @@ -942,15 +979,10 @@ or use an explicit `unsafe{ a[..] }`, if you do not want a copy of the slice.', if node.op == .decl_assign && left is ast.Ident && !left.is_mut && !left_is_lockable_dest && !right_type.has_flag(.option) && !right_type.has_flag(.result) { unwrapped_right := right.remove_par() - has_or_block := match unwrapped_right { - ast.Ident { unwrapped_right.or_expr.kind != .absent } - ast.IndexExpr { unwrapped_right.or_expr.kind != .absent } - ast.SelectorExpr { unwrapped_right.or_block.kind != .absent } - else { false } - } - - right_is_immutable_or_unwrap = has_or_block + or_expr := assign_expr_or_block(unwrapped_right) + right_is_immutable_or_unwrap = or_expr.kind != .absent && assign_or_unwrap_source_is_immutable(unwrapped_right) + && assign_or_block_default_is_safe(or_expr) } if left_sym.kind == .map && is_assign && right_sym.kind == .map && !c.inside_unsafe && !left.is_blank_ident() && right_is_lvalue && !right_is_immutable_or_unwrap diff --git a/vlib/v/checker/tests/option_map_or_unwrap_mut_default_err.out b/vlib/v/checker/tests/option_map_or_unwrap_mut_default_err.out new file mode 100644 index 00000000000000..70c350d75ccf02 --- /dev/null +++ b/vlib/v/checker/tests/option_map_or_unwrap_mut_default_err.out @@ -0,0 +1,7 @@ +vlib/v/checker/tests/option_map_or_unwrap_mut_default_err.vv:15:7: error: cannot copy map: call `move` or `clone` method (or use a reference) + 13 | } + 14 | opt := maybe_none() + 15 | x := opt or { fallback } + | ~~~ + 16 | fallback['x'] = 2 + 17 | println(x) diff --git a/vlib/v/checker/tests/option_map_or_unwrap_mut_default_err.vv b/vlib/v/checker/tests/option_map_or_unwrap_mut_default_err.vv new file mode 100644 index 00000000000000..26e9878cfd4bad --- /dev/null +++ b/vlib/v/checker/tests/option_map_or_unwrap_mut_default_err.vv @@ -0,0 +1,18 @@ +// `x := opt or { fallback }` makes `x` the `or` block's value when the option is +// empty, so if that default is a mutable map lvalue, `x` aliases it and observes +// later mutations (`fallback['x'] = 2`). The immutable-destination exemption from +// issue #27867 must not apply when the default path can return a mutable map +// lvalue, even if the option source itself is immutable. +fn maybe_none() ?map[string]int { + return none +} + +fn main() { + mut fallback := { + 'x': 1 + } + opt := maybe_none() + x := opt or { fallback } + fallback['x'] = 2 + println(x) +} From 5e63eb7f1038b4af3aadeb4b3f141e14deb24861 Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Mon, 20 Jul 2026 00:50:55 +0300 Subject: [PATCH 09/16] checker: unalias types before the or-unwrap pointer check Follow-up to the review on #27870. The immutable-source helper used raw Type.is_ptr(), so a `type Ref = &Category` receiver was classified as immutable and the map-copy guard was suppressed, even though the underlying `&Category` can mutate the same map through the original owner. Fully unalias types before the pointer check (Ident root, selector receiver, index container). The helpers are now Checker methods so they can reach the table. Adds a checker error test for the alias-to-pointer receiver. --- vlib/v/checker/assign.v | 23 +++++++++++-------- ...ion_map_or_unwrap_alias_ptr_source_err.out | 7 ++++++ ...tion_map_or_unwrap_alias_ptr_source_err.vv | 23 +++++++++++++++++++ 3 files changed, 43 insertions(+), 10 deletions(-) create mode 100644 vlib/v/checker/tests/option_map_or_unwrap_alias_ptr_source_err.out create mode 100644 vlib/v/checker/tests/option_map_or_unwrap_alias_ptr_source_err.vv diff --git a/vlib/v/checker/assign.v b/vlib/v/checker/assign.v index 149d947f7105d8..25c27e258bbddd 100644 --- a/vlib/v/checker/assign.v +++ b/vlib/v/checker/assign.v @@ -34,24 +34,27 @@ fn assign_expr_is_auto_deref(expr ast.Expr) bool { // recognised; anything unknown returns false, keeping the map-copy guard. // A pointer/reference anywhere in the chain is treated as mutable/unknown, since // an immutable pointer can still alias storage that its owner mutates later -// (e.g. `mut c := ...; p := &c; x := p.f or { ... }; c.f?[k] = v`). -fn assign_or_unwrap_source_is_immutable(expr ast.Expr) bool { +// (e.g. `mut c := ...; p := &c; x := p.f or { ... }; c.f?[k] = v`). Types are +// fully unaliased first, so a `type Ref = &T` receiver is treated as a pointer. +fn (c &Checker) assign_or_unwrap_source_is_immutable(expr ast.Expr) bool { return match expr { ast.Ident { if expr.obj is ast.Var { - !expr.is_mut() && !expr.obj.typ.is_ptr() + !expr.is_mut() && !c.table.fully_unaliased_type(expr.obj.typ).is_ptr() } else { !expr.is_mut() } } ast.SelectorExpr { - !expr.expr_type.is_ptr() && assign_or_unwrap_source_is_immutable(expr.expr) + !c.table.fully_unaliased_type(expr.expr_type).is_ptr() + && c.assign_or_unwrap_source_is_immutable(expr.expr) } ast.IndexExpr { - !expr.left_type.is_ptr() && assign_or_unwrap_source_is_immutable(expr.left) + !c.table.fully_unaliased_type(expr.left_type).is_ptr() + && c.assign_or_unwrap_source_is_immutable(expr.left) } ast.ParExpr { - assign_or_unwrap_source_is_immutable(expr.expr) + c.assign_or_unwrap_source_is_immutable(expr.expr) } else { false @@ -75,7 +78,7 @@ fn assign_expr_or_block(expr ast.Expr) ast.OrExpr { // mutable map lvalue (e.g. `or { fallback }`) then `x` aliases it. Only a fresh // value (map literal / by-value call), an immutable lvalue, or a block that // yields no value (diverges via `return`/`panic`/... or just propagates) is safe. -fn assign_or_block_default_is_safe(or_expr ast.OrExpr) bool { +fn (c &Checker) assign_or_block_default_is_safe(or_expr ast.OrExpr) bool { if or_expr.kind != .block || or_expr.stmts.len == 0 { return true } @@ -88,7 +91,7 @@ fn assign_or_block_default_is_safe(or_expr ast.OrExpr) bool { true } ast.Ident, ast.SelectorExpr, ast.IndexExpr, ast.ParExpr { - assign_or_unwrap_source_is_immutable((last as ast.ExprStmt).expr) + c.assign_or_unwrap_source_is_immutable((last as ast.ExprStmt).expr) } else { false @@ -981,8 +984,8 @@ or use an explicit `unsafe{ a[..] }`, if you do not want a copy of the slice.', unwrapped_right := right.remove_par() or_expr := assign_expr_or_block(unwrapped_right) right_is_immutable_or_unwrap = or_expr.kind != .absent - && assign_or_unwrap_source_is_immutable(unwrapped_right) - && assign_or_block_default_is_safe(or_expr) + && c.assign_or_unwrap_source_is_immutable(unwrapped_right) + && c.assign_or_block_default_is_safe(or_expr) } if left_sym.kind == .map && is_assign && right_sym.kind == .map && !c.inside_unsafe && !left.is_blank_ident() && right_is_lvalue && !right_is_immutable_or_unwrap diff --git a/vlib/v/checker/tests/option_map_or_unwrap_alias_ptr_source_err.out b/vlib/v/checker/tests/option_map_or_unwrap_alias_ptr_source_err.out new file mode 100644 index 00000000000000..af0850ceb54124 --- /dev/null +++ b/vlib/v/checker/tests/option_map_or_unwrap_alias_ptr_source_err.out @@ -0,0 +1,7 @@ +vlib/v/checker/tests/option_map_or_unwrap_alias_ptr_source_err.vv:20:9: error: cannot copy map: call `move` or `clone` method (or use a reference) + 18 | } + 19 | p := CategoryRef(&c) + 20 | x := p.translations or { panic('missing') } + | ~~~~~~~~~~~~ + 21 | c.translations?['x'] = 2 + 22 | println(x) diff --git a/vlib/v/checker/tests/option_map_or_unwrap_alias_ptr_source_err.vv b/vlib/v/checker/tests/option_map_or_unwrap_alias_ptr_source_err.vv new file mode 100644 index 00000000000000..1c7347103a7c10 --- /dev/null +++ b/vlib/v/checker/tests/option_map_or_unwrap_alias_ptr_source_err.vv @@ -0,0 +1,23 @@ +// A `type Ref = &Category` receiver is a pointer once unaliased, so or-unwrapping +// an option map through it can still alias storage mutated through the original +// owner. The immutable-source exemption from issue #27867 must unalias types +// before its pointer check, so an alias-to-pointer receiver keeps the map-copy +// guard just like a plain `&Category` receiver. +struct Category { +mut: + translations ?map[string]int +} + +type CategoryRef = &Category + +fn main() { + mut c := Category{ + translations: { + 'x': 1 + } + } + p := CategoryRef(&c) + x := p.translations or { panic('missing') } + c.translations?['x'] = 2 + println(x) +} From b599c5156a674d7c41cdc8ad88c80219dbe886b2 Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Mon, 20 Jul 2026 00:59:42 +0300 Subject: [PATCH 10/16] checker: only exempt fresh/owned or-block map defaults, not immutable lvalues MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the review on #27870. The or-block default check accepted an immutable map lvalue, but an immutable map parameter/field can alias caller-owned storage the caller mutates later, so `x := opt or { fallback }` returns a value that aliases and observes that mutation — the same copy `x := fallback` rejects. Restrict assign_or_block_default_is_safe to genuinely fresh/owned values (map literal or by-value call) or blocks that yield no value; any lvalue default now keeps the map-copy guard. Adds a checker error test for immutable parameter and immutable local defaults. --- vlib/v/checker/assign.v | 13 ++++----- ..._map_or_unwrap_immut_default_alias_err.out | 14 +++++++++ ...n_map_or_unwrap_immut_default_alias_err.vv | 29 +++++++++++++++++++ 3 files changed, 49 insertions(+), 7 deletions(-) create mode 100644 vlib/v/checker/tests/option_map_or_unwrap_immut_default_alias_err.out create mode 100644 vlib/v/checker/tests/option_map_or_unwrap_immut_default_alias_err.vv diff --git a/vlib/v/checker/assign.v b/vlib/v/checker/assign.v index 25c27e258bbddd..f25ded75576cc2 100644 --- a/vlib/v/checker/assign.v +++ b/vlib/v/checker/assign.v @@ -74,10 +74,12 @@ fn assign_expr_or_block(expr ast.Expr) ast.OrExpr { // assign_or_block_default_is_safe conservatively reports whether the value an // `or {}` block falls back to cannot alias mutable storage. `x := opt or { d }` -// makes `x` the block's value `d` when the option is empty, so if `d` is a -// mutable map lvalue (e.g. `or { fallback }`) then `x` aliases it. Only a fresh -// value (map literal / by-value call), an immutable lvalue, or a block that -// yields no value (diverges via `return`/`panic`/... or just propagates) is safe. +// makes `x` the block's value `d` when the option is empty, so if `d` is any map +// lvalue (e.g. `or { fallback }`) then `x` aliases it. Even an immutable lvalue +// is unsafe: an immutable map parameter/field can alias caller-owned storage the +// caller mutates later (the same copy `x := d` would reject). Only a fresh/owned +// value (map literal / by-value call) or a block that yields no value (diverges +// via `return`/`panic`/... or just propagates) is safe. fn (c &Checker) assign_or_block_default_is_safe(or_expr ast.OrExpr) bool { if or_expr.kind != .block || or_expr.stmts.len == 0 { return true @@ -90,9 +92,6 @@ fn (c &Checker) assign_or_block_default_is_safe(or_expr ast.OrExpr) bool { ast.MapInit, ast.CallExpr { true } - ast.Ident, ast.SelectorExpr, ast.IndexExpr, ast.ParExpr { - c.assign_or_unwrap_source_is_immutable((last as ast.ExprStmt).expr) - } else { false } diff --git a/vlib/v/checker/tests/option_map_or_unwrap_immut_default_alias_err.out b/vlib/v/checker/tests/option_map_or_unwrap_immut_default_alias_err.out new file mode 100644 index 00000000000000..1c9adce45179cf --- /dev/null +++ b/vlib/v/checker/tests/option_map_or_unwrap_immut_default_alias_err.out @@ -0,0 +1,14 @@ +vlib/v/checker/tests/option_map_or_unwrap_immut_default_alias_err.vv:13:7: error: cannot copy map: call `move` or `clone` method (or use a reference) + 11 | opt := maybe_none() + 12 | // immutable parameter fallback: `x` can alias the caller's map. + 13 | x := opt or { fallback } + | ~~~ + 14 | return x + 15 | } +vlib/v/checker/tests/option_map_or_unwrap_immut_default_alias_err.vv:23:7: error: cannot copy map: call `move` or `clone` method (or use a reference) + 21 | opt := maybe_none() + 22 | // immutable local lvalue fallback is rejected too (cannot prove freshness). + 23 | y := opt or { local } + | ~~~ + 24 | + 25 | println(pick({ diff --git a/vlib/v/checker/tests/option_map_or_unwrap_immut_default_alias_err.vv b/vlib/v/checker/tests/option_map_or_unwrap_immut_default_alias_err.vv new file mode 100644 index 00000000000000..1d8284ba849806 --- /dev/null +++ b/vlib/v/checker/tests/option_map_or_unwrap_immut_default_alias_err.vv @@ -0,0 +1,29 @@ +// An `or {}` block whose fallback is an immutable map lvalue is still unsafe: +// an immutable map parameter (or any immutable alias) can point at caller-owned +// storage the caller mutates later, so `x := opt or { fallback }` would alias it +// — the same copy `x := fallback` rejects. Only fresh/owned fallbacks (map +// literal / by-value call) are exempted. See vlang/v issue #27867. +fn maybe_none() ?map[string]int { + return none +} + +fn pick(fallback map[string]int) map[string]int { + opt := maybe_none() + // immutable parameter fallback: `x` can alias the caller's map. + x := opt or { fallback } + return x +} + +fn main() { + local := { + 'y': 9 + } + opt := maybe_none() + // immutable local lvalue fallback is rejected too (cannot prove freshness). + y := opt or { local } + + println(pick({ + 'x': 1 + })) + println(y) +} From 8f3584388459385f289a07e3aef21fa48055702c Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Mon, 20 Jul 2026 01:15:46 +0300 Subject: [PATCH 11/16] checker: filter semicolons and tighten or-block default classification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the review on #27870. assign_or_block_default_is_safe read the raw last statement of the or-block and returned safe for any non-ExprStmt, so a trailing `;` (`or { fallback; }`) made it skip the map-copy guard. Filter SemicolonStmt first (matching check_or_expr), and only treat a genuinely diverging terminator (`return`/`break`/`continue`; `panic`/`exit` are calls handled as fresh values) as safe — any other/unclassifiable last statement now keeps the guard. No separate test: `or { d; }` is rejected with 'expression evaluated but not used' on the normal compile path (identically on master), so the map-copy difference is only observable under -check, which the error-test harness does not use. --- vlib/v/checker/assign.v | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/vlib/v/checker/assign.v b/vlib/v/checker/assign.v index f25ded75576cc2..74f3d83a1900c3 100644 --- a/vlib/v/checker/assign.v +++ b/vlib/v/checker/assign.v @@ -78,18 +78,29 @@ fn assign_expr_or_block(expr ast.Expr) ast.OrExpr { // lvalue (e.g. `or { fallback }`) then `x` aliases it. Even an immutable lvalue // is unsafe: an immutable map parameter/field can alias caller-owned storage the // caller mutates later (the same copy `x := d` would reject). Only a fresh/owned -// value (map literal / by-value call) or a block that yields no value (diverges -// via `return`/`panic`/... or just propagates) is safe. +// value (map literal / by-value call), or a block that yields no value because +// it diverges (`return`/`break`/`continue`; `panic`/`exit` are calls handled +// above), is safe. Anything else — an lvalue value, or an unclassifiable last +// statement — keeps the guard. fn (c &Checker) assign_or_block_default_is_safe(or_expr ast.OrExpr) bool { - if or_expr.kind != .block || or_expr.stmts.len == 0 { + if or_expr.kind != .block { return true } - last := or_expr.stmts.last() - if last !is ast.ExprStmt { + // A trailing `;` leaves a `SemicolonStmt`; filter those out so the block's + // real value statement is classified, matching `check_or_expr`. + valid_stmts := or_expr.stmts.filter(it !is ast.SemicolonStmt) + if valid_stmts.len == 0 { return true } - return match (last as ast.ExprStmt).expr { - ast.MapInit, ast.CallExpr { + last := valid_stmts.last() + return match last { + ast.ExprStmt { + match last.expr { + ast.MapInit, ast.CallExpr { true } + else { false } + } + } + ast.Return, ast.BranchStmt { true } else { From 6565262cf83a8cf58e09ee29ed27f65efc87d6ff Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Mon, 20 Jul 2026 01:28:15 +0300 Subject: [PATCH 12/16] checker: only exempt fresh/noreturn or-block map call defaults MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the review on #27870. The or-block default check treated any CallExpr as fresh, but an arbitrary map-returning call can return a caller-owned alias (`fn id(m map) map { return m }`), so `x := opt or { id(fallback) }` aliased `fallback` and observed later mutations. Only exempt calls known to produce fresh storage — `clone`/`move` results — or `@[noreturn]` calls (`panic`/`exit`) that yield no value. Any other map-returning call now keeps the map-copy guard. Adds a checker error test for the aliasing call default. --- vlib/v/checker/assign.v | 27 ++++++++++++++----- ...n_map_or_unwrap_call_default_alias_err.out | 7 +++++ ...on_map_or_unwrap_call_default_alias_err.vv | 22 +++++++++++++++ 3 files changed, 50 insertions(+), 6 deletions(-) create mode 100644 vlib/v/checker/tests/option_map_or_unwrap_call_default_alias_err.out create mode 100644 vlib/v/checker/tests/option_map_or_unwrap_call_default_alias_err.vv diff --git a/vlib/v/checker/assign.v b/vlib/v/checker/assign.v index 74f3d83a1900c3..7c720bb3d76710 100644 --- a/vlib/v/checker/assign.v +++ b/vlib/v/checker/assign.v @@ -77,11 +77,12 @@ fn assign_expr_or_block(expr ast.Expr) ast.OrExpr { // makes `x` the block's value `d` when the option is empty, so if `d` is any map // lvalue (e.g. `or { fallback }`) then `x` aliases it. Even an immutable lvalue // is unsafe: an immutable map parameter/field can alias caller-owned storage the -// caller mutates later (the same copy `x := d` would reject). Only a fresh/owned -// value (map literal / by-value call), or a block that yields no value because -// it diverges (`return`/`break`/`continue`; `panic`/`exit` are calls handled -// above), is safe. Anything else — an lvalue value, or an unclassifiable last -// statement — keeps the guard. +// caller mutates later (the same copy `x := d` would reject). A call is unsafe +// too unless it is known to produce fresh storage — an arbitrary map-returning +// call may just return a caller-owned alias (`or { id(fallback) }`). Only a map +// literal, a `clone`/`move` call, a `@[noreturn]` call (`panic`/`exit`), or a +// block that yields no value by diverging (`return`/`break`/`continue`) is safe. +// Anything else keeps the guard. fn (c &Checker) assign_or_block_default_is_safe(or_expr ast.OrExpr) bool { if or_expr.kind != .block { return true @@ -96,7 +97,8 @@ fn (c &Checker) assign_or_block_default_is_safe(or_expr ast.OrExpr) bool { return match last { ast.ExprStmt { match last.expr { - ast.MapInit, ast.CallExpr { true } + ast.MapInit { true } + ast.CallExpr { assign_call_default_produces_fresh_map(last.expr) } else { false } } } @@ -109,6 +111,19 @@ fn (c &Checker) assign_or_block_default_is_safe(or_expr ast.OrExpr) bool { } } +// assign_call_default_produces_fresh_map reports whether a call used as an `or {}` +// default cannot alias caller-owned map storage: a `clone`/`move` result (fresh +// storage), or a `@[noreturn]` call such as `panic`/`exit` (yields no value at +// all). Any other map-returning call may return a caller-owned alias, so it is +// treated as unsafe and keeps the map-copy guard. +fn assign_call_default_produces_fresh_map(expr ast.Expr) bool { + if expr is ast.CallExpr { + return expr.is_noreturn || expr.kind in [.clone, .clone_to_depth, .move] + || (expr.is_method && expr.name in ['clone', 'move']) + } + return false +} + fn (c &Checker) auto_deref_source_type_is_pointer(expr ast.Expr) bool { if expr !is ast.Ident || c.table.cur_fn == unsafe { nil } || !expr.is_auto_deref_var() { return false diff --git a/vlib/v/checker/tests/option_map_or_unwrap_call_default_alias_err.out b/vlib/v/checker/tests/option_map_or_unwrap_call_default_alias_err.out new file mode 100644 index 00000000000000..c53aa18b8e94d8 --- /dev/null +++ b/vlib/v/checker/tests/option_map_or_unwrap_call_default_alias_err.out @@ -0,0 +1,7 @@ +vlib/v/checker/tests/option_map_or_unwrap_call_default_alias_err.vv:19:7: error: cannot copy map: call `move` or `clone` method (or use a reference) + 17 | } + 18 | opt := maybe_none() + 19 | x := opt or { id(fallback) } + | ~~~ + 20 | fallback['x'] = 2 + 21 | println(x) diff --git a/vlib/v/checker/tests/option_map_or_unwrap_call_default_alias_err.vv b/vlib/v/checker/tests/option_map_or_unwrap_call_default_alias_err.vv new file mode 100644 index 00000000000000..2219ee2f02f782 --- /dev/null +++ b/vlib/v/checker/tests/option_map_or_unwrap_call_default_alias_err.vv @@ -0,0 +1,22 @@ +// An `or {}` default that is an arbitrary map-returning call can return a +// caller-owned alias (`id` just returns its argument), so `x := opt or { id(fb) }` +// would alias `fb` and observe later mutations — the same as `or { fb }`. Only +// calls known to produce fresh storage (`clone`/`move`) or `@[noreturn]` calls +// are exempted. See vlang/v issue #27867. +fn maybe_none() ?map[string]int { + return none +} + +fn id(m map[string]int) map[string]int { + return m +} + +fn main() { + mut fallback := { + 'x': 1 + } + opt := maybe_none() + x := opt or { id(fallback) } + fallback['x'] = 2 + println(x) +} From b82d49bd9fcf40f664bca72a76d324299bcf3072 Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Mon, 20 Jul 2026 01:37:24 +0300 Subject: [PATCH 13/16] checker: restrict fresh or-block clone/move defaults to builtin map ops Follow-up to the review on #27870. CallKind and the method name alone do not distinguish the builtin map clone/move from a user-defined `clone`/`move` that returns a stored (aliasable) map, so `x := opt or { holder.clone() }` bypassed the guard. Require the clone/move call's receiver to be a map (final_sym.kind == .map) before treating it as fresh; noreturn calls still qualify. Adds a checker error test for a user-defined clone method default. --- vlib/v/checker/assign.v | 21 +++++++++----- ...n_map_or_unwrap_user_clone_default_err.out | 7 +++++ ...on_map_or_unwrap_user_clone_default_err.vv | 29 +++++++++++++++++++ 3 files changed, 49 insertions(+), 8 deletions(-) create mode 100644 vlib/v/checker/tests/option_map_or_unwrap_user_clone_default_err.out create mode 100644 vlib/v/checker/tests/option_map_or_unwrap_user_clone_default_err.vv diff --git a/vlib/v/checker/assign.v b/vlib/v/checker/assign.v index 7c720bb3d76710..1df33db358db69 100644 --- a/vlib/v/checker/assign.v +++ b/vlib/v/checker/assign.v @@ -98,7 +98,7 @@ fn (c &Checker) assign_or_block_default_is_safe(or_expr ast.OrExpr) bool { ast.ExprStmt { match last.expr { ast.MapInit { true } - ast.CallExpr { assign_call_default_produces_fresh_map(last.expr) } + ast.CallExpr { c.assign_call_default_produces_fresh_map(last.expr) } else { false } } } @@ -112,14 +112,19 @@ fn (c &Checker) assign_or_block_default_is_safe(or_expr ast.OrExpr) bool { } // assign_call_default_produces_fresh_map reports whether a call used as an `or {}` -// default cannot alias caller-owned map storage: a `clone`/`move` result (fresh -// storage), or a `@[noreturn]` call such as `panic`/`exit` (yields no value at -// all). Any other map-returning call may return a caller-owned alias, so it is -// treated as unsafe and keeps the map-copy guard. -fn assign_call_default_produces_fresh_map(expr ast.Expr) bool { +// default cannot alias caller-owned map storage: a `@[noreturn]` call such as +// `panic`/`exit` (yields no value at all), or the builtin map `clone`/`move` +// (fresh storage). Only the builtin map operations qualify — a user-defined +// function or method named `clone`/`move` (its `CallKind` also comes from the +// name) may just return an aliased map, so it is checked by the map receiver +// type. Any other map-returning call keeps the map-copy guard. +fn (c &Checker) assign_call_default_produces_fresh_map(expr ast.Expr) bool { if expr is ast.CallExpr { - return expr.is_noreturn || expr.kind in [.clone, .clone_to_depth, .move] - || (expr.is_method && expr.name in ['clone', 'move']) + if expr.is_noreturn { + return true + } + return expr.is_method && expr.name in ['clone', 'move'] + && c.table.final_sym(expr.receiver_type).kind == .map } return false } diff --git a/vlib/v/checker/tests/option_map_or_unwrap_user_clone_default_err.out b/vlib/v/checker/tests/option_map_or_unwrap_user_clone_default_err.out new file mode 100644 index 00000000000000..9d6b54e5fea4d7 --- /dev/null +++ b/vlib/v/checker/tests/option_map_or_unwrap_user_clone_default_err.out @@ -0,0 +1,7 @@ +vlib/v/checker/tests/option_map_or_unwrap_user_clone_default_err.vv:26:7: error: cannot copy map: call `move` or `clone` method (or use a reference) + 24 | } + 25 | opt := maybe_none() + 26 | x := opt or { holder.clone() } + | ~~~ + 27 | holder.m['x'] = 2 + 28 | println(x) diff --git a/vlib/v/checker/tests/option_map_or_unwrap_user_clone_default_err.vv b/vlib/v/checker/tests/option_map_or_unwrap_user_clone_default_err.vv new file mode 100644 index 00000000000000..02b1fafcd7a952 --- /dev/null +++ b/vlib/v/checker/tests/option_map_or_unwrap_user_clone_default_err.vv @@ -0,0 +1,29 @@ +// Only the builtin map `clone`/`move` produce fresh storage. A user-defined +// method named `clone` that returns a stored map is not fresh, so +// `x := opt or { holder.clone() }` would alias `holder.m` and observe later +// mutations. Such a call must keep the map-copy guard (the exemption checks the +// receiver is a map, not just the method name). See vlang/v issue #27867. +fn maybe_none() ?map[string]int { + return none +} + +struct Holder { +mut: + m map[string]int +} + +fn (h Holder) clone() map[string]int { + return h.m +} + +fn main() { + mut holder := Holder{ + m: { + 'x': 1 + } + } + opt := maybe_none() + x := opt or { holder.clone() } + holder.m['x'] = 2 + println(x) +} From 01d21af40928259916198499c9f0750145fa5a10 Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Mon, 20 Jul 2026 01:46:14 +0300 Subject: [PATCH 14/16] checker: require direct map receiver for fresh or-block clone/move defaults MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the review on #27870. Using final_sym to check the clone/move receiver still trusted an alias whose base is a map, but a map type alias can define its own clone/move (`type M = map[...]; fn (m M) clone() ...`), which fn.v resolves before the map builtin — so `x := opt or { holder.m.clone() }` aliased holder.m. Check the receiver type directly (`sym().kind == .map`): a map carries no user methods, so a direct map receiver is the builtin, while an alias receiver is `.alias` and no longer qualifies. Adds a checker error test for an alias with a user-defined clone default. --- vlib/v/checker/assign.v | 8 +++-- ...or_unwrap_alias_user_clone_default_err.out | 7 +++++ ..._or_unwrap_alias_user_clone_default_err.vv | 31 +++++++++++++++++++ 3 files changed, 43 insertions(+), 3 deletions(-) create mode 100644 vlib/v/checker/tests/option_map_or_unwrap_alias_user_clone_default_err.out create mode 100644 vlib/v/checker/tests/option_map_or_unwrap_alias_user_clone_default_err.vv diff --git a/vlib/v/checker/assign.v b/vlib/v/checker/assign.v index 1df33db358db69..9c8e38771982c9 100644 --- a/vlib/v/checker/assign.v +++ b/vlib/v/checker/assign.v @@ -116,15 +116,17 @@ fn (c &Checker) assign_or_block_default_is_safe(or_expr ast.OrExpr) bool { // `panic`/`exit` (yields no value at all), or the builtin map `clone`/`move` // (fresh storage). Only the builtin map operations qualify — a user-defined // function or method named `clone`/`move` (its `CallKind` also comes from the -// name) may just return an aliased map, so it is checked by the map receiver -// type. Any other map-returning call keeps the map-copy guard. +// name) may just return an aliased map. The receiver type must be a map +// *directly*, not via an alias: a map cannot carry user methods, but an alias +// (`type M = map[...]`) can define its own `clone`/`move`, which `fn.v` resolves +// before the map builtin. Any other map-returning call keeps the map-copy guard. fn (c &Checker) assign_call_default_produces_fresh_map(expr ast.Expr) bool { if expr is ast.CallExpr { if expr.is_noreturn { return true } return expr.is_method && expr.name in ['clone', 'move'] - && c.table.final_sym(expr.receiver_type).kind == .map + && c.table.sym(expr.receiver_type).kind == .map } return false } diff --git a/vlib/v/checker/tests/option_map_or_unwrap_alias_user_clone_default_err.out b/vlib/v/checker/tests/option_map_or_unwrap_alias_user_clone_default_err.out new file mode 100644 index 00000000000000..10af3611b10c6a --- /dev/null +++ b/vlib/v/checker/tests/option_map_or_unwrap_alias_user_clone_default_err.out @@ -0,0 +1,7 @@ +vlib/v/checker/tests/option_map_or_unwrap_alias_user_clone_default_err.vv:28:7: error: cannot copy map: call `move` or `clone` method (or use a reference) + 26 | } + 27 | opt := maybe_none() + 28 | x := opt or { holder.m.clone() } + | ~~~ + 29 | holder.m['x'] = 2 + 30 | println(x) diff --git a/vlib/v/checker/tests/option_map_or_unwrap_alias_user_clone_default_err.vv b/vlib/v/checker/tests/option_map_or_unwrap_alias_user_clone_default_err.vv new file mode 100644 index 00000000000000..1ef4f213e9722b --- /dev/null +++ b/vlib/v/checker/tests/option_map_or_unwrap_alias_user_clone_default_err.vv @@ -0,0 +1,31 @@ +// Only the *builtin* map clone/move produce fresh storage. A map type alias can +// define its own `clone`/`move`, which `fn.v` resolves before the map builtin, so +// `holder.m.clone()` here runs the user method that returns an alias of `holder.m`. +// The exemption must require a direct map receiver (not one that merely unaliases +// to a map), so this default keeps the map-copy guard. See vlang/v issue #27867. +type MapAlias = map[string]int + +fn (m MapAlias) clone() map[string]int { + return map[string]int(m) +} + +fn maybe_none() ?map[string]int { + return none +} + +struct Holder { +mut: + m MapAlias +} + +fn main() { + mut holder := Holder{ + m: MapAlias({ + 'x': 1 + }) + } + opt := maybe_none() + x := opt or { holder.m.clone() } + holder.m['x'] = 2 + println(x) +} From c31f53bca4ed5cc22332b56140e9462bcae63aa6 Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Mon, 20 Jul 2026 01:56:32 +0300 Subject: [PATCH 15/16] checker: strip parens before classifying or-block map defaults Follow-up to the review on #27870. A parenthesized fresh default (`or { (map[string]int{}) }`, `or { (fallback.clone()) }`) matched ast.ParExpr and fell into the unsafe branch, so harmless parentheses wrongly triggered the map-copy diagnostic while the unparenthesized forms compiled. remove_par() the default value before classifying it. Extends the runtime regression test with parenthesized fresh defaults. --- vlib/v/checker/assign.v | 6 ++++-- .../options/option_map_field_or_unwrap_test.v | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/vlib/v/checker/assign.v b/vlib/v/checker/assign.v index 9c8e38771982c9..df47f624b3792d 100644 --- a/vlib/v/checker/assign.v +++ b/vlib/v/checker/assign.v @@ -96,9 +96,11 @@ fn (c &Checker) assign_or_block_default_is_safe(or_expr ast.OrExpr) bool { last := valid_stmts.last() return match last { ast.ExprStmt { - match last.expr { + // harmless parentheses around the value must not change classification + default_expr := last.expr.remove_par() + match default_expr { ast.MapInit { true } - ast.CallExpr { c.assign_call_default_produces_fresh_map(last.expr) } + ast.CallExpr { c.assign_call_default_produces_fresh_map(default_expr) } else { false } } } diff --git a/vlib/v/tests/options/option_map_field_or_unwrap_test.v b/vlib/v/tests/options/option_map_field_or_unwrap_test.v index 58339f15c0180f..8addcb67ab3ccf 100644 --- a/vlib/v/tests/options/option_map_field_or_unwrap_test.v +++ b/vlib/v/tests/options/option_map_field_or_unwrap_test.v @@ -68,3 +68,18 @@ fn test_option_map_field_paren_or_unwrap() { translations := (c.translations or { panic('expected') }) assert translations['en'].title == 'Hello' } + +// Parentheses around a fresh `or {}` default must not change checker behavior: +// these fresh defaults are accepted just like their unparenthesized forms. +fn test_option_map_paren_fresh_default() { + src := get_option_map() + + a := src or { (map[string]int{}) } + assert a['a'] == 1 + + fallback := { + 'z': 9 + } + b := src or { (fallback.clone()) } + assert b['a'] == 1 +} From 73ce15c7356c3a599e6a9a9472da10e06f864278 Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Mon, 20 Jul 2026 03:25:46 +0300 Subject: [PATCH 16/16] checker: revert unsound option-map or-unwrap exemption; require clone/move MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The exemption added earlier tried to let `x := opt_map or { ... }` compile when the option source looked immutable. As the review on #27870 showed across many rounds, this cannot be made sound: V has no ownership/data-flow, so an immutable binding is not proof the unwrapped map is unaliased — e.g. `mut base := {'x':1}; opt := ?map[string]int(base); x := opt or { panic('') }` leaves `x` aliasing `base`. No local is_mut/is_ptr/receiver check can prove freshness, so restore master's behavior: an option-map or-unwrap is a map copy and keeps the `cannot copy map` guard, exactly like `m2 := m1`. For #27867, unwrap with an explicit copy: `x := (c.f or { ... }).clone()`. assign.v is now identical to master; adds a single regression test documenting that the guard applies to option-map or-unwrap forms. --- vlib/v/checker/assign.v | 137 +----------------- ...ion_map_or_unwrap_alias_ptr_source_err.out | 7 - ...tion_map_or_unwrap_alias_ptr_source_err.vv | 23 --- ...or_unwrap_alias_user_clone_default_err.out | 7 - ..._or_unwrap_alias_user_clone_default_err.vv | 31 ---- ...n_map_or_unwrap_call_default_alias_err.out | 7 - ...on_map_or_unwrap_call_default_alias_err.vv | 22 --- ..._map_or_unwrap_immut_default_alias_err.out | 14 -- ...n_map_or_unwrap_immut_default_alias_err.vv | 29 ---- .../option_map_or_unwrap_mut_alias_err.out | 14 -- .../option_map_or_unwrap_mut_alias_err.vv | 29 ---- .../option_map_or_unwrap_mut_default_err.out | 7 - .../option_map_or_unwrap_mut_default_err.vv | 18 --- .../option_map_or_unwrap_mut_source_err.out | 21 --- .../option_map_or_unwrap_mut_source_err.vv | 38 ----- ...ion_map_or_unwrap_preserves_option_err.out | 7 - ...tion_map_or_unwrap_preserves_option_err.vv | 16 -- .../option_map_or_unwrap_ptr_source_err.out | 21 --- .../option_map_or_unwrap_ptr_source_err.vv | 36 ----- ...ption_map_or_unwrap_requires_clone_err.out | 21 +++ ...option_map_or_unwrap_requires_clone_err.vv | 35 +++++ .../option_map_or_unwrap_shared_dest_err.out | 7 - .../option_map_or_unwrap_shared_dest_err.vv | 23 --- ...n_map_or_unwrap_user_clone_default_err.out | 7 - ...on_map_or_unwrap_user_clone_default_err.vv | 29 ---- .../options/option_map_field_or_unwrap_test.v | 85 ----------- 26 files changed, 57 insertions(+), 634 deletions(-) delete mode 100644 vlib/v/checker/tests/option_map_or_unwrap_alias_ptr_source_err.out delete mode 100644 vlib/v/checker/tests/option_map_or_unwrap_alias_ptr_source_err.vv delete mode 100644 vlib/v/checker/tests/option_map_or_unwrap_alias_user_clone_default_err.out delete mode 100644 vlib/v/checker/tests/option_map_or_unwrap_alias_user_clone_default_err.vv delete mode 100644 vlib/v/checker/tests/option_map_or_unwrap_call_default_alias_err.out delete mode 100644 vlib/v/checker/tests/option_map_or_unwrap_call_default_alias_err.vv delete mode 100644 vlib/v/checker/tests/option_map_or_unwrap_immut_default_alias_err.out delete mode 100644 vlib/v/checker/tests/option_map_or_unwrap_immut_default_alias_err.vv delete mode 100644 vlib/v/checker/tests/option_map_or_unwrap_mut_alias_err.out delete mode 100644 vlib/v/checker/tests/option_map_or_unwrap_mut_alias_err.vv delete mode 100644 vlib/v/checker/tests/option_map_or_unwrap_mut_default_err.out delete mode 100644 vlib/v/checker/tests/option_map_or_unwrap_mut_default_err.vv delete mode 100644 vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.out delete mode 100644 vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.vv delete mode 100644 vlib/v/checker/tests/option_map_or_unwrap_preserves_option_err.out delete mode 100644 vlib/v/checker/tests/option_map_or_unwrap_preserves_option_err.vv delete mode 100644 vlib/v/checker/tests/option_map_or_unwrap_ptr_source_err.out delete mode 100644 vlib/v/checker/tests/option_map_or_unwrap_ptr_source_err.vv create mode 100644 vlib/v/checker/tests/option_map_or_unwrap_requires_clone_err.out create mode 100644 vlib/v/checker/tests/option_map_or_unwrap_requires_clone_err.vv delete mode 100644 vlib/v/checker/tests/option_map_or_unwrap_shared_dest_err.out delete mode 100644 vlib/v/checker/tests/option_map_or_unwrap_shared_dest_err.vv delete mode 100644 vlib/v/checker/tests/option_map_or_unwrap_user_clone_default_err.out delete mode 100644 vlib/v/checker/tests/option_map_or_unwrap_user_clone_default_err.vv delete mode 100644 vlib/v/tests/options/option_map_field_or_unwrap_test.v diff --git a/vlib/v/checker/assign.v b/vlib/v/checker/assign.v index df47f624b3792d..94561a954fa5c5 100644 --- a/vlib/v/checker/assign.v +++ b/vlib/v/checker/assign.v @@ -28,111 +28,6 @@ fn assign_expr_is_auto_deref(expr ast.Expr) bool { return expr.is_auto_deref_var() } -// assign_or_unwrap_source_is_immutable conservatively reports whether the source -// of an `or {}` unwrap can never be mutated later (so a copy of its map cannot -// observe a later mutation of the original). Only the plainly-immutable roots are -// recognised; anything unknown returns false, keeping the map-copy guard. -// A pointer/reference anywhere in the chain is treated as mutable/unknown, since -// an immutable pointer can still alias storage that its owner mutates later -// (e.g. `mut c := ...; p := &c; x := p.f or { ... }; c.f?[k] = v`). Types are -// fully unaliased first, so a `type Ref = &T` receiver is treated as a pointer. -fn (c &Checker) assign_or_unwrap_source_is_immutable(expr ast.Expr) bool { - return match expr { - ast.Ident { - if expr.obj is ast.Var { - !expr.is_mut() && !c.table.fully_unaliased_type(expr.obj.typ).is_ptr() - } else { - !expr.is_mut() - } - } - ast.SelectorExpr { - !c.table.fully_unaliased_type(expr.expr_type).is_ptr() - && c.assign_or_unwrap_source_is_immutable(expr.expr) - } - ast.IndexExpr { - !c.table.fully_unaliased_type(expr.left_type).is_ptr() - && c.assign_or_unwrap_source_is_immutable(expr.left) - } - ast.ParExpr { - c.assign_or_unwrap_source_is_immutable(expr.expr) - } - else { - false - } - } -} - -// assign_expr_or_block returns the `or {}` block attached to `expr`, if any. -fn assign_expr_or_block(expr ast.Expr) ast.OrExpr { - return match expr { - ast.Ident { expr.or_expr } - ast.IndexExpr { expr.or_expr } - ast.SelectorExpr { expr.or_block } - else { ast.OrExpr{} } - } -} - -// assign_or_block_default_is_safe conservatively reports whether the value an -// `or {}` block falls back to cannot alias mutable storage. `x := opt or { d }` -// makes `x` the block's value `d` when the option is empty, so if `d` is any map -// lvalue (e.g. `or { fallback }`) then `x` aliases it. Even an immutable lvalue -// is unsafe: an immutable map parameter/field can alias caller-owned storage the -// caller mutates later (the same copy `x := d` would reject). A call is unsafe -// too unless it is known to produce fresh storage — an arbitrary map-returning -// call may just return a caller-owned alias (`or { id(fallback) }`). Only a map -// literal, a `clone`/`move` call, a `@[noreturn]` call (`panic`/`exit`), or a -// block that yields no value by diverging (`return`/`break`/`continue`) is safe. -// Anything else keeps the guard. -fn (c &Checker) assign_or_block_default_is_safe(or_expr ast.OrExpr) bool { - if or_expr.kind != .block { - return true - } - // A trailing `;` leaves a `SemicolonStmt`; filter those out so the block's - // real value statement is classified, matching `check_or_expr`. - valid_stmts := or_expr.stmts.filter(it !is ast.SemicolonStmt) - if valid_stmts.len == 0 { - return true - } - last := valid_stmts.last() - return match last { - ast.ExprStmt { - // harmless parentheses around the value must not change classification - default_expr := last.expr.remove_par() - match default_expr { - ast.MapInit { true } - ast.CallExpr { c.assign_call_default_produces_fresh_map(default_expr) } - else { false } - } - } - ast.Return, ast.BranchStmt { - true - } - else { - false - } - } -} - -// assign_call_default_produces_fresh_map reports whether a call used as an `or {}` -// default cannot alias caller-owned map storage: a `@[noreturn]` call such as -// `panic`/`exit` (yields no value at all), or the builtin map `clone`/`move` -// (fresh storage). Only the builtin map operations qualify — a user-defined -// function or method named `clone`/`move` (its `CallKind` also comes from the -// name) may just return an aliased map. The receiver type must be a map -// *directly*, not via an alias: a map cannot carry user methods, but an alias -// (`type M = map[...]`) can define its own `clone`/`move`, which `fn.v` resolves -// before the map builtin. Any other map-returning call keeps the map-copy guard. -fn (c &Checker) assign_call_default_produces_fresh_map(expr ast.Expr) bool { - if expr is ast.CallExpr { - if expr.is_noreturn { - return true - } - return expr.is_method && expr.name in ['clone', 'move'] - && c.table.sym(expr.receiver_type).kind == .map - } - return false -} - fn (c &Checker) auto_deref_source_type_is_pointer(expr ast.Expr) bool { if expr !is ast.Ident || c.table.cur_fn == unsafe { nil } || !expr.is_auto_deref_var() { return false @@ -991,38 +886,8 @@ or use an explicit `unsafe{ a[..] }`, if you do not want a copy of the slice.', } else { right.is_lvalue() } - // `x := opt_map or { ... }` unwraps an option/result into a new immutable - // variable, so `x` can never become a mutable alias of the underlying map - // and the shallow copy is safe. This mirrors how V already accepts the - // equivalent immutable `x := opt_array_field or { ... }`. Several things - // must hold for the copy to be safe, otherwise the guard is kept so - // aliasing still requires a `clone`/`move`: - // - the destination is a new immutable variable (a mutable `mut x := ...` - // or a reassignment `x = ...` could still mutate through `x`); - // - the `or` actually clears the option/result — for `map[K]?map[...]`, - // `v := m[k] or { none }` keeps the option-map type, so `v` is still an - // option handle aliasing the map in `m`; - // - the unwrapped source itself is immutable — otherwise a later mutation - // of the source (e.g. `mut opt := ...; x := opt or { ... }; opt?[k] = v`) - // would be observed through `x`. - // See vlang/v issue #27867. - // A `shared`/`atomic` destination is mutable under `lock`, so it must never - // be exempted even though it is a `:=` declaration. Its `is_mut` flag is - // already set by the parser, but check `share` explicitly so this safety - // bypass does not depend on that incidental detail. - left_is_lockable_dest := left is ast.Ident && left.info is ast.IdentVar - && left.info.share in [.shared_t, .atomic_t] - mut right_is_immutable_or_unwrap := false - if node.op == .decl_assign && left is ast.Ident && !left.is_mut && !left_is_lockable_dest - && !right_type.has_flag(.option) && !right_type.has_flag(.result) { - unwrapped_right := right.remove_par() - or_expr := assign_expr_or_block(unwrapped_right) - right_is_immutable_or_unwrap = or_expr.kind != .absent - && c.assign_or_unwrap_source_is_immutable(unwrapped_right) - && c.assign_or_block_default_is_safe(or_expr) - } if left_sym.kind == .map && is_assign && right_sym.kind == .map && !c.inside_unsafe - && !left.is_blank_ident() && right_is_lvalue && !right_is_immutable_or_unwrap + && !left.is_blank_ident() && right_is_lvalue && (!right_type.is_ptr() || (right is ast.Ident && assign_expr_is_auto_deref(right))) { // Do not allow `a = b` c.error('cannot copy map: call `move` or `clone` method (or use a reference)', diff --git a/vlib/v/checker/tests/option_map_or_unwrap_alias_ptr_source_err.out b/vlib/v/checker/tests/option_map_or_unwrap_alias_ptr_source_err.out deleted file mode 100644 index af0850ceb54124..00000000000000 --- a/vlib/v/checker/tests/option_map_or_unwrap_alias_ptr_source_err.out +++ /dev/null @@ -1,7 +0,0 @@ -vlib/v/checker/tests/option_map_or_unwrap_alias_ptr_source_err.vv:20:9: error: cannot copy map: call `move` or `clone` method (or use a reference) - 18 | } - 19 | p := CategoryRef(&c) - 20 | x := p.translations or { panic('missing') } - | ~~~~~~~~~~~~ - 21 | c.translations?['x'] = 2 - 22 | println(x) diff --git a/vlib/v/checker/tests/option_map_or_unwrap_alias_ptr_source_err.vv b/vlib/v/checker/tests/option_map_or_unwrap_alias_ptr_source_err.vv deleted file mode 100644 index 1c7347103a7c10..00000000000000 --- a/vlib/v/checker/tests/option_map_or_unwrap_alias_ptr_source_err.vv +++ /dev/null @@ -1,23 +0,0 @@ -// A `type Ref = &Category` receiver is a pointer once unaliased, so or-unwrapping -// an option map through it can still alias storage mutated through the original -// owner. The immutable-source exemption from issue #27867 must unalias types -// before its pointer check, so an alias-to-pointer receiver keeps the map-copy -// guard just like a plain `&Category` receiver. -struct Category { -mut: - translations ?map[string]int -} - -type CategoryRef = &Category - -fn main() { - mut c := Category{ - translations: { - 'x': 1 - } - } - p := CategoryRef(&c) - x := p.translations or { panic('missing') } - c.translations?['x'] = 2 - println(x) -} diff --git a/vlib/v/checker/tests/option_map_or_unwrap_alias_user_clone_default_err.out b/vlib/v/checker/tests/option_map_or_unwrap_alias_user_clone_default_err.out deleted file mode 100644 index 10af3611b10c6a..00000000000000 --- a/vlib/v/checker/tests/option_map_or_unwrap_alias_user_clone_default_err.out +++ /dev/null @@ -1,7 +0,0 @@ -vlib/v/checker/tests/option_map_or_unwrap_alias_user_clone_default_err.vv:28:7: error: cannot copy map: call `move` or `clone` method (or use a reference) - 26 | } - 27 | opt := maybe_none() - 28 | x := opt or { holder.m.clone() } - | ~~~ - 29 | holder.m['x'] = 2 - 30 | println(x) diff --git a/vlib/v/checker/tests/option_map_or_unwrap_alias_user_clone_default_err.vv b/vlib/v/checker/tests/option_map_or_unwrap_alias_user_clone_default_err.vv deleted file mode 100644 index 1ef4f213e9722b..00000000000000 --- a/vlib/v/checker/tests/option_map_or_unwrap_alias_user_clone_default_err.vv +++ /dev/null @@ -1,31 +0,0 @@ -// Only the *builtin* map clone/move produce fresh storage. A map type alias can -// define its own `clone`/`move`, which `fn.v` resolves before the map builtin, so -// `holder.m.clone()` here runs the user method that returns an alias of `holder.m`. -// The exemption must require a direct map receiver (not one that merely unaliases -// to a map), so this default keeps the map-copy guard. See vlang/v issue #27867. -type MapAlias = map[string]int - -fn (m MapAlias) clone() map[string]int { - return map[string]int(m) -} - -fn maybe_none() ?map[string]int { - return none -} - -struct Holder { -mut: - m MapAlias -} - -fn main() { - mut holder := Holder{ - m: MapAlias({ - 'x': 1 - }) - } - opt := maybe_none() - x := opt or { holder.m.clone() } - holder.m['x'] = 2 - println(x) -} diff --git a/vlib/v/checker/tests/option_map_or_unwrap_call_default_alias_err.out b/vlib/v/checker/tests/option_map_or_unwrap_call_default_alias_err.out deleted file mode 100644 index c53aa18b8e94d8..00000000000000 --- a/vlib/v/checker/tests/option_map_or_unwrap_call_default_alias_err.out +++ /dev/null @@ -1,7 +0,0 @@ -vlib/v/checker/tests/option_map_or_unwrap_call_default_alias_err.vv:19:7: error: cannot copy map: call `move` or `clone` method (or use a reference) - 17 | } - 18 | opt := maybe_none() - 19 | x := opt or { id(fallback) } - | ~~~ - 20 | fallback['x'] = 2 - 21 | println(x) diff --git a/vlib/v/checker/tests/option_map_or_unwrap_call_default_alias_err.vv b/vlib/v/checker/tests/option_map_or_unwrap_call_default_alias_err.vv deleted file mode 100644 index 2219ee2f02f782..00000000000000 --- a/vlib/v/checker/tests/option_map_or_unwrap_call_default_alias_err.vv +++ /dev/null @@ -1,22 +0,0 @@ -// An `or {}` default that is an arbitrary map-returning call can return a -// caller-owned alias (`id` just returns its argument), so `x := opt or { id(fb) }` -// would alias `fb` and observe later mutations — the same as `or { fb }`. Only -// calls known to produce fresh storage (`clone`/`move`) or `@[noreturn]` calls -// are exempted. See vlang/v issue #27867. -fn maybe_none() ?map[string]int { - return none -} - -fn id(m map[string]int) map[string]int { - return m -} - -fn main() { - mut fallback := { - 'x': 1 - } - opt := maybe_none() - x := opt or { id(fallback) } - fallback['x'] = 2 - println(x) -} diff --git a/vlib/v/checker/tests/option_map_or_unwrap_immut_default_alias_err.out b/vlib/v/checker/tests/option_map_or_unwrap_immut_default_alias_err.out deleted file mode 100644 index 1c9adce45179cf..00000000000000 --- a/vlib/v/checker/tests/option_map_or_unwrap_immut_default_alias_err.out +++ /dev/null @@ -1,14 +0,0 @@ -vlib/v/checker/tests/option_map_or_unwrap_immut_default_alias_err.vv:13:7: error: cannot copy map: call `move` or `clone` method (or use a reference) - 11 | opt := maybe_none() - 12 | // immutable parameter fallback: `x` can alias the caller's map. - 13 | x := opt or { fallback } - | ~~~ - 14 | return x - 15 | } -vlib/v/checker/tests/option_map_or_unwrap_immut_default_alias_err.vv:23:7: error: cannot copy map: call `move` or `clone` method (or use a reference) - 21 | opt := maybe_none() - 22 | // immutable local lvalue fallback is rejected too (cannot prove freshness). - 23 | y := opt or { local } - | ~~~ - 24 | - 25 | println(pick({ diff --git a/vlib/v/checker/tests/option_map_or_unwrap_immut_default_alias_err.vv b/vlib/v/checker/tests/option_map_or_unwrap_immut_default_alias_err.vv deleted file mode 100644 index 1d8284ba849806..00000000000000 --- a/vlib/v/checker/tests/option_map_or_unwrap_immut_default_alias_err.vv +++ /dev/null @@ -1,29 +0,0 @@ -// An `or {}` block whose fallback is an immutable map lvalue is still unsafe: -// an immutable map parameter (or any immutable alias) can point at caller-owned -// storage the caller mutates later, so `x := opt or { fallback }` would alias it -// — the same copy `x := fallback` rejects. Only fresh/owned fallbacks (map -// literal / by-value call) are exempted. See vlang/v issue #27867. -fn maybe_none() ?map[string]int { - return none -} - -fn pick(fallback map[string]int) map[string]int { - opt := maybe_none() - // immutable parameter fallback: `x` can alias the caller's map. - x := opt or { fallback } - return x -} - -fn main() { - local := { - 'y': 9 - } - opt := maybe_none() - // immutable local lvalue fallback is rejected too (cannot prove freshness). - y := opt or { local } - - println(pick({ - 'x': 1 - })) - println(y) -} diff --git a/vlib/v/checker/tests/option_map_or_unwrap_mut_alias_err.out b/vlib/v/checker/tests/option_map_or_unwrap_mut_alias_err.out deleted file mode 100644 index 25a48ee338dfbd..00000000000000 --- a/vlib/v/checker/tests/option_map_or_unwrap_mut_alias_err.out +++ /dev/null @@ -1,14 +0,0 @@ -vlib/v/checker/tests/option_map_or_unwrap_mut_alias_err.vv:14:17: error: cannot copy map: call `move` or `clone` method (or use a reference) - 12 | 'x': 1 - 13 | }] - 14 | mut a := xs[0] or { panic('missing') } - | ~~~~~~~~~~~~~~~~~~~~~~~ - 15 | a['x'] = 2 - 16 | -vlib/v/checker/tests/option_map_or_unwrap_mut_alias_err.vv:22:13: error: cannot copy map: call `move` or `clone` method (or use a reference) - 20 | } - 21 | } - 22 | mut b := c.translations or { panic('missing') } - | ~~~~~~~~~~~~ - 23 | b['x'] = 2 - 24 | diff --git a/vlib/v/checker/tests/option_map_or_unwrap_mut_alias_err.vv b/vlib/v/checker/tests/option_map_or_unwrap_mut_alias_err.vv deleted file mode 100644 index 8a08ef9e77742f..00000000000000 --- a/vlib/v/checker/tests/option_map_or_unwrap_mut_alias_err.vv +++ /dev/null @@ -1,29 +0,0 @@ -// Or-unwrapping an option map into a *mutable* destination would let the new -// variable alias the container/field storage, so it must still require an -// explicit `clone`/`move`. See vlang/v issue #27867 (the immutable -// `x := opt_map or { ... }` form is allowed, the mutable form is not). -struct Category { -mut: - translations ?map[string]int -} - -fn main() { - mut xs := [{ - 'x': 1 - }] - mut a := xs[0] or { panic('missing') } - a['x'] = 2 - - mut c := Category{ - translations: { - 'x': 1 - } - } - mut b := c.translations or { panic('missing') } - b['x'] = 2 - - println(xs) - println(c) - println(a) - println(b) -} diff --git a/vlib/v/checker/tests/option_map_or_unwrap_mut_default_err.out b/vlib/v/checker/tests/option_map_or_unwrap_mut_default_err.out deleted file mode 100644 index 70c350d75ccf02..00000000000000 --- a/vlib/v/checker/tests/option_map_or_unwrap_mut_default_err.out +++ /dev/null @@ -1,7 +0,0 @@ -vlib/v/checker/tests/option_map_or_unwrap_mut_default_err.vv:15:7: error: cannot copy map: call `move` or `clone` method (or use a reference) - 13 | } - 14 | opt := maybe_none() - 15 | x := opt or { fallback } - | ~~~ - 16 | fallback['x'] = 2 - 17 | println(x) diff --git a/vlib/v/checker/tests/option_map_or_unwrap_mut_default_err.vv b/vlib/v/checker/tests/option_map_or_unwrap_mut_default_err.vv deleted file mode 100644 index 26e9878cfd4bad..00000000000000 --- a/vlib/v/checker/tests/option_map_or_unwrap_mut_default_err.vv +++ /dev/null @@ -1,18 +0,0 @@ -// `x := opt or { fallback }` makes `x` the `or` block's value when the option is -// empty, so if that default is a mutable map lvalue, `x` aliases it and observes -// later mutations (`fallback['x'] = 2`). The immutable-destination exemption from -// issue #27867 must not apply when the default path can return a mutable map -// lvalue, even if the option source itself is immutable. -fn maybe_none() ?map[string]int { - return none -} - -fn main() { - mut fallback := { - 'x': 1 - } - opt := maybe_none() - x := opt or { fallback } - fallback['x'] = 2 - println(x) -} diff --git a/vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.out b/vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.out deleted file mode 100644 index 2bb3814227de76..00000000000000 --- a/vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.out +++ /dev/null @@ -1,21 +0,0 @@ -vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.vv:18:7: error: cannot copy map: call `move` or `clone` method (or use a reference) - 16 | 'x': 1 - 17 | }) - 18 | a := opt or { panic('missing') } - | ~~~ - 19 | - 20 | mut c := Category{ -vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.vv:25:9: error: cannot copy map: call `move` or `clone` method (or use a reference) - 23 | } - 24 | } - 25 | b := c.translations or { panic('missing') } - | ~~~~~~~~~~~~ - 26 | - 27 | inner := { -vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.vv:33:14: error: cannot copy map: call `move` or `clone` method (or use a reference) - 31 | 'a': ?map[string]int(inner) - 32 | } - 33 | d := m['a'] or { panic('missing') } - | ~~~~~~~~~~~~~~~~~~~~~~~ - 34 | - 35 | println(a) diff --git a/vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.vv b/vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.vv deleted file mode 100644 index c1727429fa74d1..00000000000000 --- a/vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.vv +++ /dev/null @@ -1,38 +0,0 @@ -// Or-unwrapping an option map into an immutable local is only safe when the -// *source* can never be mutated later. If the source (variable, field, or index) -// is mutable, the immutable local still aliases the same map storage and would -// observe a later mutation, so the map-copy guard must stay. See vlang/v issue -// #27867 (only immutable-source unwraps like `c := Category{}; x := c.f or {...}` -// are exempted). -struct Category { -mut: - translations ?map[string]int -} - -fn main() { - // mutable variable source (the exact case from the review): `a` aliases the - // map in `opt`, which can still be mutated later via `opt?['x'] = ...`. - mut opt := ?map[string]int({ - 'x': 1 - }) - a := opt or { panic('missing') } - - mut c := Category{ - translations: { - 'x': 1 - } - } - b := c.translations or { panic('missing') } - - inner := { - 'x': 1 - } - mut m := { - 'a': ?map[string]int(inner) - } - d := m['a'] or { panic('missing') } - - println(a) - println(b) - println(d) -} diff --git a/vlib/v/checker/tests/option_map_or_unwrap_preserves_option_err.out b/vlib/v/checker/tests/option_map_or_unwrap_preserves_option_err.out deleted file mode 100644 index b612b338015b2a..00000000000000 --- a/vlib/v/checker/tests/option_map_or_unwrap_preserves_option_err.out +++ /dev/null @@ -1,7 +0,0 @@ -vlib/v/checker/tests/option_map_or_unwrap_preserves_option_err.vv:14:14: error: cannot copy map: call `move` or `clone` method (or use a reference) - 12 | 'a': ?map[string]int(inner) - 13 | } - 14 | v := m['a'] or { none } - | ~~~~~~~~~~~ - 15 | println(v) - 16 | } diff --git a/vlib/v/checker/tests/option_map_or_unwrap_preserves_option_err.vv b/vlib/v/checker/tests/option_map_or_unwrap_preserves_option_err.vv deleted file mode 100644 index cb30dbc54de2d0..00000000000000 --- a/vlib/v/checker/tests/option_map_or_unwrap_preserves_option_err.vv +++ /dev/null @@ -1,16 +0,0 @@ -// For `map[K]?map[...]`, an `or` block that returns `none` (or another `?map`) -// does NOT clear the option: `v := m[k] or { none }` keeps the option-map type, -// so `v` is still an option handle that aliases the map stored in `m`. The -// map-copy guard must stay in that case, even though the destination is an -// immutable declaration. See vlang/v issue #27867 (only the option-clearing -// form `m[k] or { panic(...) }` is exempted). -fn main() { - inner := { - 'x': 1 - } - m := { - 'a': ?map[string]int(inner) - } - v := m['a'] or { none } - println(v) -} diff --git a/vlib/v/checker/tests/option_map_or_unwrap_ptr_source_err.out b/vlib/v/checker/tests/option_map_or_unwrap_ptr_source_err.out deleted file mode 100644 index 85eb1fcaed32c7..00000000000000 --- a/vlib/v/checker/tests/option_map_or_unwrap_ptr_source_err.out +++ /dev/null @@ -1,21 +0,0 @@ -vlib/v/checker/tests/option_map_or_unwrap_ptr_source_err.vv:13:9: error: cannot copy map: call `move` or `clone` method (or use a reference) - 11 | // immutable reference parameter: `x` aliases the caller's map, which the - 12 | // caller can still mutate. - 13 | x := p.translations or { panic('missing') } - | ~~~~~~~~~~~~ - 14 | return x - 15 | } -vlib/v/checker/tests/option_map_or_unwrap_ptr_source_err.vv:26:9: error: cannot copy map: call `move` or `clone` method (or use a reference) - 24 | // aliases `c.translations`, which is mutated afterwards through `c`. - 25 | p := &c - 26 | a := p.translations or { panic('missing') } - | ~~~~~~~~~~~~ - 27 | c.translations?['x'] = 2 - 28 | -vlib/v/checker/tests/option_map_or_unwrap_ptr_source_err.vv:31:14: error: cannot copy map: call `move` or `clone` method (or use a reference) - 29 | // pointer as an array element receiver. - 30 | arr := [&c] - 31 | b := arr[0].translations or { panic('missing') } - | ~~~~~~~~~~~~ - 32 | - 33 | println(a) diff --git a/vlib/v/checker/tests/option_map_or_unwrap_ptr_source_err.vv b/vlib/v/checker/tests/option_map_or_unwrap_ptr_source_err.vv deleted file mode 100644 index 0c6b635471d073..00000000000000 --- a/vlib/v/checker/tests/option_map_or_unwrap_ptr_source_err.vv +++ /dev/null @@ -1,36 +0,0 @@ -// An immutable pointer/reference can still alias storage that its owner mutates -// later, so or-unwrapping an option map through a pointer root (or a selector -// receiver whose type is a pointer) must keep the map-copy guard, even though the -// pointer binding itself is immutable. See vlang/v issue #27867. -struct Category { -mut: - translations ?map[string]int -} - -fn through_ref_param(p &Category) map[string]int { - // immutable reference parameter: `x` aliases the caller's map, which the - // caller can still mutate. - x := p.translations or { panic('missing') } - return x -} - -fn main() { - mut c := Category{ - translations: { - 'x': 1 - } - } - // immutable pointer to mutable data (the exact case from the review): `a` - // aliases `c.translations`, which is mutated afterwards through `c`. - p := &c - a := p.translations or { panic('missing') } - c.translations?['x'] = 2 - - // pointer as an array element receiver. - arr := [&c] - b := arr[0].translations or { panic('missing') } - - println(a) - println(b) - println(through_ref_param(&c)) -} diff --git a/vlib/v/checker/tests/option_map_or_unwrap_requires_clone_err.out b/vlib/v/checker/tests/option_map_or_unwrap_requires_clone_err.out new file mode 100644 index 00000000000000..75e5cbfc60d882 --- /dev/null +++ b/vlib/v/checker/tests/option_map_or_unwrap_requires_clone_err.out @@ -0,0 +1,21 @@ +vlib/v/checker/tests/option_map_or_unwrap_requires_clone_err.vv:22:13: error: cannot copy map: call `move` or `clone` method (or use a reference) + 20 | } + 21 | } + 22 | field := c.translations or { panic('none') } + | ~~~~~~~~~~~~ + 23 | + 24 | opt := get_opt() +vlib/v/checker/tests/option_map_or_unwrap_requires_clone_err.vv:25:14: error: cannot copy map: call `move` or `clone` method (or use a reference) + 23 | + 24 | opt := get_opt() + 25 | variable := opt or { panic('none') } + | ~~~ + 26 | + 27 | m := { +vlib/v/checker/tests/option_map_or_unwrap_requires_clone_err.vv:30:18: error: cannot copy map: call `move` or `clone` method (or use a reference) + 28 | 'k': get_opt() + 29 | } + 30 | index := m['k'] or { panic('none') } + | ~~~~~~~~~~~~~~~~~~~~ + 31 | + 32 | println(field) diff --git a/vlib/v/checker/tests/option_map_or_unwrap_requires_clone_err.vv b/vlib/v/checker/tests/option_map_or_unwrap_requires_clone_err.vv new file mode 100644 index 00000000000000..4c2e7195764cbf --- /dev/null +++ b/vlib/v/checker/tests/option_map_or_unwrap_requires_clone_err.vv @@ -0,0 +1,35 @@ +// Or-unwrapping an option map into a new variable is a map copy, so it keeps the +// `cannot copy map` guard just like `m2 := m1`: an immutable binding is not proof +// that the unwrapped map is unaliased (e.g. it can wrap a still-mutable map), so +// V requires an explicit `clone`/`move`, e.g. `x := (c.f or { ... }).clone()`. +// See vlang/v issue #27867. +struct Category { + translations ?map[string]int +} + +fn get_opt() ?map[string]int { + return { + 'a': 1 + } +} + +fn main() { + c := Category{ + translations: { + 'x': 1 + } + } + field := c.translations or { panic('none') } + + opt := get_opt() + variable := opt or { panic('none') } + + m := { + 'k': get_opt() + } + index := m['k'] or { panic('none') } + + println(field) + println(variable) + println(index) +} diff --git a/vlib/v/checker/tests/option_map_or_unwrap_shared_dest_err.out b/vlib/v/checker/tests/option_map_or_unwrap_shared_dest_err.out deleted file mode 100644 index 0a384dbe76bea4..00000000000000 --- a/vlib/v/checker/tests/option_map_or_unwrap_shared_dest_err.out +++ /dev/null @@ -1,7 +0,0 @@ -vlib/v/checker/tests/option_map_or_unwrap_shared_dest_err.vv:16:16: error: cannot copy map: call `move` or `clone` method (or use a reference) - 14 | } - 15 | } - 16 | shared b := c.translations or { panic('missing') } - | ~~~~~~~~~~~~ - 17 | lock b { - 18 | b['x'] = 2 diff --git a/vlib/v/checker/tests/option_map_or_unwrap_shared_dest_err.vv b/vlib/v/checker/tests/option_map_or_unwrap_shared_dest_err.vv deleted file mode 100644 index 4c7455173e8da6..00000000000000 --- a/vlib/v/checker/tests/option_map_or_unwrap_shared_dest_err.vv +++ /dev/null @@ -1,23 +0,0 @@ -// A `shared` (or `atomic`) destination is mutable under `lock`, so or-unwrapping -// an option map into it must keep the map-copy guard: the shallow copy would -// alias the map still stored in `c`, and `lock b { b['x'] = 2 }` could mutate it. -// The immutable-destination exemption from issue #27867 must not apply here. -struct Category { -mut: - translations ?map[string]int -} - -fn main() { - c := Category{ - translations: { - 'x': 1 - } - } - shared b := c.translations or { panic('missing') } - lock b { - b['x'] = 2 - } - println(rlock b { - b.clone() - }) -} diff --git a/vlib/v/checker/tests/option_map_or_unwrap_user_clone_default_err.out b/vlib/v/checker/tests/option_map_or_unwrap_user_clone_default_err.out deleted file mode 100644 index 9d6b54e5fea4d7..00000000000000 --- a/vlib/v/checker/tests/option_map_or_unwrap_user_clone_default_err.out +++ /dev/null @@ -1,7 +0,0 @@ -vlib/v/checker/tests/option_map_or_unwrap_user_clone_default_err.vv:26:7: error: cannot copy map: call `move` or `clone` method (or use a reference) - 24 | } - 25 | opt := maybe_none() - 26 | x := opt or { holder.clone() } - | ~~~ - 27 | holder.m['x'] = 2 - 28 | println(x) diff --git a/vlib/v/checker/tests/option_map_or_unwrap_user_clone_default_err.vv b/vlib/v/checker/tests/option_map_or_unwrap_user_clone_default_err.vv deleted file mode 100644 index 02b1fafcd7a952..00000000000000 --- a/vlib/v/checker/tests/option_map_or_unwrap_user_clone_default_err.vv +++ /dev/null @@ -1,29 +0,0 @@ -// Only the builtin map `clone`/`move` produce fresh storage. A user-defined -// method named `clone` that returns a stored map is not fresh, so -// `x := opt or { holder.clone() }` would alias `holder.m` and observe later -// mutations. Such a call must keep the map-copy guard (the exemption checks the -// receiver is a map, not just the method name). See vlang/v issue #27867. -fn maybe_none() ?map[string]int { - return none -} - -struct Holder { -mut: - m map[string]int -} - -fn (h Holder) clone() map[string]int { - return h.m -} - -fn main() { - mut holder := Holder{ - m: { - 'x': 1 - } - } - opt := maybe_none() - x := opt or { holder.clone() } - holder.m['x'] = 2 - println(x) -} diff --git a/vlib/v/tests/options/option_map_field_or_unwrap_test.v b/vlib/v/tests/options/option_map_field_or_unwrap_test.v deleted file mode 100644 index 8addcb67ab3ccf..00000000000000 --- a/vlib/v/tests/options/option_map_field_or_unwrap_test.v +++ /dev/null @@ -1,85 +0,0 @@ -// Regression test for https://github.com/vlang/v/issues/27867 -// or-unwrapping an option map (struct field, variable, index, parenthesized) -// into an immutable variable used to fail with -// `cannot copy map: call move or clone method (or use a reference)`. -// The mutable form (`mut x := m or { ... }`) still errors on purpose, see -// vlib/v/checker/tests/option_map_or_unwrap_mut_alias_err.vv. -struct Translation { - title string -} - -struct Category { - title string - translations ?map[string]Translation -} - -fn test_option_map_field_or_unwrap() { - c := Category{ - translations: { - 'en': Translation{ - title: 'Hello' - } - } - } - translations := c.translations or { panic('expected') } - assert translations['en'].title == 'Hello' -} - -fn test_option_map_field_or_unwrap_none() { - c := Category{} - translations := c.translations or { - map[string]Translation{} - } - - assert translations.len == 0 -} - -fn get_option_map() ?map[string]int { - return { - 'a': 1 - } -} - -fn test_option_map_var_or_unwrap() { - x := get_option_map() - y := x or { panic('expected') } - assert y['a'] == 1 -} - -fn test_option_map_index_or_unwrap() { - inner_map := { - 'b': 2 - } - m := { - 'a': inner_map - } - inner := m['a'] or { panic('expected') } - assert inner['b'] == 2 -} - -fn test_option_map_field_paren_or_unwrap() { - c := Category{ - translations: { - 'en': Translation{ - title: 'Hello' - } - } - } - translations := (c.translations or { panic('expected') }) - assert translations['en'].title == 'Hello' -} - -// Parentheses around a fresh `or {}` default must not change checker behavior: -// these fresh defaults are accepted just like their unparenthesized forms. -fn test_option_map_paren_fresh_default() { - src := get_option_map() - - a := src or { (map[string]int{}) } - assert a['a'] == 1 - - fallback := { - 'z': 9 - } - b := src or { (fallback.clone()) } - assert b['a'] == 1 -}