Update 'use null propagation' to understand more patterns #78992
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
CyrusNajmabadi
commented
Jun 16, 2025
|
|
||
| // we have `if (x != null) x.Y();`. Update `x.Y()` to be `x?.Y()`, then replace the entire | ||
| // if-statement with that expression statement. | ||
| var newWhenTrueStatement = CreateConditionalAccessExpression( | ||
| syntaxFacts, generator, whenPartIsNullable, whenTrueStatement, match); | ||
| Contract.ThrowIfNull(newWhenTrueStatement); | ||
|
|
||
| var isElseIf = syntaxFacts.IsElseClause(ifStatement.Parent); |
There was a problem hiding this comment.
i reordered thids pretty heavily. it had this local variable to indicate if we are in the else if (x != null) x.Foo() case. And then lots of contorted logic doing work, but throwing in random checks of this local value. So i just split things into an if/else where we keep all the logic for this case in one place, and then handle the normal if case below that.
CyrusNajmabadi
commented
Jun 16, 2025
|
|
||
| Protected Overrides Function ReplaceBlockStatements(block As ExecutableStatementSyntax, newInnerStatement As ExecutableStatementSyntax) As ExecutableStatementSyntax | ||
| Throw ExceptionUtilities.Unreachable() | ||
| End Function |
There was a problem hiding this comment.
not needed for VB. we now just call into C# to handle the entire case of else if, which means it can just inline the tryGetBlock and replaceblockstatements within PostProcessElseIf. VB has no need for it as PostProcessElseIf is never called for it.
akhera99
approved these changes
Jun 16, 2025
| // `if (<expr> != null) { <expr>.Method(); <expr> = null; }` | ||
| // | ||
| // If 'expr' is not null, then we execute the body and then end up with expr being null. So `expr?.Method(); expr = null;` | ||
| // preserves those semantics. Simialrly, if is expr is null, then `expr?.Method();` does nothing, and `expr = null` keeps it |
There was a problem hiding this comment.
Suggested change
| // preserves those semantics. Simialrly, if is expr is null, then `expr?.Method();` does nothing, and `expr = null` keeps it | |
| // preserves those semantics. Similarly, if is expr is null, then `expr?.Method();` does nothing, and `expr = null` keeps it |
Open
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Specifically, if you write:
Then this is fine to translate to:
This is a somewhat common pattern we see in cleanup methods (like Dispose) where an operation is done on a field if it is non-null, and then it is null'ed out.