types: make ownership return-alias inference terminate on recursive fns

vlib/v/tests/ownership_recursive_return_param_alias_test.v

@@ -0,0 +1,58 @@
+// vtest vflags: -d ownership
+// Recursive calls prepend the argument projection to the callee's returned parameter path
+// (`.left.name`, `.left.right.name`, ...). Return alias inference used to grow those paths
+// forever, so compiling this file with the ownership checker never finished.
+
+struct RecursiveAliasNode {
+	name  string
+	left  &RecursiveAliasNode = unsafe { nil }
+	right &RecursiveAliasNode = unsafe { nil }
+}
+
+fn recursive_alias_outermost(n &RecursiveAliasNode, go_left bool) string {
+	if go_left && !isnil(n.left) {
+		return recursive_alias_outermost(n.left, go_left)
+	}
+	if !go_left && !isnil(n.right) {
+		return recursive_alias_outermost(n.right, go_left)
+	}
+	return n.name
+}
+
+fn recursive_alias_zigzag_left(n &RecursiveAliasNode) string {
+	if isnil(n.left) {
+		return n.name
+	}
+	return recursive_alias_zigzag_right(n.left)
+}
+
+fn recursive_alias_zigzag_right(n &RecursiveAliasNode) string {
+	if isnil(n.right) {
+		return n.name
+	}
+	return recursive_alias_zigzag_left(n.right)
+}
+
+fn test_recursive_return_param_aliases_reach_fixed_point() {
+	leaf_a := &RecursiveAliasNode{
+		name: 'a'
+	}
+	leaf_b := &RecursiveAliasNode{
+		name: 'b'
+	}
+	mid := &RecursiveAliasNode{
+		name:  'mid'
+		left:  leaf_a
+		right: leaf_b
+	}
+	root := &RecursiveAliasNode{
+		name:  'root'
+		left:  mid
+		right: leaf_a
+	}
+	assert recursive_alias_outermost(root, true) == 'a'
+	assert recursive_alias_outermost(root, false) == 'a'
+	assert recursive_alias_outermost(mid, false) == 'b'
+	assert recursive_alias_zigzag_left(root) == 'b'
+	assert recursive_alias_zigzag_right(root) == 'a'
+}

vlib/v/transform/array.v

@@ -3808,6 +3808,21 @@ fn (mut t Transformer) array_map_call_result_path_origin_is_external(source type
 	if root !in origins {
 		return true
 	}
+	if source.source_is_prefix {
+		// The result aliases some storage at or below the source path, but not a known one,
+		// so keep it external when the path covering that storage, or any path below it, is.
+		prefix_path := source_path + source.source_suffix
+		if origins[array_map_local_pointer_path(prefix_path, root, origins)] {
+			return true
+		}
+		for path, external in origins {
+			if external && !path.starts_with(array_map_local_pointer_pointee_prefix)
+				&& array_map_local_path_is_possible_projection(path, prefix_path) {
+				return true
+			}
+		}
+		return false
+	}
 	effective_path := source_path + source.source_suffix + relative_suffix
 	return origins[array_map_local_pointer_path(effective_path, root, origins)]
 }

vlib/v/types/checker.v

@@ -245,6 +245,9 @@ pub:
 	arg_id        flat.NodeId
 	source_suffix string
 	target_suffix string
+	// source_is_prefix reports that the result aliases storage at or below `source_suffix`
+	// of the argument, but not a known exact path (a path widened through recursion).
+	source_is_prefix bool
 }
 
 // LocalBinding represents local binding data used by types.

vlib/v/types/checker_ownership_alias.v

