[Mono-dev] Generic Variance

[Rodrigo Kumpera]

Hi Scott, your patch now is now pretty close to been ok, only a few
remaining details are left.

@@ -2128,6 +2129,36 @@
     return (key->interface_id - element->interface_id);
 }

+static gboolean
+mono_class_is_variant_of (MonoClass *klass, MonoClass *vklass) {

This function takes arguments in the opposite order of
mono_class_is_assignable_from.
I notices now while reviewing the details of the check. It's better keep the
same ordering
of (target, source) for type query like functions [1].


+        if (container->type_params [i].flags &
GENERIC_PARAMETER_ATTRIBUTE_VARIANCE_MASK) {
+            if ((container->type_params [i].flags &
GENERIC_PARAMETER_ATTRIBUTE_CONTRAVARIANT) && !mono_class_is_assignable_from
(param_class, vparam_class))
+                return FALSE;
+            if ((container->type_params [i].flags &
GENERIC_PARAMETER_ATTRIBUTE_COVARIANT) && !mono_class_is_assignable_from
(vparam_class, param_class))
+                return FALSE;
+        } else if (param_class != vparam_class)
+            return FALSE;

Good call to change from the broken named we had internally to something
readable.


+++ mono/mini/ChangeLog    (working copy)
@@ -1,3 +1,13 @@
+2009-02-13  Scott Peterson  <lunchtimemama at gmail.com>
+
+    Contributed under the MIT/X11 license.
+
+    * mini-trampolines.cs: Use the
+    new mono_class_interface_offset_with_variance method to find interface
+    offsets.

It's better to error on the side of been to verbose than the opposite when
witting
changelog entries. Please mention that this change is required because
callsites
of variant interfaces may do through a different interface than the one the
target
implements but due to variance it's valid.

Oh, and you mention the wrong file name ;)


+2009-02-13  Scott Peterson  <lunchtimemama at gmail.com>
+
+    This adds runtime generic variance support for reference types.
+    This patch is contributed under the MIT/X11 license.
+
+    * class.c: Added mono_class_has_variant_generic_params which determins
+    if a generic class has a variant type parameter.

mono_class_has_variant_generic_params is already there.


Besides this minor issues, your patch looks great. Thanks for contributing
to mono.


Cheers,
Rodrigo



[1] It's easier to read F(A,B) a a test for the A =? B relation, where =?
could be things like
type assignable or generics variance assignable.




On Fri, Feb 13, 2009 at 1:07 AM, Scott Peterson <lunchtimemama at gmail.com>wrote:

> This patch syncs with SVN.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.ximian.com/pipermail/mono-devel-list/attachments/20090216/bedddb65/attachment.html