First draft of blog post on fixing warnings #1946

agocke · 2021-04-09T01:00:31Z

No description provided.

docs/fixing-warnings.md

Co-authored-by: Michal Strehovský <[email protected]>

docs/fixing-warnings.md

tlakollo · 2021-04-09T18:19:45Z

docs/fixing-warnings.md

+void MethodWithAssemblyLoad() { ... }
+
+[UnconditionalSuppressMessage("AssemblyLoadTrimming", "IL2026:RequiresUnreferencedCode",
+    Justification = "Everything referenced in the loaded assembly is manually preserved, so it's safe")]


Should we explain/link to how to preserve things manually? If I were to read this blog post looking for answers on how to solve my current broken app I would see that I can:

Use other function that is trim compatible (which is not always possible)

Mark my method with RequiresUnreferencedCode which changes the warning location but my app would still be broken

Preserve manually and Suppress the warning, the blog explains how to suppress the warning, but not how to preserve manually.

A good question, but not really a post I'm prepared to write.

Well just in case someone reads these comments, an XML descriptor file or DynamicDependencyAttribute could help to preserve stuff on the app

tlakollo

Looks good to me

marek-safar · 2021-04-12T10:31:52Z

docs/fixing-warnings.md

+
+In .NET Core 3.1 and 5.0 we introduced trimming as a new preview feature for self-contained .NET
+core applications. Conceptually the feature is very simple: when publishing the application the
+.NET SDK analyzes the entire application, including the .NET framework, and removes all unused


I'm not sure what "including the .NET framework" is intended to explain

I meant to emphasize that we're trimming away the core framework, but it's probably not necessary.

marek-safar · 2021-04-12T10:33:54Z

docs/fixing-warnings.md

+difficulties in safely trimming applications.
+
+The most difficult question in trimming is what is unused, or more precisely, what is used. For
+most standard C# code this is trivial -- the trimmer can easily walk method calls, field and


What is "the trimmer" ? We have not been using that term and most developers are familiar with linker term. Do we really have to confuse the community with another new name?

In all locations other than mono/linker we've been trying to avoid the term "linker" because most people understand it as the "platform linker". We use "trim tool" or sometimes "trimmer" (I would prefer "trim tool" if possible).
For the same reason we use "trimming" and not "linking" everywhere.

we've been trying to avoid the term "linker" because most people understand it as the "platform linker"

Are you sure? Few references to public documentation that clearly uses IL Linker term to describe the tool.

https://docs.microsoft.com/en-us/aspnet/core/blazor/host-and-deploy/configure-linker?view=aspnetcore-3.1

https://www.telerik.com/blogs/reduce-self-contained-app-size-with-latest-net-core-features

https://www.meziantou.net/optimizing-a-blazor-webassembly-application-size.htm

https://montemagno.com/optimizing-xamarin-apps-libraries-with-the-linker/

https://docs.telerik.com/devtools/xamarin/knowledge-base/common-understanding-ui-for-xamarin-and-linker-behavior

I'm intentionally "ignoring" the mono/Xamarin side of things - those will likely use "linker" a lot. Most of our docs and blog posts use "Trimming" - we also use it everywhere in the messages (RUC) in the framework. I don't see us going back to "linking".

That said - I understand the confusion for Xamarin users - we might want to mention this on the first use of the term.

I was not referring to trimming but to "trimmer". Even .NET Core MSDN documentation is written in the way that it uses the term since 3.1.

.NET Core 3+ ships with an IL Linker which allows you to reduce the size of the self-contained application. It automatically determines which assemblies are needed and copies only them.

I'm really not sure we have to rename everything to cause more confusion.

I would probably just not give it a name here. We can update the sentence to say "For most standard C# code this is trivial -- ~~the trimmer~~we can easily walk method calls, field and"

That said,

https://en.wikipedia.org/wiki/Linker_(computing)

This is the definition of the term used in computer engineering. It's not what IL Linker is doing. It's confusing to anyone who has ever used a linker in the past.

Reusing this term in .NET was a mistake. We should have gone with something like Remover 8000. Now Xamarin often needs to qualify the word "linker" with the word "native" when referring to what everyone outside .NET calls "linker". It's a pretty sad place to be in and puts us (the authors of .NET) into a bad light (trying to steal a well-established term).

