[Mono-dev] Generic Variance

[Marek Safar]

Hi Scott,
> This patch adds generic variance support to the C# compiler. This 
> patch does not fully support the spec in that it provides variance 
> only for reference types. This limitation is designed to reflect the 
> current (proposed) level of variance support in the VM. New error 
> codes are defined and tests for each code are provided, as well as 
> tests for variance. This patch is contributed under the MIT/X11 
> license. It is attached.
>
> NOTE: This email is a re-send. The original also had the patch inline, 
> but it was too long for the system. Blarg.
>
I think there is more work needed to update all 
conversion/overload/operator/interence rules but it probably has to wait 
until some specification is released.

The naming convention for your tests should be gtest-variance-01.cs, etc.


Comments:

+    public static bool HasVariantTypeParameter (Type type, Location loc)
+    {

HasXXX method should not issue any compiler errors, change the method 
name or behaviour.

+    public static bool IsVariantOf (Type type1, Type type2)
+    {
+#if GMCS_SOURCE
+        if (type1.IsGenericType && type2.IsGenericType) {
+            Type generic_target_type = type2.GetGenericTypeDefinition ();
+            if (type1.GetGenericTypeDefinition () == generic_target_type) {
+                Type [] t1 = type1.GetGenericArguments ();
+                Type [] t2 = type2.GetGenericArguments ();
+                int i = 0;
+                foreach (Type t in 
generic_target_type.GetGenericArguments ()) {
+                    if ((t.GenericParameterAttributes & 
GenericParameterAttributes.VarianceMask) != 0) {
+                        // FIXME this is not right
+                        if (t1[i].IsValueType || t2[i].IsValueType) {
+                            return false;
+                        }

Use TypeManager.IsValueType if you really want to check any ValueType type.

+            this.variance = variance;
+            if (variance != Variance.None && !(decl is Interface) && 
!(decl is Delegate)) {
+                Report.Error (-36, loc, "Generic variance can only be 
used with interfaces and delegates");
+            }

This check should be done in the parser.

+            GenericParameterAttributes attr = 
GenericParameterAttributes.None;
+            if (variance == Variance.Contravariant)
+                attr |= GenericParameterAttributes.Contravariant;
+            if (variance == Variance.Covariant)
+                attr |= GenericParameterAttributes.Covariant;

You are missing else for second 'if' or could they be both covariant and 
contra-variant ?

+            } else if (RootContext.Version >= LanguageVersion.Future &&
+                       (the_token == Token.IN || the_token == Token.OUT)) {
+                the_token = token ();

LanguageVersion check is not needed here. Look what we do elsewhere.

+                // FIXME this sucks, but it's the easiest way to handle 
the fact that
+                // the dummy setter parameter will violate generic 
covariance.
+                if (IsDummy)
+                    Report.DisableReporting ();
                 parameters.Resolve (ResolveContext);
+                if (IsDummy)
+                    Report.EnableReporting ();

This should really be fixed.

+opt_type_parameter_variance
+    : /* empty */ {
+        $$ = Variance.None;
+      }
+    | type_parameter_variance {
+        $$ = $1;
+      }
+    ;

We use different formating for .jay file.

+        // TODO is there a better way to handle this?
+        if (lexer.parsing_generic_declaration &&
+            RootContext.Version < LanguageVersion.Future &&
+            (name == "in" || name == "out"))
+            continue;

This should not be needed at all.


Thanks
Marek