Bug in spec for better conversion from expression

[gafter]

The spec for better conversion from expression doesn't match the implementation. The spec says

7.5.3.3 Better conversion from expression
Given an implicit conversion C1 that converts from an expression E to a type T1, and an implicit conversion C2 that converts from an expression E to a type T2, C1 is a better conversion than C2 if E does not exactly match T2 and one of the following holds:
• E exactly matches T1 (§7.5.3.4)
• T1 is a better conversion target than T2 (§7.5.3.5)

but the implementation does

            bool t1MatchesExactly = ExpressionMatchExactly(node, t1, ref useSiteDiagnostics);
            bool t2MatchesExactly = ExpressionMatchExactly(node, t2, ref useSiteDiagnostics);

            if (t1MatchesExactly)
            {
                if (!t2MatchesExactly)
                {
                    // - E exactly matches T1
                    return BetterResult.Left;
                } // NOTE NO ELSE HERE
            }
            else if (t2MatchesExactly)
            {
                // - E exactly matches T2
                return BetterResult.Right;
            }

            // - T1 is a better conversion target than T2
            return BetterConversionTarget(node, t1, conv1, t2, conv2, ref useSiteDiagnostics, out okToDowngradeToNeither);

Specifically, if both match exactly, then we test for a better conversion target. This is what is implemented:

7.5.3.3 Better conversion from expression
Given an implicit conversion C1 that converts from an expression E to a type T1, and an implicit conversion C2 that converts from an expression E to a type T2, C1 is a better conversion than C2 if one of the following holds:
• E does not exactly match T2 and E exactly matches T1 (§7.5.3.4)
• E either exactly matches both T1 and T2, or exactly matches neither T1 and T2, and also T1 is a better conversion target than T2 (§7.5.3.5)

[gafter]

This issue is preventing the proposed spec approach for out var overload resolution:

an inferred “out var” argument exactly matches (see 7.5.3.4 Exactly matching Expression) every type.

from having the effect of causing out var arguments to ignore the corresponding parameter position.

[gafter]

This appears to be mooted in the ECMA version. But the ECMA version has no concept of exactly matching expression.

[AlekseyTs]

It looks like the implementation is intentional, we would want the following scenario compiling without errors and using the second overload:

using System;
namespace ConsoleApplication2
{
    public static class Foo
    {
        public static U IfNotNull<T, U>(this T value, Func<T, U> selector)
        {
            System.Console.WriteLine("IfNotNull<T, U>(this T value, Func<T, U> selector");
            return value != null ? selector(value) : default(U);
        }
        public static U IfNotNull<T, U>(this T? source, Func<T, U> selector) where T : struct
        {
            System.Console.WriteLine("IfNotNull<T, U>(this T? source, Func<T, U> selector)");
            return source.HasValue ? selector(source.Value) : default(U);
        }
    }
    class Program
    {
        static void Main(string[] args)
        {
            double? val = null;
            var d1 = val.IfNotNull(v => v / 100);
            var d2 = Foo.IfNotNull(val, v => v / 100);
        }
    }
}

[gafter]

Indeed. Moreover, we want to implement the ECMA spec instead of the old C# 4 spec, which is quoted above in the OP.

[gafter]

Closing, as we are intending to use the ECMA spec as our base, and it does not have this problem.

[MadsTorgersen]

We cannot use the ECMA spec as our base, because it does not reflect the changes for "better betterness" that were done in C# 6.0. Instead, the ECMA spec needs to embrace the Microsoft spec when it gets updated for C# 6.0.

I agree that there is a discrepancy between how "better betterness" was spec'ed and how it's implemented. The implementation is already in use, so unless there's a strong reason not to (I don't see any), we should go with what's implemented.

That means adjusting the spec a bit, so that better conversion target is checked both when neither matches precisely, and when both do. Something like this:

7.5.3.3 Better conversion from expression
Given an implicit conversion C1 that converts from an expression E to a type T1, and an implicit conversion C2 that converts from an expression E to a type T2, C1 is a better conversion than C2 if E does not exactly match T2 and one of the following holds:
• E exactly matches T1 and E does not exactly match T2 (§7.5.3.4)
• E exactly matches both or neither of T1 and T2, and T1 is a better conversion target than T2 (§7.5.3.5)

[gafter]

@MadsTorgersen
I think you intended to delete

E does not exactly match T2 and

from the prefix to the bulleted list.

[gafter]

@MadsTorgersen That reopen the question if how you would like to handle out var in the spec.

[gafter]

Issue moved to dotnet/csharplang #363 via ZenHub