Skip to content

Commit cb106e4

Browse files
committed
v3: address PR review batch 14 — reject escaping method values, generic fn-type params, non-decimal const lengths
The checker now rejects a method value that escapes its evaluation site — returned from a factory (`fn bind(c) fn () int { return c.report }`), stored in a struct field, or put in a map — not just stored in an array (batch 11). The per-site static `_mvctx_N` receiver cannot keep several escaped callbacks distinct, so they would all invoke the last receiver (a real fix needs closure capture, which v3 lacks). Immediate callback arguments and local bindings still work; self-host and doka are unaffected. subst_generic_text (checker, generic-receiver call signatures) and substitute_generic_type_text_with_params (transform, specialized method bodies) now recurse into `fn (...) ...` parameter types, so `fn (b Box[T]) apply(cb fn (T) int)` on `Box[string]` expects and emits `fn (string) int` instead of the placeholder `fn (T) int` (which cgen would otherwise render as `fn (int) int`). Const fixed-array length folding now parses non-decimal V integer literals (hex `0x`, octal `0o`, binary `0b`, with `_` separators) via a new v_int_literal_value, so `[0xF & 6]int` and `const n = 0b1100 >> 1` fold to a numeric literal for both the length guard and the emitted C dimension instead of returning none.
1 parent d91e56e commit cb106e4

3 files changed

Lines changed: 199 additions & 13 deletions

File tree

vlib/v3/tests/type_checker_errors_test.v

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -610,11 +610,11 @@ fn test_pr_review_codegen_batch_eleven() {
610610
// reaching cgen and emitting an undefined helper. Append form:
611611
run_bad(v3_bin, 'bad_method_value_array_append',
612612
'struct Counter {\n\tid int\n}\nfn (c Counter) report() int {\n\treturn c.id\n}\nfn main() {\n\tmut cbs := []fn () int{}\n\tfor i in 0 .. 3 {\n\t\tc := Counter{\n\t\t\tid: i * 10\n\t\t}\n\t\tcbs << c.report\n\t}\n\tprintln(int_str(cbs.len))\n}\n',
613-
'cannot be stored in an array')
613+
'cannot escape its call site')
614614
// Array-literal form is rejected too.
615615
run_bad(v3_bin, 'bad_method_value_array_literal',
616616
'struct Counter {\n\tid int\n}\nfn (c Counter) report() int {\n\treturn c.id\n}\nfn main() {\n\ta := Counter{\n\t\tid: 1\n\t}\n\tb := Counter{\n\t\tid: 2\n\t}\n\tcbs := [a.report, b.report]\n\tprintln(int_str(cbs.len))\n}\n',
617-
'cannot be stored in an array')
617+
'cannot escape its call site')
618618
// The supported single-use forms still work: a method value passed directly as a
619619
// callback argument, and an `arr << int` append / `int << int` shift are not flagged.
620620
imm := run_good(v3_bin, 'good_method_value_immediate_after_guard',
@@ -661,3 +661,33 @@ fn test_pr_review_codegen_batch_thirteen() {
661661
'fn take(a [8 >>> 1]int) int {\n\treturn a[0]\n}\nfn main() {\n\t_ := take([1, 2]!)\n}\n',
662662
'cannot use')
663663
}
664+
665+
// Regression tests for the fourteenth PR-review batch (vlang/v#27557).
666+
fn test_pr_review_codegen_batch_fourteen() {
667+
v3_bin := build_v3()
668+
// Non-decimal integer literals (hex `0x`, octal `0o`, binary `0b`) fold in const
669+
// fixed-array lengths: `0xF & 6` = 6, `0b1100 >> 1` = 6, `0o17 & 8` = 8.
670+
nondec := run_good(v3_bin, 'good_non_decimal_const_fixed_array_len',
671+
'const flags = 0b1010 | 0b0100\nfn main() {\n\ta := [0xF & 6]int{}\n\tb := [0b1100 >> 1]int{}\n\tc := [0o17 & 8]int{}\n\td := [flags]int{}\n\tprintln(int_str(a.len + b.len + c.len + d.len))\n}\n')
672+
// 6 + 6 + 8 + 14 = 34
673+
assert nondec == '34'
674+
// The length guard evaluates non-decimal too: `[1, 2]` does not match `[0xF & 6]int`.
675+
run_bad(v3_bin, 'bad_non_decimal_fixed_array_literal_len',
676+
'fn take(a [0xF & 6]int) int {\n\treturn a[0]\n}\nfn main() {\n\t_ := take([1, 2]!)\n}\n',
677+
'cannot use')
678+
// A generic-receiver method with a function-type parameter substitutes the type params
679+
// inside the signature, so `Box[string].apply` expects `fn (string) int`, and a matching
680+
// callback is accepted and emitted with the right fn-pointer type.
681+
apply := run_good(v3_bin, 'good_generic_method_fn_type_param',
682+
"struct Box[T] {\n\tv T\n}\nfn (b Box[T]) apply(cb fn (T) int) int {\n\treturn cb(b.v)\n}\nfn slen(s string) int {\n\treturn s.len\n}\nfn main() {\n\tb := Box[string]{\n\t\tv: 'hello'\n\t}\n\tprintln(int_str(b.apply(slen)))\n}\n")
683+
assert apply == '5'
684+
// A method value that escapes its call site is rejected: returned from a factory, stored
685+
// in a struct field, or put in a map (the per-site static receiver can't keep several
686+
// instances distinct). Immediately-passed callbacks and local bindings still work.
687+
run_bad(v3_bin, 'bad_method_value_return_escape',
688+
'struct Counter {\n\tid int\n}\nfn (c Counter) report() int {\n\treturn c.id\n}\nfn bind(c Counter) fn () int {\n\treturn c.report\n}\nfn main() {\n\t_ := bind(Counter{\n\t\tid: 1\n\t})\n}\n',
689+
'cannot escape its call site')
690+
run_bad(v3_bin, 'bad_method_value_struct_field_escape',
691+
'struct Counter {\n\tid int\n}\nfn (c Counter) report() int {\n\treturn c.id\n}\nstruct Engine {\n\tcb fn () int\n}\nfn main() {\n\t_ := Engine{\n\t\tcb: Counter{\n\t\t\tid: 1\n\t\t}.report\n\t}\n}\n',
692+
'cannot escape its call site')
693+
}

vlib/v3/transform/monomorphize.v

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1812,6 +1812,14 @@ fn substitute_generic_type_text_with_params(typ string, args []string, params []
18121812
substitute_generic_type_text_with_params(clean[bracket_end + 1..], args, params)
18131813
}
18141814
}
1815+
if clean.starts_with('fn(') || clean.starts_with('fn (') {
1816+
// Substitute the generic params inside a function-type parameter so a specialized
1817+
// method body emits e.g. `fn (string) int`, not the placeholder `fn (T) int` (which
1818+
// cgen would otherwise render as `fn (int) int`).
1819+
if sub := subst_generic_fn_type_text(clean, args, params) {
1820+
return sub
1821+
}
1822+
}
18151823
base, nested_args, ok := generic_app_parts(clean)
18161824
if ok {
18171825
mut resolved_args := []string{}
@@ -1823,6 +1831,54 @@ fn substitute_generic_type_text_with_params(typ string, args []string, params []
18231831
return clean
18241832
}
18251833

1834+
// subst_generic_fn_type_text substitutes generic params inside a `fn (...) ...` type text,
1835+
// recursing into each parameter type and the return type. Returns none when the signature
1836+
// is malformed (unbalanced parens).
1837+
fn subst_generic_fn_type_text(clean string, args []string, params []string) ?string {
1838+
params_start := clean.index_u8(`(`) + 1
1839+
mut depth := 1
1840+
mut params_end := params_start
1841+
for params_end < clean.len {
1842+
if clean[params_end] == `(` {
1843+
depth++
1844+
} else if clean[params_end] == `)` {
1845+
depth--
1846+
if depth == 0 {
1847+
break
1848+
}
1849+
}
1850+
params_end++
1851+
}
1852+
if params_end >= clean.len {
1853+
return none
1854+
}
1855+
params_str := clean[params_start..params_end]
1856+
mut fn_parts := []string{}
1857+
if params_str.trim_space().len > 0 {
1858+
mut pdepth := 0
1859+
mut start := 0
1860+
for i := 0; i < params_str.len; i++ {
1861+
c := params_str[i]
1862+
if c == `(` || c == `[` {
1863+
pdepth++
1864+
} else if c == `)` || c == `]` {
1865+
pdepth--
1866+
} else if c == `,` && pdepth == 0 {
1867+
fn_parts << substitute_generic_type_text_with_params(params_str[start..i], args,
1868+
params)
1869+
start = i + 1
1870+
}
1871+
}
1872+
fn_parts << substitute_generic_type_text_with_params(params_str[start..], args, params)
1873+
}
1874+
ret_str := clean[params_end + 1..].trim_space()
1875+
if ret_str.len > 0 {
1876+
return 'fn(${fn_parts.join(', ')}) ${substitute_generic_type_text_with_params(ret_str,
1877+
args, params)}'
1878+
}
1879+
return 'fn(${fn_parts.join(', ')})'
1880+
}
1881+
18261882
// subst_type substitutes generic placeholders in a type-text using the currently
18271883
// active generic parameter names (so non-canonical params resolve by name). Falls
18281884
// back to positional substitution when no params are active.