I've seen this cause confusion in the past. "Linker failed. Oh, not that one, the other one."

We should phase this name out before it moves out of the niche parts of .NET (both Xamarin and Blazor-WASM are still niches).

My use of the term "trimmer" here is in its most generic sense -- it is the thing which "trims." The other documentation about a specific tool which is responsible for trimming is incorrect in this case because the only supported method of trimming applications in .NET 6 will be passing PublishTrimmed=true to the SDK. Rather than talk specifically about the SDK, I wanted to just use a generic term.

I don't really see the need to get more specific here and talk about any specific tool. The term "linker" is certainly incorrect for .NET 6 since we do not plan on shipping a separate linker tool for supported scenarios.

marek-safar · 2021-04-12T10:36:47Z

docs/fixing-warnings.md

+
+### RequiresUnreferencedCode
+
+`RequiresUnreferencedCode` is simple: it's an attribute that can be placed on methods to indicate


It could reference MSDN documentation at https://docs.microsoft.com/en-us/dotnet/api/system.diagnostics.codeanalysis.requiresunreferencedcodeattribute

marek-safar · 2021-04-12T10:37:21Z

docs/fixing-warnings.md

+
+### DynamicallyAccessedMembers
+
+`DynamicallyAccessedMembers` is usually about reflection. Unlike `RequiresUnreferencedCode`,


Add a reference to MSDN documentation for DynamicallyAccessedMembers

marek-safar · 2021-04-12T10:38:23Z

docs/fixing-warnings.md

+
+This description should cover the most common situations you end up in while trimming your
+application. Over time we'll continue to improve the diagnostic experience and provide more streamlined
+ways to make your code trim-compatible.


Over time we'll continue to improve the diagnostic experience and provide more streamlined
ways to make your code trim-compatible.

Is this claim correct?

Yup! Better warning messages, analyzers for DynamicallyAccessedMembers, better perf, etc

samsp-msft · 2021-04-12T16:25:50Z

docs/fixing-warnings.md

+the method at all when trimming and use something else which is trim-compatible. If you're
+writing a library and it's not in your control whether or not to call the method and you just
+want to communicate to *your* caller, you can also add `RequiresUnreferencedCode` to your own
+method. This silences all trimming warnings in your code, but will produce a warning whenever


Why does this produce a warning rather than an error? If the method is marked that its not going to work, and is called, then why only warn?

Because in almost all cases the method might work in some cases and might not in others. For example XmlSerializer.Serialize(object o) - it's definitely not trim compatible, but if you make sure that the type passed in is preserved enough - it will work just fine.

We use this attribute basically to say "If you ignore this, you're on your own and will have to test at runtime".

There are also cases where the method is sooo close to being trim compatible except for some weird special case - we can't mark it as compatible (since it's not 100%), but it almost always is.

And as with any other trim analysis - the linker plays it super safe, the annotations do that as well. We don't want to block people completely if we are too careful.

Pretty much everything in trimming is a warning, because many things just happen to work. For instance, if you call a method that has RUC does a lot of reflection on its parameter, but the parameter type is in user code (and therefore untrimmed) and nothing important from the framework was removed, it all just works.

Of course, this is just luck, which is why the warning is there.

samsp-msft · 2021-04-12T16:29:07Z

docs/fixing-warnings.md

