◐ Shell
clean mode source ↗

CLR enum underlying type mapping by NinoFloris · Pull Request #6581 · npgsql/npgsql

Solves half of #3303 and fixes #5591.

This brings NativeAOT compatible *unmapped* CLR enum support for reads and writes. Once merged we begin to map (by default, inferred) to the closest pg type based on the enum's underlying type:

  • int, uint => int4
  • long, ulong => int8
  • byte, sbyte, short, ushort => int2

We don't support any primitive conversions like we have for raw primitives. We can leave those until people actually ask for them. This means if users want to map an int enum into an int8 (and so on) that we do not yet support this.

Arrays are supported but not fully, see: dotnet/runtime#127249. We could consider adding these mappings on CoreCLR only. So far I've kept the functionality symmetrical, as we try to do with most mappings.

A lot of the recent work was required to land this feature:

  • Cleaning up the provider/concrete split that proliferated 'AsObject' logic all over the codebase.
  • Getting all non-generic converter dispatching to use the centralized PgConverter.Read (and so on) methods.
  • Rationalizing all PgTypeInfo variance logic to be able to confidently add the enum <-> underlying exemptions.

This PR just really threads the needle through CLR and NativeAOT quirks. The enum <-> underlying "reduced-type equivalence" is one big, weird, exception in the type system: array covariance carries it through, though generic instantiation stays invariant. Given our infra is generic it requires full manual mapping to faithfully resurface the relationship, rather than simply relying on the (allocating) boxing/unboxing helpers to do it for us.

Once past the IL stage NativeAOT brings its own problems. Its aggressive trimming does not carve out any real exceptions for these historical equivalences beyond the absolute simplest cases (nevertheless working with enums through their underlying type is the recommended way). Then just as I was rounding off the work I noticed a classic bloat issue, one that has surfaced a few times before in our NativeAOT journey. In short, there are fundamental limitations in the current IL scanner, and we are still waiting for a more capable one (dotnet/runtime#83021).

More concretely: we cannot make unmapped enum reading and writing *allocation free* on NativeAOT without incurring massive code bloat. The essential issue is that the scanner cannot clean up code blocks that were found dead or under vacuous conditionals after scanning completed. This applies to typeof(T).IsX branches, isinst branches over types that didn't get instantiated, and so forth. All of that knowledge requires the whole program view the scanner itself is building (and potentially the generic specialization view that codegen has) which all happens after it has decided the methods and types must be kept.

If we do not fortify our typeof(T).IsEnum check with RuntimeFeature.IsDynamicCodeSupported we would root all that enum dispatching logic for every T that flows through, and as they are dispatching stubs, that's basically everything. We can't use any of our usual tricks here - like putting the code behind virtual dispatch - because there are no actual rooted instantations over the user's TEnum. The entire point was that we didn't need to know about those types in Npgsql. Finally, on the more exotic end there is no 'unsafe but guaranteed' equivalence of virtual table layout between T = enum and its underlying T = int either. Which means we cannot 'safely' do Unsafe.As<PgConverter<IntEnum>(PgConverter<int>) and be guaranteed their virtual slots line up such that we end up in the virtual method we intended to call. As a result NativeAOT support is a bit worse than I would have wanted, but it sits on the edge of what's reasonably possible.

Suffice to say this ended up being an even more hilariously painful feature to land than I initially assumed, but it's type safe, trim safe, and complete with as much compositional support as the runtime provides guarantees for.