Skip to content

Conversation

@AlekseyTs
Copy link
Contributor

@AlekseyTs AlekseyTs commented Sep 24, 2025

Related to #78968

Relates to test plan #76130

@AlekseyTs AlekseyTs requested review from jcouv and jjonescz September 24, 2025 22:17
@AlekseyTs AlekseyTs requested a review from a team as a code owner September 24, 2025 22:17
@AlekseyTs AlekseyTs added the Feature - Extension Everything The extension everything feature label Sep 24, 2025
@AlekseyTs AlekseyTs marked this pull request as draft September 25, 2025 03:27
@jcouv jcouv mentioned this pull request Sep 25, 2025
16 tasks
@AlekseyTs AlekseyTs marked this pull request as ready for review September 25, 2025 21:44
@AlekseyTs
Copy link
Contributor Author

@jcouv, @jjonescz, @dotnet/roslyn-compiler Please review.

@jcouv jcouv self-assigned this Sep 26, 2025
@AlekseyTs
Copy link
Contributor Author

@jcouv, @jjonescz, @dotnet/roslyn-compiler For a second review.

static bool hasRefToRefStructThis(MethodSymbol? method)
{
return method?.TryGetThisParameter(out var thisParameter) == true &&
return method?.TryGetThisParameter(out var thisParameter) == true && thisParameter is not null &&
Copy link
Member

@jjonescz jjonescz Sep 26, 2025

Choose a reason for hiding this comment

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

Consider nullable-enabling TryGetThisParameter to catch these kinds of mistakes sooner in the future #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Technically this wouldn't help because on this code path null result was unexpected before my change to the behavior of the API.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Annotated TryGetThisParameter

@AlekseyTs
Copy link
Contributor Author

@jcouv Please review

static bool hasRefToRefStructThis(MethodSymbol? method)
{
return method?.TryGetThisParameter(out var thisParameter) == true &&
return method?.TryGetThisParameter(out var thisParameter) == true && thisParameter is not null &&
Copy link
Member

@jcouv jcouv Sep 29, 2025

Choose a reason for hiding this comment

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

Should Binder.HasThis(...) method be adjusted as well? Never mind, I see we already have #Closed

Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

LGTM Thanks (commit 4) with one question

@AlekseyTs AlekseyTs enabled auto-merge (squash) September 29, 2025 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-Compilers Feature - Extension Everything The extension everything feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants