Skip to content

Extensions: resolve some follow-up comments on signature conflicts and betterness #78442

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

Merged
merged 2 commits into from
May 7, 2025

Conversation

jcouv
Copy link
Member

@jcouv jcouv commented May 5, 2025

From discussion in WG, the implementation follows the design that was discussed in LDM for signature conflicts and for betterness, and the implications were understood. This PR is a shout test, in case you find that any of the interesting scenarios should warrant a design change.

Relates to test plan #76130

@jcouv jcouv self-assigned this May 5, 2025
@jcouv jcouv added Area-Compilers Feature - Extension Everything The extension everything feature labels May 5, 2025
@ghost ghost added the untriaged Issues and PRs which have not yet been triaged by a lead label May 5, 2025
@jcouv jcouv marked this pull request as ready for review May 5, 2025 20:48
@jcouv jcouv requested a review from a team as a code owner May 5, 2025 20:48
@jcouv jcouv requested a review from MadsTorgersen May 5, 2025 20:52
@AlekseyTs
Copy link
Contributor

AlekseyTs commented May 6, 2025

    Assert.Equal("System.Int32 E.<>E__0.P { get; }", model.GetSymbolInfo(memberAccess).Symbol.ToTestDisplayString());

It is hard to say which property is used since both extensions are in the same class. Perhaps ToDisplayString will be more user-friendly. #Pending


Refers to: src/Compilers/CSharp/Test/Emit3/Semantics/ExtensionTests.cs:28083 in ba65636. [](commit_id = ba65636, deletion_comment = False)

@AlekseyTs
Copy link
Contributor

AlekseyTs commented May 6, 2025

    Assert.Equal("System.Int32 E.<>E__1.P { get; }", model.GetSymbolInfo(memberAccess).Symbol.ToTestDisplayString());

Similar comment #Pending


Refers to: src/Compilers/CSharp/Test/Emit3/Semantics/ExtensionTests.cs:28114 in ba65636. [](commit_id = ba65636, deletion_comment = False)

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 1)

@jcouv jcouv enabled auto-merge (squash) May 6, 2025 22:51
@jcouv jcouv merged commit 0ee3dc1 into dotnet:main May 7, 2025
23 of 24 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone May 7, 2025
@jcouv jcouv deleted the extensions-tracked branch May 7, 2025 02:19
@jcouv jcouv added the Test Test failures in roslyn-CI label May 7, 2025
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 Test Test failures in roslyn-CI untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants