[One .NET] fix more Mono.Android.dll linker warnings #6270
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
As discovered in dotnet/linker#2265, adding |
Context: dotnet#5652 There are two remaining linker warnings when building with: > dotnet build -c Release -p:SuppressTrimAnalysisWarnings=false ... src\Mono.Android\Android.Runtime\ResourceIdManager.cs(37,6): warning IL2026: Android.Runtime.ResourceIdManager.GetResourceTypeFromAssembly(Assembly): Using method 'System.Reflection.Assembly.GetType(String)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Types might be removed. src\Mono.Android\Java.Interop\JavaObjectExtensions.cs(136,4): warning IL2026: Java.Interop.JavaObjectExtensions.GetHelperType(Type,String): Using method 'System.Reflection.Assembly.GetType(String)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Types might be removed. src\Mono.Android\Java.Interop\JavaObjectExtensions.cs(131,5): warning IL2026: Java.Interop.JavaObjectExtensions.GetHelperType(Type,String): Using method 'System.Reflection.Assembly.GetType(String)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Types might be removed. The two issues here: 1. `ResourceIdManager` uses reflection for the implementation of `Resource.designer.cs`. Specify `UnconditionalSuppressMessage` to ignore the warning, because the assembly hitting the warning is the "root assembly". The methods in question will not be removed by the linker. 2. `JavaObjectExtensions.GetHelperType` dynamically looks up `Invoker` types. Specify `UnconditionalSuppressMessage` to ignore the warning, because the `MarkJavaObjects` linker step preserves them. Additionally, I renamed the method to `GetInvokerType()` and hardcoded `"Invoker"` within the method. This should prevent us from messing this up if a new "helper" type is added in addition to `Invoker`. Unfortunately, even after all the issues are solved. `ILLink` still appears to emit warnings: Java.Interop.dll warning IL2104: Assembly 'Java.Interop' produced trim warnings. For more information see https://aka.ms/dotnet-illink/libraries Mono.Android.dll warning IL2104: Assembly 'Mono.Android' produced trim warnings. For more information see https://aka.ms/dotnet-illink/libraries Adding `-p:TrimmerSingleWarn=false` shows an additional 30 warnings. There is potentially an issue with how these warnings are presented, filed at: dotnet/linker#2265
jonathanpeppers
force-pushed
the
mono.android-illink-warnings
branch
from
ea75e92
to
0c5753d
Compare
jonathanpeppers
changed the title
jonathanpeppers
requested review from
dellis1972 and
jonpryor
as code owners
dellis1972
approved these changes
Sep 9, 2021
jonathanpeppers
commented
Sep 9, 2021
Comment on lines
-127
to
+128
| internal static Type? GetHelperType (Type type, string suffix) | ||
| // typeof(Foo) -> FooInvoker | ||
| // typeof(Foo<>) -> FooInvoker`1 | ||
| [UnconditionalSuppressMessage ("Trimming", "IL2026", Justification = "*Invoker types are preserved by the MarkJavaObjects linker step.")] | ||
| internal static Type? GetInvokerType (Type type) |
There was a problem hiding this comment.
@jonpryor this was the part, I wanted to check it's OK to rename this method.
I thought allowing any string suffix through here could introduce a future linker issue, because MarkJavaObjects only preserves *Invoker and *Adapter right now.
There was a problem hiding this comment.
This just raises a related question: what's a *Adapter type, and why is it being preserved? I forget. :-(
I think for "consistency" we should consider updating src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/MarkJavaObjects.cs to likewise rename GetHelperType() to, or otherwise introduce, GetInvokerType(). At least then our internal terminology would be consistent.
There was a problem hiding this comment.
This appears to be the commit which introduce the Adapter suffix; it was apparently for interfaces: https://github.com/xamarin/monodroid/commit/ff043c0281fbe1fefb3e83b7ccda1eca791c3fb0
There was a problem hiding this comment.
I suspect what happened is we "unified" the Adapter and Invoker suffixes into Invoker, but I can't easily find such a commit.
There was a problem hiding this comment.
Looks like Adapter was removed in generator output, with Invoker as the replacement, as part of https://github.com/xamarin/monodroid/commit/efeff037dfc4d4c60ad87fd6d6b22d9af45d7583
So it looks like there's no need to preserve Adapter-suffixed types, not since 2011.
There was a problem hiding this comment.
Updated the MarkJavaObjects step. I wonder if we'll see any .apk size differences?
|
We can probably ignore the test failure here: Can keep an eye on this, if it keeps happening. |
Context: https://github.com/dotnet/linker/issues/2265
Context: https://github.com/xamarin/xamarin-android/issues/5652
There are two remaining linker warnings when building with
`$(SuppressTrimAnalysisWarnings)`=false:
> dotnet build -c Release -p:SuppressTrimAnalysisWarnings=false
…
src\Mono.Android\Android.Runtime\ResourceIdManager.cs(37,6): warning IL2026: Android.Runtime.ResourceIdManager.GetResourceTypeFromAssembly(Assembly): Using method 'System.Reflection.Assembly.GetType(String)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Types might be removed.
src\Mono.Android\Java.Interop\JavaObjectExtensions.cs(136,4): warning IL2026: Java.Interop.JavaObjectExtensions.GetHelperType(Type,String): Using method 'System.Reflection.Assembly.GetType(String)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Types might be removed.
src\Mono.Android\Java.Interop\JavaObjectExtensions.cs(131,5): warning IL2026: Java.Interop.JavaObjectExtensions.GetHelperType(Type,String): Using method 'System.Reflection.Assembly.GetType(String)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Types might be removed.
The two issues here:
1. `ResourceIdManager` uses reflection for the implementation of
`Resource.designer.cs`. Use `[UnconditionalSuppressMessage]` to
ignore the warning, because the assembly hitting the warning is
the "root assembly", which is *not linked*. The methods in
question will not be removed by the linker.
2. `JavaObjectExtensions.GetHelperType()` dynamically looks up
`*Invoker` types. Use `[UnconditionalSuppressMessage]` to ignore
the warning, because the `MarkJavaObjects()` linker step preserves
them. Additionally, I renamed the method to `GetInvokerType()`
and hardcoded `"Invoker"` within the method. This should prevent
us from messing this up if a new "helper" type is added in
addition to `Invoker`.
Unfortunately, even after all the issues are solved. `ILLink` still
appears to emit warnings:
Java.Interop.dll warning IL2104: Assembly 'Java.Interop' produced trim warnings. For more information see https://aka.ms/dotnet-illink/libraries
Mono.Android.dll warning IL2104: Assembly 'Mono.Android' produced trim warnings. For more information see https://aka.ms/dotnet-illink/libraries
Adding `-p:TrimmerSingleWarn=false` shows an additional 30 warnings. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context: #5652
There are two remaining linker warnings when building with:
The two issues here:
ResourceIdManageruses reflection for the implementation ofResource.designer.cs. SpecifyUnconditionalSuppressMessagetoignore the warning, because the assembly hitting the warning is the
"root assembly". The methods in question will not be removed by the
linker.
JavaObjectExtensions.GetHelperTypedynamically looks upInvokertypes. Specify
UnconditionalSuppressMessageto ignore thewarning, because the
MarkJavaObjectslinker step preserves them.Additionally, I renamed the method to
GetInvokerType()andhardcoded
"Invoker"within the method. This should prevent usfrom messing this up if a new "helper" type is added in addition to
Invoker.Unfortunately, even after all the issues are solved.
ILLinkstillappears to emit warnings:
Adding
-p:TrimmerSingleWarn=falseshows an additional 30 warnings.There is potentially an issue with how these warnings are presented,
filed at:
dotnet/linker#2265