search

dotnet / winforms

Windows Forms is a .NET UI framework for building Windows desktop applications.
MIT License
4.35k stars 965 forks source link

Epic - Make WinForms trim compatible #4649

Open agocke opened 3 years ago

agocke commented 3 years ago

The TrimTest project tracks a simple WinForms application for trimming purposes. This project generates 2535 trim warnings - Default_Trimmed_Warnings.txt.

Below issues track the work related to getting the warnings addressed.

Linked to this epic

RussKie commented 3 years ago

From an offline chat with @agocke:

(These) warnings effectively represent reflection that the toolchain can't figure out. In most cases the warning is probably about a use of a particular reflection method and will tell you that the System.Type you're operating on isn't marked with the same requirements as the reflection method requires. After finding the source of the System.Type you have two options:

  1. The type is statically known, like typeof or even a string literal with the full name
  2. The type is retrieved dynamically, like from an arbitrary object.GetType() or from a name from the user

In case (1), this is likely just an annotation problem. You need to use the DynamicallyAccessedMembers attribute to walk back through the code path, and mark the type as requiring the specific capabilities. When you get back to the original typeof call or whatever, the toolchain will be able to figure out what it needs to preserve on the type.

In case (2), this is fundamentally not linker safe. There's no guarantee that the thing you're reflecting against won't be trimmed, and the member won't be gone at runtime. For that it's usually recommended that you use RequiresUnreferencedCodeAttribute on the method. It silences the method warnings, but produces a warning for anyone calling that method. You can keep doing this until you bubble it up to the public APIs, where users will get a warning that the method is not linker safe.

Useful docs:

weltkante commented 3 years ago

I find no mention of COM interop in the links, the warnings about COM interop not being supported are worrying (IL2050: xxx declares a parameter with COM marshalling. Correctness of COM interop cannot be guaranteed after trimming. Interfaces and interface members might be removed.) since this is fundamental functionality to communicate with Windows (WinForms without clipboard, drag'n'drop, RichEdit, accessibility etc. would not be the same)

Furthermore the text mentions trimming of COM interfaces yet only reports P/Invoke signatures. That has me worried that ComImport is not recognized by the linker and also gets broken (otherwise the comment about trimming interface members makes not much sense). Trimming members of ComImport interfaces is definitiely not allowed (changes method order which is fatal for IUnknown based interfaces) and I'd expect this to be easily recognizable (just check the attribute) so I'm confused what the warning means in the first place.

I really hope these are just false positives and COM interop is supported, or at least can be supported by custom annotations, otherwise I don't think WinForms can be made compatible.

merriemcgaw commented 2 years ago

Adding this to our dotnet/planning/issues/27 theme

kant2002 commented 2 years ago

I did try trim recent version of WinForms and what are the log which I receive 640 rows. with_design.txt

Most issues which I see is from Design namespace which for most application does not make sense. Also I do not think that in practice that would make sense to target that namespace without additional work on Designer which happens in private.

So I remove from log these warnings and receive smaller file of 260 rows. without_design.txt

I look again, and next target for questionable warnings would be Converters which is mostly used during design-time. Also DefaultValueAttribute somewhat irrelevant for "trimming-app".

So I would like to ask questions

RussKie commented 2 years ago

Most issues which I see is from Design namespace which for most application does not make sense. Also, I do not think that in practice that would make sense to target that namespace without additional work on Designer which happens in private.

