From 5c7bf0743f57c5a7bb69cec962074861f49b99bd Mon Sep 17 00:00:00 2001 From: Jake Potrebic Date: Mon, 31 Jul 2023 22:27:48 -0700 Subject: [PATCH 1/2] Map lambda params from their source method --- .../src/main/kotlin/codebook.gradle.kts | 1 + .../io/papermc/codebook/lvt/LvtNamer.java | 58 ++++++++++++------- 2 files changed, 38 insertions(+), 21 deletions(-) diff --git a/build-logic/src/main/kotlin/codebook.gradle.kts b/build-logic/src/main/kotlin/codebook.gradle.kts index 80e3045..d587fd1 100644 --- a/build-logic/src/main/kotlin/codebook.gradle.kts +++ b/build-logic/src/main/kotlin/codebook.gradle.kts @@ -8,6 +8,7 @@ plugins { } repositories { + mavenLocal() mavenCentral() maven("https://maven.parchmentmc.org") { name = "ParchmentMC" diff --git a/src/main/java/io/papermc/codebook/lvt/LvtNamer.java b/src/main/java/io/papermc/codebook/lvt/LvtNamer.java index 666e184..d6ef00e 100644 --- a/src/main/java/io/papermc/codebook/lvt/LvtNamer.java +++ b/src/main/java/io/papermc/codebook/lvt/LvtNamer.java @@ -27,7 +27,8 @@ import dev.denwav.hypo.asm.AsmMethodData; import dev.denwav.hypo.core.HypoContext; import dev.denwav.hypo.hydrate.generic.HypoHydration; -import dev.denwav.hypo.hydrate.generic.MethodClosure; +import dev.denwav.hypo.hydrate.generic.LambdaClosure; +import dev.denwav.hypo.hydrate.generic.LocalClassClosure; import dev.denwav.hypo.model.data.ClassData; import dev.denwav.hypo.model.data.FieldData; import dev.denwav.hypo.model.data.HypoKey; @@ -100,14 +101,15 @@ private void fillNames0(final MethodData method) throws IOException { // If it does, we need to keep track of the LVTs we inherit @Nullable AsmMethodData outerMethod = null; int @Nullable [] outerMethodParamLvtIndices = null; - final @Nullable List> lambdaCalls = method.get(HypoHydration.LAMBDA_CALLS); + @Nullable LambdaClosure lambda = null; + final @Nullable List lambdaCalls = method.get(HypoHydration.LAMBDA_CALLS); // Only track synthetic, non-synthetic means a method reference which does not behave as a closure (does not // capture LVT) if (lambdaCalls != null && method.isSynthetic()) { - for (final MethodClosure lambdaCall : lambdaCalls) { + for (final LambdaClosure lambdaCall : lambdaCalls) { // lambdaCall.getClosure() -> The lambda method // lambdaCall.getContainingMethod() -> The outer method - if (lambdaCall.getClosure().equals(method)) { + if (lambdaCall.getLambda().equals(method)) { outerMethod = (AsmMethodData) lambdaCall.getContainingMethod(); if (outerMethod.equals(method)) { // lambdas can be recursive @@ -115,6 +117,7 @@ private void fillNames0(final MethodData method) throws IOException { continue; } outerMethodParamLvtIndices = lambdaCall.getParamLvtIndices(); + lambda = lambdaCall; // there can only be 1 outer method break; } @@ -132,11 +135,11 @@ private void fillNames0(final MethodData method) throws IOException { // A method cannot be both a lambda expression and a local class, so if we've already determined an outer // method, there's nothing to do here. Set innerClassFieldNames = Set.of(); - @Nullable MethodClosure localClassClosure = null; + @Nullable LocalClassClosure localClassClosure = null; int @Nullable [] innerClassOuterMethodParamLvtIndices = null; localCheck: if (outerMethod == null) { - final @Nullable List> localClasses = parentClass.get(HypoHydration.LOCAL_CLASSES); + final @Nullable List localClasses = parentClass.get(HypoHydration.LOCAL_CLASSES); if (localClasses == null || localClasses.isEmpty()) { break localCheck; } @@ -175,7 +178,7 @@ private void fillNames0(final MethodData method) throws IOException { // The only way to handle this kin of clash it to go back up and fix the // local variable, we can't fix it here. fixOuterScopeName( - localClassClosure, + new ClosureInfo(localClassClosure.getContainingMethod(), localClassClosure.getParamLvtIndices()), outerLvt.name, RootLvtSuggester.determineFinalName(outerLvt.name, scopedNames), outerLvt.index); @@ -183,10 +186,20 @@ private void fillNames0(final MethodData method) throws IOException { } } - final Optional methodMapping = this.mappings + Optional methodMapping = this.mappings .getClassMapping(parentClass.name()) .flatMap(c -> c.getMethodMapping(method.name(), method.descriptorText())); + boolean usingLambdaMethodMapping = false; + if (methodMapping.isEmpty() && lambda != null && lambda.getInterfaceMethod() != null) { + final LambdaClosure finalLambda = lambda; + methodMapping = this.mappings.getClassMapping(lambda.getInterfaceMethod().parentClass().name()) + .flatMap(c -> c.getMethodMapping(finalLambda.getInterfaceMethod().name(), finalLambda.getInterfaceMethod().descriptorText())); + if (methodMapping.isPresent()) { + usingLambdaMethodMapping = true; + } + } + // If there's no LVT table there's nothing for us to process if (node.localVariables == null) { // interface / abstract methods don't have LVT @@ -281,6 +294,7 @@ private void fillNames0(final MethodData method) throws IOException { int usedNameIndex = 0; final @Nullable UsedLvtName[] usedNames = new UsedLvtName[node.localVariables.size()]; + final int parameterMappingIdxOffset = usingLambdaMethodMapping ? ourCapturedLvts.length - 1 : 0; outer: for (final LocalVariableNode lvt : node.localVariables) { if (lvt.index == 0 && !method.isStatic()) { @@ -306,7 +320,7 @@ private void fillNames0(final MethodData method) throws IOException { } final @Nullable String paramName = methodMapping - .flatMap(m -> m.getParameterMapping(lvt.index)) + .flatMap(m -> m.getParameterMapping(lvt.index - parameterMappingIdxOffset)) .map(Mapping::getDeobfuscatedName) .orElse(null); @@ -368,8 +382,8 @@ private static void _collectAllFields(final ClassData container, final ClassData } private static void fixOuterScopeName( - final MethodClosure parentDef, final String badName, final String newName, final int lvtIndex) { - final MethodData containing = parentDef.getContainingMethod(); + final ClosureInfo closureInfo, final String badName, final String newName, final int lvtIndex) { + final MethodData containing = closureInfo.containing(); final MethodNode node = ((AsmMethodData) containing).getNode(); for (final LocalVariableNode lvt : node.localVariables) { @@ -392,32 +406,34 @@ private static void fixOuterScopeName( } } - final @Nullable List> lambdas = containing.get(HypoHydration.LAMBDA_CALLS); - final @Nullable List> localClasses = containing.get(HypoHydration.LOCAL_CLASSES); - final ArrayList> innerClosures = new ArrayList<>( + final @Nullable List lambdas = containing.get(HypoHydration.LAMBDA_CALLS); + final @Nullable List localClasses = containing.get(HypoHydration.LOCAL_CLASSES); + final ArrayList innerClosureContainingMethods = new ArrayList<>( (lambdas != null ? lambdas.size() : 0) + (localClasses != null ? localClasses.size() : 0)); if (lambdas != null) { - for (final MethodClosure lambda : lambdas) { + for (final LambdaClosure lambda : lambdas) { if (!lambda.getContainingMethod().equals(containing)) { - innerClosures.add(lambda); + innerClosureContainingMethods.add(new ClosureInfo(lambda.getContainingMethod(), lambda.getParamLvtIndices())); } } } if (localClasses != null) { - for (final MethodClosure localClass : localClasses) { + for (final LocalClassClosure localClass : localClasses) { if (!localClass.getContainingMethod().equals(containing)) { - innerClosures.add(localClass); + innerClosureContainingMethods.add(new ClosureInfo(localClass.getContainingMethod(), localClass.getParamLvtIndices())); } } } - for (final MethodClosure closure : innerClosures) { - if (closure.getParamLvtIndices().length > lvtIndex) { - fixOuterScopeName(closure, badName, newName, closure.getParamLvtIndices()[lvtIndex]); + for (final ClosureInfo closure : innerClosureContainingMethods) { + if (closure.paramLvtIndices().length > lvtIndex) { + fixOuterScopeName(closure, badName, newName, closure.paramLvtIndices()[lvtIndex]); } } } + private record ClosureInfo(MethodData containing, int[] paramLvtIndices) { } + private static String packageName(final ClassData classData) { final String name = classData.name(); final int lastIndex = name.lastIndexOf('/'); From 40b8361cab7a7c4c9aff654138a21bff2128ee00 Mon Sep 17 00:00:00 2001 From: Jake Potrebic Date: Mon, 31 Jul 2023 22:31:43 -0700 Subject: [PATCH 2/2] Update for latest Hypo changes --- .../src/main/kotlin/codebook.gradle.kts | 1 - .../io/papermc/codebook/lvt/LvtNamer.java | 28 ++++++------------- .../io/papermc/codebook/pages/FixJarPage.java | 21 +++++++++++--- .../codebook/pages/InspectJarPage.java | 6 +++- 4 files changed, 31 insertions(+), 25 deletions(-) diff --git a/build-logic/src/main/kotlin/codebook.gradle.kts b/build-logic/src/main/kotlin/codebook.gradle.kts index d587fd1..80e3045 100644 --- a/build-logic/src/main/kotlin/codebook.gradle.kts +++ b/build-logic/src/main/kotlin/codebook.gradle.kts @@ -8,7 +8,6 @@ plugins { } repositories { - mavenLocal() mavenCentral() maven("https://maven.parchmentmc.org") { name = "ParchmentMC" diff --git a/src/main/java/io/papermc/codebook/lvt/LvtNamer.java b/src/main/java/io/papermc/codebook/lvt/LvtNamer.java index d6ef00e..dca7245 100644 --- a/src/main/java/io/papermc/codebook/lvt/LvtNamer.java +++ b/src/main/java/io/papermc/codebook/lvt/LvtNamer.java @@ -101,7 +101,6 @@ private void fillNames0(final MethodData method) throws IOException { // If it does, we need to keep track of the LVTs we inherit @Nullable AsmMethodData outerMethod = null; int @Nullable [] outerMethodParamLvtIndices = null; - @Nullable LambdaClosure lambda = null; final @Nullable List lambdaCalls = method.get(HypoHydration.LAMBDA_CALLS); // Only track synthetic, non-synthetic means a method reference which does not behave as a closure (does not // capture LVT) @@ -117,7 +116,6 @@ private void fillNames0(final MethodData method) throws IOException { continue; } outerMethodParamLvtIndices = lambdaCall.getParamLvtIndices(); - lambda = lambdaCall; // there can only be 1 outer method break; } @@ -178,7 +176,8 @@ private void fillNames0(final MethodData method) throws IOException { // The only way to handle this kin of clash it to go back up and fix the // local variable, we can't fix it here. fixOuterScopeName( - new ClosureInfo(localClassClosure.getContainingMethod(), localClassClosure.getParamLvtIndices()), + new ClosureInfo( + localClassClosure.getContainingMethod(), localClassClosure.getParamLvtIndices()), outerLvt.name, RootLvtSuggester.determineFinalName(outerLvt.name, scopedNames), outerLvt.index); @@ -186,20 +185,10 @@ private void fillNames0(final MethodData method) throws IOException { } } - Optional methodMapping = this.mappings + final Optional methodMapping = this.mappings .getClassMapping(parentClass.name()) .flatMap(c -> c.getMethodMapping(method.name(), method.descriptorText())); - boolean usingLambdaMethodMapping = false; - if (methodMapping.isEmpty() && lambda != null && lambda.getInterfaceMethod() != null) { - final LambdaClosure finalLambda = lambda; - methodMapping = this.mappings.getClassMapping(lambda.getInterfaceMethod().parentClass().name()) - .flatMap(c -> c.getMethodMapping(finalLambda.getInterfaceMethod().name(), finalLambda.getInterfaceMethod().descriptorText())); - if (methodMapping.isPresent()) { - usingLambdaMethodMapping = true; - } - } - // If there's no LVT table there's nothing for us to process if (node.localVariables == null) { // interface / abstract methods don't have LVT @@ -294,7 +283,6 @@ private void fillNames0(final MethodData method) throws IOException { int usedNameIndex = 0; final @Nullable UsedLvtName[] usedNames = new UsedLvtName[node.localVariables.size()]; - final int parameterMappingIdxOffset = usingLambdaMethodMapping ? ourCapturedLvts.length - 1 : 0; outer: for (final LocalVariableNode lvt : node.localVariables) { if (lvt.index == 0 && !method.isStatic()) { @@ -320,7 +308,7 @@ private void fillNames0(final MethodData method) throws IOException { } final @Nullable String paramName = methodMapping - .flatMap(m -> m.getParameterMapping(lvt.index - parameterMappingIdxOffset)) + .flatMap(m -> m.getParameterMapping(lvt.index)) .map(Mapping::getDeobfuscatedName) .orElse(null); @@ -413,14 +401,16 @@ private static void fixOuterScopeName( if (lambdas != null) { for (final LambdaClosure lambda : lambdas) { if (!lambda.getContainingMethod().equals(containing)) { - innerClosureContainingMethods.add(new ClosureInfo(lambda.getContainingMethod(), lambda.getParamLvtIndices())); + innerClosureContainingMethods.add( + new ClosureInfo(lambda.getContainingMethod(), lambda.getParamLvtIndices())); } } } if (localClasses != null) { for (final LocalClassClosure localClass : localClasses) { if (!localClass.getContainingMethod().equals(containing)) { - innerClosureContainingMethods.add(new ClosureInfo(localClass.getContainingMethod(), localClass.getParamLvtIndices())); + innerClosureContainingMethods.add( + new ClosureInfo(localClass.getContainingMethod(), localClass.getParamLvtIndices())); } } } @@ -432,7 +422,7 @@ private static void fixOuterScopeName( } } - private record ClosureInfo(MethodData containing, int[] paramLvtIndices) { } + private record ClosureInfo(MethodData containing, int[] paramLvtIndices) {} private static String packageName(final ClassData classData) { final String name = classData.name(); diff --git a/src/main/java/io/papermc/codebook/pages/FixJarPage.java b/src/main/java/io/papermc/codebook/pages/FixJarPage.java index e3e05df..1fb85b4 100644 --- a/src/main/java/io/papermc/codebook/pages/FixJarPage.java +++ b/src/main/java/io/papermc/codebook/pages/FixJarPage.java @@ -22,8 +22,6 @@ package io.papermc.codebook.pages; -import static java.util.Objects.requireNonNullElse; - import com.google.common.collect.Iterables; import dev.denwav.hypo.asm.AsmClassData; import dev.denwav.hypo.asm.AsmConstructorData; @@ -42,6 +40,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.List; +import java.util.Set; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; import org.checkerframework.checker.nullness.qual.Nullable; @@ -111,8 +110,22 @@ private static void addAnnotations(final AsmClassData classData) { continue; } - final @Nullable MethodData targetMethod = method.get(HypoHydration.SYNTHETIC_SOURCE); - final MethodData baseMethod = requireNonNullElse(targetMethod, method); + final MethodData baseMethod; + final @Nullable Set syntheticSources = method.get(HypoHydration.SYNTHETIC_SOURCES); + if (syntheticSources == null) { + baseMethod = method; + } else { + outer: + { + for (final MethodData targetMethod : syntheticSources) { + if (targetMethod.parentClass().equals(method.parentClass())) { + baseMethod = targetMethod; + break outer; + } + } + return; + } + } if (baseMethod.superMethod() != null) { final MethodNode node = ((AsmMethodData) method).getNode(); diff --git a/src/main/java/io/papermc/codebook/pages/InspectJarPage.java b/src/main/java/io/papermc/codebook/pages/InspectJarPage.java index 3af1b80..1b10284 100644 --- a/src/main/java/io/papermc/codebook/pages/InspectJarPage.java +++ b/src/main/java/io/papermc/codebook/pages/InspectJarPage.java @@ -37,6 +37,7 @@ import dev.denwav.hypo.hydrate.HydrationManager; import dev.denwav.hypo.mappings.ChangeChain; import dev.denwav.hypo.mappings.MappingsCompletionManager; +import dev.denwav.hypo.mappings.contributors.CopyLambdaParametersDown; import dev.denwav.hypo.mappings.contributors.CopyMappingsDown; import dev.denwav.hypo.mappings.contributors.CopyRecordParameters; import io.papermc.codebook.exceptions.UnexpectedException; @@ -120,7 +121,10 @@ public void exec() { // Fill in any missing mapping information final MappingSet completedMappings = ChangeChain.create() - .addLink(CopyMappingsDown.createWithoutOverwrite(), CopyRecordParameters.create()) + .addLink( + CopyMappingsDown.createWithoutOverwrite(), + CopyLambdaParametersDown.createWithoutOverwrite(), + CopyRecordParameters.create()) .applyChain(lorenzMappings, MappingsCompletionManager.create(ctx)); this.bind(ParamMappings.KEY).to(completedMappings);