eiriktsarpalis / PolyType

Practical generic programming for C#
https://eiriktsarpalis.github.io/PolyType/
MIT License
145 stars 6 forks source link

Support for netstandard2.0 #49

Open AArnott opened 1 day ago

AArnott commented 1 day ago

I have a branch where I'm exploring this now. Besides the "simple" breaks that just require #if to avoid undefined APIs, I see these high-level problems:

  1. Default interface implementations, such as IObjectTypeShape defining

    TypeShapeKind ITypeShape.Kind => TypeShapeKind.Object;

    This could presumably be resolved by making the default implementation conditional on .NET 8, such that folks supporting .NET Standard 2.0 will have to implement the member themselves. Annoying (for them) but not a big deal. And not a breaking change going from netstandard2.0 to .NET 8 either, I think.

  2. static abstract interface members. This is the IShapeable<T> elephant in the room.
eiriktsarpalis commented 1 day ago

It might make sense to enumerate the pain points we know such a backport will encounter. Starting with a few obvious ones:

AArnott commented 1 day ago

It's funny. You have exactly one #if NET8_0 already in your code... but the code around it doesn't compile outside .NET 8 anyway. 😜

eiriktsarpalis commented 1 day ago

If you look at the git history, the library used to support more targets and I was very close to completing a ns2.0 backport. The appeal of IShapeable<T> made me drop that effort altogether though..

AArnott commented 1 day ago

For the static abstract interface member...

Is there a way that is NativeAOT compliant for code (possibly reflection) to go from a type defined in some arbitrary assembly to another type in that same assembly by some well-known name, or possibly by an assembly attribute?

For example in MessagePack 3.0, we can dynamically discover source generated formatters by going from data type T to typeof(T).Assembly.GetCustomAttributes<SourceGeneratedResolverAttribute>() to find [assembly: SourceGeneratedResolver(typeof(TheResolver))]. Then we activate TheResolver from that assembly (using reflection) and cast it to an interface and ask it where the formatter for T is.

Does NativeAOT forbid reflecting over assembly-level attributes or invoking a type referenced by that attribute? If not, then maybe we can use that instead of IShapeable<T> for netstandard2.0.

eiriktsarpalis commented 1 day ago

Default interface implementations, such as IObjectTypeShape defining

These are largely superficial and can be replaced with a little bit of boilerplate in the implementing classes. FWIW converting the abstractions from interfaces to classes isn't entirely straightforward since the abstractions hierarchy contains diamond (if you factor in that every shape abstraction has a non-generic variant).

Does NativeAOT forbid reflecting over assembly-level attributes or invoking a type referenced by that attribute? If not, then maybe we can use that instead of IShapeable for netstandard2.0.

I don't believe that it does, the only concern is that attribute annotations can be trimmed. I'm not sure if assembly-level attributes do get trimmed though. @eerhardt might know.

The witness pattern already used in net8.0 might be more amenable to something that is supported in netstandard2.0. We could for example have an interface similar to IShapeable<T> that effectively does the same job without its method being static. Let's call it IShapeable2<T> for lack of a better name. As before you could source generate against a witness like so:

[GenerateShape<MyPoco>]
partial struct Witness;

Which augments Witness with an implementation of IShapeable2<T>. On the application layer you could then expose a consuming API like so:

public static string Serialize<T, TWitness>(T? value) where TWitness : IShapeable<T>, new()
{
     return Cache<T, TWitness>.Converter.Serialize(value);
}

private static class Cache<T, TWitness> where TWitness : IShapeable<T>, new()
{
    public MyConverter<T> Converter = CreateConverterFromTypeShape(new TWitness().GetShape());
}
eiriktsarpalis commented 1 day ago

Another related, but technically independent concern is minimum language version supported by the source generator. Officially, ns2.0 isn't supported in language versions going beyond 7.x but in practice the latest versions do support it. Still, it makes sense to require a minimum version and emit a diagnostic if it isn't met. We need C# 9 at least to support init properties, but I think we can afford to be more assertive and require C# 12 as the minimum (so that we get nice things like collection expression support).

eerhardt commented 1 day ago

I don't believe that it does, the only concern is that attribute annotations can be trimmed. I'm not sure if assembly-level attributes do get trimmed though. @eerhardt might know.

In general, attributes don't get trimmed. So if you can get a member in a trim-compatible way, you can get the attributes off it. If you have a Type, you can get its Assembly and get the attributes off it. The warnings will guide you on what is incompatible.

If the attribute references another Type, you can annotate the attribute to preserve the constructors of that Type so you can create new instances in a trim-compatible way. For example:

https://github.com/dotnet/runtime/blob/524d217d52780cf9b0bce33a91e8d7266447d5bb/src/libraries/System.ObjectModel/src/System/ComponentModel/TypeConverterAttribute.cs#L35-L59

