Unsafe evolution: symbol display by jjonescz · Pull Request #84706 · dotnet/roslyn

jjonescz · 2026-07-30T12:45:33Z

Include unsafe in modifiers produced by SymbolDisplay. In a follow-up PR, I'd like to use this to update PublicApiAnalyzer to include unsafe in its signatures.

Test plan: #81207

Microsoft Reviewers: Open in CodeFlow

azure-pipelines · 2026-07-30T12:45:51Z

Azure Pipelines: Successfully started running 1 pipeline(s). 1 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run.

Copilot

Pull request overview

This PR updates C# symbol display so that, when SymbolDisplayMemberOptions.IncludeModifiers is requested and the containing module uses the updated memory safety rules, members that are explicitly caller-unsafe (CallerUnsafeMode.Explicit) include the unsafe modifier in ToDisplayString() output. The accompanying tests extend existing Unsafe Evolution coverage to assert the new display behavior across a variety of member kinds.

Changes:

Emit unsafe in SymbolDisplayVisitor.AddMemberModifiersIfNeeded for explicitly caller-unsafe members under updated memory safety rules.
Extend UnsafeEvolutionTests with validators/assertions verifying modifier display differences between legacy vs updated rule sets (methods, properties/accessors, indexers, events, ctors, fields, and extern cases).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/Compilers/CSharp/Test/CSharp15/UnsafeEvolutionTests.cs	Adds targeted assertions/validators ensuring `ToDisplayString(...IncludeModifiers...)` includes `unsafe` only under updated memory safety rules and only for explicitly caller-unsafe members.
src/Compilers/CSharp/Portable/SymbolDisplay/SymbolDisplayVisitor.Members.cs	Adds `unsafe` keyword emission in member modifier display when `GetCallerUnsafeMode(...) == CallerUnsafeMode.Explicit` and the containing module uses updated memory safety rules.

azure-pipelines · 2026-07-31T14:21:24Z

Azure Pipelines: Successfully started running 1 pipeline(s). 1 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

AlekseyTs · 2026-07-31T14:33:41Z

                    AddSpace();
                }

+                if (symbol is Symbols.PublicModel.Symbol { UnderlyingSymbol: { ContainingModule.UseUpdatedMemorySafetyRules: true } internalSymbol } &&


UnderlyingSymbol

We prefer to implement SymbolDisplay through public API

I know, but I've seen this pattern of using internal symbols elsewhere in SymbolDisplayVisitor.

Also, public API review already approved a bool-only shape of the public API and I think I want to only add unsafe modifier for CallerUnsafeMode.Explicit. I guess I could check whether the ContainingModule uses updated memory safety rules when a public API for that is available. Or we could revisit this in API review.

I think it should be fine to proceed with internal symbols access with a follow-up issue.

AlekseyTs · 2026-07-31T14:34:37Z

                }

+                if (symbol is Symbols.PublicModel.Symbol { UnderlyingSymbol: { ContainingModule.UseUpdatedMemorySafetyRules: true } internalSymbol } &&
+                    internalSymbol.GetCallerUnsafeMode(ConsList<FieldSymbol>.Empty) == CallerUnsafeMode.Explicit)


We probably need a new display option to add the modifier.

SymbolDisplayMemberOptions.IncludeModifiers seems appropriate for this.

Isn't there already a behavior for this flag?

Sure, it's not a new flag. I guess we could document this as a breaking change. Although it's a new behavior in a sense - unsafe will be only added for new code (code that opts into unsafe evolution and uses the keyword to annotate caller-unsafe members).

To be clear, SymbolDisplayMemberOptions.IncludeModifiers is the flag this is already gated on (see line 934 above).

Although it's a new behavior in a sense - unsafe will be only added for new code (code that opts into unsafe evolution and uses the keyword to annotate caller-unsafe members).

This sounds reasonable to me. At the very least this should be said explicitly. Either in a comment here, or in PR description. Or both.

