Process Span<>/ReadOnlySpan<>/.Contains() cases in LINQ Expression

[SergeiPavlov]

.NET9 C# compiler in Preview mode sometime generates Span<> or ReadOnlySpan<>.Contains() instead of IEnumerable.Contains() call.

for example in such case:

    private List<string> ReadOnlySpanContains_GetCustomers(string[] customerNames) =>
      (from c in Session.Query.All<Customer>() where customerNames.Contains(c.FirstName) select c.FirstName).ToList();

We should correctly process it.
In this fix, we just apply .ToArray() to ReadOnlySpan<>

[snaumenko-st]

I'm not sure that this is the best way to solve the issue. I believe it will behave worse than before the aforementioned compiler optimization.
Have you tried to discover the options to opt-out of it in some other way?
Or maybe we can file an issue to the .NET team for it?
Can these issues be related? dotnet/docs#43949, dotnet/roslyn#73743 or is it something else?

There is a breaking change in C# v14 though, seems to be potentially related too: https://github.com/dotnet/roslyn/blob/main/docs/compilers/CSharp/Compiler%20Breaking%20Changes%20-%20DotNet%2010.md#spant-and-readonlyspant-overloads-are-applicable-in-more-scenarios-in-c-14-and-newer

If it's the case, maybe we should convert these args to IEnumerable instead of calling ToArray?

[SergeiPavlov]

If it's the case, maybe we should convert these args to IEnumerable instead of calling ToArray?

I agree that the solution is sub-optimal.
But actually the case is rare (just a few Monolith tests failed because of ReadOnlySpan<> in Expression).

My aim is make to make DO compatible with the new C# compilers.
Optimization is a separate task.

Yes, the issue seems related to the mentioned Roslyn/.NET issues. But I have not much of time to analyze details.
While .ToArray() does work, lets use it.

And Yes, we can opt-out the ReadOnlySpan<> feature in the Monolith for a while by disabling Preview mode.
On the other side Preview is desirable in future branch, like 73quasar do detect other potential issues

[snaumenko-st]

While .ToArray() does work, lets use it.

It doesn't seem to be a good solution, but a dirty hack :) Actually, I really don't like it. It really doesn't solve a problem but potentially leads us to other similar bugs later, e.g. when new overloads will be introduced or/and preferred resolution order will be changed.

On the other side Preview is desirable in future branch, like 73quasar do detect other potential issues

AFAIK Preview mode is for .NET10 at the moment, and it doesn't make much sense to make changes, which make things worse just for compatibility with it. IMHO we have enough time to analyze the RC and fix it before .NET10 goes out.

[SergeiPavlov]

IMHO we have enough time to analyze the RC and fix it before .NET10 goes out.

It is unreal for us to solve all DO problems.
The deep understanding of its internals is required.
My strategy is: "find a workaround and forget".

Only some hotspots require increased attention.

[botinko]

One of the option is to prohibit preview compiler for a while. Dotnet 9 support ends 12 may 2026. Are we continue bumping dotnet version to 10 ?

[SergeiPavlov]

Are we continue bumping dotnet version to 10 ?

It is very desirable to detect problems on early stage.
Then we will have enough time to fix them before .NET10 release
That is why I want to keep Preview enabled at least on one main release branch

And it is much easier to work when we have the Application compatible with all contemporary versions of .NET/compilers

[snaumenko-st]

That is why I want to keep Preview enabled at least on one main release branch

Maybe we can wait at least until the next main release? My gut tells me it can be fixed by someone soon and there will be no need to implement this PR at all.

[SergeiPavlov]

Maybe we can wait at least until the next main release? My gut tells me it can be fixed by someone soon and there will be no need to implement this PR at all.

What do you mean as main release? SDK 9.200 or 10.0 ?
anyway reverting is easy

[snaumenko-st]

What do you mean as main release? SDK 9.200 or 10.0 ?

I mean Preview compiler is not production ready and could be a subject to change at any time until 10.0. Why do we need to implement changes for unstable things like this one?

[snaumenko-st]

OK, looks like the impact is not really big. However we should revert it as soon as possible if something changes.

[SergeiPavlov]

Why do we need to implement changes for unstable things like this one?

To lower amount of noise.
When we don't see failed tests caused by this issue the other issues will be more prominent