[Proposal]: Ignore overloads with ref struct parameters within Expression lambdas

[cston]

Summary

Ignore overloads with ref struct parameters when binding within Expression lambdas.

Motivation

C#13 adds support for params span, and C# preview adds first-class span types. Both of these features allow overload resolution to prefer overloads with span type parameters in more cases than with earlier language versions.

However, within an Expression, binding to a member that requires a ref struct instance may result in a compile-time or runtime error, and is therefore a breaking change.

Example 1: params span, see #110592

Expression<Func<string, string, string>> expr =
    (x, y) => string.Join("", x, y); // String.Join(string?, params ReadOnlySpan<string?>)

Example 2: First-class span types, see #109757

Expression<Func<string[], string, bool>> expr =
    (x, y) => x.Contains(y); // System.MemoryExtensions.Contains<T>(this ReadOnlySpan<T>, T)

var f = expr.Compile(preferInterpretation: true); // Exception

Design

12.6.4.2 Applicable function member could be updated as follows:

• A static method is only applicable if the method group results from a simple_name or a member_access through a type.
• ...
• Within an Expression, if any parameters of the candidate method, other than the implicit instance method receiver, may have a ref struct type, the candidate is not applicable.

Note that the disqualifying parameter:

• May be a params parameter
• May be an optional parameter
• May have a generic parameter type with allows ref struct

Drawbacks

Ignoring certain overloads may be a breaking change itself, in limited scenarios where ref struct instances are supported within an Expression currently. Examples?

Alternatives

No compiler change; add IDE fixer

Make no additional compiler changes. Instead, require callers to rewrite Expression instances to work around the overload resolution changes. An IDE fixer could be provided to rewrite calls for these cases.

Support ref struct arguments within Expression lambdas

The Expression interpreter relies on reflection, so supporting these cases would require supporting ref struct arguments in reflection or rewriting parts of the interpreter.

Design meetings

• https://github.com/dotnet/csharplang/blob/main/meetings/2024/LDM-2024-06-17.md#params-span-breaks
• https://github.com/dotnet/csharplang/blob/main/meetings/2024/LDM-2024-12-04.md#conversions-in-expression-trees

[cston]

cc @jaredpar, @jjonescz

[jaredpar]

Ignoring certain overloads may be a breaking change itself, in limited scenarios where ref struct instances are supported within an Expression currently. Examples?

Believe that there are a few limited cases where ref struct can be used in expression trees today. @jjonescz likely remembers the examples. I'm optimistic that in the existing cases if we implemented this change it would just fall back to a more appropriate overload.

[jjonescz]

See dotnet/roslyn#74143.

Example (outputs "1" now, will output "2" after the change):

using System;
using System.Collections.Generic;
using System.Linq.Expressions;

C.R(() => C.M(new string[0]));

static class C
{
    public static void R(Expression<Action> e) => e.Compile()();
    
    public static void M(ReadOnlySpan<string> x) => Console.Write(1);
    public static void M(IEnumerable<string> x) => Console.Write(2);
}

Of course, one can come up also with examples that will break with compilation errors, e.g., if the overloads aren't equivalent:

using System;
using System.Collections.Generic;
using System.Linq.Expressions;

C.R(() => C.M(new string[0]));

static class C
{
    public static void R(Expression<Func<string>> e) => e.Compile()();
    
    public static string M(ReadOnlySpan<string> x) { Console.Write(1); return ""; }
    public static int M(IEnumerable<string> x) { Console.Write(2); return 0; }
}

[jjonescz]

I now realize the proposal is to make span overloads completely inapplicable, so the break is even simpler - just have one overload taking a span type. But even if we changed the proposal to only prefer non-span overloads in expression trees, the examples above would still be breaking changes (albeit less severe ones).

[hez2010]

This would be a huge breaking change. ref structs in ExpressionTree works from the very beginning since we introduced ref struct, and there're already many code relying on such behavior, though they don't support NativeAOT.

Expression<Func<string[], string, bool>> expr =
    (x, y) => Foo.Contains(x, y);
Console.WriteLine(expr.Compile()(["hello", "world"], "hello")); // prints True under CoreCLR

class Foo
{
    internal static bool Contains(ReadOnlySpan<string> ss, string s) => ss.Contains(s);
}

I don't see any reason for regressing such valid scenarios in CoreCLR just because someone suddenly wants to use ExpressionTree in NativeAOT. And blocking this now will lead to another breaking change in the future if we want to bring the support for ref struct in the ExpressionTree interpreter later.

IMO we should keep it as is, and fix dotnet/runtime#45152 and dotnet/runtime#96160 instead.

[jaredpar]

I don't see any reason for regressing such valid scenarios in CoreCLR just because someone suddenly wants to use ExpressionTree in NativeAOT.

The primary source of issues is EFCore, not NativeAOT.

ExpressionTree works from the very beginning since we introduced ref struct,

It works in limited scenarios, not all of them, which is part of the core issue here. The support is hard to describe and trips customers up quite regularly.

[hez2010]

The primary source of issues is EFCore, not NativeAOT.

There're also many use cases outside EFCore, I have seen many code use ExpressionTree to "accelerate" reflection because the performance of a compiled expression is far better than using reflection. And they also use ExpressionTree to workaround the issue where you cannot use ref structs in reflection, by compling a lambda which takes arrays as input and calls into the method which has Spans as parameters.