The designer codebase has its own copies of several runtime types (for various reasons). Most of the types that are present in this codebase can and are used in runtime scenarios (see https://github.com/dotnet/winforms/issues?q=label%3A%22area%3A+designer+support%22+sort%3Acreated-desc), e.g., editors and type converters are invoked via the PropertyGrid control. Some other types from the "designer" namespace can be used in general-purpose designers, such as a report viewer control (see https://github.com/dotnet/winforms/issues/4908 for more info).

RussKie commented 2 years ago
  • Can whole issue be splitted in something more manageable. For example I see "design" "typeconverters" "binding", "resources" areas which can be worked separately. Please correct me if I wrong here. Since this issue looks so big, nobody want to bother, but if split issue on something smaller, it would be easier to handle.

Right now this epic isn't high on the team's backlog, and you're paving the road :) (and we are very appreciative of your work, thank you). Which means you can split and dice it any way you wish.

with_design.txt contains only the following warnings: IL2050, IL2094, IL2093, IL2092, IL2046, IL2026, IL2072, IL2075, IL2067, IL2057, IL2062, IL2070, IL2111, IL2087. without_design.txt contains all the same list without IL2087. Maybe the work can be split by addressing each warning type?

kant2002 commented 2 years ago

with_design.txt contains only the following warnings: IL2050, IL2094, IL2093, IL2092, IL2046, IL2026, IL2072, IL2075, IL2067, IL2057, IL2062, IL2070, IL2111, IL2087. without_design.txt contains all the same list without IL2087.

That's interesting observation. I did not notice that.

Maybe the work can be split by addressing each warning type?

I would like (for now) discuss what's in principle should be done. Kind of meta-discussion. I would like to get rid of undocumented _SuppressWinFormsTrimError property introduced in https://github.com/dotnet/sdk/pull/19409/files . That cannot happens until team decide that earlier issues no longer a problem. I have only vague understanding why this block was introduced. If only reasons stated here https://github.com/dotnet/sdk/pull/16892/files then with ComWrappers work ongoing, that block can be removed and resurrect warning for example. If something else, then would be good if it is spelled out explicitly.

My understanding is that System.Windows.Forms.Design assembly provide supplementary code for WinForms designer. They provide different kind of actions which can happens with control, hints, etc. That's important part of experience for WinForms, but that code mostly run in Designer app. So trimming block does not applied to that code IMO.

e.g., editors and type converters are invoked via the PropertyGrid control.

Yes. That's kind of problems which I would like to surface in my attempt to assess what's blocking this issue. I completely forgot about UITypeEditor and friends which is used a lot inside PropertyGrid and lives in Design namespace.

However we need to port the general purpose designer infrastructure – i.e. API that enable building "a designer that the user can embed in their application". For example, a user might have a business application that contains a report designer feature, which can use our "general designer" framework.

From where I am now, I cannot assess, is this kind of applications can prevent removal of _SuppressWinFormsTrimError or not. I looking for smallest possible amount of work, so I would like to probe what you consider acceptable, and what not. If by a chance I can do not annotate Design namespace, I would prefer not to.

jkotas commented 2 years ago

How that Design namespace issues can be hidden from trim warnings?

Make sure that the code from Design namespace is not getting rooted and that it gets trimmed in a typical WinForms apps. Code that gets trimmed won't produce warnings.

https://docs.microsoft.com/en-us/dotnet/core/deploying/trimming/prepare-libraries-for-trimming is a general guidance for how to prepare libraries for trimming.

JeremyKuhne commented 2 years ago

Comment on tooling: https://github.com/dotnet/winforms/pull/7256#issuecomment-1152138817

mikola-akbal commented 1 year ago

Please, make Windows Forms compatible with trimming.

mikola-akbal commented 1 year ago

Can you estimate when Native AOT will support Windows Forms? Time or version of .NET.

KlausLoeffelmann commented 7 months ago

@merriemcgaw, just to get this Epic on your radar for planning. I think due to the scope, it's not a good candidate for tracking it your projects, but I just wanted to ping you that we're maintaining this.

Also @elachlan FYI (I am sure, he wants to be in the loop).

KlausLoeffelmann commented 7 months ago

@mikola-akbal: Just as an update: we have started work in the runtime a while ago, and I am starting to investigate Binding from the runtime and the Out-Of-Proc Designer side.

JOULTICOA commented 4 months ago

Bless all of you working on this. If you've been in the weeds its easy to forget how revolutionary (and massive) this is.

Q: If this doesn't quite make it completely for 9 STS, will that mean that it won't make it into 10 LTS? or will it probably end up in 10 LTS 90% complete regardless?