AlekseyTs · 2026-07-31T14:36:40Z

Done with a quick glance (commit 6)

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

jjonescz · 2026-08-06T06:59:00Z

@333fred @RikkiGibson for reviews, thanks

333fred

LGTM, assuming we address Aleksey's comment about documentation.

jjonescz · 2026-08-11T08:00:42Z

assuming we address Aleksey's comment about documentation.

That should have been already addressed in commit b738bc2

jjonescz · 2026-08-11T08:00:50Z

@RikkiGibson for a second review, thanks

jjonescz · 2026-08-17T09:50:33Z

@RikkiGibson for a second review, thanks

RikkiGibson · 2026-08-18T18:27:00Z

+        var localFunctions = tree.GetRoot().DescendantNodes().OfType<LocalFunctionStatementSyntax>().Select(s => model.GetDeclaredSymbol(s)!).ToArray();
+        Assert.Equal(2, localFunctions.Length);
+
+        AssertEx.Equal("void M1()", localFunctions[0].ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat.AddMemberOptions(SymbolDisplayMemberOptions.IncludeModifiers)));


I didn't follow why this display is missing the unsafe modifier, when, the declaration has the modifier, and new unsafe rules are being used.

Local functions are for some reason excluded from SymbolDisplayMemberOptions.IncludeModifiers:

roslyn/src/Compilers/CSharp/Portable/SymbolDisplay/SymbolDisplayVisitor.Members.cs

Line 936 in b738bc2

(containingType.TypeKind != TypeKind.Interface && !IsEnumMember(symbol) && !IsLocalFunction(symbol))))

RikkiGibson · 2026-08-18T18:29:49Z

+                AssertEx.Equal("int C.P1", comp.GetMember("C.P1").ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat.AddMemberOptions(SymbolDisplayMemberOptions.IncludeModifiers)));
+                AssertEx.Equal("unsafe int C.P1.get", comp.GetMember("C.get_P1").ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat.AddMemberOptions(SymbolDisplayMemberOptions.IncludeModifiers)));
+                AssertEx.Equal("int C.P2", comp.GetMember("C.P2").ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat.AddMemberOptions(SymbolDisplayMemberOptions.IncludeModifiers)));
+                AssertEx.Equal("int C.P2.get", comp.GetMember("C.get_P2").ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat.AddMemberOptions(SymbolDisplayMemberOptions.IncludeModifiers)));


nit: Consider also checking the display of the setter.

RikkiGibson

LGTM with some minor nits/questions. Feel free to address in follow up or similar if desired.

Discussed a bit offline and noted that it would be good for presence of 'unsafe' modifier in Quick Info, when we get to that, to indicate that the item requires unsafe context to be used. So, for example, it would be good to include it in the display there, for a legacy method with pointers in signature.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/Compilers/CSharp/Portable/SymbolDisplay/SymbolDisplayVisitor.Members.cs:964

The new comment suggests unsafe is only shown for members explicitly annotated with the unsafe keyword, but the condition uses symbol.RequiresUnsafeContext, which can also be true for implicitly-requires-unsafe members (e.g., pointers in signature via CallerUnsafeMode.Implicit). Consider updating the comment to describe the actual behavior so future readers don’t infer a narrower contract than the code implements.

                // unsafe is added only for new code (code that opts into unsafe evolution and uses the keyword to annotate caller-unsafe members)

jjonescz · 2026-08-20T13:45:26Z

@RikkiGibson or @333fred for another look, thanks; I needed to fixup a name after merge

Unsafe evolution: symbol display

39cb3ca

jjonescz added 4 commits July 31, 2026 12:27

Merge branch 'main' into Unsafe-46-SymbolDisplay

df575cf

Unsafe evolution: fixup conflicting merges

72385f7

Fixup merge

7c8082f

Revert safe modifier display

48ad311

jjonescz added Area-Compilers Feature - Unsafe Evolution labels Jul 31, 2026