@@ -0,0 +1,63 @@
+module types
+
+// This file is compiled with and without `-d ownership`, so that its pure helpers can be
+// unit tested by the regular compiler. A `-d ownership` build selects the ownership
+// compiler, which would also run ownership analysis over the whole `types` module.
+
+const ownership_unknown_pointer_index_alias = '<unknown-index-alias>'
+
+// ownership_alias_chain_borrows_indexed_storage reports whether `rhs_name` reaches indexed
+// storage (or an unknown index alias) through the recorded pointer index aliases.
+fn ownership_alias_chain_borrows_indexed_storage(aliases map[string]string, rhs_name string) bool {
+	// Follow the complete alias chain (`arr -> val -> t[k]`). Cycles represent unresolved
+	// alias state, so treat them conservatively as borrowed storage.
+	mut cur := rhs_name
+	mut seen := map[string]bool{}
+	for {
+		if seen[cur] {
+			return true
+		}
+		seen[cur] = true
+		source := aliases[cur] or { return false }
+		if source == ownership_unknown_pointer_index_alias {
+			return true
+		}
+		if source.contains('[') {
+			return true
+		}
+		cur = source
+	}
+	return false
+}
+
+// ownership_return_param_call_source composes, in function `caller`, a callee's returned
+// parameter path behind the projection of the argument passed for that parameter: when the
+// callee returns `.name` of its parameter, `callee(p.next)` returns `.next.name` of `p`. It also
+// returns whether the result is a prefix source, which aliases storage at or below the path
+// rather than that exact path, and the functions that composed the path (`callee_via` lists
+// those of the callee's path).
+//
+// Around a call cycle the argument projection is prepended again every round (`.next.name`,
+// `.next.next.name`, ...), so the return fixed point would never converge, and it would grow
+// exponentially for a function that recurses through several fields. A cycle shows as a
+// callee path that `caller` already composed; such a call yields a prefix source at the
+// argument projection. Every other composition extends `via` by a function that is not in it
+// yet, which bounds the exact paths, and keeps acyclic call chains exact.
+fn ownership_return_param_call_source(caller string, arg_suffix string, callee_suffix string, callee_is_prefix bool, callee_via []string) (string, bool, []string) {
+	if caller in callee_via {
+		return arg_suffix, true, [caller]
+	}
+	return arg_suffix + callee_suffix, callee_is_prefix, ownership_return_param_via(callee_via,
+		caller)
+}
+
+// ownership_return_param_via returns `via` extended by `caller`, unless it already lists it.
+fn ownership_return_param_via(via []string, caller string) []string {
+	if caller in via {
+		return via
+	}
+	mut out := []string{cap: via.len + 1}
+	out << via
+	out << caller
+	return out
+}

vlib/v/types/checker_ownership_alias_test.v

@@ -1,4 +1,3 @@
-// vtest vflags: -d ownership
 module types
 
 fn test_long_pointer_index_alias_chain_is_borrowed() {
@@ -22,3 +21,117 @@ fn test_long_pointer_index_alias_chain_is_borrowed() {
 	}
 	assert ownership_alias_chain_borrows_indexed_storage(cycle, 'left')
 }
