fix: handle InvocationExpression pattern for MemoryExtensions.Contains (.NET 10)#79
Closed
korchak-aleksandr wants to merge 13 commits intomainfrom
Closed
fix: handle InvocationExpression pattern for MemoryExtensions.Contains (.NET 10)#79korchak-aleksandr wants to merge 13 commits intomainfrom
korchak-aleksandr wants to merge 13 commits intomainfrom
Conversation
- TargetFramework: netstandard2.0 → net8.0 - LangVersion: latest → 12 - PackageVersion: 10.7.2 → 10.8.0 - Microsoft.Data.SqlClient: 5.1.0 → 6.1.4 - Removed BCL packages: System.Reflection.Emit, System.Runtime, System.Runtime.Extensions, System.ComponentModel.Annotations, System.Security.Permissions - SecurityUtils: removed CAS (ReflectionPermission) — no-op on .NET 5+ - SYSLIB0011 (BinaryFormatter): suppressed — legacy DB serialization path - Tests: updated to net8.0, suppressed CS0169 QueryConverter: support MemoryExtensions.Contains from .NET 10 In .NET 10, array.Contains(value) in expression trees compiles to MemoryExtensions.Contains(ReadOnlySpan<T>.op_Implicit(array), value) instead of Enumerable.Contains(array, value). Added handling to translate it to SQL IN clause the same way. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…fix CI - Add Directory.Build.props: TargetFramework=net8.0, LangVersion=12 - Add Directory.Packages.props: centralized package versions - Restore original SecurityUtils.cs (add System.Security.Permissions 8.0.0 package; suppress SYSLIB0003 — CAS is a no-op on .NET 5+, types still compile with the compat package) - Simplify csproj files: remove properties now in Directory.Build.props - Update pull-request.yml: setup-dotnet@v4, dotnet-version 8.0.x
- Microsoft.NET.Test.Sdk: 17.4.1 → 18.3.0 - MSTest.TestAdapter/TestFramework: 3.0.2 → 3.11.1 - Moq: 4.18.4 → 4.20.72
Tests verify that array.Contains(value) in LINQ queries translates to SQL IN clause on both net8.0 and net10.0. On .NET 10, the compiler generates MemoryExtensions.Contains — the fix in QueryConverter ensures it produces identical SQL to Enumerable.Contains on .NET 8. Also update CI to install both .NET 8 and .NET 10 SDKs.
- Add <TargetFramework /> to clear Directory.Build.props default - Set AppendTargetFrameworkToOutputPath=true to avoid DLL overwrites
More robust: matches any conversion to ReadOnlySpan<T>, not just the specific op_Implicit generated by the current C# compiler.
…ork per project - Directory.Build.props: LangVersion, Company, Authors, Copyright, PackageLicenseExpression - Each csproj now declares its own TargetFramework/TargetFrameworks - Remove duplicate Company/Authors/Copyright from individual csproj files - Remove leftover Product/NeutralLanguage from test csproj
…s in .NET 10 In .NET 10, array.Contains(value) in LINQ expression trees may generate two patterns: 1. MethodCallExpression(op_Implicit, [array]) — for simple local captures 2. InvocationExpression(ConstantExpression(pre_compiled_delegate), []) — when array is captured in a nested closure (e.g. passed as a method parameter) The original 10.8.0 fix only handled pattern 1. This fix adds: - TryExtractArrayFromSpanExpression() helper that handles both patterns - For pattern 2: extracts the array from the delegate's closure target via reflection - Broader condition check: mc.Arguments[0].Type is ReadOnlySpan<T> (not just MethodCallExpression) New test ArrayContains_PassedAsParameter_TranslatesToSqlIn reproduces pattern 2.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
10.8.0(PR #76) fixedMemoryExtensions.Containsfor the simple case, but missed a second compiler pattern.In .NET 10,
array.Contains(value)in LINQ expression trees generates two patterns depending on closure structure:MethodCallExpression(op_Implicit, [array])— simple local variable capture ✅ fixed in 10.8.0InvocationExpression(ConstantExpression(pre_compiled_delegate), [])— nested closure (e.g. array passed as method parameter) ❌ still threwNotSupportedExceptionThe
VisitInvocationhandler tried toDynamicInvokethe delegate butReadOnlySpan<T>is a ref struct and cannot be returned via reflection — causing the crash.Fix
Added
TryExtractArrayFromSpanExpression()helper that handles both patterns:MethodCallExpression(op_Implicit, [array_expr])→ returnarray_exprInvocationExpression(ConstantExpression(delegate), [])→ extract the array from the delegate's closure target via reflectionThe entry condition is also broadened: checks
mc.Arguments[0].Type is ReadOnlySpan<T>instead of requiring a specificMethodCallExpressionshape.New test
ArrayContains_PassedAsParameter_TranslatesToSqlIn— reproduces pattern 2 (array captured in nested method scope), was RED before this fix.Test plan
dotnet build— 0 errors, 0 warningsdotnet test— 63 passed, 17 skipped (net8.0 + net10.0)