Skip to content

Commit 0213c4a

Browse files
committed
[class-parse] Update parameter names backwards
Context: 8ccb837 Context: dotnet/android-libraries#413 Context: https://discord.com/channels/732297728826277939/732297837953679412/902301741159182346 Context: https://repo1.maven.org/maven2/org/jetbrains/kotlin/kotlin-stdlib/1.5.31/kotlin-stdlib-1.5.31.jar Context: https://discord.com/channels/732297728826277939/732297837953679412/902554256035426315 dotnet/android-libraries#413 ran into an issue: D:\a\1\s\generated\org.jetbrains.kotlin.kotlin-stdlib\obj\Release\net6.0-android\generated\src\Kotlin.Coroutines.AbstractCoroutineContextElement.cs(100,8): error CS1002: ; expected The offending line: var this = Java.Lang.Object.GetObject<Java.Lang.Object> (native_this, JniHandleOwnership.DoNotTransfer); (Assigning to `this` makes for a very weird error message.) This was eventually tracked down to commit 8ccb837; @jpobst wrote: > previously it produced: > > <parameter name="initial" type="R" jni-type="TR;" /> > <parameter name="operation" type="kotlin.jvm.functions.Function2&lt;? super R, ? super kotlin.coroutines.CoroutineContext.Element, ? extends R&gt;" /> > > now it produces: > > <parameter name="this" type="R" jni-type="TR;" /> > <parameter name="initial" type="kotlin.jvm.functions.Function2&lt;? super R, ? super kotlin.coroutines.CoroutineContext.Element, ? extends R&gt;" /> The (a?) "source" of the problem is that Kotlin is "weird": it emits a Java method with signature: /* partial */ class AbstractCoroutineContextElement { public Object fold(Object initial, Function2 operation); } However, the local variables table declares *three* local variables: 1. `this` of type `kotlin.coroutines.CoroutineContext.Element` 2. `initial` of type `java.lang.Object` 3. `operation` of type `Function2` This is an instance method, so normally we would skip the first local variable, as "normally" the first local variable of an instance method has the same type as the declaring type. The "weirdness" with Kotlin is that the first local parameter type is *not* the same as the declaring type, it's of a "random" implemented interface type! % mono class-parse.exe --dump kotlin/coroutines/AbstractCoroutineContextElement.class … ThisClass: Utf8("kotlin/coroutines/AbstractCoroutineContextElement") … 3: fold (Ljava/lang/Object;Lkotlin/jvm/functions/Function2;)Ljava/lang/Object; Public Code(13, Unknown[LineNumberTable](6), LocalVariableTableAttribute( LocalVariableTableEntry(Name='this', Descriptor='Lkotlin/coroutines/CoroutineContext$Element;', StartPC=0, Index=0), LocalVariableTableEntry(Name='initial', Descriptor='Ljava/lang/Object;', StartPC=0, Index=1), LocalVariableTableEntry(Name='operation', Descriptor='Lkotlin/jvm/functions/Function2;', StartPC=0, Index=2))) Signature(<R:Ljava/lang/Object;>(TR;Lkotlin/jvm/functions/Function2<-TR;-Lkotlin/coroutines/CoroutineContext$Element;+TR;>;)TR;) RuntimeInvisibleParameterAnnotationsAttribute(Parameter0(), Parameter1(Annotation('Lorg/jetbrains/annotations/NotNull;', {}))) … (Here, we "expect" the `this` local variable to be of type `kotlin.coroutines.AbstractCoroutineContextElement`, but it is instead of type `kotlin.coroutines.CoroutineContext.Element`.) This "type mismatch" means that our logic to skip the first local variable doesn't actually skip the first local variable. But wait, Kotlin can throw differently weird stuff at us, too. Enter [inline and reified type parameters][0], TODO EXPL. To better address these scenarios, relax and rework the logic in `MethodInfo.UpdateParametersFromLocalVariables()`: instead of requiring that we know the "start" offset between the local variable names and the parameters (previous world order), instead look for a "run" of local variables which have the same types, in the same order, as the descriptor parameters. If there are extra local variables, *ignore them*. This allows to "cleanly" handle `fold()`: when checking for a matching "run" of `{ Object, Function2 }`, we'll skip the `this` parameter and nicely align on the `initial` parameter. This does mean that if a method has no descriptor parameters, it'll match *everything*, we we arguably have less validation occurring, but @jonpryor isn't convinced we were gaining anything from that. [0]: https://medium.com/swlh/inline-and-reified-type-parameters-in-kotlin-c7585490e103
1 parent 79744f6 commit 0213c4a

