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

Add support for covariant returns #619

Merged
merged 8 commits into from
Jul 19, 2022
Merged

Add support for covariant returns #619

merged 8 commits into from
Jul 19, 2022

Commits on May 14, 2022

  1. Add passing test for proxying empty record type

    @stakx
    stakx committed May 14, 2022
    Configuration menu
    Copy the full SHA
    9a84a79 View commit details
    Browse the repository at this point in the history

  2. Add failing test for proxying derived record type 

    On .NET 6+ this fails with a `TypeLoadException`:

> Return type in method `DerivedEmptyRecord.<Clone>$()` [...] is not
> compatible with base type method `EmptyRecord.<Clone>$()`

From dotnet/efcore#26602 (comment):

> The C# compiler changed the emit strategy for records in .NET 6 to
> take advantage of covariant returns. The Clone method now always has a
> return type that matches the containing type. Covariant returns were
> added to the runtime and language in .NET 5 but records didn't take
> advantage of them (just ran out of time). In .NET 6 though we finished
> off that feature and added it to records.

So we'll need to add support for covariant returns to DynamicProxy.
    @stakx
    stakx committed May 14, 2022
    Configuration menu
    Copy the full SHA
    2cf79ee View commit details
    Browse the repository at this point in the history

  3. Minimal bugfix: relax return type equality check 

    The previous commit's test fails due to DynamicProxy treating the base
and derived record classes' `<Clone>$` methods as two distinct methods
(because they differ in their exact return type), instead of as a single
overridden method. We can change this by adjusting `MethodSignature-
Comparer` such that it also accepts assignment-compatible return types.
    @stakx
    stakx committed May 14, 2022
    Configuration menu
    Copy the full SHA
    d27ef43 View commit details
    Browse the repository at this point in the history

  4. Check for PreserveBaseOverridesAttribute 

    Covariant returns were introduced with .NET 5. For all earlier runtimes,
the last commit may have relaxed method signature comparison too much.

We can resolve this by checking for a specific custom attribute that
.NET compilers are expected to put on override methods using covariant
returns.

Reference:
https://github.com/dotnet/runtime/blob/main/docs/design/features/covariant-return-methods.md
    @stakx
    stakx committed May 14, 2022
    Configuration menu
    Copy the full SHA
    4bc7ba2 View commit details
    Browse the repository at this point in the history

  5. MethodSignatureComparer.Equals should be commutative 

    So far, we simply assumed that `x` := override method, and `y` := over-
ridden method, but there is actually no guarantee for that.
    @stakx
    stakx committed May 14, 2022
    Configuration menu
    Copy the full SHA
    9ed7fa3 View commit details
    Browse the repository at this point in the history

  6. Extract method MethodSignatureComparer.EqualReturnTypes

    @stakx
    stakx committed May 14, 2022
    Configuration menu
    Copy the full SHA
    36c608a View commit details
    Browse the repository at this point in the history

  7. Test covariant returns a little more thoroughly

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

  8. Update changelog

    @stakx
    stakx committed May 14, 2022
    Configuration menu
    Copy the full SHA
    01c2c6a View commit details
    Browse the repository at this point in the history