Emit safe keyword on generated UnsafeAccessor methods under updated memory safety rules - #39001
Open
jkoritzinsky wants to merge 2 commits into
Open
jkoritzinsky wants to merge 2 commits into
jkoritzinsky wants to merge 2 commits into
Conversation
… memory safety rules When the C# "updated memory safety rules" (a.k.a. unsafe evolution) feature is enabled, generated `[UnsafeAccessor]` `static extern` methods in the precompiled-query code generation pipeline now get the `safe` contextual keyword added to their modifiers. Adds an internal `MemorySafetyRules` helper (adapted from dotnet/runtime's System.Runtime.InteropServices source generator shim) that detects whether a Compilation was parsed with the updated memory safety rules enabled, first checking the (currently experimental) Roslyn `CSharpCompilationOptions.MemorySafetyRulesVersion` API reflectively, and falling back to the `updated-memory-safety-rules` entry in the parse options' `Features` dictionary when that API is not available on the referenced Roslyn version. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The compatibility fallback issue and missing enabled-path assertions remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review tier: Lite
Findings: 1
Open (2)
What changed in this PR
Updates precompiled-query generation to emit safe on generated [UnsafeAccessor] methods when updated memory safety rules are enabled.
Changes:
- Added Roslyn-compatible memory safety rule detection.
- Threaded the setting through query translators.
- Added detection tests.
| File | Description |
|---|---|
test/EFCore.Design.Tests/Query/MemorySafetyRulesTest.cs |
Tests memory safety rule detection. |
src/EFCore.Design/Query/Internal/RuntimeModelLinqToCSharpSyntaxTranslator.cs |
Forwards the translator setting. |
src/EFCore.Design/Query/Internal/PrecompiledQueryCodeGenerator.cs |
Supplies compilation-based detection. |
src/EFCore.Design/Query/Internal/MemorySafetyRules.cs |
Implements compatibility detection. |
src/EFCore.Design/Query/Internal/LinqToCSharpSyntaxTranslator.cs |
Emits the safe modifier. |
…, add translator test for safe-keyword path - UseUpdatedMemorySafetyRules now checks the updated-memory-safety-rules parse feature even when MemorySafetyRulesVersion is available but returns Version1, since Roslyn treats the feature as an opt-in fallback in that case. - Added a LinqToCSharpSyntaxTranslator test that constructs the translator with useUpdatedMemorySafetyRules: true and asserts the safe modifier is present on the generated UnsafeAccessor declaration (when SafeKeyword is recognized). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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
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.
When C#'s "updated memory safety rules" (a.k.a. unsafe evolution) feature is enabled, generated
[UnsafeAccessor]static externmethods in the precompiled-query code generation pipeline now get thesafecontextual keyword added to their modifiers.Adds an internal
MemorySafetyRuleshelper (adapted from dotnet/runtime'sSystem.Runtime.InteropServicessource generator shim) that detects whether aCompilationwas parsed with the updated memory safety rules enabled:CSharpCompilationOptions.MemorySafetyRulesVersionAPI reflectively.updated-memory-safety-rulesentry in the parse options'Featuresdictionary when that API is not available on the referenced Roslyn version.LinqToCSharpSyntaxTranslatorgains auseUpdatedMemorySafetyRulesconstructor parameter (defaulting tofalse, preserving existing behavior), andPrecompiledQueryCodeGeneratorcomputes the flag once from itsCompilationand threads it through. The NativeAOT compiled-model generator (CSharpRuntimeModelCodeGenerator) is unaffected, since it generates code via string-building from reflection and has noCompilation/parse-options signal available.Added
MemorySafetyRulesTestcovering flag detection; all existingEFCore.Design.Tests(1288 tests) pass with no regressions.