File tree

1 file changed

+38
-32
lines changed

1 file changed

+38
-32
lines changed

src/Xamarin.Android.Tools.Bytecode/Methods.cs

+38-32
Original file line numberDiff line numberDiff line change
@@ -165,44 +165,50 @@ void UpdateParametersFromLocalVariables (ParameterInfo[] parameters)
165165
return;
166166

167167
var names = locals.LocalVariables.Where (p => p.StartPC == 0).ToList ();
168-
int namesStart = 0;
169-
if (!AccessFlags.HasFlag (MethodAccessFlags.Static) &&
170-
names.Count > namesStart &&
171-
names [namesStart].Descriptor == DeclaringType.FullJniName) {
172-
namesStart++; // skip `this` parameter
173-
}
174-
if (!DeclaringType.IsStatic &&
175-
IsConstructor &&
176-
names.Count > namesStart &&
177-
DeclaringType.InnerClass != null && DeclaringType.InnerClass.OuterClassName != null &&
178-
names [namesStart].Descriptor == "L" + DeclaringType.InnerClass.OuterClassName + ";") {
179-
namesStart++; // "outer `this`", for non-static inner classes
180-
}
181-
if (!DeclaringType.IsStatic &&
182-
IsConstructor &&
183-
names.Count > namesStart &&
184-
DeclaringType.TryGetEnclosingMethodInfo (out var declaringClass, out var _, out var _) &&
185-
names [namesStart].Descriptor == "L" + declaringClass + ";") {
186-
namesStart++; // "outer `this`", for non-static inner classes
168+
169+
int parametersCount = GetDeclaredParametersCount (parameters);
170+
171+
if (!FindParameterRun (parameters, parametersCount, names, out int namesStart)) {
172+
CheckDescriptorVariablesToLocalVariables (parameters, parameters.Length, names, 0);
173+
return;
187174
}
188175

189-
// For JvmOverloadsConstructor.<init>.(LJvmOverloadsConstructor;IILjava/lang/String;)V
190-
if (namesStart > 0 &&
191-
names.Count > namesStart &&
192-
parameters.Length > 0 &&
193-
names [namesStart].Descriptor != parameters [0].Type.BinaryName &&
194-
names [namesStart-1].Descriptor == parameters [0].Type.BinaryName) {
195-
namesStart--;
176+
for (int i = 0; i < parametersCount; ++i) {
177+
if ((i + namesStart) > names.Count)
178+
break;
179+
parameters [i].Name = names [namesStart+i].Name;
180+
CheckLocalVariableTypeToDescriptorType (i, parameters, names, namesStart);
196181
}
197182

198-
int parametersCount = GetDeclaredParametersCount (parameters);
199183
CheckDescriptorVariablesToLocalVariables (parameters, parametersCount, names, namesStart);
184+
}
200185

201-
int max = Math.Min (parametersCount, names.Count - namesStart);
202-
for (int i = 0; i < max; ++i) {
203-
parameters [i].Name = names [namesStart+i].Name;
204-
CheckLocalVariableTypeToDescriptorType (i, parameters, names, namesStart);
186+
bool FindParameterRun (ParameterInfo[] parameters, int parametersCount, List<LocalVariableTableEntry> names, out int namesStart)
187+
{
188+
namesStart = 0;
189+
190+
if (parametersCount == 0) {
191+
return true;
192+
}
193+
194+
for (int ni = 0; ni < names.Count; ni++) {
195+
if ((names.Count - ni) < parametersCount) {
196+
return false;
197+
}
198+
bool valid = true;
199+
for (int i = 0; i < parametersCount; ++i) {
200+
if (parameters [i].Type.BinaryName != names [ni+i].Descriptor) {
201+
valid = false;
202+
break;
203+
}
204+
}
205+
if (valid) {
206+
namesStart = ni;
207+
return true;
208+
}
205209
}
210+
211+
return false;
206212
}
207213

208214
LocalVariableTableAttribute GetLocalVariables ()
@@ -277,7 +283,7 @@ void CheckDescriptorVariablesToLocalVariables (ParameterInfo[] parameters, int p
277283
{
278284
if (AccessFlags.HasFlag (MethodAccessFlags.Synthetic))
279285
return;
280-
if ((names.Count - namesStart) == parametersCount)
286+
if ((names.Count - namesStart) >= parametersCount)
281287
return;
282288
if (IsEnumCtor)
283289
return;

0 commit comments

Comments
 (0)