vlib/v3/types/checker.v

Lines changed: 111 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1670,6 +1670,56 @@ fn is_decimal_int_literal(s string) bool {
16701670
return true
16711671
}
16721672

1673+
// v_int_literal_value parses a complete V integer literal — decimal, hex (`0x`), octal
1674+
// (`0o`), or binary (`0b`), with optional `_` digit separators — to its value. Returns
1675+
// none when `s` is not a whole integer literal (a const name, an expression, etc.), so
1676+
// const-length folding accepts `0xF & 6` / `[0b1100 >> 1]int`, not just decimal text.
1677+
fn v_int_literal_value(s string) ?int {
1678+
if s.len == 0 {
1679+
return none
1680+
}
1681+
t := s.replace('_', '')
1682+
if t.len == 0 {
1683+
return none
1684+
}
1685+
mut base := 10
1686+
mut digits := t
1687+
if t.len >= 2 && t[0] == `0` {
1688+
c := t[1]
1689+
if c == `x` || c == `X` {
1690+
base = 16
1691+
digits = t[2..]
1692+
} else if c == `o` || c == `O` {
1693+
base = 8
1694+
digits = t[2..]
1695+
} else if c == `b` || c == `B` {
1696+
base = 2
1697+
digits = t[2..]
1698+
}
1699+
}
1700+
if digits.len == 0 {
1701+
return none
1702+
}
1703+
mut value := 0
1704+
for ch in digits {
1705+
mut d := 0
1706+
if ch >= `0` && ch <= `9` {
1707+
d = int(ch - `0`)
1708+
} else if ch >= `a` && ch <= `f` {
1709+
d = int(ch - `a`) + 10
1710+
} else if ch >= `A` && ch <= `F` {
1711+
d = int(ch - `A`) + 10
1712+
} else {
1713+
return none
1714+
}
1715+
if d >= base {
1716+
return none
1717+
}
1718+
value = value * base + d
1719+
}
1720+
return value
1721+
}
1722+
16731723
// is_bare_generic_param reports whether is bare generic param applies in types.
16741724
fn is_bare_generic_param(typ string) bool {
16751725
return typ.len == 1 && typ[0] >= `A` && typ[0] <= `Z`
@@ -2080,12 +2130,17 @@ fn (mut tc TypeChecker) check_node(id flat.NodeId) {
20802130
tc.check_select_stmt(node)
20812131
return
20822132
}
2083-
// A method value stored in an array escapes the single-use guarantee of its per-site
2084-
// static receiver, so reject `[obj.method]` literals and `arr << obj.method` appends.
2133+
// A method value stored in a container escapes the single-use guarantee of its per-site
2134+
// static receiver, so reject `[obj.method]` / `arr << obj.method` / `{'k': obj.method}`.
20852135
if node.kind == .array_literal {
20862136
for i in 0 .. node.children_count {
20872137
tc.reject_stored_method_value(tc.a.child(&node, i))
20882138
}
2139+
} else if node.kind == .map_init {
2140+
// children alternate key, value, key, value, ...; check the value positions.
2141+
for j := 1; j < node.children_count; j += 2 {
2142+
tc.reject_stored_method_value(tc.a.child(&node, j))
2143+
}
20892144
} else if node.kind == .infix && node.op == .left_shift && node.children_count >= 2 {
20902145
if unwrap_pointer(tc.resolve_type(tc.a.child(&node, 0))) is Array {
20912146
tc.reject_stored_method_value(tc.a.child(&node, 1))
@@ -2516,6 +2571,12 @@ fn (mut tc TypeChecker) resolve_lvalue_type(lhs_id flat.NodeId) Type {
25162571

25172572
// check_return validates check return state for types.
25182573
fn (mut tc TypeChecker) check_return(id flat.NodeId, node flat.Node) {
2574+
// A returned method value escapes the function, where its per-site static receiver
2575+
// can't keep multiple returned callbacks distinct (a factory `fn bind(c) fn () int {
2576+
// return c.report }`); reject it rather than emitting invalid C.
2577+
for i in 0 .. node.children_count {
2578+
tc.reject_stored_method_value(tc.a.child(&node, i))
2579+
}
25192580
expected := tc.cur_fn_ret_type
25202581
if expected is Void {
25212582
if node.children_count > 0 && tc.should_diagnose(id) {
@@ -4465,6 +4526,9 @@ fn (mut tc TypeChecker) check_struct_init(id flat.NodeId, node flat.Node) {
44654526
continue
44664527
}
44674528
value_id := tc.a.child(&field, 0)
4529+
// A method value stored in a struct field escapes the evaluation site (several
4530+
// instances from the same `Foo{cb: obj.method}` site would share one receiver).
4531+
tc.reject_stored_method_value(value_id)
44684532
mut expected := Type(void_)
44694533
if field.value.len > 0 {
44704534
mut found := false
@@ -4527,14 +4591,17 @@ fn (tc &TypeChecker) expr_is_method_value(id flat.NodeId) bool {
45274591
return false
45284592
}
45294593

4530-
// reject_stored_method_value reports a clear error when a method value is stored into an
4531-
// array, where the per-site static receiver slot cannot keep multiple instances distinct
4532-
// (e.g. `cbs << obj.method` in a loop would make every callback use the last receiver).
4533-
// Without this the value reaches cgen and emits C referencing an unsupported helper.
4594+
// reject_stored_method_value reports a clear error when a method value escapes its
4595+
// evaluation site — stored in an array/map/struct field or returned. The per-site static
4596+
// receiver slot cannot keep several live instances distinct, so a factory like
4597+
// `fn bind(c Counter) fn () int { return c.report }` (or storage in a loop) would make
4598+
// every escaped callback use the last receiver; without this the value also reaches cgen
4599+
// and emits C referencing an unsupported helper. Pass method values directly as a
4600+
// callback argument instead (a real closure capture is not yet supported).
45344601
fn (mut tc TypeChecker) reject_stored_method_value(id flat.NodeId) {
45354602
if tc.expr_is_method_value(id) && tc.should_diagnose(id) {
45364603
tc.record_error(.assignment_mismatch,
4537-
'a method value (`obj.method`) cannot be stored in an array (no closure capture); pass it directly as a callback argument',
4604+
'a method value (`obj.method`) cannot escape its call site (no closure capture); pass it directly as a callback argument',
45384605
id)
45394606
}
45404607
}
@@ -5354,8 +5421,8 @@ pub fn (tc &TypeChecker) const_int_value(name string, seen []string) ?int {
53545421
return tc.const_int_expr(expr_id, next_seen)
53555422
}
53565423
}
5357-
if is_decimal_int_literal(name) {
5358-
return name.int()
5424+
if v := v_int_literal_value(name) {
5425+
return v
53595426
}
53605427
// Simple const arithmetic in string form, e.g. a fixed-array size `[SEGS + 1]`,
53615428
// `[SEGS+1]`, `[segs / 2]`, `[segs % 4]` or `[2 * (segs + 1)]`. A length wrapped
@@ -5453,8 +5520,8 @@ fn (tc &TypeChecker) const_int_expr(id flat.NodeId, seen []string) ?int {
54535520
node := tc.a.nodes[int(id)]
54545521
match node.kind {
54555522
.int_literal {
5456-
if is_decimal_int_literal(node.value) {
5457-
return node.value.int()
5523+
if v := v_int_literal_value(node.value) {
5524+
return v
54585525
}
54595526
}
54605527
.ident {
@@ -7597,6 +7664,39 @@ fn subst_generic_text(typ string, args []string, params []string) string {
75977664
1..], args, params)
75987665
}
75997666
}
7667+
if clean.starts_with('fn(') || clean.starts_with('fn (') {
7668+
// A function-type parameter (`fn (T) int`) carries the generic params in its own
7669+
// signature; substitute each parameter and the return type so a `Box[string].apply`
7670+
// callback is expected as `fn (string) int`, not the unsubstituted `fn (T) int`.
7671+
params_start := clean.index_u8(`(`) + 1
7672+
mut depth := 1
7673+
mut params_end := params_start
7674+
for params_end < clean.len {
7675+
if clean[params_end] == `(` {
7676+
depth++
7677+
} else if clean[params_end] == `)` {
7678+
depth--
7679+
if depth == 0 {
7680+
break
7681+
}
7682+
}
7683+
params_end++
7684+
}
7685+
if params_end < clean.len {
7686+
mut fn_parts := []string{}
7687+
params_str := clean[params_start..params_end]
7688+
if params_str.trim_space().len > 0 {
7689+
for part in split_params(params_str) {
7690+
fn_parts << subst_generic_text(normalize_fn_type_param_text(part), args, params)
7691+
}
7692+
}
7693+
ret_str := clean[params_end + 1..].trim_space()
7694+
if ret_str.len > 0 {
7695+
return 'fn(${fn_parts.join(', ')}) ${subst_generic_text(ret_str, args, params)}'
7696+
}
7697+
return 'fn(${fn_parts.join(', ')})'
7698+
}
7699+
}
76007700
bracket := clean.index_u8(`[`)
76017701
if bracket > 0 {
76027702
bracket_end := find_matching_bracket(clean, bracket)

0 commit comments

Comments
 (0)