Unsafe evolution: avoid errors inside nameof - #84325
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adjusts the C# binder so “unsafe member access” diagnostics are not produced while binding inside a nameof(...) argument, and adds regression tests to ensure unsafe declarations/members (including fields) don’t trigger updated memory-safety diagnostics in that context.
Changes:
- Skip
ReportDiagnosticsIfUnsafeMemberAccesswhenBinder.IsInsideNameofis true, preventing unsafe-member-operation diagnostics from being reported innameofarguments. - Add new compiler tests covering
nameofoverunsafealias/local function/type declarations and member names, validating behavior under both legacy and updated memory safety rules.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Compilers/CSharp/Portable/Binder/Binder_Unsafe.cs | Early-exits unsafe member access diagnostic reporting when binding within nameof. |
| src/Compilers/CSharp/Test/CSharp15/UnsafeEvolutionTests.cs | Adds regression coverage ensuring unsafe-related diagnostics aren’t reported inside nameof (including for fields), across rule modes. |
|
|
||
| private void ReportDiagnosticsIfUnsafeMemberAccess<T>(DiagnosticBag diagnostics, Symbol symbol, T arg, Func<T, Location?> location) | ||
| { | ||
| if (IsInsideNameof) |
There was a problem hiding this comment.
It looks like we have BinderFlags.SuppressUnsafeDiagnostics. Also, it looks like in one of the recent changes we set BinderFlags.UnsafeRegion flag on a binder to achieve the same goal for params collections declarations. I think we should simply stick to setting proper BinderFlags for NameOfBinder. Also, we probably should standardize on BinderFlags.SuppressUnsafeDiagnostics for the purpose of suppressing diagnostics. #Closed
There was a problem hiding this comment.
That's what I looked at first, but it doesn't seem to be as simple in NameofBinder. It computes the fact whether we are in nameof operator lazily, via some non-trivial logic, hence I don't think we can just statically set a flag when creating the binder.
|
Done with review pass (commit 1) |
The preview C# compiler currently reports CS9363 ("must be used in an
unsafe context") for nameof references to fields whose signature contains
pointers, even though nameof never dereferences them. These references
live in MemberData iterator methods, which can never be an unsafe context,
so marking the methods unsafe does not help. Replace the affected
nameof(...) usages with equivalent string literals.
This is fixed upstream by dotnet/roslyn#84325 ("Unsafe evolution: avoid
errors inside nameof"), so this workaround can be reverted once that
change flows into the repo's compiler.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Test plan: #81207
Unsafe errors were already not reported for most members inside nameof (because binding there doesn't get to the point of "use" where those diagnostics are reported) except for fields.
Microsoft Reviewers: Open in CodeFlow