-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Support null-conditional operator when the return value is a pointer type #80694
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…havior Co-authored-by: CyrusNajmabadi <[email protected]>
- Modified Binder_Expressions.cs to allow pointer types in null-conditional expressions - Changed check from IsPointerOrFunctionPointer() to IsFunctionPointer() to allow regular pointers - Added condition to skip Nullable<T> wrapping for pointer types - Pointers can represent null as the zero value, so no need for Nullable<T> - Function pointers are still correctly rejected - Added comprehensive tests covering various pointer scenarios - All existing tests continue to pass Co-authored-by: CyrusNajmabadi <[email protected]>
} | ||
|
||
// The resulting type must be either a reference type T or Nullable<T> | ||
// The resulting type must be either a reference type T, Nullable<T>, or a pointer type. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dotnet/roslyn-compiler i think this is correct. The spec says this: https://github.com/dotnet/csharpstandard/blob/standard-v6/standard/expressions.md#1177-null-conditional-member-access
A null_conditional_member_access expression E is of the form P?.A. Let T be the type of the expression P.A. The meaning of E is determined as follows:
Otherwise the type of E is T, and the meaning of E is the same as the meaning of:
- ((object)P == null) ? null : P.A
- Except that P is evaluated only once.
So my reading from teh spec is that this should be legal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/dotnet/csharpstandard/blob/draft-v8/standard/expressions.md#1288-null-conditional-member-access has revised the wording somewhat but I think Cyrus's interpretation still holds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The latest docs show this:
Let T be the type of the expression P.A.
...
Otherwise the type of E is T, and the meaning of E is the same as the meaning of:
((object)P == null) ? (T)null : P.A
Except that P is evaluated only once.
graph, symbol); | ||
} | ||
|
||
[Fact] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot these should have WorkItem attributes on them. The WorkItem should only have one argument. The url to the issue this is a test for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added WorkItem attributes to all 7 test methods with the issue URL. (commit: work in progress)
Added WorkItem attribute with issue URL to all 7 test methods as requested in code review. Co-authored-by: CyrusNajmabadi <[email protected]>
…ithub.com/dotnet/roslyn into copilot/support-null-conditional-operator
@dotnet/roslyn-compiler ptal. |
I think I agree with Cyrus's interpretation of the spec. I was suspicious that this scenario went from disallowed by spec to allowed by spec when the spec was rewritten in csharpstandard. I went and looked, and my interpretation is the same with both the old and new spec language. See the old spec for
It looks like past interpretations were that pointers are non-nullable value types. On that reasoning, it's invalid for a conditional-access's value to be of a pointer type, because However, both the old and new specs say that pointers are not value types:
I think the motivation for actually making the change is low. I don't think it's clear that the spec authors had pointers in mind when the |
Summary
This PR adds support for the null-conditional operator (
?.
) when the return value is a pointer type, fixing the issue where code likebyte* ptr = x?.Ptr;
was incorrectly rejected with error CS8978.Changes Made
1. Modified
Binder_Expressions.cs
:IsPointerOrFunctionPointer()
toIsFunctionPointer()
to allow regular pointers while still rejecting function pointersNullable<T>
wrapping for pointer types since pointers can already represent null2. Added 7 comprehensive tests in
NullConditionalAssignmentTests.cs
:PointerReturnType_Simple
: Basic functionality testPointerReturnType_WithUsage
: Runtime behavior verificationPointerReturnType_IntPointer
: Tests withint*
PointerReturnType_VoidPointer
: Tests withvoid*
PointerReturnType_Chained
: Chained access scenariosPointerReturnType_StatementContext
: Statement context usageFunctionPointerReturnType_StillRejected
: Verifies function pointers are still rejected[WorkItem("https://github.com/dotnet/roslyn/issues/7502")]
attributesTechnical Details
Nullable<T>
wrapping needed= null
initialization in tests avoids CS0649 warnings for cleaner test outputOriginal prompt
Fixes #7502
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.