CSHARP-5749: Support C# 14 changes that result in overloads now binding MemoryExtensions extension methods #1802
Conversation
…ng MemoryExtensions extension methods
rstam
added
feature
There was a problem hiding this comment.
Pull Request Overview
This PR addresses C# 14 compiler changes that now bind array-based extension method calls (like Contains and SequenceEqual) to MemoryExtensions instead of Enumerable. The driver's LINQ provider cannot translate these MemoryExtensions methods, so the solution rewrites expression trees to use the original Enumerable methods before translation.
Key changes:
- Introduced
LinqExpressionPreprocessoras a centralized preprocessing step that performs CLR compatibility rewrites and partial evaluation - Added
ClrCompatExpressionRewriterto detect and rewriteMemoryExtensionscalls back toEnumerableequivalents - Replaced all direct
PartialEvaluator.EvaluatePartiallycalls withLinqExpressionPreprocessor.Preprocess
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| LinqExpressionPreprocessor.cs | New preprocessor that chains CLR compatibility rewrites with partial evaluation |
| ClrCompatExpressionRewriter.cs | New rewriter that converts MemoryExtensions method calls back to Enumerable equivalents |
| MemoryExtensionsMethod.cs | Reflection helpers for identifying MemoryExtensions methods |
| EnumerableMethod.cs | Added references to Enumerable.Contains and SequenceEqual overloads with comparers |
| TypeExtensions.cs | Added IsSpanOf and IsReadOnlySpanOf helper methods |
| MethodInfoExtensions.cs | Added parameter and generic argument checking helpers |
| LinqProviderAdapter.cs | Updated to use new preprocessor instead of direct PartialEvaluator calls |
| ExpressionToExecutableQueryTranslator.cs | Updated to use new preprocessor |
| ExpressionToSetStageTranslator.cs | Updated to use new preprocessor |
| LookupMethodToPipelineTranslator.cs | Updated to use new preprocessor |
| GroupingWithOutputExpressionStageDefinitions.cs | Updated to use new preprocessor |
| PredicateTranslatorTests.cs | Updated test to use new preprocessor |
| LegacyPredicateTranslatorTests.cs | Updated test to use new preprocessor |
| CSharp5749Tests.cs | New test file validating the MemoryExtensions to Enumerable rewriting works correctly |
Comments suppressed due to low confidence (1)
src/MongoDB.Driver/Linq/Linq3Implementation/Misc/ClrCompatExpressionRewriter.cs:1
- Incorrect method reference: should use
SequenceEqualWithReadOnlySpanAndReadOnlySpanAndComparersince this branch handles the 3-parameter overload with a comparer, not the 2-parameter overload.
/* Copyright 2010-present MongoDB Inc.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| var result = collection.AsQueryable().Single(Rewrite((C x) => ratings.SequenceEqual(x.Ratings))); | ||
|
|
||
| result.Id.Should().Be(3); |
There was a problem hiding this comment.
Inconsistent indentation: this line uses 7 spaces instead of 8 spaces like the surrounding code.
| result.Id.Should().Be(3); | |
| result.Id.Should().Be(3); |
| namespace MongoDB.Driver.Linq.Linq3Implementation.Misc; | ||
|
|
||
| /// <summary> | ||
| /// This visitor rewrites expressions where new features of .NET CLR or |
There was a problem hiding this comment.
Could you please refer to particular features instead of "new", as the term "new" will be outdated pretty soon.
There was a problem hiding this comment.
I'm just removing the word "new" from the comment.
Listing features doesn't feel appropriate for the one sentence summary.
The code itself documents in detail what this class does.
| using MongoDB.Driver.TestHelpers; | ||
| using Xunit; | ||
|
|
||
| namespace MongoDB.Driver.Tests.Linq.Linq3Implementation.Jira; |
There was a problem hiding this comment.
Should we add unit tests for ClrCompatExpressionRewriter? That will have expressions as an input and output.
There was a problem hiding this comment.
That would be redundant, we already have integration tests that indirectly test this. This is common for LINQ tests, where we rely on the integration tests to also tests functionality lower down.
Otherwise we end up testing things more than once.
| ]; | ||
| } | ||
|
|
||
| private Expression<Func<T, bool>> Rewrite<T>(Expression<Func<T, bool>> predicate) |
There was a problem hiding this comment.
Am I right to think that we have this rewriter only because C# 14 is not released yet and we cannot bump our yet? Should we hold this PR until the release and actually use the features in the tests?
There was a problem hiding this comment.
We want to get this out ahead of switching to c# 14. At some point in tje future we can remove the test rewriter.
| namespace MongoDB.Driver.Linq.Linq3Implementation.Misc | ||
| { | ||
| internal static class MethodInfoExtensions | ||
| { |
There was a problem hiding this comment.
Instead of Has can we use TryGet given that this has out parameters that it tries to get? It would follow many other .net methods like TryGetValue on Dictionary.
E g. TryGetSingleGenericArgument
There was a problem hiding this comment.
I would prefer to keep the existing names.
The Linq processor has many existing methods that are named IsXyz, HasXyz, ImplementsXyz etc.. many of which also have out parameters.
Naming a method TryXyz puts the emphasis on the operation and whether it succeeds or fails.
In the LINQ processor we aren't thinking in terms of success or failure, we are thinking in terms of asking questions like "is", "has", "implements" etc... the out parameters are less important than the question being asked.
| ]; | ||
| } | ||
|
|
||
| private Expression<Func<T, bool>> Rewrite<T>(Expression<Func<T, bool>> predicate) |
There was a problem hiding this comment.
We want to get this out ahead of switching to c# 14. At some point in tje future we can remove the test rewriter.
3 participants
Handle C# 14 now binding some overloads to MemoryExtensions extension methods by rewriting the Expression tree to use the original Enumerable extension methods that C# used to bind to.