Skip to content

Conversation

@DoctorKrolic
Copy link
Contributor

Redo of #74218
Related to #73894

@DoctorKrolic DoctorKrolic requested a review from a team as a code owner October 26, 2025 13:59
@dotnet-policy-service dotnet-policy-service bot added the Community The pull request was submitted by a contributor who is not a Microsoft employee. label Oct 26, 2025
@AlekseyTs
Copy link
Contributor

AlekseyTs commented Oct 27, 2025

Done with review pass (commit 1), tests were not looked at. It looks like the change affects scenarios that fall outside of the stated goal of this PR. I suggest making adjustments to affect only error scenarios with target typed new on the right. #Closed

@DoctorKrolic
Copy link
Contributor Author

This PR is based on the following semantics of ??= operator:

  • If left type is a reference type, the right-hand side should also be that reference type
  • If left type is nullable value type, the right-hand side should be an underlying value type of that nullable type

This semantics is independent on what shape the right-hand side is. Therefore, limiting this recovery to only new() scenarios seems like an overfitting to me. If you look at tests, I heavily verified other target-typed expressions to ensure they get assigned a type since they are most likely to benifit from this change in IDE scenarios even if specific bugs where not discovered yet. But even for non-target-typed scenarios this recovery makes sense since now compiler can insert additional conversions when necessary, which are gonna be present in the success scenarios anyway. Look at ErrorRecovery_ReferenceType_ImplicitConversion and ErrorRecovery_NullableValueType_ImplicitConversion for such examples

@AlekseyTs
Copy link
Contributor

AlekseyTs commented Nov 3, 2025

If left type is nullable value type, the right-hand side should be an underlying value type of that nullable type

I do not think this is accurate. #Closed

@AlekseyTs
Copy link
Contributor

AlekseyTs commented Nov 3, 2025

Done with review pass (commit 2), tests were not looked at. Some of the previously made comments still look relevant. Also, an alternative way to suppress cascading diagnostics was suggested. #Closed

@AlekseyTs
Copy link
Contributor

AlekseyTs commented Nov 19, 2025

@DoctorKrolic It looks like PR validation is failing #Closed

@DoctorKrolic DoctorKrolic requested a review from a team as a code owner November 20, 2025 18:41
Copy link
Contributor

@AlekseyTs AlekseyTs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (commit 10)

@AlekseyTs AlekseyTs requested a review from a team November 21, 2025 20:10
@AlekseyTs
Copy link
Contributor

@dotnet/roslyn-compiler For a second review on a community PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-Compilers Community The pull request was submitted by a contributor who is not a Microsoft employee.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants