Null-conditional assignment 1 #75821
Conversation
dotnet-issue-labeler
bot
added
Area-Compilers
untriaged
…gnment' into nca-1
|
@CyrusNajmabadi ready for a high-level review pass. |
| // We should consume suppression '!'s which are in the middle of the 'whenNotNull', but not at the end. | ||
| // For example, 'a?.b!.c' should be a cond-access whose RHS is '.b!.c', | ||
| // while 'a?.b!' should be a suppression-expr containing a cond-access 'a?.b'. | ||
| using var beforeSuppressionsResetPoint = GetDisposableResetPoint(resetOnDispose: false); |
There was a problem hiding this comment.
Using a reset point lets us unify the "check" and "act" steps of deciding to include the '!'s we found in the current cond-access node or not. This lets us avoid having to write a "dry-run" version of the assignment operator parsing code, which advances the token stream and potentially merges tokens found in it.
src/Compilers/CSharp/Test/Semantic/Semantics/NullConditionalAssignmentTests.cs
Outdated
Show resolved
Hide resolved
| rs?.RF = ref i; // 1 | ||
|
|
||
| RS? nrs = rs; // 2 | ||
| nrs?.RF = ref i; // 3 |
There was a problem hiding this comment.
That's an interesting thought, as the assignment itself is fine. Left side doesn't need to be a variable in this case. It just doesn't work because a ref struct can't be nullable.
|
|
||
| static void M(C c) | ||
| { | ||
| c?.E += () => { Console.Write(1); }; |
| } | ||
|
|
||
| [Fact] | ||
| public void TypeParameter_03() |
| [Fact] | ||
| public void Increment_LeftMemberAccess() | ||
| { | ||
| // Increment/decrement of a conditional access is not supported. |
| public void Increment_LeftMemberAccess() | ||
| { | ||
| // Increment/decrement of a conditional access is not supported. | ||
| string source = "a?.b++"; |
| [Fact] | ||
| public void Parentheses_Assignment_LHS_01() | ||
| { | ||
| UsingExpression("(c?.F) = 1"); |
There was a problem hiding this comment.
Perhaps pass LanguageVersion as a parameter in each case.
[Theory]
[CombinatorialData]
public void Parentheses_Assignment_LHS_01(
[CombinatorialValues(LanguageVersion.CSharp13, LanguageVersion.Preview)]
LanguageVersion languageVersion)There was a problem hiding this comment.
sure, probably the best way is by a Theory/TheoryData which is shared between all the tests.
| [InlineData(SyntaxKind.PercentEqualsToken)] | ||
| [InlineData(SyntaxKind.EqualsToken)] | ||
| [InlineData(SyntaxKind.QuestionQuestionEqualsToken)] | ||
| public void VariousAssignmentKinds_LeftMemberAccess(SyntaxKind kind) |
There was a problem hiding this comment.
I didn't feel like setting up the combinatorial data for all the syntax kinds and language versions here so just kept the local functions approach.
| [InlineData(SyntaxKind.PercentEqualsToken)] | ||
| [InlineData(SyntaxKind.EqualsToken)] | ||
| [InlineData(SyntaxKind.QuestionQuestionEqualsToken)] | ||
| public void LangVersion_03(SyntaxKind kind) |
There was a problem hiding this comment.
It occurs to me that if a new assignment kind is added later, it will be gated behind its own LangVersion check, which is at least as strict as this one.
| @@ -10944,6 +10944,10 @@ private BoundConditionalAccess BindConditionalAccessExpression(ConditionalAccess | |||
|
|
|||
| var conditionalAccessBinder = new BinderWithConditionalReceiver(this, receiver); | |||
| var access = conditionalAccessBinder.BindValue(node.WhenNotNull, diagnostics, BindValueKind.RValue); | |||
| if (access.Syntax is AssignmentExpressionSyntax) | |||
| { | |||
| CheckFeatureAvailability(node.WhenNotNull, MessageID.IDS_FeatureNullConditionalAssignment, diagnostics); | |||
There was a problem hiding this comment.
Consider placing the diagnostics just on the =. It's generally nicer than a big squiggle on something large.
Co-authored-by: Cyrus Najmabadi <[email protected]>
RikkiGibson
merged commit 7c84cba
into
dotnet:features/null-conditional-assignment
Relates to test plan #75554