Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: dotnet/java-interop
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: 32635fd
Choose a base ref
...
head repository: dotnet/java-interop
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 7dc270d
Choose a head ref
  • 2 commits
  • 12 files changed
  • 2 contributors

Commits on Feb 7, 2022

  1. [Xamarin.Android.Tools.Bytecode] Improve Kotlin metadata matching (#946)

    Fixes: #945
    
    Context: dotnet/android-libraries#436 (comment)
    
    Consider the following Kotlin function:
    
    	fun get (index: Int) : UInt { … }
    
    When `get` is compiled with Kotlin 1.5, `class-parse` would emit:
    
    	<method abstract="false" final="true" name="get-pVg5ArA" return="uint" jni-return="I" static="true" visibility="public" jni-signature="([II)I">
    	  <parameter name="$this" type="int[]" jni-type="[I" />
    	  <parameter name="index" type="int" jni-type="I" />
    	</method>
    
    However, when `get` is compiled with Kotlin 1.6, `class-parse` sees:
    
    	<method abstract="false" final="true" name="get-pVg5ArA" return="int" jni-return="I" static="true" visibility="public" jni-signature="([II)I">
    	  <parameter name="arg0" type="int[]" jni-type="[I" />
    	  <parameter name="index" type="int" jni-type="I" />
    	</method>
    
    Note that the name of the first `int[]` parameter changes from
    `$this` to `arg0`, and the return type changed from `uint` to `int`.
    
    The parameter name change is annoying; the return type change is
    an ABI break, and makes many package updates impossible.
    
    What happened?
    
    Recall commit 71afce5: Kotlin-compiled `.class` files contain a
    `kotlin.Metadata` type-level annotation blob, which contains a
    Protobuf stream containing Kotlin metadata for the type.
    The `kotlin.Metadata` annotation contains information about Kotlin
    functions, and `class-parse` needs to associate the information about
    the Kotlin functions to Java methods.  This can be tricky because
    there is not an explicit way to do this, and one must rely on trying
    to match function names and parameter types.  Previously, this was
    accomplished by looping through the parameters and comparing types.
    
    However, the `kotlin.Metadata` annotation only includes "user created"
    method parameters, while the Java method may include "compiler created"
    method parameters.  We attempted to skip these by ignoring Java
    parameters whose names begin with a dollar sign (ex `$this`).
    
    This "worked" most of the time.
    
    This broke in Kotlin 1.6, as the compiler generated method parameters
    stopped using `$` for compiler-generated parameter names.
    algorithm.
    
    We need something better.
    
    The `kKotlin.Metadata` annotation provides a `JvmSignature` property --
    which matches the `JvmName` property which we already use -- which
    sounds promising.  However, `JvmSignature` is sometimes `null`, and
    sometimes it doesn't actually match.  But it's closer.
    
    So now we try to match with signatures.
    
      * If `JvmSignature` matches the Java signature we use that.
    
      * If `JvmSignature` is `null`, calculate it from the Kotlin
        parameters and try to match with that.
        * Note it wants unsigned types as `Lkotlin/UInt;` and not the
          primitive encoding (`I`).
    
      * If Kotlin metadata defines a `ReceiverType`, add that as the
        first parameter to the signature.
    
    Using `kotlin-stdlib-1.5.31.jar` as a baseline, we get much better
    results:
    
    | Version         | Matched Functions |   Unmatched Functions |
    | --------------- | ----------------: | --------------------: |
    | main @ 32635fd |               946 |                   154 |
    | This PR         |              1100 |                     0 |
    
    Now that we can match Kotlin functions to Java methods, we still have
    to match up the parameters so we can apply parameter-level transforms.
    Do this by removing Java method parameters from the front and back of
    the list to account for cases where the compiler adds "hidden"
    parameters:
    
      * Remove Kotlin `ReceiverType` from beginning of list.
      * Remove Kotlin `Lkotlin/coroutines/Continuation;` from end of list.
        * Note this _could_ be a legitimate user supplied parameter so we
          try to restrict it to "unnamed" parameters.
      * Remove the `this` parameter added to `static` functions.
    
    Note: the test classes `DeepRecursiveKt.class`,
    `DeepRecursiveScope.class`, and `UByteArray.class` are pulled from
    [`kotlin-stdlib-1.5.31.jar`][0].
    
    [0]: https://mvnrepository.com/artifact/org.jetbrains.kotlin/kotlin-stdlib/1.5.31
    jpobst authored Feb 7, 2022
    Configuration menu
    Copy the full SHA
    f91b077 View commit details
    Browse the repository at this point in the history

Commits on Feb 8, 2022

  1. [build] set $(ProduceReferenceAssemblyInOutDir)=True (#949)

    Context: https://docs.microsoft.com/dotnet/core/compatibility/sdk/6.0/write-reference-assemblies-to-obj
    Context: dotnet/msbuild#7355
    
    Using the latest internal builds of Visual Studio, you hit the build
    error when building xamarin-android:
    
    	build-tools\create-packs\Directory.Build.targets(28,5): error MSB4018: The "CreateFrameworkListFile" task failed unexpectedly. [build-tools\create-packs\Microsoft.Android.Ref.proj]
    	build-tools\create-packs\Directory.Build.targets(28,5): error MSB4018: System.IO.DirectoryNotFoundException: Could not find a part of the path 'external\Java.Interop\bin\Debug-net6.0\ref\Java.Interop.dll'.
    
    This was a breaking change in MSBuild:
    
    > ### Old behavior
    > Since reference assemblies were added, the .NET SDK has written
    > reference assemblies to the `ref` directory in the `OutDir`
    > directory of the compilation.
    > 
    > ### New behavior
    > Now, reference assemblies are written to the `refint` directory of
    > the `IntermediateOutputPath` directory by default, like many other
    > intermediate artifacts.
    > 
    > ### Reason for change
    > Reference assemblies are generally not run-time assets, and so don't
    > belong in the `OutDir` directory by default.
    
    Since we *are* using the reference assembly as build output, I think
    we *should* put the file in `bin`.
    
    Let's set `$(ProduceReferenceAssemblyInOutDir)`=True to get the
    old behavior.
    
    After this change goes in, we'll need other changes in
    xamarin/xamarin-android.  We might also be able to use
    `$(TargetRefPath)` in some way, but that isn't needed here.
    jonathanpeppers authored Feb 8, 2022
    Configuration menu
    Copy the full SHA
    7dc270d View commit details
    Browse the repository at this point in the history
Loading