From 9d1130b359b901635ee9d64bd5fe4b6d6b5cbaa4 Mon Sep 17 00:00:00 2001 From: hl-valdemar Date: Thu, 25 Jun 2026 07:43:43 +0200 Subject: [PATCH] self-referential c record imports --- compiler/cimport/libclang.odin | 5 ++++ compiler/types/types.odin | 27 ++++++++++++++++----- compiler_tests.odin | 14 +++++++++++ examples/interop/recursive/app/main.bro | 16 ++++++++++++ examples/interop/recursive/include/native.h | 4 +++ testbed/cstdio.bro | 4 +++ testbed/cstdio.h | 1 + testbed/main.bro | 2 -- 8 files changed, 65 insertions(+), 8 deletions(-) create mode 100644 examples/interop/recursive/app/main.bro create mode 100644 examples/interop/recursive/include/native.h create mode 100644 testbed/cstdio.bro create mode 100644 testbed/cstdio.h diff --git a/compiler/cimport/libclang.odin b/compiler/cimport/libclang.odin index f669561..cf7dcdc 100644 --- a/compiler/cimport/libclang.odin +++ b/compiler/cimport/libclang.odin @@ -415,6 +415,11 @@ populate_record :: proc(ctx: ^Context, index: u32, declaration: CXCursor) { ctx.result.records[index].kind = .Union if ctx.api.get_cursor_kind(definition) == CXCursor_UnionDecl else .Struct ctx.result.records[index].size = u64(size) ctx.result.records[index].alignment = u32(alignment) + // Mark in-progress before visiting fields: a self-referential field (e.g. the + // `struct __sFILE *` inside FILE) re-enters populate_record for this same record; + // the top guard now short-circuits instead of recursing forever. The final line + // recomputes the real value from `reason`. + ctx.result.records[index].complete = true field_ctx := Record_Field_Context{ctx=ctx, record=index} _ = ctx.api.visit_children(definition, visit_record_field, &field_ctx) ctx.result.records[index].complete = len(ctx.result.records[index].reason) == 0 diff --git a/compiler/types/types.odin b/compiler/types/types.odin index db4bf11..d7e013f 100644 --- a/compiler/types/types.odin +++ b/compiler/types/types.odin @@ -640,9 +640,23 @@ is_c_signature_type :: proc(value: Type, store: ^Store, allow_void := false) -> (is_c_struct(value, store) && is_runtime_value(value, store)) } -contains_distinct :: proc(value: Type, store: ^Store, depth := 0) -> bool { - if depth > 256 { - return true +// contains_distinct reports whether a `distinct` type is reachable from `value` +// (by value, behind a pointer, through fields/params/children). A pointer to a +// distinct type still counts — distinct types do not cross the C ABI even behind +// indirection. The `seen` set makes the graph walk terminate on self-referential +// records (e.g. `?*mut Node` inside `Node`), which previously recursed until the +// depth cap and wrongly reported `true`. +contains_distinct :: proc(value: Type, store: ^Store) -> bool { + seen: [dynamic]Type + defer delete(seen) + return contains_distinct_seen(value, store, &seen) +} + +contains_distinct_seen :: proc(value: Type, store: ^Store, seen: ^[dynamic]Type) -> bool { + for visited in seen { + if visited == value { + return false + } } item, ok := node(store, value) if !ok { @@ -651,21 +665,22 @@ contains_distinct :: proc(value: Type, store: ^Store, depth := 0) -> bool { if item.kind == .Distinct { return true } + append(seen, value) if item.kind == .Struct || item.kind == .Union { for field in fields_for(store, value) { - if contains_distinct(field.type, store, depth+1) { + if contains_distinct_seen(field.type, store, seen) { return true } } } if item.kind == .Function { for param in params_for(store, value) { - if contains_distinct(param.type, store, depth+1) { + if contains_distinct_seen(param.type, store, seen) { return true } } } - return is_valid(item.child) && contains_distinct(item.child, store, depth+1) + return is_valid(item.child) && contains_distinct_seen(item.child, store, seen) } is_c_integer_promotion_candidate :: proc(value: Type) -> bool { diff --git a/compiler_tests.odin b/compiler_tests.odin index bacfbf9..afdc130 100644 --- a/compiler_tests.odin +++ b/compiler_tests.odin @@ -1972,6 +1972,20 @@ by_value_c_records_and_unions_compile_and_link :: proc(t: ^testing.T) { testing.expect_value(t, state.exit_code, 1) } +@(test) +self_referential_c_records_import_and_compile :: proc(t: ^testing.T) { + // `struct Node { struct Node *next; int value; }`: the importer must not recurse + // forever populating the self-referential record, and the checker must accept the + // self-referential `?*mut Node` field as C-layout-compatible. + output := "/tmp/brolang-test-recursive" + defer _ = os.remove(output) + c_options := cimport.Options{include_paths=[]string{"examples/interop/recursive/include"}} + status := compiler_core.compile_package("examples/interop/recursive/app", output, nil, target.DEFAULT, c_options) + testing.expect_value(t, status, 0) + state := run_executable(output) + testing.expect_value(t, state.exit_code, 0) +} + @(test) unsupported_c_header_members_diagnose_only_when_referenced :: proc(t: ^testing.T) { output := "/tmp/brolang-test-header-unsupported" diff --git a/examples/interop/recursive/app/main.bro b/examples/interop/recursive/app/main.bro new file mode 100644 index 0000000..ccd7b20 --- /dev/null +++ b/examples/interop/recursive/app/main.bro @@ -0,0 +1,16 @@ +native :: import "../include/native.h" + +# Exercises a self-referential imported C record (`struct Node { struct Node *next; int value; }`): +# the importer must not recurse forever populating it, and the checker must accept the +# self-referential `?*mut Node` field as C-layout-compatible. + +main :: func() i32 { + node native.Node = native.Node { next = none, value = 7 } + if node.next |_| { + return 1 + } + if node.value == 7 { + return 0 + } + return 2 +} diff --git a/examples/interop/recursive/include/native.h b/examples/interop/recursive/include/native.h new file mode 100644 index 0000000..2920733 --- /dev/null +++ b/examples/interop/recursive/include/native.h @@ -0,0 +1,4 @@ +typedef struct Node { + struct Node *next; + int value; +} Node; diff --git a/testbed/cstdio.bro b/testbed/cstdio.bro new file mode 100644 index 0000000..2e6353c --- /dev/null +++ b/testbed/cstdio.bro @@ -0,0 +1,4 @@ +# generated by brolang translate-c from testbed/cstdio.h + +printf :: c_func(arg0 ?*c_char, ...) c_int + diff --git a/testbed/cstdio.h b/testbed/cstdio.h new file mode 100644 index 0000000..563ef4e --- /dev/null +++ b/testbed/cstdio.h @@ -0,0 +1 @@ +int printf(const char *format, ...); diff --git a/testbed/main.bro b/testbed/main.bro index dbfed2c..1864162 100644 --- a/testbed/main.bro +++ b/testbed/main.bro @@ -1,5 +1,3 @@ -printf :: c_func(format *c_char, ...) c_int - PlayerID :: distinct u32 Tier :: enum(u8) {