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

Consider method accessibility in interface resolution #81409

Merged
merged 2 commits into from
Feb 2, 2023

Conversation

MichalStrehovsky
Copy link
Member

Fixes dotnet/corert#1986 (yep, all the way to CoreRT repo).

We finally found an instance where this matters - in MAUI. Non-public methods never implement an interface by name+sig combo. We were getting the ProtectedDerived case in the newly added test wrong.

Cc @dotnet/ilc-contrib
Cc @ivanpovazan

Fixes dotnet/corert#1986 (yep, all the way to CoreRT repo).

We finally found an instance where this matters - in MAUI. Non-public methods never implement an interface by name+sig combo. We got the `ProtectedDerived` case in the newly added test wrong.
@MichalStrehovsky
Copy link
Member Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Thanks!

@MichalStrehovsky
Copy link
Member Author

I'll need to clean up the failures in the extra-platform runs. Looks like there's extra fallout from #80896.

There were many failures in the extra-platform run of that PR because those runs were broken by the previous dataflow changes (e.g. https://dev.azure.com/dnceng-public/public/_build/results?buildId=150101&view=logs&jobId=f50bdeb8-70f9-5fe0-0c12-4bd43ab59b51&j=f50bdeb8-70f9-5fe0-0c12-4bd43ab59b51&t=3941b02a-393b-543b-8813-9a3e5bea473a).

I did not see the failures that are showing up in this run in that log.

But now that I look at the previous log, it ends with:

    0 Warning(s)
    100 Error(s)

and that's a suspiciously round number. It's possible Helix is just giving up once it reaches 100 failures? That's annoying. We need to start running extra-platforms on all NativeAOT changes to prevent ending up in this state again.

@MichalStrehovsky
Copy link
Member Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@MichalStrehovsky MichalStrehovsky merged commit 60fa29b into dotnet:main Feb 2, 2023
@MichalStrehovsky MichalStrehovsky deleted the fix1986 branch February 2, 2023 05:27
@ghost ghost locked as resolved and limited conversation to collaborators Mar 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Accessibility not respected by the interface resolution algorithm
2 participants