Skip to content

Conversation

bholmes
Copy link
Contributor

@bholmes bholmes commented May 10, 2021

Addressing 4 issues for GetInterfaceMap and default interface methods

  • Only methods marked as virtual on on interface should be added to the
    interface map. (no static or instance)
  • If the found target method is ambiguous (a diamond) the target is
    null.
  • If the found target method's class in an interface, then the target
    class is the interface class, else it is the class of the RuntimeType
    (aka this)
  • If the found target method is abstract (reabstraction) then the
    target is null.

Fixes #34389

Addressing 4 issues for GetInterfaceMap and default interface methods
 - Only methods marked as virtual on on interface should be added to the
   interface map. (no static or instance)
 - If the found target method is ambiguous (a diamond) the target is
   null.
 - If the found target method's class in an interface, then the target
   class is the interface class, else it is the class of the RuntimeType
   (aka this)
 - If the found target method is abstract (reabstraction) then the
   target is null.

Fixes dotnet#34389
@ghost ghost added the area-VM-reflection-mono Reflection issues specific to MonoVM label May 10, 2021
@ghost
Copy link

ghost commented May 10, 2021

Tagging subscribers to this area: @CoffeeFlux
See info in area-owners.md if you want to be subscribed.

Issue Details

Addressing 4 issues for GetInterfaceMap and default interface methods

  • Only methods marked as virtual on on interface should be added to the
    interface map. (no static or instance)
  • If the found target method is ambiguous (a diamond) the target is
    null.
  • If the found target method's class in an interface, then the target
    class is the interface class, else it is the class of the RuntimeType
    (aka this)
  • If the found target method is abstract (reabstraction) then the
    target is null.

Fixes #34389

Author: bholmes
Assignees: -
Labels:

area-System.Reflection-mono

Milestone: -

@bholmes
Copy link
Contributor Author

bholmes commented May 10, 2021

I would also like to see this in the mono/mono repo as well. AFAICT the only difference is some calls require the domain argument.

@marek-safar
Copy link
Contributor

@lambdageek please review

@lambdageek
Copy link
Member

/azp run runtime

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ghost
Copy link

ghost commented Jun 2, 2021

Hello @lambdageek!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit 5d64814 into dotnet:main Jun 3, 2021
@lambdageek
Copy link
Member

/azp run sync-runtime-to-mono

@ghost ghost locked as resolved and limited conversation to collaborators Jul 3, 2021
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-VM-reflection-mono Reflection issues specific to MonoVM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CoreCLR tests on Mono runtime : crash running reflection/DefaultInterfaceMethods/GetInterfaceMapConsumer/GetInterfaceMapConsumer.sh
3 participants