Merge pull request #7208 from Taylbr/poly-proc-fix

Fix: Polymorphic procedures resolving to nil + further fixes

src/check_expr.cpp

@@ -410,6 +410,15 @@ gb_internal bool find_or_generate_polymorphic_procedure(CheckerContext *old_c, E
 	}
 
 	if (!src->Proc.is_polymorphic || src->Proc.is_poly_specialized) {
+		// NOTE: polymorphic procedure check not idempotent without this
+		if (src->Proc.is_poly_specialized && base_entity->Procedure.generated_from_polymorphic) {
+			if (are_types_identical(src, dst)) {
+				if (poly_proc_data) {
+					poly_proc_data->gen_entity = base_entity;
+				}
+				return true;
+			}
+		}
 		return false;
 	}
 
@@ -457,9 +466,7 @@ gb_internal bool find_or_generate_polymorphic_procedure(CheckerContext *old_c, E
 	scope->flags |= ScopeFlag_Proc;
 	nctx.scope = scope;
 	nctx.allow_polymorphic_types = true;
-	if (nctx.polymorphic_scope == nullptr) {
-		nctx.polymorphic_scope = scope;
-	}
+	nctx.polymorphic_scope = scope;
 
 
 	auto *pt = &src->Proc;
@@ -1023,17 +1030,6 @@ gb_internal bool check_is_assignable_to_with_score(CheckerContext *c, Operand *o
 		return false;
 	}
 
-	// Handle polymorphic procedure used as default parameter
-	if (operand->mode == Addressing_Value && is_type_proc(type) && is_type_proc(operand->type)) {
-		Entity *e = entity_from_expr(operand->expr);
-		if (e != nullptr && e->kind == Entity_Procedure && is_type_polymorphic(e->type) && !is_type_polymorphic(type)) {
-			// Special case: Allow a polymorphic procedure to be used as default value for concrete proc type
-			// during the initial check. It will be properly instantiated when actually used.
-			if (score_) *score_ = assign_score_function(1);
-			return true;
-		}
-	}
-
 	i64 score = check_distance_between_types(c, operand, type, allow_array_programming);
 	if (score >= 0) {
 		if (score_) *score_ = assign_score_function(score, is_variadic);

src/check_type.cpp

@@ -1791,6 +1791,7 @@ gb_internal ParameterValue handle_parameter_value(CheckerContext *ctx, Type *in_
 					if (e->kind == Entity_Procedure) {
 						param_value.kind = ParameterValue_Constant;
 						param_value.value = exact_value_procedure(e->identifier);
+						param_value.proc_entity = e;
 						add_entity_use(ctx, e->identifier, e);
 					} else {
 						if (e->flags & EntityFlag_Param) {
@@ -2143,8 +2144,12 @@ gb_internal Type *check_get_params(CheckerContext *ctx, Scope *scope, Ast *_para
 						// This is just to add the error message to determine_type_from_polymorphic which
 						// depends on valid position information
 						op.expr = _params;
-						op.mode = Addressing_Invalid;
-						op.type = t_invalid;
+
+						// NOTE(taylbr): Can still have valid type with null expr. Needed for resolving
+						if (op.mode == Addressing_Invalid || op.type == nullptr) {
+							op.mode = Addressing_Invalid;
+							op.type = t_invalid;
+						}
 					}
 					if (is_type_polymorphic_type) {
 						type = determine_type_from_polymorphic(ctx, type, op);

src/entity.cpp

@@ -111,6 +111,7 @@ enum ParameterValueKind {
 struct ParameterValue {
 	ParameterValueKind kind;
 	Ast *original_ast_expr;
+	Entity *proc_entity;
 	union {
 		ExactValue value;
 		Ast *ast_value;

src/llvm_backend_proc.cpp

@@ -4785,6 +4785,12 @@ gb_internal lbValue lb_build_builtin_proc(lbProcedure *p, Ast *expr, TypeAndValu
 gb_internal lbValue lb_handle_param_value(lbProcedure *p, Type *parameter_type, ParameterValue const &param_value, TypeProc *procedure_type, Ast* call_expression) {
 	switch (param_value.kind) {
 	case ParameterValue_Constant:
+		if (param_value.proc_entity != nullptr && is_type_proc(parameter_type)) {
+			lbValue v = lb_find_procedure_value_from_entity(p->module, param_value.proc_entity);
+			if (v.value != nullptr) {
+				return lb_emit_conv(p, v, parameter_type);
+			}
+		}
 		if (is_type_constant_type(parameter_type)) {
 			auto res = lb_const_value(p->module, parameter_type, param_value.value);
 			return res;

tests/issues/run.bat

@@ -26,6 +26,7 @@ set COMMON=-define:ODIN_TEST_FANCY=false -file -vet -strict-style -ignore-unused
 ..\..\..\odin build ..\test_issue_5097.odin %COMMON% || exit /b
 ..\..\..\odin build ..\test_issue_5097-2.odin %COMMON% || exit /b
 ..\..\..\odin build ..\test_issue_5265.odin %COMMON% || exit /b
+..\..\..\odin build ..\test_issue_5573.odin %COMMON% 2>&1 | find /c "Error:" | findstr /x "2" || exit /b
 ..\..\..\odin test ..\test_issue_5699.odin %COMMON%  || exit /b
 ..\..\..\odin test ..\test_issue_6068.odin %COMMON%  || exit /b
 ..\..\..\odin test ..\test_issue_6101.odin %COMMON%  || exit /b
@@ -37,6 +38,7 @@ set COMMON=-define:ODIN_TEST_FANCY=false -file -vet -strict-style -ignore-unused
 ..\..\..\odin test ..\test_pr_6470.odin -define:TEST_EXPECT_FAILURE=true %COMMON% 2>&1 | find /c "Error:" | findstr /x "1" || exit /b
 ..\..\..\odin test ..\test_pr_6476.odin %COMMON%  || exit /b
 ..\..\..\odin check ..\test_issue_6484.odin -no-entry-point %COMMON%  || exit /b
+..\..\..\odin test ..\test_issue_6753.odin %COMMON%  || exit /b
 ..\..\..\odin check ..\test_issue_6874.odin %COMMON% 2>&1 | find /c "Error:" | findstr /x "1" || exit /b
 ..\..\..\odin check ..\test_issue_6979.odin -no-entry-point %COMMON%  || exit /b
 ..\..\..\odin build ..\test_issue_7037.odin %COMMON% -o:none  || exit /b

tests/issues/run.sh

@@ -34,6 +34,12 @@ $ODIN build ../test_issue_5043.odin $COMMON
 $ODIN build ../test_issue_5097.odin $COMMON
 $ODIN build ../test_issue_5097-2.odin $COMMON
 $ODIN build ../test_issue_5265.odin $COMMON
+if [[ $($ODIN build ../test_issue_5573.odin $COMMON 2>&1 >/dev/null | grep -c "Error:") -eq 2 ]] ; then
+	echo "SUCCESSFUL 1/1"
+else
+	echo "SUCCESSFUL 0/1"
+	exit 1
+fi
 $ODIN test ../test_issue_5699.odin $COMMON
 $ODIN test ../test_issue_6068.odin $COMMON
 $ODIN test ../test_issue_6101.odin $COMMON
@@ -76,6 +82,7 @@ else
 	exit 1
 fi
 $ODIN check ../test_issue_6484.odin -no-entry-point $COMMON_CHECK
+$ODIN test ../test_issue_6753.odin $COMMON
 if [[ $($ODIN check ../test_issue_6874.odin $COMMON_CHECK 2>&1 >/dev/null | grep -c "Error:") -eq 1 ]] ; then
 	echo "SUCCESSFUL 1/1"
 else

tests/issues/test_issue_5573.odin

@@ -0,0 +1,18 @@
+// Tests issue #5573 https://github.com/odin-lang/Odin/issues/5573
+package test_issues
+
+poly :: proc(x: $T) -> string {
+	return "poly"
+}
+
+takes_concrete :: proc(f: proc(a: int, b: f32, c: rawptr) -> ^int) {
+}
+
+main :: proc() {
+	// should error - wrong arity, wrong parameter types, wrong return type
+	mismatched: proc(a: int, b: f32, c: rawptr) -> ^int = poly
+	_ = mismatched
+
+	// should error - same, as a procedure argument
+	takes_concrete(poly)
+}

tests/issues/test_issue_6753.odin

@@ -0,0 +1,64 @@
+// test issue for #6753 https://github.com/odin-lang/odin/issues/6753
+package test_issues
+import "core:testing"
+import "core:fmt"
+
+foo_concrete :: proc(x: int, g: proc(int) -> int) -> int {
+	return g(x)
+}
+foo_impossible :: proc(x: int, g: proc(int, int) -> string) -> string {
+	return "impossible"
+}
+foo_group :: proc {
+	foo_concrete,
+	foo_impossible,
+}
+
+f_poly :: proc(x: $T) -> T { return x }
+foo_poly :: proc(x: $T, g: proc(T) -> T = f_poly) -> T {
+	return g(x)
+}
+
+@test
+test_issue_6753_ambiguous_poly_argument :: proc (t: ^testing.T) {
+	testing.expect_value(t, foo_group(1, f_poly), 1) // should be no ambiguity whether foo_concrete or foo_impossible
+}
+
+@test
+test_issue_6753_default_poly_proc :: proc (t: ^testing.T) {
+	testing.expect_value(t, foo_poly(1), 1)
+}
+
+@test
+test_issue_6753_parapoly_proc_variable :: proc(t: ^testing.T) {
+	p: proc(int) -> int = f_poly
+	testing.expect(t, p != nil, "polymorphic procedure was not instantiated")
+	testing.expect_value(t, p(123), 123)
+}
+
+// -- Fixing above led to some new bugs surfacing --
+@test
+test_issue_6753_parapoly_proc_as_argument :: proc(t: ^testing.T) {
+	testing.expect(t, foo_concrete(123, f_poly) == 123, "failed to pass poly proc as argument")
+}
+
+@test
+test_issue_6753_parapoly_with_default_proc_same_generic_type_T :: proc(t: ^testing.T) {
+	testing.expect_value(t, foo_poly(123), 123)
+	testing.expect_value(t, foo_poly(123, f_poly), 123)
+}
+
+// all together now
+describe :: proc(x: $T) -> string { return fmt.tprintf("#%v", x) }
+describe_bytes :: proc(x: []byte) -> string { return "bytes" }
+
+bar_poly  :: proc(x: $T, g: proc(x: T) -> string = describe) -> string { return g(x) }
+bar_bytes :: proc(x: []byte, g: proc(x: []byte) -> string) -> string { return g(x) }
+bar_group :: proc { bar_poly, bar_bytes }
+
+@test
+test_issue_6753_parapoly_default_in_group :: proc(t: ^testing.T) {
+	testing.expect_value(t, bar_group(123, describe), "#123")
+	testing.expect_value(t, bar_group("hi"), "#hi")
+	testing.expect_value(t, bar_group([]byte{1, 2}, describe_bytes), "bytes")
+}