Skip to content
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

Fix LVT index calculation for long and double parameters #7

Merged
merged 2 commits into from
Dec 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 30 additions & 13 deletions api/src/main/java/net/neoforged/jst/api/PsiHelper.java
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package net.neoforged.jst.api;

import com.intellij.lang.jvm.types.JvmPrimitiveTypeKind;
import com.intellij.psi.PsiClass;
import com.intellij.psi.PsiLambdaExpression;
import com.intellij.psi.PsiMethod;
import com.intellij.psi.PsiModifier;
import com.intellij.psi.PsiParameter;
import com.intellij.psi.PsiParameterListOwner;
import com.intellij.psi.PsiPrimitiveType;
import com.intellij.psi.PsiTypes;
import com.intellij.psi.SyntaxTraverser;
import com.intellij.psi.util.CachedValueProvider;
Expand Down Expand Up @@ -114,28 +116,43 @@ private static boolean isNonStaticInnerClassConstructor(PsiMethod method) {
return false;
}

public static int getBinaryIndex(PsiParameter psiParameter, int index) {
var declarationScope = psiParameter.getDeclarationScope();
if (declarationScope instanceof PsiMethod psiMethod) {
/**
* Gets the local variable table indices of the parameters for the given method
* or lambda expression
*/
public static int[] getParameterLvtIndices(PsiParameterListOwner methodOrLambda) {

// Account for hidden parameters before the first actual parameter
int currentIndex = 0;
if (methodOrLambda instanceof PsiMethod psiMethod) {
if (!psiMethod.hasModifierProperty(PsiModifier.STATIC)) {
index++; // this pointer
currentIndex++; // this pointer
}

// Try to account for hidden parameters only present in bytecode since the
// mapping data refers to parameters using those indices
if (isEnumConstructor(psiMethod)) {
index += 2;
currentIndex += 2;
} else if (isNonStaticInnerClassConstructor(psiMethod)) {
index += 1;
currentIndex += 1;
}
}

return index;
} else if (declarationScope instanceof PsiLambdaExpression psiLambda) {
// Naming lambdas doesn't really work
return index;
} else {
return -1;
var parameters = methodOrLambda.getParameterList().getParameters();
var lvti = new int[parameters.length];
for (int i = 0; i < lvti.length; i++) {
lvti[i] = currentIndex++;
// double and long use 2 slots in the LVT
if (parameters[i].getType() instanceof PsiPrimitiveType primitiveType) {
var kind = primitiveType.getKind();
if (kind == JvmPrimitiveTypeKind.LONG || kind == JvmPrimitiveTypeKind.DOUBLE) {
currentIndex++;
}
}
}

return lvti;

}

public static boolean isRecordConstructor(PsiMethod psiMethod) {
Expand Down
1 change: 1 addition & 0 deletions cli/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ dependencies {

testImplementation platform("org.junit:junit-bom:$junit_version")
testImplementation 'org.junit.jupiter:junit-jupiter'
testImplementation "org.assertj:assertj-core:$assertj_version"
}

test {
Expand Down
52 changes: 31 additions & 21 deletions cli/src/test/java/net/neoforged/jst/cli/PsiHelperTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import java.io.IOException;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;

class PsiHelperTest {
Expand Down Expand Up @@ -66,13 +67,12 @@ void testSignature() {
}

@Test
void testMethodParameterIndex() {
var firstParam = ctor.getParameterList().getParameter(0);
int index = PsiHelper.getBinaryIndex(firstParam, 0);
// Binary parameters are:
void testMethodParameterIndices() {
// LVT of method should be:
// 0) this
// 1) first method parameter
assertEquals(1, index);
assertThat(PsiHelper.getParameterLvtIndices(ctor))
.containsExactly(1);
}
}

Expand All @@ -95,12 +95,11 @@ void testSignature() {
}

@Test
void testMethodParameterIndex() {
var firstParam = ctor.getParameterList().getParameter(0);
int index = PsiHelper.getBinaryIndex(firstParam, 0);
void testMethodParameterIndices() {
// Binary parameters are:
// 0) first method parameter
assertEquals(0, index);
assertThat(PsiHelper.getParameterLvtIndices(ctor))
.containsExactly(0);
}
}

Expand All @@ -124,15 +123,14 @@ void testSignature() {
}

@Test
void testMethodParameterIndex() {
var firstParam = ctor.getParameterList().getParameter(0);
int index = PsiHelper.getBinaryIndex(firstParam, 0);
void testMethodParameterIndices() {
// Binary parameters are:
// 0) this
// 1) enum literal name
// 2) enum literal ordinal
// 3) first method parameter
assertEquals(3, index);
assertThat(PsiHelper.getParameterLvtIndices(ctor))
.containsExactly(3);
}
}

Expand All @@ -157,14 +155,13 @@ void testSignature() {
}

@Test
void testMethodParameterIndex() {
var firstParam = ctor.getParameterList().getParameter(0);
int index = PsiHelper.getBinaryIndex(firstParam, 0);
void testMethodParameterIndices() {
// Binary parameters are:
// 0) this
// 1) outer class pointer
// 2) first method parameter
assertEquals(2, index);
assertThat(PsiHelper.getParameterLvtIndices(ctor))
.containsExactly(2);
}
}

Expand All @@ -189,16 +186,29 @@ void testSignature() {
}

@Test
void testMethodParameterIndex() {
var firstParam = ctor.getParameterList().getParameter(0);
int index = PsiHelper.getBinaryIndex(firstParam, 0);
void testMethodParameterIndices() {
// Binary parameters are:
// 0) this
// 1) first method parameter
assertEquals(1, index);
assertThat(PsiHelper.getParameterLvtIndices(ctor))
.containsExactly(1);
}
}

@Test
void testLvtIndicesForPrimitiveTypes() {
var m = parseSingleMethod("""
class Outer {
static void m(byte p1, short p2, int p3, long p4, float p5, double p6, boolean p7) {
}
}
""");

assertEquals("(BSIJFDZ)V", PsiHelper.getBinaryMethodSignature(m));
assertThat(PsiHelper.getParameterLvtIndices(m))
.containsExactly(0, 1, 2, 3, 5, 6, 8);
}

private PsiMethod parseSingleMethod(@Language("JAVA") String javaCode) {
return parseSingleElement(javaCode, PsiMethod.class);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ public void visitElement(@NotNull PsiElement element) {
Map<String, String> renamedParameters = new HashMap<>();
List<String> parameterOrder = new ArrayList<>();

PsiParameter[] parameters = psiMethod.getParameterList().getParameters();
var parameters = psiMethod.getParameterList().getParameters();
var parametersLvtIndices = PsiHelper.getParameterLvtIndices(psiMethod);
boolean hadReplacements = false;
for (int i = 0; i < parameters.length; i++) {
var psiParameter = parameters[i];
Expand All @@ -90,7 +91,7 @@ public void visitElement(@NotNull PsiElement element) {

// Parchment stores parameter indices based on the index of the parameter in the actual compiled method
// to account for synthetic parameter not found in the source-code, we must adjust the index accordingly.
var jvmIndex = PsiHelper.getBinaryIndex(psiParameter, i);
var jvmIndex = parametersLvtIndices[i];

var paramData = methodData.getParameter(jvmIndex);
// Optionally replace the parameter name, but skip record constructors, since those could have
Expand Down
1 change: 1 addition & 0 deletions tests/data/param_indices/expected/TestClass.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ public class TestClass {
public TestClass(int mapped) {}
public void instanceMethod(int mapped) {}
public static void staticMethod(int mapped) {}
public static void staticMethodWithLongAndDouble(int mappedInt, long mappedLong, double mappedDouble, float mappedFloat) {}
public class InnerClass {
public InnerClass(int mapped) {}
public void instanceMethod(int mapped) {}
Expand Down
22 changes: 22 additions & 0 deletions tests/data/param_indices/parchment.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,28 @@
"name": "mapped"
}
]
},
{
"name": "staticMethodWithLongAndDouble",
"descriptor": "(IJDF)V",
"parameters": [
{
"index": 0,
"name": "mappedInt"
},
{
"index": 1,
"name": "mappedLong"
},
{
"index": 3,
"name": "mappedDouble"
},
{
"index": 5,
"name": "mappedFloat"
}
]
}
]
},
Expand Down
1 change: 1 addition & 0 deletions tests/data/param_indices/source/TestClass.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ public class TestClass {
public TestClass(int p) {}
public void instanceMethod(int p) {}
public static void staticMethod(int p) {}
public static void staticMethodWithLongAndDouble(int p1, long p2, double p3, float p4) {}
public class InnerClass {
public InnerClass(int p) {}
public void instanceMethod(int p) {}
Expand Down