AArnott commented 1 day ago

We could for example have an interface similar to IShapeable<T> that effectively does the same job without its method being static.

Yes, that's clever. And it pretty much preserves the user exposed API... for the 2 arity methods anyway. For the 1 arity methods, things break down because the 'witness' class is the data type itself, which now must carry a default constructor and be created ... to learn about its shape and how to properly create it. So that kinda forces us into a world where types may not generally want to be their own witnesses.

The assembly attribute approach can move all witnesses onto one type in the assembly, thus potentially avoiding the need for 2-arity generic methods entirely. And it would work on any runtime. But you lose the compile-time type safety of limiting input types to your methods to only those for which a shape is available.
But we could solve that by applying an empty interface to the annotated type and test for that interface with a generic type constraint.

I think we can afford to be more assertive and require C# 12 as the minimum

I'm totally good with that. For MessagePack-C# source generation we require C# 9 IIRC. And CsWin32 required C# 9+. I fully support requiring newer language versions on older targets. Help give users reasons to use a better language version, all the way. :)

AArnott commented 1 day ago

@eerhardt as trimming advances across runtime versions, what are the odds that things that previously were not trimmed gets trimmed in the future, possibly breaking an API design that depends on those things not being trimmed?

eiriktsarpalis commented 1 day ago

We could for example have an interface similar to IShapeable<T> that effectively does the same job without its method being static.

Yes, that's clever. And it pretty much preserves the user exposed API... for the 2 arity methods anyway. For the 1 arity methods, things break down because the 'witness' class is the data type itself, which now must carry a default constructor and be created ... to learn about its shape and how to properly create it. So that kinda forces us into a world where types may not generally want to be their own witnesses.

Would it be a workable compromise for framework applications though? The default constructor could be filled in by the source generator if not already present.

An alternative approach is that we forsake type safety altogether in older targets and expose methods like the following:

[RequiresUnreferencedCode, RequiresDynamicCode]
public static string Serialize<T>(T? value) => Serialize(value, ReflectionTypeShapeProvider.Default);
public static string Serialize<T>(T? value, ITypeShapeProvider provider)
{
    // caches converters using a ConditionalWeakTable keyed on the ITypeShapeProvider
    // throws an exception if the provider doesn't contain a shape for T
}

#if NET8_0_OR_GREATER
// Usual APIs based on static abstracts
public static string Serialize<T>(T? value) where T : IShapeable<T>
public static string Serialize<T, TWitness>(T? value) where TWitness : IShapeable<T>
#endif
eerhardt commented 1 day ago

@eerhardt as trimming advances across runtime versions, what are the odds that things that previously were not trimmed gets trimmed in the future, possibly breaking an API design that depends on those things not being trimmed?

The possibility is low, but non-zero. If this were to happen, it would be an "extra opt-in" case, it wouldn't be the default behavior (excluding places that are considered "bugs"). For example, https://github.com/dotnet/runtime/blob/main/src/coreclr/nativeaot/docs/reflection-free-mode.md is a mode where no reflection is allowed at all.

However, the principle here is that the trimmer must warn you when something like this happens. If you are using an API that is no longer guaranteed to work, you will get a warning alerting you to it. For the core reflection APIs, they have been pretty well established and we haven't been changing this behavior lately. Check out https://github.com/dotnet/runtime/issues/103934 for more discussion on trimming attributes.

AArnott commented 1 day ago

Would it be a workable compromise for framework applications though?

I don't know... requiring two explicit type args and the concept of a witness class to every single use on Framework seems quite unfortunate.

The default constructor could be filled in by the source generator if not already present.

I'm pretty against adding a default constructor to a user type. First of all, that could often fail because the user type contains non-nullable ref type fields/properties that a default constructor would not initialize.

An alternative approach is that we forsake type safety altogether in older targets and expose methods like the following:

I think offering a set of APIs like that, with some APIs only available on .NET 8, is perfectly feasible. But I hope we can find a good solution that avoids reflection costs on Framework.

The assembly-level attribute idea + an empty interface sounds like it should work on every runtime and provide type safety, with only one type argument everywhere (except where witnesses must be provided because the data type and shape are in different assemblies).

eiriktsarpalis commented 1 day ago

But I hope we can find a good solution that avoids reflection costs on Framework.

What reflection costs do you have in mind?

AArnott commented 1 day ago

Those incurred by ReflectionTypeShapeProvider.Default.

Minimal reflection to look at assembly-level attributes, and only once per assembly, doesn't seem too bad.

eiriktsarpalis commented 1 day ago

It should still be possible to pass an ITypeShapeProvider that is source generated though, right?

AArnott commented 1 day ago

Sure. I don't object to a method that takes a shape provider parameter. I would just like to offer an easier method to call by default for both Framework and .NET.