IDataRecord.GetFieldType for arrays is not implementable with new DynamicallyAccessedMemberTypes contstraints

[idigra]

Describe the bug

Trying to migrate existing codebase to .NET 7 (rc1).
The codebase contains implementation of https://learn.microsoft.com/en-us/dotnet/api/system.data.idatarecord.getfieldtype?view=net-7.0#system-data-idatarecord-getfieldtype(system-int32) , which is now added DynamicallyAccessedMemberTypes constraints. One of the cell implementations of the data record is of a generic array type, which previously used simple typeof(T).MakeArrayType(). However MakeArrayType has no dynamic access constraints, thus I can't use it as a return value.

To Reproduce

Sample pseudo code:

    internal class Record : IDataRecord
    {
        private ICell[] columns;
        // ...
        [return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties | DynamicallyAccessedMemberTypes.PublicFields)]
        public Type GetFieldType(int i)
        {
            return this.columns[i].ColumnType;
        }
    }

    internal interface ICell
    {
        Type ColumnType
        {
            [return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties | DynamicallyAccessedMemberTypes.PublicFields)]
            get;
        }
    }

    internal class ArrayCell<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties | DynamicallyAccessedMemberTypes.PublicFields)] T>
    {
        Type ColumnType
        {
            [return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties | DynamicallyAccessedMemberTypes.PublicFields)]
            get => typeof(T).MakeArrayType(); // <-- IL2073 here
        }
    }

Exceptions (if any)

Compile error:
Error IL2073 'Microsoft.Data.DeltaLake.Types.ArrayCell.ColumnType.get' method return value does not satisfy 'DynamicallyAccessedMemberTypes.PublicFields', 'DynamicallyAccessedMemberTypes.PublicProperties' requirements. The return value of method 'System.Type.MakeArrayType()' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.

Further technical details

• Include the output of dotnet --info

    .NET SDK:
     Version:   7.0.100-rc.1.22431.12
     Commit:    f1cf61e1c0

    Runtime Environment:
     OS Name:     Windows
     OS Version:  10.0.19044
     OS Platform: Windows
     RID:         win10-x64
     Base Path:   C:\Program Files\dotnet\sdk\7.0.100-rc.1.22431.12\

    Host:
      Version:      7.0.0-rc.1.22426.10
      Architecture: x64
      Commit:       06aceb7015

    .NET SDKs installed:
      3.1.423 [C:\Program Files\dotnet\sdk]
      5.0.209 [C:\Program Files\dotnet\sdk]
      5.0.214 [C:\Program Files\dotnet\sdk]
      5.0.406 [C:\Program Files\dotnet\sdk]
      5.0.408 [C:\Program Files\dotnet\sdk]
      6.0.304 [C:\Program Files\dotnet\sdk]
      6.0.400 [C:\Program Files\dotnet\sdk]
      6.0.401 [C:\Program Files\dotnet\sdk]
      7.0.100-preview.7.22377.5 [C:\Program Files\dotnet\sdk]
      7.0.100-rc.1.22431.12 [C:\Program Files\dotnet\sdk]

    .NET runtimes installed:
      Microsoft.AspNetCore.App 3.1.23 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
      Microsoft.AspNetCore.App 3.1.29 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
      Microsoft.AspNetCore.App 5.0.15 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
      Microsoft.AspNetCore.App 5.0.17 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
      Microsoft.AspNetCore.App 6.0.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
      Microsoft.AspNetCore.App 6.0.9 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
      Microsoft.AspNetCore.App 7.0.0-preview.7.22376.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
      Microsoft.AspNetCore.App 7.0.0-rc.1.22427.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
      Microsoft.NETCore.App 3.1.23 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
      Microsoft.NETCore.App 3.1.29 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
      Microsoft.NETCore.App 5.0.15 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
      Microsoft.NETCore.App 5.0.17 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
      Microsoft.NETCore.App 6.0.8 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
      Microsoft.NETCore.App 6.0.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
      Microsoft.NETCore.App 7.0.0-preview.7.22375.6 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
      Microsoft.NETCore.App 7.0.0-rc.1.22426.10 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
      Microsoft.WindowsDesktop.App 3.1.23 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
      Microsoft.WindowsDesktop.App 3.1.29 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
      Microsoft.WindowsDesktop.App 5.0.15 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
      Microsoft.WindowsDesktop.App 5.0.17 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
      Microsoft.WindowsDesktop.App 6.0.8 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
      Microsoft.WindowsDesktop.App 6.0.9 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
      Microsoft.WindowsDesktop.App 7.0.0-preview.7.22377.1 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
      Microsoft.WindowsDesktop.App 7.0.0-rc.1.22427.1 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

    Other architectures found:
      x86   [C:\Program Files (x86)\dotnet]
        registered at [HKLM\SOFTWARE\dotnet\Setup\InstalledVersions\x86\InstallLocation]

    Environment variables:
      Not set

    global.json file:
      Not found

    Learn more:
      https://aka.ms/dotnet/info

    Download .NET:
      https://aka.ms/dotnet/download

