Always show IL2026 for IsTrimmable assemblies

[sbomer]

This lets RUC warnings intentionally left in IsTrimmable-attributed assemblies survive collapsing, so that the user-readable message is shown.

This is a workaround for the lack of a more well-defined model for "global" warnings (such as RUC methods behind a disabled feature switch, or RUC code that's part of an override which may be called virtually). In the framework libraries we took the approach of moving RUC to helper methods that always have a direct callsite in managed code, and we gave the attribute a message indicating the global problem. These warnings will no longer be collapsed, but they still have a message and origin mentioning a call from non-RUC to RUC inside of the library.

Addresses part of #2004.

[vitek-karas]

Looks good but please try to run the linker on an app with for example enabled startup hook, to see if it actually works the way we think it should.

I assume the followup is to remove the detailed message origin and maybe only report an assembly, or maybe no origin if collapsing is turned on for the affected assembly. We will also probably have to change the warning message in some way.

[mateoatr]

LGTM. Should we use an out MessageContainer param in TryLogSingleWarning instead of going through LogWarning a second time?

[sbomer]

I assume the followup is to remove the detailed message origin and maybe only report an assembly, or maybe no origin if collapsing is turned on for the affected assembly.

@vitek-karas I wasn't planning on doing this - the warning is still an IL2026 which has a documented message format, and that message doesn't make much sense without an origin - unless we wanted to give it a different warning code? Then I'm not sure what would be a good message/origin. The linker doesn't really have any more context except that the RUC method was called.

Only printing the RUC user message without an origin might be a bit mysterious as to where the warning is coming from. This feels like it would be better handled by specialized warning codes for different circumstances along the lines we've been discussing.

cc @eerhardt in case you have opinions on this.

[sbomer]

Should we use an out MessageContainer param in TryLogSingleWarning instead of going through LogWarning a second time?

I think I'd want it to go through LogWarning again because it's kind of a separate warning. Any of the usual warning filtering should be applied - for example if there were a way to suppress assembly-level warnings, it should be possible to suppress IL2104.

[sbomer]

please try to run the linker on an app with for example enabled startup hook, to see if it actually works the way we think it should.

before:

sven@localhost StartupHook]$ dotnet publish -r linux-x64 -p:PublishTrimmed=true -p:StartupHookSupport=true
Microsoft (R) Build Engine version 17.0.0-preview-21304-01+a5ea6d2ca for .NET
Copyright (C) Microsoft Corporation. All rights reserved.

  Determining projects to restore...
  Restored /home/sven/tmp/StartupHook/StartupHook.csproj (in 209 ms).
  You are using a preview version of .NET. See: https://aka.ms/dotnet-core-preview
  StartupHook -> /home/sven/tmp/StartupHook/bin/Debug/net6.0/linux-x64/StartupHook.dll
/home/sven/.nuget/packages/microsoft.netcore.app.runtime.linux-x64/6.0.0-preview.6.21306.1/runtimes/linux-x64/lib/net6.0/System.Private.CoreLib.dll : warning IL2104: Assembly 'System.Private.CoreLib' produced trim warnings. For more information see https://aka.ms/dotnet-illink/libraries [/home/sven/tmp/StartupHook/StartupHook.csproj]
  Optimizing assemblies for size, which may change the behavior of the app. Be sure to test after publishing. See: https://aka.ms/dotnet-illink
  StartupHook -> /home/sven/tmp/StartupHook/bin/Debug/net6.0/linux-x64/publish/

after:

[sven@localhost StartupHook]$ dotnet publish -r linux-x64 -p:PublishTrimmed=true -p:StartupHookSupport=true -p:ILLinkTasksAssembly=/home/sven/linker/artifacts/bin/ILLink.Tasks/Debug/net5.0/ILLink.Tasks.dll
Microsoft (R) Build Engine version 17.0.0-preview-21304-01+a5ea6d2ca for .NET
Copyright (C) Microsoft Corporation. All rights reserved.

  Determining projects to restore...
  Restored /home/sven/tmp/StartupHook/StartupHook.csproj (in 222 ms).
  You are using a preview version of .NET. See: https://aka.ms/dotnet-core-preview
  StartupHook -> /home/sven/tmp/StartupHook/bin/Debug/net6.0/linux-x64/StartupHook.dll
ILLink : Trim analysis warning IL2026: System.StartupHookProvider.ProcessStartupHooks(): Using method 'System.StartupHookProvider.CallStartupHook(StartupHookProvider.StartupHookNameOrPath)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. The StartupHookSupport feature switch has been enabled for this app which is being trimmed. Startup hook code is not observable by the trimmer and so required assemblies, types and members may be removed. [/home/sven/tmp/StartupHook/StartupHook.csproj]
  Optimizing assemblies for size, which may change the behavior of the app. Be sure to test after publishing. See: https://aka.ms/dotnet-illink
  StartupHook -> /home/sven/tmp/StartupHook/bin/Debug/net6.0/linux-x64/publish/

[eerhardt]

cc @eerhardt in case you have opinions on this.

I don't think I have hugely strong opinions on it. On one hand I do like it says

System.StartupHookProvider.ProcessStartupHooks(): Using method 'System.StartupHookProvider.CallStartupHook(StartupHookProvider.StartupHookNameOrPath)....,

so a savvy developer could go looking into our code for more information about this (comments in the code, git blame, linked issues and PRs, etc).

I almost think the only real piece of that message that doesn't provide value in this case is which has 'RequiresUnreferencedCodeAttribute' . Beyond those 3 words, I think the message reads pretty well.

[vitek-karas]

Definitely a much better experience. And I would be perfectly OK shipping it this way.

One day I think we should improve on this:

• For "normal" developer the warning source is "magic" - either nothing at all, or just CoreLib would probably be better
• The "technical" part might be interesting but only for people who are willing to dig into it, which is probably not very common.

Ideally I think the warning should be basically:

ILLink : Trim analysis warning IL2026: The StartupHookSupport feature switch has been enabled for this app which is being trimmed. Startup hook code is not observable by the trimmer and so required assemblies, types and members may be removed. http://dot.net/illink/startuphook.

That is for cases where we collapse warnings. If I disable warning collapse, I should get the warning in the current form with the detailed technical description and true origin.