From 147542b5cc97abbebb4f95cb32377af2c0d4f523 Mon Sep 17 00:00:00 2001 From: Harold Brenes Date: Mon, 23 Mar 2026 20:55:44 -0400 Subject: [PATCH 1/5] Allow pointers to types which have subtype fields at offset 0 to be assignable in proc parameters. ```odin // Virtual interface IFoo :: struct { foo: proc( self: ^IFoo ), } // Implements IFoo interface Foo :: struct { using vt: IFoo, name: string, } // Implement interface via `Foo` Foo_Impl :: IFoo { // `self` of type `^Foo` (not `^IFoo`) is now accepted as a valid parameter. foo = proc( self: ^Foo ) { ... }, } ``` --- src/check_expr.cpp | 52 +++++++++++++++++++++++++++++++++++++++++++++- src/types.cpp | 36 ++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 1 deletion(-) diff --git a/src/check_expr.cpp b/src/check_expr.cpp index 9044d8346..597c641a2 100644 --- a/src/check_expr.cpp +++ b/src/check_expr.cpp @@ -667,6 +667,7 @@ gb_internal bool find_or_generate_polymorphic_procedure_from_parameters(CheckerC gb_internal bool check_type_specialization_to(CheckerContext *c, Type *specialization, Type *type, bool compound, bool modify_type); gb_internal bool is_polymorphic_type_assignable(CheckerContext *c, Type *poly, Type *source, bool compound, bool modify_type); gb_internal bool check_cast_internal(CheckerContext *c, Operand *x, Type *type); +gb_internal bool check_proc_params_assignable(CheckerContext *c, Type *x, Type *y); #define MAXIMUM_TYPE_DISTANCE 10 @@ -927,8 +928,12 @@ gb_internal i64 check_distance_between_types(CheckerContext *c, Operand *operand add_entity_use(c, operand->expr, e); return 4; } + + if (is_type_proc(src) && check_proc_params_assignable(c, dst, src)) { + return 4; + } } - + if (is_type_complex_or_quaternion(dst)) { Type *elem = base_complex_elem_type(dst); if (are_types_identical(elem, base_type(src))) { @@ -1052,6 +1057,51 @@ gb_internal bool internal_check_is_assignable_to(Type *src, Type *dst) { return check_is_assignable_to(nullptr, &x, dst); } +gb_internal bool check_proc_params_assignable(CheckerContext *c, Type *dst, Type *src) { + GB_ASSERT(dst->kind == Type_Proc); + GB_ASSERT(src->kind == Type_Proc); + + if (!dst->Proc.params || !src->Proc.params) { + return false; + } + + if (!are_types_identical(src->Proc.results, dst->Proc.results)) { + return false; + } + + auto& dst_tuple = dst->Proc.params->Tuple; + auto& src_tuple = src->Proc.params->Tuple; + + if (dst_tuple.variables.count == src_tuple.variables.count && dst_tuple.is_packed == src_tuple.is_packed) { + for_array(i, dst_tuple.variables) { + Entity *edst = dst_tuple.variables[i]; + Entity *esrc = src_tuple.variables[i]; + + if (edst->kind != esrc->kind || !are_types_identical(edst->type, esrc->type)) { + + // Pointers to subtype fields that are at byte offset 0 are OK + if (edst->type->kind == Type_Pointer && esrc->type->kind == Type_Pointer && + is_type_struct(esrc->type->Pointer.elem) && + check_is_assignable_to_using_offset_zero_subtype(esrc->type->Pointer.elem, edst->type->Pointer.elem)) { + continue; + } + + return false; + } + + if (edst->kind == Entity_Constant && !compare_exact_values(Token_CmpEq, edst->Constant.value, esrc->Constant.value)) { + // NOTE(bill): This is needed for polymorphic procedures + return false; + } + } + + return true; + } + + return false; +} + + gb_internal AstPackage *get_package_of_type(Type *type) { for (;;) { if (type == nullptr) { diff --git a/src/types.cpp b/src/types.cpp index f4b708e57..6d033eeb3 100644 --- a/src/types.cpp +++ b/src/types.cpp @@ -4922,6 +4922,42 @@ gb_internal isize check_is_assignable_to_using_subtype(Type *src, Type *dst, isi return 0; } +gb_internal bool check_is_assignable_to_using_offset_zero_subtype(Type *src, Type *dst) { + + Type *src_struct = base_type(src); + if (!is_type_struct(src_struct)) { + return false; + } + + // We check multiple fields in case of #raw_union, + // but exit on the first field that is not at offset 0. + for_array(i, src_struct->Struct.fields) { + Entity *f = src_struct->Struct.fields[i]; + if (f->kind != Entity_Variable || (f->flags&EntityFlags_IsSubtype) == 0) { + continue; + } + + Type *field_type = nullptr; + i64 offset = type_offset_of(src_struct, i, &field_type); + + // Only allowed if the subtype field shared the same address as its container + if (offset != 0) { + return false; + } + + if (are_types_identical(field_type, dst)) { + return true; + } + + // Check parent if the field type is a struct + if (check_is_assignable_to_using_offset_zero_subtype(field_type, dst)) { + return true; + } + } + + return false; +} + gb_internal bool is_type_subtype_of(Type *src, Type *dst) { if (are_types_identical(src, dst)) { return true; From 4f6caf19f01701d794a4a1fd5b05494edd5dc069 Mon Sep 17 00:00:00 2001 From: Harold Brenes Date: Mon, 23 Mar 2026 21:15:12 -0400 Subject: [PATCH 2/5] Ensure checking for proc property equality before checking param assignability --- src/check_expr.cpp | 2 +- src/types.cpp | 13 ++++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/check_expr.cpp b/src/check_expr.cpp index 597c641a2..d6c91ce94 100644 --- a/src/check_expr.cpp +++ b/src/check_expr.cpp @@ -929,7 +929,7 @@ gb_internal i64 check_distance_between_types(CheckerContext *c, Operand *operand return 4; } - if (is_type_proc(src) && check_proc_params_assignable(c, dst, src)) { + if (is_type_proc(src) && are_proc_properties_identical(dst,src) && check_proc_params_assignable(c, dst, src)) { return 4; } } diff --git a/src/types.cpp b/src/types.cpp index 6d033eeb3..ffd0fc196 100644 --- a/src/types.cpp +++ b/src/types.cpp @@ -3058,6 +3058,13 @@ gb_internal bool are_types_identical_unique_tuples(Type *x, Type *y) { return are_types_identical_internal(x, y, true); } +gb_internal bool are_proc_properties_identical(Type *x, Type *y) { + return x->Proc.calling_convention == y->Proc.calling_convention && + x->Proc.c_vararg == y->Proc.c_vararg && + x->Proc.variadic == y->Proc.variadic && + x->Proc.diverging == y->Proc.diverging && + x->Proc.optional_ok == y->Proc.optional_ok; +} gb_internal bool are_types_identical_internal(Type *x, Type *y, bool check_tuple_names) { if (x == y) { @@ -3262,11 +3269,7 @@ gb_internal bool are_types_identical_internal(Type *x, Type *y, bool check_tuple break; case Type_Proc: - return x->Proc.calling_convention == y->Proc.calling_convention && - x->Proc.c_vararg == y->Proc.c_vararg && - x->Proc.variadic == y->Proc.variadic && - x->Proc.diverging == y->Proc.diverging && - x->Proc.optional_ok == y->Proc.optional_ok && + return are_proc_properties_identical(x, y) && are_types_identical_internal(x->Proc.params, y->Proc.params, check_tuple_names) && are_types_identical_internal(x->Proc.results, y->Proc.results, check_tuple_names); From 7a017d2ecd78b8eaf8c5d32841beeba2ccaf5406 Mon Sep 17 00:00:00 2001 From: Harold Brenes Date: Tue, 24 Mar 2026 15:37:04 -0400 Subject: [PATCH 3/5] Add test case for implicit cast pointer to offset zero subtype field proc param --- tests/issues/run.bat | 2 + tests/issues/run.sh | 7 +++ tests/issues/test_issue_xxxx.odin | 79 +++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+) create mode 100644 tests/issues/test_issue_xxxx.odin diff --git a/tests/issues/run.bat b/tests/issues/run.bat index 3db9d48c2..3c722ccc7 100644 --- a/tests/issues/run.bat +++ b/tests/issues/run.bat @@ -32,6 +32,8 @@ set COMMON=-define:ODIN_TEST_FANCY=false -file -vet -strict-style -ignore-unused ..\..\..\odin test ..\test_issue_6165.odin %COMMON% || exit /b ..\..\..\odin build ..\test_issue_6240.odin %COMMON% 2>&1 | find /c "Error:" | findstr /x "3" || exit /b ..\..\..\odin build ..\test_issue_6401.odin %COMMON% 2>&1 | find /c "Error:" | findstr /x "3" || exit /b +..\..\..\odin test ..\test_issue_xxxx.odin %COMMON% || exit /b +..\..\..\odin build ..\test_issue_xxxx.odin -define:TEST_EXPECT_FAILURE=true %COMMON% 2>&1 | find /c "Error:" | findstr /x "1" || exit /b @echo off diff --git a/tests/issues/run.sh b/tests/issues/run.sh index 01e6a6a28..67530a1d7 100755 --- a/tests/issues/run.sh +++ b/tests/issues/run.sh @@ -49,6 +49,13 @@ else echo "SUCCESSFUL 0/1" exit 1 fi +$ODIN test ../test_issue_xxxx.odin $COMMON +if [[ $($ODIN test ../test_issue_xxxx.odin -define:TEST_EXPECT_FAILURE=true $COMMON 2>&1 >/dev/null | grep -c "Error:") -eq 1 ]] ; then + echo "SUCCESSFUL 1/1" +else + echo "SUCCESSFUL 0/1" + exit 1 +fi set +x popd diff --git a/tests/issues/test_issue_xxxx.odin b/tests/issues/test_issue_xxxx.odin new file mode 100644 index 000000000..4d20ef356 --- /dev/null +++ b/tests/issues/test_issue_xxxx.odin @@ -0,0 +1,79 @@ +// Tests PR #xxxx https://github.com/odin-lang/Odin/issues/xxxx +package test_issues + +TEST_EXPECT_FAILURE :: #config(TEST_EXPECT_FAILURE, false) + +// Interfaces +IFoo :: struct { + foo: proc(self: ^IFoo) -> string, +} + +IBar :: struct { + bar: proc(self: ^IBar) -> string, +} + + +// Virtual table holders +Foo :: struct { + using vt: IFoo, +} + +// This is OK, but be careful! +Foo_Bar :: struct #raw_union { + using vt_foo: IFoo, + using vt_bar: IBar, +} + +// Implementation via Foo +Foo_Impl :: IFoo { + foo = proc(self: ^Foo) -> string { + return "Foo" + }, +} + +// Implementations via Foo_Bar +Foo_Bar_Foo_Impl :: IFoo { + foo = proc(self: ^Foo_Bar) -> string { + return "Foo_Bar: Foo" + }, +} + +Foo_Bar_Bar_Impl :: IBar { + bar = proc(self: ^Foo_Bar) -> string { + return "Foo_Bar: Bar" + }, +} + +when TEST_EXPECT_FAILURE { + // Will not be allowed in to be used in an implementation: + // The interface and implementation do not share the same address. + Invalid_Foo :: struct { + x: int, + using vt: IFoo, + } + + Invalid_Foo_Impl :: IFoo { + // Will not compile: + foo = proc(self: ^Invalid_Foo) -> string { + return "" + }, + } +} + +import "core:testing" + +@test +test_const_array_fill_assignment :: proc(t: ^testing.T) { + foo := Foo { + vt = Foo_Impl, + } + testing.expect_value(t, foo->foo(), "Foo") + + foo_bar := Foo_Bar { + vt_foo = Foo_Bar_Foo_Impl, + } + testing.expect_value(t, foo_bar->foo(), "Foo_Bar: Foo") + + foo_bar.vt_bar = Foo_Bar_Bar_Impl + testing.expect_value(t, foo_bar->bar(), "Foo_Bar: Bar") +} From b66e65e7f89d8c6ce426079581de84bfdbda9e58 Mon Sep 17 00:00:00 2001 From: Harold Brenes Date: Tue, 24 Mar 2026 15:40:29 -0400 Subject: [PATCH 4/5] Rename temp test name given PR number --- tests/issues/run.bat | 4 ++-- tests/issues/run.sh | 4 ++-- tests/issues/{test_issue_xxxx.odin => test_pr_6470.odin} | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) rename tests/issues/{test_issue_xxxx.odin => test_pr_6470.odin} (95%) diff --git a/tests/issues/run.bat b/tests/issues/run.bat index 3c722ccc7..be04148a6 100644 --- a/tests/issues/run.bat +++ b/tests/issues/run.bat @@ -32,8 +32,8 @@ set COMMON=-define:ODIN_TEST_FANCY=false -file -vet -strict-style -ignore-unused ..\..\..\odin test ..\test_issue_6165.odin %COMMON% || exit /b ..\..\..\odin build ..\test_issue_6240.odin %COMMON% 2>&1 | find /c "Error:" | findstr /x "3" || exit /b ..\..\..\odin build ..\test_issue_6401.odin %COMMON% 2>&1 | find /c "Error:" | findstr /x "3" || exit /b -..\..\..\odin test ..\test_issue_xxxx.odin %COMMON% || exit /b -..\..\..\odin build ..\test_issue_xxxx.odin -define:TEST_EXPECT_FAILURE=true %COMMON% 2>&1 | find /c "Error:" | findstr /x "1" || exit /b +..\..\..\odin test ..\test_pr_6470.odin %COMMON% || exit /b +..\..\..\odin build ..\test_pr_6470.odin -define:TEST_EXPECT_FAILURE=true %COMMON% 2>&1 | find /c "Error:" | findstr /x "1" || exit /b @echo off diff --git a/tests/issues/run.sh b/tests/issues/run.sh index 67530a1d7..996007cd7 100755 --- a/tests/issues/run.sh +++ b/tests/issues/run.sh @@ -49,8 +49,8 @@ else echo "SUCCESSFUL 0/1" exit 1 fi -$ODIN test ../test_issue_xxxx.odin $COMMON -if [[ $($ODIN test ../test_issue_xxxx.odin -define:TEST_EXPECT_FAILURE=true $COMMON 2>&1 >/dev/null | grep -c "Error:") -eq 1 ]] ; then +$ODIN test ../test_pr_6470.odin $COMMON +if [[ $($ODIN test ../test_pr_6470.odin -define:TEST_EXPECT_FAILURE=true $COMMON 2>&1 >/dev/null | grep -c "Error:") -eq 1 ]] ; then echo "SUCCESSFUL 1/1" else echo "SUCCESSFUL 0/1" diff --git a/tests/issues/test_issue_xxxx.odin b/tests/issues/test_pr_6470.odin similarity index 95% rename from tests/issues/test_issue_xxxx.odin rename to tests/issues/test_pr_6470.odin index 4d20ef356..d2427b335 100644 --- a/tests/issues/test_issue_xxxx.odin +++ b/tests/issues/test_pr_6470.odin @@ -1,4 +1,4 @@ -// Tests PR #xxxx https://github.com/odin-lang/Odin/issues/xxxx +// Tests PR #6470 https://github.com/odin-lang/Odin/pull/6470 package test_issues TEST_EXPECT_FAILURE :: #config(TEST_EXPECT_FAILURE, false) From 35b4c42f87963a2e7522c05cad9979a6b0dee3a1 Mon Sep 17 00:00:00 2001 From: Harold Brenes Date: Tue, 24 Mar 2026 16:07:59 -0400 Subject: [PATCH 5/5] Use `odin test` for `test_pr_6470.odin` on windows --- tests/issues/run.bat | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/issues/run.bat b/tests/issues/run.bat index be04148a6..f1a1c48c0 100644 --- a/tests/issues/run.bat +++ b/tests/issues/run.bat @@ -33,7 +33,7 @@ set COMMON=-define:ODIN_TEST_FANCY=false -file -vet -strict-style -ignore-unused ..\..\..\odin build ..\test_issue_6240.odin %COMMON% 2>&1 | find /c "Error:" | findstr /x "3" || exit /b ..\..\..\odin build ..\test_issue_6401.odin %COMMON% 2>&1 | find /c "Error:" | findstr /x "3" || exit /b ..\..\..\odin test ..\test_pr_6470.odin %COMMON% || exit /b -..\..\..\odin build ..\test_pr_6470.odin -define:TEST_EXPECT_FAILURE=true %COMMON% 2>&1 | find /c "Error:" | findstr /x "1" || exit /b +..\..\..\odin test ..\test_pr_6470.odin -define:TEST_EXPECT_FAILURE=true %COMMON% 2>&1 | find /c "Error:" | findstr /x "1" || exit /b @echo off