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) |
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