jjonescz requested a lite review from Copilot July 31, 2026 14:10

Copilot started reviewing on behalf of jjonescz July 31, 2026 14:10 View session

Merge branch 'main' into Unsafe-46-SymbolDisplay

3447cec

Copilot AI reviewed Jul 31, 2026

View reviewed changes

jjonescz marked this pull request as ready for review July 31, 2026 14:21

jjonescz requested a review from a team as a code owner July 31, 2026 14:21

Copilot AI review requested due to automatic review settings July 31, 2026 14:21

Copilot started reviewing on behalf of jjonescz July 31, 2026 14:21 View session

Copilot AI reviewed Jul 31, 2026

View reviewed changes

AlekseyTs reviewed Jul 31, 2026

View reviewed changes

Improve code and comments

b738bc2

Copilot AI review requested due to automatic review settings July 31, 2026 15:11

Copilot started reviewing on behalf of jjonescz July 31, 2026 15:11 View session

Copilot AI reviewed Jul 31, 2026

View reviewed changes

jjonescz requested a review from 333fred August 3, 2026 08:28

jjonescz requested a review from RikkiGibson August 10, 2026 08:59

333fred approved these changes Aug 10, 2026

View reviewed changes

RikkiGibson reviewed Aug 18, 2026

View reviewed changes

RikkiGibson approved these changes Aug 18, 2026

View reviewed changes

Merge branch 'main' into Unsafe-46-SymbolDisplay

73391c3

Copilot AI review requested due to automatic review settings August 19, 2026 13:27

Copilot started reviewing on behalf of jjonescz August 19, 2026 13:32 View session

jjonescz added 2 commits August 19, 2026 15:33

Fixup after merge

0d5bae8

Improve tests

1623779

jjonescz requested a review from RikkiGibson August 19, 2026 13:38

Copilot AI reviewed Aug 19, 2026

View reviewed changes

Comment thread src/Compilers/CSharp/Portable/SymbolDisplay/SymbolDisplayVisitor.Members.cs

Copilot AI review requested due to automatic review settings August 19, 2026 13:40

Copilot started reviewing on behalf of jjonescz August 19, 2026 13:40 View session

Copilot AI reviewed Aug 19, 2026

View reviewed changes

RikkiGibson approved these changes Aug 20, 2026

View reviewed changes

jjonescz merged commit 7de096b into dotnet:main Aug 20, 2026
21 checks passed

jjonescz deleted the Unsafe-46-SymbolDisplay branch August 20, 2026 17:28

This was referenced Aug 20, 2026

[release/11.0.1xx] Source code updates from dotnet/roslyn dotnet/dotnet#8449

Merged

[main] Source code updates from dotnet/roslyn dotnet/dotnet#8504

Merged

davidwengier mentioned this pull request Aug 24, 2026

Update roslyn to 5.11.0-1.26424.9 dotnet/vscode-csharp#9701

Merged

jjonescz added this to the 18.11 milestone Aug 25, 2026

jjonescz mentioned this pull request Sep 1, 2026

Unsafe evolution: display unsafe modifier on interface members too #85118

Merged

Conversation

jjonescz commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Microsoft Reviewers: Open in CodeFlow

Uh oh!

azure-pipelines Bot commented Jul 30, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

azure-pipelines Bot commented Jul 31, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jjonescz Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs commented Jul 31, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

jjonescz commented Aug 6, 2026

Uh oh!

333fred left a comment

Choose a reason for hiding this comment

Uh oh!

jjonescz commented Aug 11, 2026

Uh oh!

jjonescz commented Aug 11, 2026

Uh oh!

jjonescz commented Aug 17, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RikkiGibson left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

jjonescz commented Aug 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

jjonescz commented Jul 30, 2026 •

edited

Loading

jjonescz Jul 31, 2026 •

edited

Loading

AlekseyTs Jul 31, 2026 •

edited

Loading