Skip to content

Conversation

@lambdageek
Copy link
Member

When there are covariant return overrides, we check the vtable slot in every parent class for signature compatibility with the proposed override.

However if one of the ancestor classes is abstract, it could have an abstract override for the method:

    class Base {
      public virtual Base Method() => this;
    }
    public abstract Intermediate : Base {
      public override abstract Base Method();
    }
    public Leaf : Intermediate {
      public override Leaf Method() => this;
    }

In this case when we're checking that Leaf.Method is compatible with the vtable slot in Intermediate, we will see a null method (since Intermediate.Method is abstract). In such cases we can skip over the class and continue with its parent.

Fixes
#76312

When there are covariant return overrides, we check the vtable slot in every
parent class for signature compatability with the proposed override.

However if one of the ancestor classes is abstract, it could have an
abstract override for hte method:

    class Base {
      public virtual Base Method() => this;
    }
    public abstract Intermediate : Base {
      public override abstract Base Method();
    }
    public Leaf : Intermediate {
      public override Leaf Method() => this;
    }

In this case when we're checking that Leaf.Method is compatible with
the vtable slot in Intermediate, we will see a null method (since
Intermediate is abstract).  In such cases we can skip over the class
and continue with its parent.

Fixes
dotnet#76312
@ghost ghost added the area-VM-meta-mono label Sep 28, 2022
@ghost ghost assigned lambdageek Sep 28, 2022
@lambdageek
Copy link
Member Author

I think we should backport this for net7 and net6

@lambdageek
Copy link
Member Author

/azp run runtime-extra-platforms

@lambdageek
Copy link
Member Author

/backport to release/7.0

@github-actions
Copy link
Contributor

Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/3146190259

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@lambdageek
Copy link
Member Author

/backport to release/6.0

@github-actions
Copy link
Contributor

Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/3146255970

@lambdageek
Copy link
Member Author

runtime-extra-platforms (Build tvOS arm64 Release AllSubsets_Mono) is failing repeatedly while trying to set up a tunnel to the device:

[10:39:13] dbug: [TCP tunnel] Xamarin.Hosting: Attempting USB tunnel between the port 61033 on the device and the port 61033 (27118) on the mac: 61

I think it's an infra problem, not my PR...

@lambdageek lambdageek merged commit 90d9034 into dotnet:main Sep 30, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Oct 31, 2022
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.

2 participants