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.

Sign up for GitHub

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

Jump to bottom

[class] DIMs in the vtable of abstract classes may be overridden #82556

Merged
merged 5 commits into from
Feb 28, 2023
Merged

[class] DIMs in the vtable of abstract classes may be overridden #82556

merged 5 commits into from
Feb 28, 2023

Conversation

lambdageek
Copy link
Member

@lambdageek lambdageek commented Feb 23, 2023 •
edited
Loading

pretend the slot is empty in abstract classes and allow concrete subclasses classes to fill it in.

Related to #81882

@lambdageek
[class] Don't put abstract methods into the vtable
8d0f42f 
leave the slot NULL and allow leaf classes to fill it in.

Related to dotnet#81882
@ghost ghost assigned lambdageek Feb 23, 2023
@lambdageek

This comment was marked as resolved.

Sign in to view
@lambdageek lambdageek marked this pull request as ready for review February 23, 2023 21:29
@lambdageek lambdageek requested a review from BrzVlad February 23, 2023 21:30
@lambdageek lambdageek changed the title [class] Don't put abstract methods into the vtable [class] Don't put DIMs into the vtable of abstract classes Feb 23, 2023
@lambdageek
Copy link
Member Author

Failures are related

@lambdageek lambdageek changed the title [class] Don't put DIMs into the vtable of abstract classes [class] DIMs into the vtable of abstract classes may be overridden Feb 24, 2023
@lambdageek
Change approach: treat slot as empty if it's filled in with a DIM
f69766c 
Classes may override an interface method slot if it was filled in with
a DIM, just as if it was empty.
@lambdageek lambdageek changed the title [class] DIMs into the vtable of abstract classes may be overridden [class] DIMs in the vtable of abstract classes may be overridden Feb 24, 2023
@lambdageek
Copy link
Member Author

lambdageek commented Feb 24, 2023 •
edited
Loading

Changed the approach. actually putting NULL in the vtable broke Type.GetInterfaceMap(Type). Instead just treat the slot as empty if it currently has a DIM filling it.

@lambdageek lambdageek closed this Feb 24, 2023
@lambdageek lambdageek reopened this Feb 24, 2023
This was referenced Feb 24, 2023
Closed
Closed
@lambdageek lambdageek merged commit 4862343 into dotnet:main Feb 28, 2023
@lambdageek lambdageek mentioned this pull request Mar 2, 2023
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Mar 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@thaystg thaystg thaystg approved these changes

@vargaz vargaz Awaiting requested review from vargaz

@marek-safar marek-safar Awaiting requested review from marek-safar

@BrzVlad BrzVlad Awaiting requested review from BrzVlad

Assignees

@lambdageek lambdageek

Labels
area-VM-meta-mono
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lambdageek @thaystg