Unsafe evolution: unsafe fields#83694
Open
jjonescz wants to merge 3 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the C# compiler’s “unsafe evolution” implementation so that fields can be explicitly marked unsafe under the updated memory safety rules, making field access require an unsafe context via RequiresUnsafeAttribute/CallerUnsafeMode.
Changes:
- Add field support to
RequiresUnsafeAttributeand update compiler tests to validate unsafe field behavior and metadata. - Implement
CallerUnsafeModefor fields across source, metadata, and wrapper/synthesized field symbols (including synthesis and decoding ofRequiresUnsafeAttribute). - Adjust binder behavior so
unsafeon declarations does not introduce an unsafe region under updated rules, and emit diagnostics (not just DEBUG asserts) when inline-array element fields are caller-unsafe.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Compilers/Test/Utilities/CSharp/CSharpTestBase.cs | Updates the test definition of RequiresUnsafeAttribute to allow Field targets. |
| src/Compilers/CSharp/Test/CSharp15/UnsafeEvolutionTests.cs | Adds coverage for unsafe fields, including metadata validation for PE fields. |
| src/Compilers/CSharp/Portable/Symbols/Tuples/TupleFieldSymbol.cs | Propagates CallerUnsafeMode from the underlying field for tuple element fields. |
| src/Compilers/CSharp/Portable/Symbols/Tuples/TupleErrorFieldSymbol.cs | Explicitly sets CallerUnsafeMode to None for error tuple fields. |
| src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedLambdaCacheFieldSymbol.cs | Sets synthesized lambda cache fields to CallerUnsafeMode.None. |
| src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedFieldSymbol.cs | Sets synthesized fields to CallerUnsafeMode.None. |
| src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedEnumValueFieldSymbol.cs | Sets enum backing fields to CallerUnsafeMode.None. |
| src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedBackingFieldSymbol.cs | Sets auto-property backing fields to CallerUnsafeMode.None. |
| src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedPrimaryConstructorParameterBackingFieldSymbol.cs | Sets primary-ctor backing fields to CallerUnsafeMode.None. |
| src/Compilers/CSharp/Portable/Symbols/SubstitutedFieldSymbol.cs | Delegates CallerUnsafeMode to the underlying field for substituted fields. |
| src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberFieldSymbol.cs | Implements explicit caller-unsafe fields under updated rules and synthesizes RequiresUnsafeAttribute. |
| src/Compilers/CSharp/Portable/Symbols/Source/SourceEnumConstantSymbol.cs | Ensures enum constants remain CallerUnsafeMode.None. |
| src/Compilers/CSharp/Portable/Symbols/Source/FieldSymbolWithAttributesAndModifiers.cs | Reports ERR_RequiresUnsafeAttributeInSource when RequiresUnsafeAttribute is applied to fields in source. |
| src/Compilers/CSharp/Portable/Symbols/Retargeting/RetargetingFieldSymbol.cs | Delegates CallerUnsafeMode to the underlying field for retargeting wrappers. |
| src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEFieldSymbol.cs | Decodes/filter-caches RequiresUnsafeAttribute from metadata and exposes it via CallerUnsafeMode. |
| src/Compilers/CSharp/Portable/Symbols/FieldSymbol.cs | Removes the previous “always None” default to allow field-specific CallerUnsafeMode. |
| src/Compilers/CSharp/Portable/Symbols/AnonymousTypes/SynthesizedSymbols/AnonymousType.FieldSymbol.cs | Sets anonymous type backing fields to CallerUnsafeMode.None. |
| src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/StateMachineFieldSymbol.cs | Sets state machine fields to CallerUnsafeMode.None. |
| src/Compilers/CSharp/Portable/Lowering/ClosureConversion/LambdaCapturedVariable.cs | Sets captured-variable fields to CallerUnsafeMode.None. |
| src/Compilers/CSharp/Portable/Binder/Binder_Flags.cs | Updates unsafe-region introduction rules under updated memory safety rules. |
| src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs | Emits diagnostics for unsafe inline-array element field access. |
| src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs | Emits diagnostics for unsafe inline-array conversions via caller-unsafe element fields. |
jjonescz
changed the title
unsafe fieldsunsafe fields
Member
Author
|
@333fred @AlekseyTs for reviews, thanks |
Member
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s), but failed to run 1 pipeline(s). |
Member
Author
|
@333fred @AlekseyTs for reviews, thanks |
333fred
reviewed
May 18, 2026
| { | ||
| get | ||
| { | ||
| if (ContainingModule.UseUpdatedMemorySafetyRules && |
Member
There was a problem hiding this comment.
If the containing module uses updated rules, shouldn't we fall into this branch always, then differentiate based on the declaration modifiers? As currently written, int* field; as a field in source will always be either CallerUnsafeMode.Explicit or CallerUnsafeMode.Implicit, whereas I'd expect it to be CallerUnsafeMode.None in such a case. SourceMethodSymbol does not do this: void M(int* i) would be CallerUnsafeMode.None when the new rules are turned on.
| { | ||
| MarshalAsAttributeDecoder<FieldWellKnownAttributeData, AttributeSyntax, CSharpAttributeData, AttributeLocation>.Decode(ref arguments, AttributeTargets.Field, MessageProvider.Instance); | ||
| } | ||
| else if (attribute.IsTargetAttribute(AttributeDescription.RequiresUnsafeAttribute)) |
Member
There was a problem hiding this comment.
We should probably just add RequiresUnsafeAttribute to ReservedAttributes below.
Member
Author
There was a problem hiding this comment.
This is a slightly better diagnostics experience though since it tells you what to use instead (the unsafe keyword) which the generic diagnostic cannot. We have specialized diagnostics like this for other features too.
Member
There was a problem hiding this comment.
I see FixedSizeBuffer_SafeContext; is there a test with an unsafe fixed-size buffer?
|
|
||
| public override ImmutableArray<CustomModifier> RefCustomModifiers => ImmutableArray<CustomModifier>.Empty; | ||
|
|
||
| internal override CallerUnsafeMode CallerUnsafeMode => CallerUnsafeMode.None; |
Member
There was a problem hiding this comment.
Suggested change
| internal override CallerUnsafeMode CallerUnsafeMode => CallerUnsafeMode.None; | |
| internal sealed override CallerUnsafeMode CallerUnsafeMode => CallerUnsafeMode.None; |
Member
Author
There was a problem hiding this comment.
The containing class is already sealed.
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.
Test plan: #81207
Relevant speclet section: https://github.com/dotnet/csharplang/blob/cd938182639870e8f8fa7c7a9cf0542a1eeeac5b/proposals/unsafe-evolution.md#fields
Microsoft Reviewers: Open in CodeFlow