Type inference for lambdas in collection expressions

[ufcpp]

Version Used:

Visual Studio 17.10.0 Preview 5.0

Steps to Reproduce:

using System.Collections;

A a =
[
    _ => { }, // CS8917
];

class A : IEnumerable
{
    public void Add(Action<int> handler) { }
    IEnumerator IEnumerable.GetEnumerator() => throw new NotImplementedException();
}

Diagnostic Id:

CS8917

Expected Behavior:

No error.

Actual Behavior:

No error in Visual Studio 17.10 Preview 4, but CS8917 in 17.10 Preview 5.
On the other hand, it can be compiled if you use a collection initializer: A a = new() { _ => { } };.

[cston]

This looks to be a regression from 17.9.

In 17.9, for collection types implementing IEnumerable but not IEnumerable<T>, the compiler assumed the elements were convertible to the iteration type object when determining convertibility. In 17.10, the compiler checks that each element is implicitly convertible to object. For this example, that conversion fails because the type of the delegate cannot be inferred.

cc @jaredpar, @CyrusNajmabadi

[CyrusNajmabadi]

What rule changed that caused this? I thought we were effectively rolling back to the 17.9 rules (just looking for the presence of an Add method now).

[cston]

The Add requirement was relaxed in 17.10p4 but the check that each element is implicitly convertible to the iteration type was retained. In 17.9, that check was skipped if the target type implemented IEnumerable but not IEnumerable<T>.

That additional check in 17.10 that the element is implicitly convertible to object (for the IEnumerable case) can fail when the element is target typed, and relies on a strongly-typed Add method parameter type.

[cston]

@ufcpp, thanks for reporting this issue.

The break was the result of an intentional design change for collection expressions targeting types that implement IEnumerable (and that do not have a strongly-typed enumerator) to require that all elements are implicitly convertible to the iteration type object (see LDM-2024-01-08). That change provided consistency with other target types where we already required collection expression elements to be implicitly convertible to the iteration type of the target.

We had not recognized your breaking change case previously. I’ve added an item based on your example to the breaking change documentation for 17.10 (see #73278).

Did you hit this issue with an existing collection type? We're curious how common this case might be. Thanks.

[ufcpp]

Yes, this change broke my team's code.

Extracting the main points, we had the following classes. It used collection initialisers for event handler registration.

using System.Collections;

Reciever reciever = new()
{
    (sender, args) =>
    {

    },
    (sender, args, args2) =>
    {

    }
};

class Reciever : IEnumerable
{
    public void Recieve()
    {
        RecievedEventArgs args = new();
        _onRecieved1?.Invoke(this, args);

        if (_onRecieved2 is { } r)
        {
            RecievedEventArgs2 args2 = new(); // high computational cost
            _onRecieved2?.Invoke(this, args, args2);
        }
    }

    private Action<Reciever, RecievedEventArgs>? _onRecieved1;
    private Action<Reciever, RecievedEventArgs, RecievedEventArgs2>? _onRecieved2;

    public void Add(Action<Reciever, RecievedEventArgs> onRecieved) => _onRecieved1 += onRecieved;

    public void Add(Action<Reciever, RecievedEventArgs, RecievedEventArgs2> onRecieved) => _onRecieved2 += onRecieved;

    IEnumerator IEnumerable.GetEnumerator() => throw new NotImplementedException("dummy for collection initializers");
}

// normal args
struct RecievedEventArgs;

// args with high computational cost
struct RecievedEventArgs2;

VS 17.9 suggested IDE0028 (and 17.10 still makes this suggestion). With this applied, we had the code like the following.

Reciever reciever =
[
    (sender, args) =>
    {

    },
    (sender, args, args2) =>
    {

    }
];

Since there are two overloads of Add, Implementing IEnumerable<T> does not solve the problem. The collection builder is also helpless.

[CyrusNajmabadi]

• Yup. This is just not a case we want to with currently. Will look into ways to make this work in the future. for now, stick with collection initializers here.

Sorry about the hassle!