Skip to content
Open
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
fa25149
checker: fix or-unwrapping an option map field/var/index (fix #27867)
medvednikov Jul 19, 2026
756eced
checker: address review — only relax the map-copy guard for immutable…
medvednikov Jul 19, 2026
2a6f7dd
checker: address review — only exempt or-unwrap that clears the option
medvednikov Jul 19, 2026
8c4713c
checker: address review — keep map-copy guard for mutable option sources
medvednikov Jul 19, 2026
66afd39
tests: use the review's exact mutable-source form in the alias regres…
medvednikov Jul 19, 2026
b870fb7
checker: reject or-unwrap through immutable pointers to mutable data
medvednikov Jul 19, 2026
6448418
checker: never exempt shared/atomic or-unwrap destinations from map-c…
medvednikov Jul 19, 2026
e65b20e
checker: keep map-copy guard when or-block default returns a mutable …
medvednikov Jul 19, 2026
5e63eb7
checker: unalias types before the or-unwrap pointer check
medvednikov Jul 19, 2026
b599c51
checker: only exempt fresh/owned or-block map defaults, not immutable…
medvednikov Jul 19, 2026
8f35843
checker: filter semicolons and tighten or-block default classification
medvednikov Jul 19, 2026
6565262
checker: only exempt fresh/noreturn or-block map call defaults
medvednikov Jul 19, 2026
b82d49b
checker: restrict fresh or-block clone/move defaults to builtin map ops
medvednikov Jul 19, 2026
01d21af
checker: require direct map receiver for fresh or-block clone/move de…
medvednikov Jul 19, 2026
c31f53b
checker: strip parens before classifying or-block map defaults
medvednikov Jul 19, 2026
73ce15c
checker: revert unsound option-map or-unwrap exemption; require clone…
medvednikov Jul 20, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
135 changes: 134 additions & 1 deletion vlib/v/checker/assign.v
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,109 @@ 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()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep guard for immutable option-map aliases

Fresh evidence in this revision is that this helper still returns true for any non-mut, non-pointer ast.Var, treating immutability as ownership. An immutable option map can still be a shallow wrapper around mutable storage, e.g. mut base := {'x': 1}; opt := ?map[string]int(base); x := opt or { panic('missing') }; base['x'] = 2; the new bypass suppresses the map-copy error at the unwrap and x observes the later mutation. Keep requiring clone/move unless the unwrapped source is proven fresh/owned, not merely immutable.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved by reverting the exemption in 73ce15c. You're right, and this is the terminal version of the whole thread: assign_or_unwrap_source_is_immutable treated immutability as ownership, and those aren't the same — opt := ?map(base) is an immutable binding over base's mutable storage. There's no local is_mut/is_ptr/receiver/provenance check that can prove freshness (it needs whole-program data-flow V doesn't have), and applied rigorously the "prove fresh" requirement rejects essentially all lvalue sources — i.e. it collapses to keeping the guard.

So I've restored master's behavior: an option-map or-unwrap is a map copy and keeps the cannot copy map diagnostic, exactly like m2 := m1. checker/assign.v is now identical to master (0-line diff); all the exemption helpers and their probe tests are removed, and I left a single regression test documenting that the guard applies to the option-map or-unwrap forms.

For #27867 the unwrap is written with an explicit copy — x := (c.f or { ... }).clone() — which compiles today.

} 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 {
match last.expr {
ast.MapInit { true }
ast.CallExpr { c.assign_call_default_produces_fresh_map(last.expr) }
else { false }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Strip parens before classifying safe defaults

When the or block's value is parenthesized, this match sees an ast.ParExpr and falls into the unsafe case, so valid fresh defaults like x := opt or { (map[string]int{}) } or x := opt or { (fallback.clone()) } still hit the map-copy diagnostic even though the unparenthesized forms are accepted. Unwrap last.expr.remove_par() before this classification so harmless parentheses do not change checker behavior.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in c31f53b. remove_par() the default value before classifying it:

ast.ExprStmt {
    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 }
    }
}

Now or { (map[string]int{}) } and or { (fallback.clone()) } compile like their unparenthesized forms, while parenthesized unsafe defaults stay guarded — or { (fallback) } and or { (holder.clone()) } still emit cannot copy map (remove_par exposes the lvalue/user-call, which then fails the freshness check). Extended the runtime regression test with the parenthesized fresh cases. Full compiler_errors_test.v and all 216 vlib/v/tests/options/ tests pass.

(This one was a false-negative rather than an aliasing hole — thanks.) The standing source-side opt := ?map(mutable_base) alias is still open and unfixable locally; revert or auto-clone remain the only ways to fully close this out.

}
}
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
Expand Down Expand Up @@ -886,8 +989,38 @@ 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
&& !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)',
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
23 changes: 23 additions & 0 deletions vlib/v/checker/tests/option_map_or_unwrap_alias_ptr_source_err.vv
Original file line number Diff line number Diff line change
@@ -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)
}
Original file line number Diff line number Diff line change
@@ -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)
Original file line number Diff line number Diff line change
@@ -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)
}
Original file line number Diff line number Diff line change
@@ -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)
Original file line number Diff line number Diff line change
@@ -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)
}
Original file line number Diff line number Diff line change
@@ -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({
Original file line number Diff line number Diff line change
@@ -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)
}
14 changes: 14 additions & 0 deletions vlib/v/checker/tests/option_map_or_unwrap_mut_alias_err.out
Original file line number Diff line number Diff line change
@@ -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 |
29 changes: 29 additions & 0 deletions vlib/v/checker/tests/option_map_or_unwrap_mut_alias_err.vv
Original file line number Diff line number Diff line change
@@ -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)
}
7 changes: 7 additions & 0 deletions vlib/v/checker/tests/option_map_or_unwrap_mut_default_err.out
Original file line number Diff line number Diff line change
@@ -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)
18 changes: 18 additions & 0 deletions vlib/v/checker/tests/option_map_or_unwrap_mut_default_err.vv
Original file line number Diff line number Diff line change
@@ -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)
}
21 changes: 21 additions & 0 deletions vlib/v/checker/tests/option_map_or_unwrap_mut_source_err.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
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)
Loading
Loading