Merge pull request #6470 from harold-b/hb.allow-offset-zero-subtype-ptr-proc-param

Allow pointers to types which have subtype fields at offset 0 to be assignable in proc parameters.

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) && are_proc_properties_identical(dst,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) {

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);
 
@@ -4922,6 +4925,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;

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_pr_6470.odin %COMMON%  || 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
 

tests/issues/run.sh

@@ -49,6 +49,13 @@ else
 	echo "SUCCESSFUL 0/1"
 	exit 1
 fi
+$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"
+	exit 1
+fi
 set +x
 
 popd

tests/issues/test_pr_6470.odin

@@ -0,0 +1,79 @@
+// Tests PR #6470 https://github.com/odin-lang/Odin/pull/6470
+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")
+}