+
+fn test_return_param_call_source_keeps_acyclic_chains_exact() {
+	// `inner(p)` returns `p.left.name`, and `outer(p)` returns `inner(p.left)`: the repeated
+	// `.left` projection is not a cycle.
+	mut suffix, mut is_prefix, mut via := ownership_return_param_call_source('outer', '.left',
+		'.left.name', false, ['inner'])
+	assert suffix == '.left.left.name'
+	assert !is_prefix
+	assert via == ['inner', 'outer']
+	// Depth alone never widens a path.
+	suffix, is_prefix, via = ownership_return_param_call_source('a', '.a', '.b.c.d.e.f', false,
+		['b', 'c', 'd', 'e'])
+	assert suffix == '.a.b.c.d.e.f'
+	assert !is_prefix
+	// A prefix source stays a prefix source when it is composed further.
+	suffix, is_prefix, via = ownership_return_param_call_source('a', '.a', '.b', true, ['b'])
+	assert suffix == '.a.b'
+	assert is_prefix
+	assert via == ['b', 'a']
+}
+
+fn test_return_param_call_source_widens_call_cycles_to_prefixes() {
+	// Direct recursion: the callee's own path lists the caller.
+	mut suffix, mut is_prefix, mut via := ownership_return_param_call_source('last', '.left',
+		'.name', false, ['last'])
+	assert suffix == '.left'
+	assert is_prefix
+	assert via == ['last']
+	// Mutual recursion: `zig` composed the path that `zag` returns to it.
+	suffix, is_prefix, via = ownership_return_param_call_source('zig', '[0]', '.alt.name', false,
+		['zag', 'zig', 'zag'])
+	assert suffix == '[0]'
+	assert is_prefix
+	assert via == ['zig']
+}
+
+fn test_return_param_via_lists_each_function_once() {
+	assert ownership_return_param_via([], 'a') == ['a']
+	assert ownership_return_param_via(['a'], 'b') == ['a', 'b']
+	assert ownership_return_param_via(['a', 'b'], 'a') == ['a', 'b']
+}
+
+struct ReturnSourceCallSite {
+	caller     string
+	callee     string
+	arg_suffix string
+}
+
+struct ReturnSource {
+	suffix    string
+	is_prefix bool
+	via       []string
+}
+
+// return_sources_fixed_point runs the return alias fixed point of `sites` from base sources that
+// return `.name` of the parameter, and returns the rounds it took with the sources per function.
+fn return_sources_fixed_point(sites []ReturnSourceCallSite, fns []string) (int, map[string]map[string]ReturnSource) {
+	mut sources := map[string]map[string]ReturnSource{}
+	for f in fns {
+		sources[f] = {
+			'.name': ReturnSource{'.name', false, [f]}
+		}
+	}
+	mut rounds := 0
+	for changed := true; changed; rounds++ {
+		assert rounds < 20
+		changed = false
+		for site in sites {
+			for _, callee_source in sources[site.callee].clone() {
+				suffix, is_prefix, via := ownership_return_param_call_source(site.caller,
+					site.arg_suffix, callee_source.suffix, callee_source.is_prefix, callee_source.via)
+				key := suffix + if is_prefix { '*' } else { '' }
+				if key !in sources[site.caller] {
+					sources[site.caller][key] = ReturnSource{suffix, is_prefix, via}
+					changed = true
+				}
+			}
+		}
+	}
+	return rounds, sources
+}
+
+// Mutual recursion through two fields each: `a(p)` returns `b(p.left)`, `b(p.right)` or `p.name`,
+// and `b(p)` returns `a(p.left)`, `a(p.right)` or `p.name`. Without widening, the returned paths
+// of the return fixed point grow forever.
+fn test_return_param_call_sources_reach_fixed_point_for_mutual_recursion() {
+	rounds, sources := return_sources_fixed_point([
+		ReturnSourceCallSite{'a', 'b', '.left'},
+		ReturnSourceCallSite{'a', 'b', '.right'},
+		ReturnSourceCallSite{'b', 'a', '.left'},
+		ReturnSourceCallSite{'b', 'a', '.right'},
+	], ['a', 'b'])
+	assert rounds < 10
+	assert sources['a'].len < 16
+	assert sources['b'].len < 16
+	// The paths that did not go around the cycle stay exact.
+	assert !sources['a']['.left.name'].is_prefix
+	assert !sources['a']['.right.name'].is_prefix
+	assert sources['a']['.left*'].is_prefix
+}
+
+// `outer(p)` returns `inner(p.left)`, `inner(p)` returns `leaf(p.left)`, and `leaf(p)` returns
+// `leaf(p.next)` or `p.name`: only the recursion of `leaf` is widened.
+fn test_return_param_call_sources_widen_only_the_cycle() {
+	_, sources := return_sources_fixed_point([
+		ReturnSourceCallSite{'outer', 'inner', '.left'},
+		ReturnSourceCallSite{'inner', 'leaf', '.left'},
+		ReturnSourceCallSite{'leaf', 'leaf', '.next'},
+	], ['leaf'])
+	assert sources['leaf'].keys().sorted() == ['.name', '.next*']
+	assert sources['inner'].keys().sorted() == ['.left.name', '.left.next*']
+	assert sources['outer'].keys().sorted() == ['.left.left.name', '.left.left.next*']
+	assert sources['outer']['.left.left.name'].via == ['leaf', 'inner', 'outer']
+}