+}
+void M2(Type type)
+{
+    // Trim analysis warning IL2070: net6.Program.M2(Type): 'this' argument does not satisfy


Move out the error into a block to show what the compiler would say

Sorry, not sure what you mean by this. You mean extract the comment into a separate code block?

Yes, and you can mark it as console, something like:

void M1() { M2(typeof(System.Tuple)); } void M2(Type type) { var methods = type.GetMethods(); ... }

If you compile, you'll see the following warning:

Trim analysis warning IL2070: net6.Program.M2(Type): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicMethods' in call to 'System.Type.GetMethods()'. The parameter 'type' of method 'net6.Program.M2(Type)' 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.

samsp-msft · 2021-04-12T16:30:14Z

docs/fixing-warnings.md

+`DynamicallyAccessMembers` come in? What happens if the reflection is split across methods?
+
+```C#
+void M1()


M1 seems wrong to me, Method1 would read better

samsp-msft · 2021-04-12T16:31:35Z

A couple of niggles, but LGTM.

agocke · 2021-04-12T17:41:05Z

@samsp-msft Any idea where this should go? I'd like to put it somewhere more accessible than the linker docs. Either MS docs or blog post. Thoughts?

samsp-msft · 2021-04-12T18:16:44Z

@samsp-msft Sam Spencer FTE Any idea where this should go? I'd like to put it somewhere more accessible than the linker docs. Either MS docs or blog post. Thoughts?

It reads more like a blog post than docs, but doesn't have the preamble of a blog post. I'd suggest either

Turn it into an update on trimming blog post - put a bit more up front about changes in trimming
or put it in the trimming docs - https://docs.microsoft.com/en-us/dotnet/core/deploying/trimming-options
ideally, both

agocke · 2021-04-16T22:19:43Z

@samsp-msft I think this is ready. Not sure what kind of preface you would want to add, but can we start moving this to a blog site?

Co-authored-by: Michal Strehovský <[email protected]> Commit migrated from dotnet/linker@4ef464a

First draft of blog post on fixing warnings

660e3e4

agocke requested review from sbomer and vitek-karas April 9, 2021 01:00

agocke requested a review from marek-safar as a code owner April 9, 2021 01:00

MichalStrehovsky reviewed Apr 9, 2021

View reviewed changes

docs/fixing-warnings.md Outdated Show resolved Hide resolved

docs/fixing-warnings.md Outdated Show resolved Hide resolved

agocke and others added 3 commits April 9, 2021 01:29

Update docs/fixing-warnings.md

4942438

Co-authored-by: Michal Strehovský <[email protected]>

Update docs/fixing-warnings.md

c98be03

Co-authored-by: Michal Strehovský <[email protected]>

Update fixing-warnings.md

d08a430

vitek-karas reviewed Apr 9, 2021

View reviewed changes

docs/fixing-warnings.md Outdated Show resolved Hide resolved

vitek-karas reviewed Apr 9, 2021

View reviewed changes

docs/fixing-warnings.md Outdated Show resolved Hide resolved

sbomer reviewed Apr 9, 2021

View reviewed changes

docs/fixing-warnings.md Outdated Show resolved Hide resolved

tlakollo reviewed Apr 9, 2021

View reviewed changes

docs/fixing-warnings.md Outdated Show resolved Hide resolved

agocke added 2 commits April 9, 2021 10:27

PR comments

1acd4df

Readability

0680936

tlakollo reviewed Apr 9, 2021

View reviewed changes

vitek-karas approved these changes Apr 9, 2021

View reviewed changes

tlakollo approved these changes Apr 9, 2021

View reviewed changes

marek-safar reviewed Apr 12, 2021

View reviewed changes

samsp-msft reviewed Apr 12, 2021

View reviewed changes

Add references to docs

4dfa391

sbomer mentioned this pull request Apr 13, 2021

Document new linker options and trimming libraries dotnet/docs#23766

Merged

Update fixing-warnings.md

81c095a

marek-safar approved these changes Apr 13, 2021

View reviewed changes

agocke added 2 commits April 13, 2021 12:55

Feedback

54b8832

Add link to previous blog post

f47a479

agocke merged commit 4ef464a into dotnet:main Apr 16, 2021

agocke deleted the fixing-warnings branch April 16, 2021 22:15

agocke added a commit to dotnet/runtime that referenced this pull request Nov 16, 2022

First draft of blog post on fixing warnings (dotnet/linker#1946)

cd7a8ba

Co-authored-by: Michal Strehovský <[email protected]> Commit migrated from dotnet/linker@4ef464a


		### RequiresUnreferencedCode

		`RequiresUnreferencedCode` is simple: it's an attribute that can be placed on methods to indicate


		### DynamicallyAccessedMembers

		`DynamicallyAccessedMembers` is usually about reflection. Unlike `RequiresUnreferencedCode`,

First draft of blog post on fixing warnings #1946

First draft of blog post on fixing warnings #1946

Uh oh!

Conversation

agocke commented Apr 9, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tlakollo Apr 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tlakollo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samsp-msft commented Apr 12, 2021

Uh oh!

agocke commented Apr 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

samsp-msft commented Apr 12, 2021

Uh oh!

agocke commented Apr 16, 2021

Uh oh!

Uh oh!

tlakollo Apr 9, 2021 •

edited

Loading

agocke commented Apr 12, 2021 •

edited

Loading