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

Prevent embedding of interfaces that contain non-abstract members. #34171

Merged

Conversation

AlekseyTs
Copy link
Contributor

No description provided.

@AlekseyTs AlekseyTs requested a review from a team as a code owner March 15, 2019 20:07
@AlekseyTs AlekseyTs added the Feature - Default Interface Impl Default Interface Implementation label Mar 15, 2019
Diagnostic(ErrorCode.ERR_DefaultInterfaceImplementationInNoPIAType, "ITest33").WithArguments("ITest33").WithLocation(6, 13)
);
}
}
Copy link
Member

@333fred 333fred Mar 15, 2019

Choose a reason for hiding this comment

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

Would it be interesting to test this combination:
ITest33 with method declaration. ITest34 inerits ITest33, implements the method. UsePia test with both ITest33 and ITest34. #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.

Would it be interesting to test this combination:
ITest33 with method declaration. ITest34 inerits ITest33, implements the method. UsePia test with both ITest33 and ITest34.

Added tests.


In reply to: 266136460 [](ancestors = 266136460)


[ComImport()]
[Guid(""f9c2d51d-4f44-45f0-9eda-c9d599b58279"")]
public interface ITest33
Copy link
Member

@jcouv jcouv Mar 15, 2019

Choose a reason for hiding this comment

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

Is it worth testing an interface with a nested type (interface or abstract class), since they are new scenarios? Looks like we expect an error for that (ERR_NoPIANestedType). #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.

Is it worth testing an interface with a nested type (interface or abstract class), since they are new scenarios? Looks like we expect an error for that (ERR_NoPIANestedType).

These are not new scenarios because VB supports types nested in interfaces. The ERR_NoPIANestedType is also covered in NoPia_06.


In reply to: 266158754 [](ancestors = 266158754)

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

@jcouv jcouv self-assigned this Mar 15, 2019
@jcouv jcouv added this to the 16.1.P1 milestone Mar 15, 2019
Copy link
Member

@333fred 333fred 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 2)

@AlekseyTs AlekseyTs merged commit 3cc1c9a into dotnet:features/DefaultInterfaceImplementation Mar 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants