Skip to content
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

Exclude explicitly implemented interface methods from proxy #8992

Merged
merged 2 commits into from May 22, 2024

Conversation

alrz
Copy link
Member

@alrz alrz commented May 10, 2024

Fix #8991

Microsoft Reviewers: Open in CodeFlow

@@ -81,7 +81,12 @@ private List<ProxyMethodDescription> GetMethods(INamedTypeSymbol symbol)
{
foreach (var method in iface.GetDeclaredInstanceMembers<IMethodSymbol>())
{
if (methods.TryGetValue(method, out _))
if (method.MethodKind == MethodKind.ExplicitInterfaceImplementation)
Copy link
Member Author

Choose a reason for hiding this comment

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

I think this should also check for a body, explicit impls can be used for reabstraction in interface type hierarchies.

Copy link
Member

Choose a reason for hiding this comment

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

I am not sure it's necessary: the generated proxy should implement every method explicitly (I added a comment below this line). Any overrides should take effect on the callee side naturally (correct me if you find that I'm wrong here)

@alrz
Copy link
Member Author

alrz commented May 10, 2024

What sort of tests do we need here? I'd appreciate some pointers on existing unit tests so I can follow from there. Also not sure about namings and the location these interfaces should be declared in.

@ReubenBond for review, thanks.

@ReubenBond ReubenBond self-assigned this May 16, 2024
@ReubenBond
Copy link
Member

ReubenBond commented May 22, 2024

I fixed the merge conflict. LGTM. Thanks for the PR

@ReubenBond ReubenBond merged commit 63236c3 into dotnet:main May 22, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: Invalid code generated for interface with explicit implementations
2 participants