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

Switch to explicit implementation for redeclared events #618

Merged
merged 5 commits into from
May 14, 2022
Merged

Switch to explicit implementation for redeclared events #618

merged 5 commits into from
May 14, 2022

Commits on May 12, 2022

  1. Redeclaring an event can produce invalid metadata 

    The test being added here fails PEVerify with "Event has a duplicate".
    @stakx
    stakx committed May 12, 2022
    Configuration menu
    Copy the full SHA
    f04a993 View commit details
    Browse the repository at this point in the history

  2. Switch to explicit impl for redeclared events 

    The failing test from the previous commit causes the following metadata
to be emitted:

    .property instance class [mscorlib]System.Action`1<bool> Property()
    {
        .get instance class [mscorlib]System.Action`1<bool>
                         Castle.Proxies.IDerivedProxy::get_Property()
    }

    .property instance class [mscorlib]System.Action Property()
    {
        .get instance class [mscorlib]System.Action
                         Castle.Proxies.IDerivedProxy::get_Property()
    }

Obviously we need to switch to explicit implementation for one of these
events in order to avoid a name collision. We do that by changing the
"collision detection" such that it does not take into account events'
return types; only their name matters.
    @stakx
    stakx committed May 12, 2022
    Configuration menu
    Copy the full SHA
    b56a43f View commit details
    Browse the repository at this point in the history

  3. Same issue might exist for redeclared properties 

    Since events and properties have very similar metadata, the same issue
we just solved for events might be present for properties; let's test
this suspicion.
    @stakx
    stakx committed May 12, 2022
    Configuration menu
    Copy the full SHA
    5c80d1d View commit details
    Browse the repository at this point in the history

  4. PEVerify detects no duplicate for redeclared props 

    This might be a PEVerify bug.

Given that the C# compiler would force us to implement either the base
or derived property explicitly (in a class implementing `IDerived`),
let's do the same in DynamicProxy. Use the same approach as for events.
    @stakx
    stakx committed May 12, 2022
    Configuration menu
    Copy the full SHA
    1f125cd View commit details
    Browse the repository at this point in the history

  5. Update the changelog

    @stakx
    stakx committed May 12, 2022
    Configuration menu
    Copy the full SHA
    142e028 View commit details
    Browse the repository at this point in the history