vala: Improve check of delegate assignments and initializers

Fixes https://gitlab.gnome.org/GNOME/vala/issues/875

9 files changed, 88 insertions, 32 deletions

diff --git a/codegen/valaccodedelegatemodule.vala b/codegen/valaccodedelegatemodule.vala
index 35d5c4342..64a83411f 100644
--- a/codegen/valaccodedelegatemodule.vala
+++ b/codegen/valaccodedelegatemodule.vala
@@ -294,8 +294,8 @@ public class Vala.CCodeDelegateModule : CCodeArrayModule {
 			} else {
 				// use first delegate parameter as instance
 				if (d_params.size == 0 || m.closure) {
-					Report.error (node != null ? node.source_reference : null, "Cannot create delegate without target for instance method or closure");
-					arg = new CCodeConstant ("NULL");
+					Report.error (node != null ? node.source_reference : null, "internal: Cannot create delegate wrapper");
+					arg = new CCodeInvalidExpression ();
 				} else {
 					arg = new CCodeIdentifier (get_ccode_name (d_params.get (0)));
 					i = 1;
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 7b78d25ee..3a7d6d01e 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -282,8 +282,11 @@ TESTS = \
 	delegates/fields.vala \
 	delegates/fields-no-target.vala \
 	delegates/incompatible.test \
+	delegates/incompatible-assignment.test \
+	delegates/incompatible-initializer.test \
 	delegates/incompatible-target.test \
 	delegates/instance-method-to-no-target.test \
+	delegates/instance-method-to-no-target-2.test \
 	delegates/lambda-mixed-instance-static.vala \
 	delegates/lambda-shared-closure.vala \
 	delegates/member-target-destroy.vala \
diff --git a/tests/delegates/incompatible-assignment.test b/tests/delegates/incompatible-assignment.test
new file mode 100644
index 000000000..1cd8c9fac
--- /dev/null
+++ b/tests/delegates/incompatible-assignment.test
@@ -0,0 +1,15 @@
+Invalid Code
+
+[CCode (has_target = false)]
+delegate void Foo (string s);
+
+class Bar {
+	public void foo () {
+	}
+}
+
+void main () {
+	var bar = new Bar ();
+	Foo foo;
+	foo = bar.foo;
+}
diff --git a/tests/delegates/incompatible-initializer.test b/tests/delegates/incompatible-initializer.test
new file mode 100644
index 000000000..0a23d4aa9
--- /dev/null
+++ b/tests/delegates/incompatible-initializer.test
@@ -0,0 +1,14 @@
+Invalid Code
+
+[CCode (has_target = false)]
+delegate void Foo (string s);
+
+class Bar {
+	public void foo () {
+	}
+}
+
+void main () {
+	var bar = new Bar ();
+	Foo foo = bar.foo;
+}
diff --git a/tests/delegates/instance-method-to-no-target-2.test b/tests/delegates/instance-method-to-no-target-2.test
new file mode 100644
index 000000000..124fbdb5e
--- /dev/null
+++ b/tests/delegates/instance-method-to-no-target-2.test
@@ -0,0 +1,15 @@
+Invalid Code
+
+[CCode (has_target = false)]
+delegate void Foo ();
+
+class Bar {
+	public void foo () {
+	}
+}
+
+void main () {
+	var bar = new Bar ();
+	Foo foo;
+	foo = bar.foo;
+}
diff --git a/vala/valaassignment.vala b/vala/valaassignment.vala
index a19228dbe..9b5c7da5b 100644
--- a/vala/valaassignment.vala
+++ b/vala/valaassignment.vala
@@ -294,23 +294,18 @@ public class Vala.Assignment : Expression {
 				error = true;
 				Report.error (source_reference, "`length' field of fixed length arrays is read-only");
 				return false;
-			} else if (ma.symbol_reference is Variable && right.value_type == null) {
+			} else if (ma.symbol_reference is Variable && right.value_type is MethodType) {
 				unowned Variable variable = (Variable) ma.symbol_reference;
 
-				if (right.symbol_reference is Method &&
-				    variable.variable_type is DelegateType) {
-					unowned Method m = (Method) right.symbol_reference;
-					unowned DelegateType dt = (DelegateType) variable.variable_type;
-					unowned Delegate cb = dt.delegate_symbol;
-
+				if (variable.variable_type is DelegateType) {
 					/* check whether method matches callback type */
-					if (!cb.matches_method (m, dt)) {
+					if (!right.value_type.compatible (variable.variable_type)) {
+						unowned Method m = (Method) right.symbol_reference;
+						unowned Delegate cb = ((DelegateType) variable.variable_type).delegate_symbol;
 						error = true;
-						Report.error (source_reference, "declaration of method `%s' doesn't match declaration of callback `%s'".printf (m.get_full_name (), cb.get_full_name ()));
+						Report.error (source_reference, "Declaration of method `%s' is not compatible with delegate `%s'".printf (m.get_full_name (), cb.get_full_name ()));
 						return false;
 					}
-
-					right.value_type = variable.variable_type.copy ();
 				} else {
 					error = true;
 					Report.error (source_reference, "Assignment: Invalid assignment attempt");
diff --git a/vala/valadelegate.vala b/vala/valadelegate.vala
index 813a73479..200b2dbc2 100644
--- a/vala/valadelegate.vala
+++ b/vala/valadelegate.vala
@@ -165,27 +165,33 @@ public class Vala.Delegate : TypeSymbol, Callable {
 
 		bool first = true;
 		foreach (Parameter param in parameters) {
+			DataType method_param_type;
 			/* use first callback parameter as instance parameter if
 			 * an instance method is being compared to a static
 			 * callback
 			 */
 			if (first && m.binding == MemberBinding.INSTANCE && !has_target) {
 				first = false;
-				continue;
-			}
-
-			/* method is allowed to accept less arguments */
-			if (!method_params_it.next ()) {
-				break;
+				method_param_type = SemanticAnalyzer.get_data_type_for_symbol (m.parent_symbol);
+			} else {
+				/* method is allowed to accept less arguments */
+				if (!method_params_it.next ()) {
+					break;
+				}
+				method_param_type = method_params_it.get ().variable_type;
 			}
 
 			// method is allowed to accept arguments of looser types (weaker precondition)
-			var method_param = method_params_it.get ();
-			if (!param.variable_type.get_actual_type (dt, null, this).stricter (method_param.variable_type)) {
+			if (!param.variable_type.get_actual_type (dt, null, this).stricter (method_param_type)) {
 				return false;
 			}
 		}
 
+		// delegate without target for instance method or closure
+		if (first && m.binding == MemberBinding.INSTANCE && !has_target) {
+			return false;
+		}
+
 		/* method may not expect more arguments */
 		if (method_params_it.next ()) {
 			return false;
diff --git a/vala/valalocalvariable.vala b/vala/valalocalvariable.vala
index e334301c5..f441e16b8 100644
--- a/vala/valalocalvariable.vala
+++ b/vala/valalocalvariable.vala
@@ -162,27 +162,22 @@ public class Vala.LocalVariable : Variable {
 		}
 
 		if (initializer != null && !initializer.error) {
-			if (initializer.value_type == null) {
+			if (initializer.value_type is MethodType) {
 				if (!(initializer is MemberAccess) && !(initializer is LambdaExpression)) {
 					error = true;
 					Report.error (source_reference, "expression type not allowed as initializer");
 					return false;
 				}
 
-				if (initializer.symbol_reference is Method &&
-				    variable_type is DelegateType) {
-					var m = (Method) initializer.symbol_reference;
-					var dt = (DelegateType) variable_type;
-					var cb = dt.delegate_symbol;
-
+				if (variable_type is DelegateType) {
 					/* check whether method matches callback type */
-					if (!cb.matches_method (m, dt)) {
+					if (!initializer.value_type.compatible (variable_type)) {
+						unowned Method m = (Method) initializer.symbol_reference;
+						unowned Delegate cb = ((DelegateType) variable_type).delegate_symbol;
 						error = true;
-						Report.error (source_reference, "declaration of method `%s' doesn't match declaration of callback `%s'".printf (m.get_full_name (), cb.get_full_name ()));
+						Report.error (source_reference, "Declaration of method `%s' is not compatible with delegate `%s'".printf (m.get_full_name (), cb.get_full_name ()));
 						return false;
 					}
-
-					initializer.value_type = variable_type;
 				} else {
 					error = true;
 					Report.error (source_reference, "expression type not allowed as initializer");
diff --git a/vala/valapointertype.vala b/vala/valapointertype.vala
index 2a26f92cb..6efbad9bd 100644
--- a/vala/valapointertype.vala
+++ b/vala/valapointertype.vala
@@ -113,6 +113,19 @@ public class Vala.PointerType : DataType {
 		base_type.accept (visitor);
 	}
 
+	public override bool stricter (DataType type2) {
+		if (type2 is PointerType) {
+			return compatible (type2);
+		}
+
+		if (base_type is VoidType) {
+			// void* can hold reference types
+			return type2 is ReferenceType;
+		}
+
+		return base_type.stricter (type2);
+	}
+
 	public override void replace_type (DataType old_type, DataType new_type) {
 		if (base_type == old_type) {
 			base_type = new_type;