Linker doesn't preserve type forwarder for a nested type in a second type forwarder

[vitek-karas]

Repro:

app.csproj

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net6.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
    <TrimMode>copyused</TrimMode>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Azure.Identity" Version="1.5.0" />
    <PackageReference Include="Azure.ResourceManager.Resources" Version="1.0.0-beta.2" />
  </ItemGroup>
</Project>

app.cs

using Azure.ResourceManager;

typeof(ArmClientOptions).Assembly.GetTypes();

Publish the app with trimming enabled and run it. It fails with Could not load type 'Enumerator' from assembly 'Microsoft.Bcl.AsyncInterfaces, Version=1.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51'

The reason is that Azure.ResourceManager.dll has a type reference to a System.Runtime.CompilerServices.ConfiguredCancelableAsyncEnumerable``1/Enumerator in Microsoft.Bcl.AsyncInterfaces. This assembly has a type forwarder for this nested type which points to netstandard. And netstandard has a type forward to corelib.

After trimming only the type forwarder in Microsoft.Bcl.AsyncInterfaces is kept. The one in netstandard is removed. Note that the parent type is kept in both cases, it's only the nested type forwarder which is removed.

Also notes that this runs the linker in copyused mode.

[vitek-karas]

The actual problem is that setup like this, the project trims the assemblies as a mix of "copy" and "copyused" actions. This is bound to break. "copyused" makes the assumption that all type refs are rewritten to point to implementation assembly (as it doesn't preserve type forwarders by default and it doesn't mark them). On the other hand "copy" can't rewrite the type refs.

The workaround is to set TrimmerDefaultAction=copyused.

[marek-safar]

This looks to me like something worth servicing fix, thought?

[vitek-karas]

I don't think this is worth servicing. I honestly even question if we should fix it.

Going forward we don't want people to use "copyused". It's not the default anymore, the only way to get there is through TrimMode=copyused. So maybe what we should do is simply add a warning that you're using "copyused" and there are problems.

As for this specific issue - as far as I can tell, type forwarders were always problematic. It's just that .NET 6 behaves differently from .NET 5, but they're both wrong in certain cases.

I guess it's a discussion of importance - how important is it to make it work, given it's not the default and we generally don't encourage people to use it.

[marek-safar]

ok, I missed there is custom setting of

<TrimMode>copyused</TrimMode>

Should we start issuing a warning about this mode in .NET7 so we can remove it in .NET8?

[vitek-karas]

This is actually a bug for "link" mode as well.

The scenario is:

• "copy" assembly test which has a reference to Namespace.Type.Nested (nested type on Type) and Namespace.OtherType.Nested from CopyForwarder assembly.
• "copy" assembly CopyForwarder which has a type forwarder for Namespace.Type.Nested and Namespace.OtherType.Nested which points to LinkForwarder
• "link" assembly LinkForwarder which has a type forwarder for Namespace.Type.Nested which points to Implementation but also has a type forwarder for Namespace.OtherType.Nested which points to Implementation.
• "link" assembly Implementation with the right implementations for the type forwards.

Trimming this setup will actually remove one of the nested type forwarders in the LinkForwarder (it should keep both).
This is a combination of two problems:

• There's a bug in the linker in

  linker/src/linker/Linker.Steps/MarkStep.cs

  Line 1446 in c90ed0a

  markingHelpers.MarkForwardedScope (new TypeReference (exportedType.Namespace, exportedType.Name, assembly.MainModule, exportedType.Scope));

  . We create a type ref for a forwarder, but the way it's done it will only work for non-nested type forwarders. Nested type forwarders will end up with a TypeRef("", "Nested", module, assembly) with no DeclaringType. So they look like a type without a namespace.
• There's a bug in Cecil: Type references with empty namespace can resolve to nested type forwarders jbevain/cecil#812. Where such a type ref actually resolves to the first nested type forwarder with the right name. So linker doesn't fail, it just ends up marking potentially the wrong type forwarder.

The bug exists both in "copyused" and "link" modes.

[vitek-karas]

Test for this issue in the linker is #2372.
Test for the issue in Cecil is in vitek-karas/cecil@1d9104a.

Note that fixing the problem in linker doesn't depend on the Cecil fix. The Cecil bug just makes it so that the bug in the linker doesn't cause failures. If the linker creates a correct type ref with declaring type set, it will work even on current cecil correctly.

[vitek-karas]

The fix for this has been merged into the 6.0.2xx branch and should appear in 6.0.200 SDK soon.