[vala/wip/issue548: 7/8] codegen: Use properly checked implicit interface implementations

[Rico Tzschichholz <ricotz src gnome org>]

commit 7dcf56146e3e3cd4dd2129786c1e64e1e13ea4c0
Author: Rico Tzschichholz <ricotz ubuntu com>
Date:   Fri Nov 30 21:26:15 2018 +0100

    codegen: Use properly checked implicit interface implementations
    
    Collect implicit interface implementations in AST and avoid doing the same
    checks twice.
    
    This caused double vfunc assignments if an implementation is provided while
    a method with the same name is available in prerequisite class.
    
    See https://bugzilla.gnome.org/show_bug.cgi?id=536863
    and https://bugzilla.gnome.org/show_bug.cgi?id=652098
    
    Fixes https://gitlab.gnome.org/GNOME/vala/issues/548

 codegen/valagtypemodule.vala                       | 34 ++++++--------------
 tests/Makefile.am                                  |  2 ++
 tests/objects/classes-implicit-implementation.vala | 36 ++++++++++++++++++++++
 tests/objects/classes-interfaces.vala              | 27 ++++++++++++++++
 vala/valaclass.vala                                |  8 +++++
 5 files changed, 83 insertions(+), 24 deletions(-)
---
diff --git a/codegen/valagtypemodule.vala b/codegen/valagtypemodule.vala
index bb971c1c4..3dd0f2c48 100644
--- a/codegen/valagtypemodule.vala
+++ b/codegen/valagtypemodule.vala
@@ -1472,32 +1472,18 @@ public class Vala.GTypeModule : GErrorModule {
                }
 
                // connect inherited implementations
-               foreach (Method m in iface.get_methods ()) {
-                       if (m.is_abstract) {
-                               Method cl_method = null;
-                               var base_class = cl;
-                               while (base_class != null && cl_method == null) {
-                                       cl_method = base_class.scope.lookup (m.name) as Method;
-                                       base_class = base_class.base_class;
-                               }
-                               if (base_class != null && cl_method.parent_symbol != cl) {
-                                       // method inherited from base class
-
-                                       var base_method = cl_method;
-                                       if (cl_method.base_interface_method != null) {
-                                               base_method = cl_method.base_interface_method;
-                                       } else if (cl_method.base_method != null) {
-                                               //FIXME should this ever be possible here?
-                                               base_method = cl_method.base_method;
-                                       }
+               var it = cl.get_implicit_implementations ().map_iterator ();
+               while (it.next ()) {
+                       Method m = it.get_key ();
+                       if (m.parent_symbol == iface) {
+                               Method base_method = it.get_value ();
 
-                                       generate_method_declaration (base_method, cfile);
+                               generate_method_declaration (base_method, cfile);
 
-                                       CCodeExpression cfunc = new CCodeIdentifier (get_ccode_name 
(base_method));
-                                       cfunc = cast_method_pointer (base_method, cfunc, iface);
-                                       var ciface = new CCodeIdentifier ("iface");
-                                       ccode.add_assignment (new CCodeMemberAccess.pointer (ciface, 
get_ccode_vfunc_name (m)), cfunc);
-                               }
+                               CCodeExpression cfunc = new CCodeIdentifier (get_ccode_name (base_method));
+                               cfunc = cast_method_pointer (m, cfunc, iface);
+                               var ciface = new CCodeIdentifier ("iface");
+                               ccode.add_assignment (new CCodeMemberAccess.pointer (ciface, 
get_ccode_vfunc_name (m)), cfunc);
                        }
                }
 
diff --git a/tests/Makefile.am b/tests/Makefile.am
index f62e5f2c3..a62998b1b 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -254,6 +254,8 @@ TESTS = \
        objects/chainup.vala \
        objects/class_only.vala \
        objects/classes.vala \
+       objects/classes-interfaces.vala \
+       objects/classes-implicit-implementation.vala \
        objects/compact-class.vala \
        objects/compact-class-destructor.vala \
        objects/constructor-variadic.test \
diff --git a/tests/objects/classes-implicit-implementation.vala 
b/tests/objects/classes-implicit-implementation.vala
new file mode 100644
index 000000000..3931b34bc
--- /dev/null
+++ b/tests/objects/classes-implicit-implementation.vala
@@ -0,0 +1,36 @@
+interface IFoo : Object {
+       public abstract int foo ();
+}
+
+interface IBar : Object {
+}
+
+class Bar : Object {
+       public int foo () {
+               assert_not_reached ();
+               return -1;
+       }
+}
+
+class Baz : Object {
+       public int foo () {
+               return 23;
+       }
+}
+
+class Foo : Bar, IFoo {
+       public int foo () {
+               return 42;
+       }
+}
+
+class Faz : Baz, IFoo, IBar {
+}
+
+void main () {
+       var foo = new Foo ();
+       assert (foo.foo () == 42);
+
+       var baz = new Baz ();
+       assert (baz.foo () == 23);
+}
diff --git a/tests/objects/classes-interfaces.vala b/tests/objects/classes-interfaces.vala
new file mode 100644
index 000000000..9f2474653
--- /dev/null
+++ b/tests/objects/classes-interfaces.vala
@@ -0,0 +1,27 @@
+class Base : Object {
+       public void foo () {
+       }
+}
+
+interface IFoo : Base {
+       public abstract string foo ();
+}
+
+interface IBar : Base {
+       public abstract int foo ();
+}
+
+class Manam : Base, IFoo, IBar {
+       public int IBar.foo () {
+               return 23;
+       }
+       public string IFoo.foo () {
+               return "foo";
+       }
+}
+
+void main () {
+       var manam = new Manam ();
+       assert (((IFoo) manam).foo () == "foo");
+       assert (((IBar) manam).foo () == 23);
+}
diff --git a/vala/valaclass.vala b/vala/valaclass.vala
index 81bc6ba91..419e72d93 100644
--- a/vala/valaclass.vala
+++ b/vala/valaclass.vala
@@ -110,6 +110,7 @@ public class Vala.Class : ObjectTypeSymbol {
        private bool? _is_singleton;
 
        private List<DataType> base_types = new ArrayList<DataType> ();
+       private HashMap<Method,Method> implicit_implementations = new HashMap<Method,Method> ();
 
        /**
         * Specifies the default construction method.
@@ -305,6 +306,10 @@ public class Vala.Class : ObjectTypeSymbol {
                }
        }
 
+       public HashMap<Method,Method> get_implicit_implementations () {
+               return implicit_implementations;
+       }
+
        /**
         * Adds the specified property as a member to this class.
         *
@@ -748,6 +753,9 @@ public class Vala.Class : ObjectTypeSymbol {
                                                                                impl.version.check 
(source_reference);
                                                                                impl.used = true;
                                                                                implemented = true;
+                                                                               if 
(impl.base_interface_method == null) {
+                                                                                       
implicit_implementations.set (m, impl);
+                                                                               }
                                                                                break;
                                                                        }
                                                                }