From 06214ffb9a7645616fed5594860ff62e48d2ff1c Mon Sep 17 00:00:00 2001 From: Jonathan Pobst Date: Mon, 15 Apr 2024 13:27:16 -1000 Subject: [PATCH] [Xamain.Android.Tools.Bytecode] Hide private Kotlin default ctors (#1206) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Context: f6c12ba30048748572af57654b218c60bc9ff95f `Kotlin.Stdlib` contains [`kotlin.io.encoding.Base64`][0], a public class which is not intended to be instantiated by external users. Thus it only contains a `private` default constructor: // Kotlin public open /* partial */ class Base64 private constructor( internal val isUrlSafe: Boolean, internal val isMimeScheme: Boolean ) { // … public companion object Default : Base64(isUrlSafe = false, isMimeScheme = false) { // … } } which compiles into the equivalent Java code: // Java public /* partial */ class Base64 { private Base64 (boolean isUrlSafe, boolean isMimeScheme) {…} public Base64(boolean isUrlSafe, boolean isMimeScheme, kotlin.jvm.internal.DefaultConstructorMarker $constructor_marker) {…} // ^ synthetic constructor } Previously (f6c12ba3) we believed that a synthetic default constructor would always end in an `int, DefaultConstructorMarker)` parameter pair, however this one does not. This synthetic constructor causes us to generate some bizarre "usable but shouldn't be used" constructors: // C# partial class Base64 { [Register (".ctor", "(ZZLkotlin/jvm/internal/DefaultConstructorMarker;)V", "")] public unsafe Base64 (bool isUrlSafe, bool isMimeScheme, global::Kotlin.Jvm.Internal.DefaultConstructorMarker? _constructor_marker) : base (IntPtr.Zero, JniHandleOwnership.DoNotTransfer) { // … } } Fix this by extending our Kotlin default constructor marker detection logic to look for any synthetic constructor whose final parameter is `DefaultConstructorMarker` to handle this additional case. With the change, `Kotlin.IO.Encoding.Base64` no longer generates a `public` constructor. [0]: https://github.com/JetBrains/kotlin/blob/3fbb7bc92086bdf3bde123a8f774bce25b19ef37/libraries/stdlib/src/kotlin/io/encoding/Base64.kt --- .../Kotlin/KotlinUtilities.cs | 10 ++++---- .../KotlinFixupsTests.cs | 22 ++++++++++++++++++ .../PrivateDefaultConstructor$Default.class | Bin 0 -> 829 bytes .../kotlin/PrivateDefaultConstructor.class | Bin 0 -> 1221 bytes .../kotlin/PrivateDefaultConstructor.kt | 5 ++++ 5 files changed, 31 insertions(+), 6 deletions(-) create mode 100644 tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin/PrivateDefaultConstructor$Default.class create mode 100644 tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin/PrivateDefaultConstructor.class create mode 100644 tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin/PrivateDefaultConstructor.kt diff --git a/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinUtilities.cs b/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinUtilities.cs index bf68a8c8e..e27b04c09 100644 --- a/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinUtilities.cs +++ b/src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinUtilities.cs @@ -97,8 +97,7 @@ public static string GetMethodNameWithoutUnsignedSuffix (string name) public static bool IsDefaultConstructorMarker (this MethodInfo method) { - // A default constructor is synthetic and always has an int and a - // DefaultConstructorMarker as its final 2 parameters. + // A default constructor is synthetic and has a DefaultConstructorMarker as its final parameter. if (method.Name != "") return false; @@ -107,12 +106,11 @@ public static bool IsDefaultConstructorMarker (this MethodInfo method) var parameters = method.GetParameters (); - if (parameters.Length < 2) + if (parameters.Length < 1) return false; - // Parameter list ends with `int, DefaultConstructorMarker`. - return parameters [parameters.Length - 2].Type.TypeSignature == "I" && - parameters [parameters.Length - 1].Type.TypeSignature == "Lkotlin/jvm/internal/DefaultConstructorMarker;"; + // Parameter list ends with `DefaultConstructorMarker`. + return parameters [parameters.Length - 1].Type.TypeSignature == "Lkotlin/jvm/internal/DefaultConstructorMarker;"; } // Sometimes the Kotlin provided JvmSignature is null (or unhelpful), so we need to construct one ourselves diff --git a/tests/Xamarin.Android.Tools.Bytecode-Tests/KotlinFixupsTests.cs b/tests/Xamarin.Android.Tools.Bytecode-Tests/KotlinFixupsTests.cs index 292ea5f2a..e92deeb95 100644 --- a/tests/Xamarin.Android.Tools.Bytecode-Tests/KotlinFixupsTests.cs +++ b/tests/Xamarin.Android.Tools.Bytecode-Tests/KotlinFixupsTests.cs @@ -92,6 +92,28 @@ public void HideDefaultConstructorMarker () Assert.False (ctor_3p.AccessFlags.HasFlag (MethodAccessFlags.Public)); } + [Test] + public void HidePrivateDefaultConstructorMarker () + { + var klass = LoadClassFile ("PrivateDefaultConstructor.class"); + + // init (int isFoo) + var ctor_1p = klass.Methods.Single (m => m.Name == "" && m.GetParameters ().Length == 1); + + // init (int isFoo, DefaultConstructorMarker p1) + var ctor_2p = klass.Methods.Single (m => m.Name == "" && m.GetParameters ().Length == 2); + + Assert.True (ctor_2p.AccessFlags.HasFlag (MethodAccessFlags.Public)); + + KotlinFixups.Fixup ([klass]); + + // Assert that the normal constructor is still public + Assert.False (ctor_1p.AccessFlags.HasFlag (MethodAccessFlags.Public)); + + // Assert that the synthetic "DefaultConstructorMarker" constructor has been marked private + Assert.False (ctor_2p.AccessFlags.HasFlag (MethodAccessFlags.Public)); + } + [Test] public void HideImplementationMethod () { diff --git a/tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin/PrivateDefaultConstructor$Default.class b/tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin/PrivateDefaultConstructor$Default.class new file mode 100644 index 0000000000000000000000000000000000000000..26045198cb34676d4a637ebbc629b42b348b4224 GIT binary patch literal 829 zcmbVKO>fgc5Ph>w~+L3 zCw>V3fKw$7DC&_Pg_uoJaX>l1!SnHUo@eIm{`~d*2Y@3yChWXP^+;OvQk}~@vYo_a zHqHAsN$YdS2N7xu%Y1l*;!|z3eMayadp$zC@u53RY@|)|(Kh;u>?Or0VP7gUg(Ul+g;f7dbVRitMNariU9;}5H{-lKO29Hr_&G){_U59wQItO zvT`7;peZX4x4KXvp|y?Xp9T1})xlG7bV!71M7rE?s@`s*`bB9MTKFB(A;| zMMb#MHeqYAAl%QNt4r8ksN2ENvb>#Spa`p7ZPe>LK2zy?c@{CU-c9;4>d92Qd=8dF zV^rFSWR@wul;B;Gr+xKGJ3{SkZmf<~PiLAL&y7i}w3-z~i}fwIrP&L<+ahoOfU_1n zjsSn(qf0D*n-)A|>k7b>9k%6J2?DGD6=J$W9Q_Gv6#j03(sfKaHYbctH;2klSBBFVx0onqhJ?*!8lvpMTv(lj(kAVN_{eYZ z*%zZ`i6&;*qd&@cZp+-m!ow0cr|0yX^L^*L7ykVH`5V9n9x>$he0#u!^}_nV`;Mr3 zZXo=ATX=pFjKS#e0XH4)9+|IN9jh&ph%u;Jwrh*04Dn)fwZRZ8t~OFgAgLn`jUizN z+n&e3nknd*oPdH96^oB{z>u$xh1_J&!`4EN+ip0w$xtaa>s?Pcwrh3li>1Q#}`VD5Tix9>P4^z<3I z9ZT>-E;!|s-XKO7gGncPQs7z0X>kNA*!;G$E6(WJ$*6`huask_pH;<(@6Sfj+tXp3G8(nc$F+o^%DELL(H6!^yVezS zclP_PuzOa`9oT{0a;#_PyU_)!dWRN6x^BDHZok*E{5PCx7&3LQ&7B7KZ5fY}shaCr ze%0YYV9~YIUweJOZEf4KAveNlTtLAxi5w>}6gno-oH&~lspOZ*XXJe+AE)<;Fk7bR zB}EGP$jp0~2G#K<{kV8%Kq!|*W z@0j{R579WuL@FuAq@sdoj6uIk;ukePrfLbRmh#7#m1Lrhr6~I!SRDb&4X=i@G+YDy zHG~whVKw3WLhc(8Xl6bQvN2fSvf$ghC&UuYPgL% h^oU{&t0*0S1-OfQ2hakf1KbawhgJ<>1egx6_zyC>2P6Oh literal 0 HcmV?d00001 diff --git a/tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin/PrivateDefaultConstructor.kt b/tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin/PrivateDefaultConstructor.kt new file mode 100644 index 000000000..15c0719cd --- /dev/null +++ b/tests/Xamarin.Android.Tools.Bytecode-Tests/kotlin/PrivateDefaultConstructor.kt @@ -0,0 +1,5 @@ +public open class PrivateDefaultConstructor private constructor (internal val isFoo: Boolean) { + init { } + + public companion object Default : PrivateDefaultConstructor (isFoo = false) { } +}