[vala/staging] vala: Improve check of delegate assignments and initializers

[Rico Tzschichholz <ricotz src gnome org>]

commit 6690ea0e3c6f0d81d849e13548efc8c0809149cc
Author: Rico Tzschichholz <ricotz ubuntu com>
Date:   Fri Nov 8 16:17:58 2019 +0100

    vala: Improve check of delegate assignments and initializers
    
    Fixes https://gitlab.gnome.org/GNOME/vala/issues/875

 codegen/valaccodedelegatemodule.vala               |  4 ++--
 tests/Makefile.am                                  |  3 +++
 tests/delegates/incompatible-assignment.test       | 15 +++++++++++++++
 tests/delegates/incompatible-initializer.test      | 14 ++++++++++++++
 .../delegates/instance-method-to-no-target-2.test  | 15 +++++++++++++++
 vala/valaassignment.vala                           | 17 ++++++-----------
 vala/valadelegate.vala                             | 22 ++++++++++++++--------
 vala/valalocalvariable.vala                        | 17 ++++++-----------
 vala/valapointertype.vala                          | 13 +++++++++++++
 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;