• The IDE (VS / VS Code/ VS4Mac) you're running on, and its version

    17.4.0 Preview 2.0

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[marcpopMSFT]

Old issue triage: Looks like runtime calls so routing there.

[dotnet-policy-service]

Tagging subscribers to this area: @SamMonoRT, @dotnet/efteam
See info in area-owners.md if you want to be subscribed.

[roji]

Apologies this took so long to get answered.

I'm not quite sure why you're calling MakeArrayType() from within IDataRecord.GetFieldType(), what T represents in your code above and and how exactly the code connects together (partial snippets are always hard to follow).

But assuming you do need to return an array for elements of type T, instead of using MakeArrayType(), write out code that returns the correct array type for each element type supported by your provider (so if T is int, return int[]; if T is string, return string[]`, and so on). This indeed makes the code bigger, but allows the trimmer to know exactly which array types might be needed, and so need to be preserved.

Otherwise, if you don't wish to make your provider trimming-compatible at this time (or need a temporary workaround), you can suppress the warning here and mark the provider as trimming-unsafe, by adding [RequiresUnreferencedCode] on entry types to your provider (e.g. the DbConnection, DbFactory implementation).

As this issue has been open for very long, I'll go ahead and close it; but feel free to post back here if the issue is still relevant.

[idigra]

@roji I'm not sure that the mitigation suggestions are applicable for us.

• This issue is blocking us from reducing the size of library that is exported to other consumers. Today we avoid trimming exactly due to issues like this one, thus avoid trimming is not the solution.
• For the same reason, just adding RequiresUnreferencedCode in consumers' code wouldn't solve the issue. Once we trim, users code will fail, with or without this attribute.
• Limitting our code to closed list of types: I tried to look at the code, it's too complicated to say for sure that we don't have types propagated from outside. Anyway even if it's possible, it's going to be a big refactoring hence the request still stands.

About relevance: There's a long-standing request from consumers to reduce library size. This is not high-priority one, and it's not the only issue that needed to be mitigated, thus I haven't pressed all this time to solve. However this kind of requests sometimes increased in priority, and given the time it takes to solve and distribute the fix it will be nice if it can be solved as part of your regular feature request flow. I'll be grateful for reopening the issue.

[roji]

This issue is blocking us from reducing the size of library that is exported to other consumers. Today we avoid trimming exactly due to issues like this one, thus avoid trimming is not the solution.

I'm very confused by this statement... The purpose of trimming is precisely to allow unneeded code to get removed. If you want to allow your users to trim their application, then you need to make your application trimming-compatible, and that includes writing your code in a way that's friendly to the trimmer.

In any case, there's also the issue of NativeAOT; I'm not sure if you're aiming to support NativeAOT or just trimming (ideally your library would support both). MakeArrayType() is inherently incompatible with NativeAOT, as it has [RequiresDynamicCode].

To summarize, if you aim for your provider to be compatible with modern .NET application deployment (NativeAOT, trimming), you need to stop using certain methods/patterns in your code.

Limitting our code to closed list of types: I tried to look at the code, it's too complicated to say for sure that we don't have types propagated from outside.

What you're saying is that effectively it may be possible for a user to pass in their own arbitrary type, and for your implementation of IDataRecord.GetFieldType() to need to return an array of that arbitrary type? That is highly unusual, and I'd be interested in understanding more about that. In general, I'm not quite sure why your GetFieldType() needs to return an array - that method is generally supposed to return the simple element type column, and not an array over that; if the column itself contains arrays (i.e. each row has an array in that column), then I'd expect T itself to already represent that, again obviating the need for MakeArrayType. Could you provide a bit more context on that?

Anyway even if it's possible, it's going to be a big refactoring hence the request still stands.

Making existing libraries compatible with trimming/NativeAOT unfortunately frequently indeed requires non-trivial changes and refactorings. In any case, the [DAM] attribute on IDataRecord is there for a purpose - there's various code layered on top (in DbDataReader, thanks @vonzshik) that relies on it - so it cannot simply be removed. It might be possible to investigate removing it, but since MakeArrayType is in any case incompatible with NativeAOT, I'd highly recommend planning for getting rid of that call.

Reopening to look into possibly removing [DAM] from IDbRecord.GetFieldType (and DbDataReader), but I personally won't have time to look into this any time soon. If anyone wants to try, please do.