Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Java.Interop] suppress IL3050 with #pragma #1201

Merged
merged 1 commit into from
Mar 11, 2024

Conversation

jonathanpeppers
Copy link
Member

Context: dotnet/android#8758 (comment)
Context: #1192

In 7d1e705 and b8f6f88, we suppressed IL3050, an AOT-related warning with:

// FIXME: https://github.com/xamarin/java.interop/issues/1192
[UnconditionalSuppressMessage ("AOT", "IL3050")]
// Problematic code here

We don't immediately plan to support NativeAOT on Android in .NET 9, so it would be nice if we could suppress the warning for now. But if anyone tried to use this code in a NativeAOT context, they could get a warning.

So instead we should use:

// FIXME: https://github.com/xamarin/java.interop/issues/1192
#pragma warning disable IL3050
// Problematic code here
#pragma warning restore IL3050

This will prevent us from introducing new IL3050 and related warnings, but if anyone consumes Java.Interop.dll from NativeAOT -- they will see the warning.

Context: dotnet/android#8758 (comment)
Context: dotnet#1192

In 7d1e705 and b8f6f88, we suppressed IL3050, an AOT-related
warning with:

    // FIXME: dotnet#1192
    [UnconditionalSuppressMessage ("AOT", "IL3050")]
    // Problematic code here

We don't immediately *plan* to support NativeAOT on Android in .NET 9,
so it would be nice if we could suppress the warning *for now*. But if
anyone tried to use this code in a NativeAOT context, they could get a
warning.

So instead we should use:

    // FIXME: dotnet#1192
    #pragma warning disable IL3050
    // Problematic code here
    #pragma warning restore IL3050

This will prevent us from introducing new `IL3050` and related
warnings, but if anyone consumes `Java.Interop.dll` from NativeAOT --
they will see the warning.
@jonpryor
Copy link
Member

jonpryor commented Mar 7, 2024

One the one hand, this "works"; the build for samples/Hello-NativeAOTFromJNI now has IL3050 warnings: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=9160228&view=logs&j=4ab9874f-3b82-50ef-2a6b-4340d0043621&t=3f720a70-9e37-509d-b752-3ff7ec647839

…/src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs(665): AOT analysis warning IL3050: Java.Interop.JniRuntime.JniValueManager.<GetObjectArrayMarshaler>g__MakeGenericMethod|41_0(MethodInfo,Type): Using member 'System.Reflection.MethodInfo.MakeGenericMethod(Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
…/src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs(381): AOT analysis warning IL3050: Java.Interop.JniRuntime.JniValueManager.<GetInvokerType>g__MakeGenericType|31_1(Type,Type[]): Using member 'System.Type.MakeGenericType(Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
…/src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs(789): AOT analysis warning IL3050: Java.Interop.JavaPeerableValueMarshaler.CreateParameterToManagedExpression(JniValueMarshalerContext,ParameterExpression,ParameterAttributes,Type): Using member 'System.Linq.Expressions.Expression.Call(Expression,String,Type[],Expression[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. Calling a generic method requires dynamic code generation. This can be suppressed if the method is not generic.
…/src/Java.Interop/Java.Interop/JniRuntime.JniTypeManager.cs(284): AOT analysis warning IL3050: Java.Interop.JniRuntime.JniTypeManager.MakeGenericType(Type,Type): Using member 'System.Type.MakeGenericType(Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
…/src/Java.Interop/Java.Interop/JniRuntime.JniTypeManager.cs(277): AOT analysis warning IL3050: Java.Interop.JniRuntime.JniTypeManager.MakeArrayType(Type): Using member 'System.Type.MakeArrayType()' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The code for an array of the specified type might not be available.

On the other hand, I'm not quite sure how to fix these warnings?

@jonathanpeppers
Copy link
Member Author

I think the way to fix would be to "generate code", such that you'd perform the same calls without System.Reflection. That would be a lot of work for some of these...

I was playing with trimming issues like this here:

@jonpryor jonpryor merged commit 45437e2 into dotnet:main Mar 11, 2024
4 checks passed
@jonathanpeppers jonathanpeppers deleted the IL3050 branch March 11, 2024 18:46
jonpryor pushed a commit to dotnet/android that referenced this pull request Mar 13, 2024
Change: dotnet/java-interop@3436a30...5bca8ad

  * dotnet/java-interop@5bca8ad6: [build] Automatically add NullableAttributes.cs for netstandard2.0 (dotnet/java-interop#1188)
  * dotnet/java-interop@45437e22: [Java.Interop] suppress IL3050 with `#pragma` (dotnet/java-interop#1201)
  * dotnet/java-interop@1c9c8c9c: [Java.Interop.Tools.Maven] Initial commit. (dotnet/java-interop#1179)

With dotnet/java-interop@5bca8ad6, all instances of nullable
attributes types defined in `NullableAttributes.cs` are now `internal`.

However, `Xamarin.Android.Build.Tasks` was consuming the `public`
types from its reference to `Java.Interop.Tools.JavaCallableWrappers`,
so this change resulted in a build break:

	MavenExtensions.cs(26,32): error CS0122: 'NotNullWhenAttribute' is inaccessible due to its protection level
	MamJsonParser.cs(92,43): error CS0122: 'NotNullWhenAttribute' is inaccessible due to its protection level
	MamJsonParser.cs(92,81): error CS0122: 'NotNullWhenAttribute' is inaccessible due to its protection level

We can apply almost the same logic from dotnet/java-interop@5bca8ad6
to `xamarin-android`; the difference is that neither of the
`netstandard2.0` projects in xamarin-android that need the attributes
have `$(Nullable)='enabled'` and thus our `Condition` fails.  

(`Xamarin.Android.Build.Tasks.csproj` and
`Xamarin.Android.Tools.JavadocImporter.csproj` include `.cs` files from
`external/Java.Interop` that have been NRT annotated and thus need the
nullable attribute types.)

We add `$(Nullable)='annotations'` to the `@(Compile)` which
allows one to use NRT syntax but does not emit any NRT warnings since
these assemblies have not been converted.  We then modify the
`NullableAttributes.cs` inclusion logic to additionally key off the
`$(Nullable)=annotations` value.

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Jonathan Pobst <[email protected]>
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants