-
Notifications
You must be signed in to change notification settings - Fork 53
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.Tools.Expressions] Add Java.Interop.Tools.Expressions #1046
Merged
jonpryor
merged 3 commits into
dotnet:main
from
jonpryor:jonp-cecil-expression-compiler
Feb 28, 2023
Merged
[Java.Interop.Tools.Expressions] Add Java.Interop.Tools.Expressions #1046
jonpryor
merged 3 commits into
dotnet:main
from
jonpryor:jonp-cecil-expression-compiler
Feb 28, 2023
Commits on Feb 23, 2023
-
[Java.Interop.Tools.Expressions] Add Java.Interop.Tools.Expressions
Fixes: dotnet#616 Context: dotnet#14 Context: ff4053c Context: da5d1b8 Context: 4787e01 Context: 41ba348 Remember `jnimarshalmethod-gen` (176240d)? And it's crazy idea to use the `System.Linq.Expressions`-based custom marshaling infrastructure (ff4053c, da5d1b8) to generate JNI marshal methods at build/packaging time. And how we had to back burner it because it depended upon `System.Reflection.Emit` being able to write assemblies to disk, which is a feature that never made it to .NET Core, and is still lacking as of .NET 7? Add `src/Java.Interop.Tools.Expressions`, which contains code which uses Mono.Cecil to compile `Expression<T>` expressions to IL. Then update `jnimarshalmethod-gen` to use it! ~~ Usage ~~ % dotnet bin/Debug-net7.0/jnimarshalmethod-gen.dll \ bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll \ -v -v --keeptemp \ --jvm /Library/Java/JavaVirtualMachines/microsoft-11.jdk/Contents/Home/lib/jli/libjli.dylib \ -o _x \ -L bin/TestDebug-net7.0 \ -L /usr/local/share/dotnet/shared/Microsoft.NETCore.App/7.0.0 First param is assembly to process; `Java.Interop.Export-Tests.dll` is handy because that's what the `run-test-jnimarshal` target in `Makefile` processed. * `-v -v` is *really* verbose output * `--keeptemp` is keep temporary files, in this case `_x/Java.Interop.Export-Tests.dll.cecil`. * `--jvm PATH` is the path to the JVM library to load+use. * `-o DIR` is where to place output files; this will create `_x/Java.Interop.Export-Tests.dll`. * `-L DIR` adds `DIR` to library resolution paths; this adds `bin/TestDebug/net7.0` (dependencies of `Java.Interop.Export-Tests.dll`) and `Microsoft.NETCore.App/7.0.0-rc.1.22422.12` (net7 libs). By default the directories containing input assemblies and the directory containing `System.Private.CoreLib.dll` are part of the default `-L` list. When running in-tree, e.g. AzDO pipeline execution, `--jvm PATH` will attempt to read the path in `bin/Build*/JdkInfo.props` a'la `TestJVM` (002dea4). This allows an in-place update in `core-tests.yaml` which does: dotnet bin/Debug-net7.0/jnimarshalmethod-gen.dll \ bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll \ -v -v --keeptemp -o bin/TestDebug-net7.0 ~~ Using `jnimarshalmethod-gen` output ~~ What does `jnimarshalmethod-gen` *do*? % ikdasm bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll > beg.il % ikdasm _x/Java.Interop.Export-Tests.dll > end.il % git diff --no-index beg.il end.il # https://gist.github.com/jonpryor/b8233444f2e51043732bea922f6afc81 is a ~1KB diff which shows, paraphrasing greatly: public partial class ExportTest { partial class '__<$>_jni_marshal_methods' { static IntPtr funcIJavaObject (IntPtr jnienv, IntPtr this) => … // … [JniAddNativeMethodRegistration] static void __RegisterNativeMembers (JniNativeMethodRegistrationArguments args) => … } } internal delegate long _JniMarshal_PP_L (IntPtr jnienv, IntPtr self); // … wherein `ExportTest._<$>_jni_marshal_methods` and the `_JniMarshal*` delegate types are added to the assembly. This also unblocks the desire stated in 4787e01: > For `Java.Base`, @jonpryor wants to support the custom marshaling > infrastructure introduced in 77a6bf8. This would allow types to > participate in JNI marshal method ("connector method") generation > *at runtime*, allowing specialization based on the current set of > types and assemblies. What can we do with this `jnimarshalmethod-gen` output? Use it! First, make sure the tests work: % dotnet test --logger "console;verbosity=detailed" bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll … Passed! - Failed: 0, Passed: 17, Skipped: 0, Total: 17, Duration: 103 ms - Java.Interop.Export-Tests.dll (net7.0) Then update/replace the unit test assembly with `jnimarshalmethod-gen` output: % \cp _x/Java.Interop.Export-Tests.dll bin/TestDebug-net7.0 % dotnet test --logger "console;verbosity=detailed" bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll … Total tests: 17 Passed: 17 `core-tests.yaml` has been updated to do this. ~~ One-Off Tests ~~ One-off tests: ensure that the generated assembly can be decompiled: % ikdasm bin/TestDebug-net7.0/Java.Interop.Tools.Expressions-Tests-ExpressionAssemblyBuilderTests.dll % monodis bin/TestDebug-net7.0/Java.Interop.Tools.Expressions-Tests-ExpressionAssemblyBuilderTests.dll % ikdasm _x/Java.Interop.Export-Tests.dll % monodis _x/Java.Interop.Export-Tests.dll # which currently fails :-() Re-enable most of `Java.Interop.Export-Tests.dll` for .NET 7; see 41ba348, which disabled those tests. To verify the generated IL, use the [dotnet-ilverify][0] tool: dotnet tool install --global dotnet-ilverify Usage of which is "weird": $HOME/.dotnet/tools/ilverify _x/Java.Interop.Export-Tests.dll \ --tokens --system-module System.Private.CoreLib \ -r 'bin/TestDebug-net7.0/*.dll' \ -r '/usr/local/share/dotnet/shared/Microsoft.NETCore.App/7.0.0/*.dll' All Classes and Methods in /Volumes/Xamarin-Work/src/xamarin/Java.Interop/_x/Java.Interop.Export-Tests.dll Verified. # no errors! where: * `--tokens`: Include metadata tokens in error messages. * `--system-module NAME`: set the "System module name". Defaults to `mscorlib`, which is wrong for .NET 5+, so this must be set to `System.Private.CoreLib` (no `.dll` suffix!). * `-r FILE-GLOB`: Where to resolve assembly references for the input assembly. Fortunately file globs are supported… ~~ Removing `System.Private.CoreLib` ~~ `System.Private.CoreLib.dll` is *private*; it's not part of the public assembly surface area, so you can't use `csc -r:System.Private.CoreLib …` and expect it to work. This makes things interesting because *at runtime* everything "important" is in `System.Private.CoreLib.dll`, like `System.Object`. Specifically, if we do the "obvious" thing and do: newTypeDefinition.BaseType = assemblyDefinition.MainModule .ImportReference (typeof (object)); you're gonna have a bad type, because the resulting IL for `newTypeDefinition` will have a base class of `[System.Private.CoreLib]System.Object`, which isn't usable. Fix this by: 1. Writing the assembly to a `Stream`. 2. Reading the `Stream` from (1) 3. Fixing all member references and assembly references so that `System.Private.CoreLib` is not referenced. If `jnimarshalmethod-gen.dll --keeptemp` is used, then a `.cecil` file is created with the contents of (1). Additionally, and unexpectedly -- [jbevain/cecil#895][1] -- Mono.Cecil adds a reference to the assembly being modified. Remove the declaring assembly from `AssemblyReferences`. [0]: https://www.nuget.org/packages/dotnet-ilverify [1]: jbevain/cecil#895
Configuration menu - View commit details
-
Copy full SHA for 6247614 - Browse repository at this point
Copy the full SHA 6247614View commit details -
Configuration menu - View commit details
-
Copy full SHA for 180978c - Browse repository at this point
Copy the full SHA 180978cView commit details
Commits on Feb 24, 2023
-
Configuration menu - View commit details
-
Copy full SHA for 7de7daf - Browse repository at this point
Copy the full SHA 7de7dafView commit details
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.