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

Assertion failed: The class hierarchy should declare the method #103268

Closed
MichalStrehovsky opened this issue Jun 11, 2024 · 2 comments · Fixed by #105266
Closed

Assertion failed: The class hierarchy should declare the method #103268

MichalStrehovsky opened this issue Jun 11, 2024 · 2 comments · Fixed by #105266
Labels
area-System.Reflection disabled-test The test is disabled in source code against the issue
Milestone

Comments

@MichalStrehovsky
Copy link
Member

This is just an assert guarding some invariant that the reflection stack relies on. The test "works" with a release build of the runtime, but whatever invariant it's guarding obviously doesn't hold.

Ran into this while writing tests for #103220, going to disable newly added tests on this.

Process terminated. Assertion failed.
The class hierarchy should declare the method
   at System.Delegate.GetMethodImpl()
   at System.MulticastDelegate.GetMethodImpl()
   at Program.<Main>$(String[] args)

Hit for the following program:

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Reflection;
using System.Runtime.CompilerServices;
using Internal;

foreach (var o in Create_ClosedDelegate_TestData())
    Console.WriteLine(((Delegate)o[0]).Method.ToString());


static IEnumerable<object[]> Create_ClosedDelegate_TestData()
{
    const string TestNamespace = nameof(System);

    IGenericInterfaceForDiagnosticMethodInfoTests<object> og = new GenericClassForDiagnosticMethodInfoTests<object>();

        yield return new object[] {
                (Action)og.NonGenericDefaultMethod,
                nameof(IGenericInterfaceForDiagnosticMethodInfoTests<object>.NonGenericDefaultMethod) ,
                TestNamespace + nameof(IGenericInterfaceForDiagnosticMethodInfoTests<object>) + "`1"
                };

    yield return new object[] {
                (Action)og.GenericDefaultMethod<object>,
                nameof(IGenericInterfaceForDiagnosticMethodInfoTests<object>.GenericDefaultMethod),
                TestNamespace + nameof(IGenericInterfaceForDiagnosticMethodInfoTests<object>) + "`1"
            };
}

interface IGenericInterfaceForDiagnosticMethodInfoTests<T>
{
    void NonGenericMethod();
    void GenericMethod<U>();

    void NonGenericDefaultMethod() { }
    void GenericDefaultMethod<U>() { }
}

class GenericClassForDiagnosticMethodInfoTests<T> : IGenericInterfaceForDiagnosticMethodInfoTests<T>
{
    void IGenericInterfaceForDiagnosticMethodInfoTests<T>.GenericMethod<U>() => throw new NotImplementedException();
    void IGenericInterfaceForDiagnosticMethodInfoTests<T>.NonGenericMethod() => throw new NotImplementedException();
    public void Method1() { }
    public void Method2() { }
}
@MichalStrehovsky MichalStrehovsky added area-System.Reflection disabled-test The test is disabled in source code against the issue labels Jun 11, 2024
@ericstj
Copy link
Member

ericstj commented Jul 18, 2024

Assertion is here:

// RCWs don't need to be "strongly-typed" in which case we don't find a base type
// that matches the declaring type of the method. This is fine because interop needs
// to work with exact methods anyway so declaringType is never shared at this point.
Debug.Assert(currentType != null || _target.GetType().IsCOMObject, "The class hierarchy should declare the method");

Seems to me like this assertion doesn't handle DIMs. Should it just be updated to account to allow DIMs @AaronRobinsonMSFT @jkotas?

@ericstj ericstj added this to the 10.0.0 milestone Jul 18, 2024
@AaronRobinsonMSFT
Copy link
Member

@ericstj Yes, this is a DIM issue. A naive fix would be to update the assert to check if the targetType is an interface. I'm not sure we have a better explicit check for a DIM. I can submit a PR if everyone is okay with this.

Debug.Assert(
    currentType != null
    || _target.GetType().IsCOMObject
    || targetType.IsInterface, "The class hierarchy should declare the method");

@AaronRobinsonMSFT AaronRobinsonMSFT modified the milestones: 10.0.0, 9.0.0 Jul 22, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Aug 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Reflection disabled-test The test is disabled in source code against the issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants