Skip to content

Commit 82e947b

Browse files
authored
cgen: stabilize auxiliary type symbol hashes (#27570)
1 parent bd87753 commit 82e947b

4 files changed

Lines changed: 281 additions & 40 deletions

File tree

vlib/v/gen/c/array.v

Lines changed: 31 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1229,6 +1229,17 @@ fn (mut g Gen) gen_array_sorted(node ast.CallExpr) {
12291229
g.writeln(';')
12301230
}
12311231

1232+
// array_sort_expr_key returns a C-identifier-safe, lossless key for a sort expression.
1233+
fn array_sort_expr_key(expr_key string) string {
1234+
hex_digits := '0123456789abcdef'
1235+
mut b := strings.new_builder(expr_key.len * 2)
1236+
for ch in expr_key.bytes() {
1237+
b.write_u8(hex_digits[int(ch >> 4)])
1238+
b.write_u8(hex_digits[int(ch & 0x0f)])
1239+
}
1240+
return b.str()
1241+
}
1242+
12321243
// `users.sort(a.age < b.age)`
12331244
fn (mut g Gen) gen_array_sort(node ast.CallExpr) {
12341245
// println('filter s="${s}"')
@@ -1251,21 +1262,15 @@ fn (mut g Gen) gen_array_sort(node ast.CallExpr) {
12511262
// `users.sort(a.age > b.age)`
12521263
// Generate a comparison function for a custom type
12531264
elem_stype := g.styp(elem_type)
1254-
mut compare_fn := 'compare_${g.unique_file_path_hash}_${elem_stype.replace('*', '_ptr')}'
1265+
mut compare_fn := 'compare_${g.stable_type_symbol_hash(elem_type)}_${elem_stype.replace('*',
1266+
'_ptr')}'
12551267
mut comparison_type := g.unwrap(ast.void_type)
12561268
mut left_expr, mut right_expr := '', ''
12571269
mut use_lambda := false
12581270
mut lambda_fn_name := ''
12591271
// the only argument can only be an infix expression like `a < b` or `b.field > a.field`
12601272
if node.args.len == 0 {
12611273
comparison_type = g.unwrap(elem_type.set_nr_muls(0))
1262-
rlock g.array_sort_fn {
1263-
if compare_fn in g.array_sort_fn {
1264-
g.gen_array_sort_call(node,
1265-
g.ensure_array_sort_qsort_adapter(compare_fn, elem_type), left_is_array)
1266-
return
1267-
}
1268-
}
12691274
left_expr = '*a'
12701275
right_expr = '*b'
12711276
} else if node.args[0].expr is ast.LambdaExpr {
@@ -1288,23 +1293,18 @@ fn (mut g Gen) gen_array_sort(node ast.CallExpr) {
12881293
}
12891294
comparison_type = g.unwrap(comparison_left_type.set_nr_muls(0))
12901295
left_name := infix_expr.left.str()
1296+
expr_key := '${left_name}\n${infix_expr.op.str()}\n${infix_expr.right.str()}'
12911297
if left_name.len > 1 {
12921298
compare_fn += '_by' +
12931299
left_name[1..].replace_each(['.', '_', '[', '_', ']', '_', "'", '_', '"', '_', '(', '', ')', '', ',', '', '/', '_'])
12941300
}
1301+
compare_fn += '_expr_${array_sort_expr_key(expr_key)}'
12951302
// is_reverse is `true` for `.sort(a > b)` and `.sort(b < a)`
12961303
is_reverse := (left_name.starts_with('a') && infix_expr.op == .gt)
12971304
|| (left_name.starts_with('b') && infix_expr.op == .lt)
12981305
if is_reverse {
12991306
compare_fn += '_reverse'
13001307
}
1301-
rlock g.array_sort_fn {
1302-
if compare_fn in g.array_sort_fn {
1303-
g.gen_array_sort_call(node,
1304-
g.ensure_array_sort_qsort_adapter(compare_fn, elem_type), left_is_array)
1305-
return
1306-
}
1307-
}
13081308
if left_name.starts_with('a') != is_reverse {
13091309
left_expr = g.expr_string(infix_expr.left)
13101310
right_expr = g.expr_string(infix_expr.right)
@@ -1328,8 +1328,16 @@ fn (mut g Gen) gen_array_sort(node ast.CallExpr) {
13281328

13291329
// Register a new custom `compare_xxx` function for qsort()
13301330
// TODO: move to checker
1331+
mut already_generated := false
13311332
lock g.array_sort_fn {
1332-
g.array_sort_fn << compare_fn
1333+
already_generated = compare_fn in g.array_sort_fn
1334+
if !already_generated {
1335+
g.array_sort_fn << compare_fn
1336+
}
1337+
}
1338+
if already_generated {
1339+
g.gen_array_sort_call(node, '${compare_fn}_qsort_adapter', left_is_array)
1340+
return
13331341
}
13341342

13351343
stype_arg := g.styp(elem_type)
@@ -1380,13 +1388,15 @@ fn (g &Gen) array_sort_fn_visibility() string {
13801388

13811389
fn (mut g Gen) ensure_array_sort_qsort_adapter(compare_fn string, elem_type ast.Type) string {
13821390
qsort_compare_fn := '${compare_fn}_qsort_adapter'
1383-
rlock g.array_sort_wrappers {
1384-
if qsort_compare_fn in g.array_sort_wrappers {
1385-
return qsort_compare_fn
1391+
mut already_generated := false
1392+
lock g.array_sort_wrappers {
1393+
already_generated = qsort_compare_fn in g.array_sort_wrappers
1394+
if !already_generated {
1395+
g.array_sort_wrappers << qsort_compare_fn
13861396
}
13871397
}
1388-
lock g.array_sort_wrappers {
1389-
g.array_sort_wrappers << qsort_compare_fn
1398+
if already_generated {
1399+
return qsort_compare_fn
13901400
}
13911401
elem_stype := g.styp(elem_type)
13921402
g.sort_fn_definitions.writeln('${g.array_sort_fn_visibility()}int ${qsort_compare_fn}(const void* a, const void* b) {')

vlib/v/gen/c/cgen.v

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -140,9 +140,7 @@ mut:
140140
tmp_var_ptr map[string]bool // indicates if the tmp var passed to or_block() is a ptr
141141
labeled_loops map[string]&ast.Stmt
142142
contains_ptr_cache map[ast.Type]bool
143-
boehm_keep_decl map[string]bool
144-
boehm_keep_gen map[string]bool
145-
boehm_keep_busy map[string]bool
143+
boehm_keep_gen shared map[string]bool
146144
inner_loop &ast.Stmt = unsafe { nil }
147145
cur_indexexpr []int // list of nested indexexpr which generates array_set/map_set
148146
shareds map[int]string // types with hidden mutex for which decl has been emitted
@@ -433,9 +431,7 @@ pub fn gen(files []&ast.File, mut table ast.Table, pref_ &pref.Preferences) GenO
433431
has_debugger: 'v.debug' in table.modules
434432
reflection_strings: &reflection_strings
435433
generated_map_key_fns: map[ast.Type]bool{}
436-
boehm_keep_decl: map[string]bool{}
437434
boehm_keep_gen: map[string]bool{}
438-
boehm_keep_busy: map[string]bool{}
439435
closure_frame_arg_tmps: map[int]string{}
440436
generic_parts_cache: []i8{len: table.type_symbols.len}
441437
unwrap_generic_cache: map[u64]ast.Type{}
@@ -1092,9 +1088,7 @@ fn cgen_process_one_file_cb(mut p pool.PoolProcessor, idx int, wid int) voidptr
10921088
has_debugger: 'v.debug' in global_g.table.modules
10931089
reflection_strings: global_g.reflection_strings
10941090
generated_map_key_fns: map[ast.Type]bool{}
1095-
boehm_keep_decl: map[string]bool{}
1096-
boehm_keep_gen: map[string]bool{}
1097-
boehm_keep_busy: map[string]bool{}
1091+
boehm_keep_gen: global_g.boehm_keep_gen
10981092
closure_frame_arg_tmps: map[int]string{}
10991093
generic_parts_cache: []i8{len: global_g.table.type_symbols.len}
11001094
unwrap_generic_cache: map[u64]ast.Type{}
@@ -8681,18 +8675,19 @@ fn (mut g Gen) boehm_collect_keep_alive_helper_name(typ ast.Type) string {
86818675
if styp.ends_with('_ptr') {
86828676
return ''
86838677
}
8684-
fn_name := '__v_boehm_collect_keepalive_${g.unique_file_path_hash}_${styp.replace('*', '_ptr').replace(' ', '_')}'
8685-
if g.boehm_keep_gen[fn_name] {
8686-
return fn_name
8687-
}
8688-
if !g.boehm_keep_decl[fn_name] {
8689-
g.definitions.writeln('static inline int ${fn_name}(${styp}* it, voidptr* out, int idx);')
8690-
g.boehm_keep_decl[fn_name] = true
8678+
fn_name := '__v_boehm_collect_keepalive_${g.stable_type_symbol_hash(resolved_typ)}_${styp.replace('*',
8679+
'_ptr').replace(' ', '_')}'
8680+
mut should_generate := false
8681+
lock g.boehm_keep_gen {
8682+
if fn_name !in g.boehm_keep_gen {
8683+
g.boehm_keep_gen[fn_name] = true
8684+
should_generate = true
8685+
}
86918686
}
8692-
if g.boehm_keep_busy[fn_name] {
8687+
if !should_generate {
86938688
return fn_name
86948689
}
8695-
g.boehm_keep_busy[fn_name] = true
8690+
g.definitions.writeln('static inline int ${fn_name}(${styp}* it, voidptr* out, int idx);')
86968691
mut sb := strings.new_builder(256)
86978692
sb.writeln('static inline int ${fn_name}(${styp}* it, voidptr* out, int idx) {')
86988693
match sym.kind {
@@ -8776,8 +8771,6 @@ fn (mut g Gen) boehm_collect_keep_alive_helper_name(typ ast.Type) string {
87768771

87778772
sb.writeln('}')
87788773
g.auto_fn_definitions << sb.str()
8779-
g.boehm_keep_gen[fn_name] = true
8780-
g.boehm_keep_busy.delete(fn_name)
87818774
return fn_name
87828775
}
87838776

vlib/v/gen/c/coutput_test.v

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -633,6 +633,90 @@ fn test_array_sort_with_compare_uses_stable_sort_adapters() {
633633
assert normalized.contains('_qsort_adapter);')
634634
}
635635

636+
fn test_array_sort_expression_key_avoids_sanitized_name_collisions() {
637+
os.chdir(vroot) or {}
638+
test_dir := os.join_path(os.vtmp_dir(), 'coutput_array_sort_expr_collision_${os.getpid()}')
639+
os.mkdir_all(test_dir)!
640+
defer {
641+
os.rmdir_all(test_dir) or {}
642+
}
643+
os.write_file(os.join_path(test_dir, 'main.v'),
644+
['module main', '', 'struct Inner {', '\tbar int', '}', '', 'struct Item {', '\tfoo Inner', '\tfoo_bar int', '\tlabel string', '}', '', 'fn main() {', '\tprintln(sort_by_nested())', '\tprintln(sort_by_flat())', '}'].join('\n') +
645+
'\n')!
646+
os.write_file(os.join_path(test_dir, 'nested.v'),
647+
['module main', '', 'fn sort_by_nested() string {', "\tmut items := [Item{ foo: Inner{ bar: 2 }, foo_bar: 1, label: 'nested-wrong' }, Item{ foo: Inner{ bar: 1 }, foo_bar: 2, label: 'nested-ok' }]", '\titems.sort(a.foo.bar < b.foo.bar)', '\treturn items[0].label', '}'].join('\n') +
648+
'\n')!
649+
os.write_file(os.join_path(test_dir, 'flat.v'),
650+
['module main', '', 'fn sort_by_flat() string {', "\tmut items := [Item{ foo: Inner{ bar: 1 }, foo_bar: 2, label: 'flat-wrong' }, Item{ foo: Inner{ bar: 2 }, foo_bar: 1, label: 'flat-ok' }]", '\titems.sort(a.foo_bar < b.foo_bar)', '\treturn items[0].label', '}'].join('\n') +
651+
'\n')!
652+
pexe := os.join_path(test_dir, 'sort_expr_collision')
653+
cmd := '${os.quoted_path(vexe)} -o ${os.quoted_path(pexe)} ${os.quoted_path(test_dir)}'
654+
compilation := os.execute(cmd)
655+
ensure_compilation_succeeded(compilation, cmd)
656+
res := os.execute(os.quoted_path(pexe))
657+
assert res.exit_code == 0
658+
assert res.output.trim_space().replace('\r\n', '\n') == 'nested-ok\nflat-ok'
659+
}
660+
661+
// generated_c_symbol_with_prefix extracts one generated C symbol from compiler output.
662+
fn generated_c_symbol_with_prefix(output string, prefix string) string {
663+
idx := output.index(prefix) or { return '' }
664+
rest := output[idx..]
665+
mut end := 0
666+
for end < rest.len && (rest[end].is_letter() || rest[end].is_digit() || rest[end] == `_`) {
667+
end++
668+
}
669+
return rest[..end]
670+
}
671+
672+
fn test_auxiliary_c_symbols_use_stable_type_hashes() {
673+
os.chdir(vroot) or {}
674+
test_dir := os.join_path(os.vtmp_dir(), 'coutput_stable_type_symbol_hash_${os.getpid()}')
675+
dir_a := os.join_path(test_dir, 'a')
676+
dir_b := os.join_path(test_dir, 'b')
677+
os.mkdir_all(dir_a)!
678+
os.mkdir_all(dir_b)!
679+
defer {
680+
os.rmdir_all(test_dir) or {}
681+
}
682+
source_lines := [
683+
'module main',
684+
'',
685+
'struct Item {',
686+
'\tname string',
687+
'\trank int',
688+
'}',
689+
'',
690+
'fn main() {',
691+
"\tmut items := [Item{'b', 2}, Item{'a', 1}, Item{'c', 3}]",
692+
'\titems.sort(a.rank < b.rank)',
693+
'\tmut nested := [items]',
694+
'\tprintln("\${nested[0][0].name}")',
695+
'}',
696+
]
697+
source := source_lines.join('\n') + '\n'
698+
path_a := os.join_path(dir_a, 'main.v')
699+
path_b := os.join_path(dir_b, 'main.v')
700+
os.write_file(path_a, source)!
701+
os.write_file(path_b, source)!
702+
cmd_a := '${os.quoted_path(vexe)} -prod -gc boehm_full_opt -o - ${os.quoted_path(path_a)}'
703+
cmd_b := '${os.quoted_path(vexe)} -prod -gc boehm_full_opt -o - ${os.quoted_path(path_b)}'
704+
compilation_a := os.execute(cmd_a)
705+
compilation_b := os.execute(cmd_b)
706+
ensure_compilation_succeeded(compilation_a, cmd_a)
707+
ensure_compilation_succeeded(compilation_b, cmd_b)
708+
compare_a := generated_c_symbol_with_prefix(compilation_a.output, 'compare_')
709+
compare_b := generated_c_symbol_with_prefix(compilation_b.output, 'compare_')
710+
keepalive_a := generated_c_symbol_with_prefix(compilation_a.output,
711+
'__v_boehm_collect_keepalive_')
712+
keepalive_b := generated_c_symbol_with_prefix(compilation_b.output,
713+
'__v_boehm_collect_keepalive_')
714+
assert compare_a != ''
715+
assert keepalive_a != ''
716+
assert compare_a == compare_b
717+
assert keepalive_a == keepalive_b
718+
}
719+
636720
fn test_veb_implicit_ctx_alias_uses_user_context_name() {
637721
os.chdir(vroot) or {}
638722
test_source := os.join_path(os.vtmp_dir(), 'coutput_veb_implicit_ctx_alias.vv')

0 commit comments

Comments
 (0)