Skip to content

Commit

Permalink
improve handling of synthetic parameters and parameter names
Browse files Browse the repository at this point in the history 
These 2 changes in JDK 18 and JDK 21 require significant changes to
how synthetic/mandated parameters and parameters names are handled:

- https://bugs.openjdk.org/browse/JDK-8271623
- https://bugs.openjdk.org/browse/JDK-8292275

Therefore, with this commit, the `Indexer` no longer stores just method
parameter name arrays. Instead, for each `MethodInfo` from currently
processed class, it stores a `MethodParamList`, which contains 2 lists
of `ParamData`, one based on the `MethodParameters` bytecode attribute
and the other based on the local variable table. Those lists contain,
for each method parameter, its name and synthetic/mandated flag.

When the `Indexer` is done parsing a method, the `MethodParamList`
is finished, which means computing:

- whether the method is a constructor whose first parameter is the
  enclosing instance synthetic/mandated parameter;
- list of parameters that are possibly named (synthetic/mandated
  parameters are ignored).

This information is then used:

- to compute the final array of parameter names (preferring the proper
  name over the debug name);
- as part of a heuristic that figures out if a constructor belongs
  to an "inner" class (in case of local/anonymous classes, that in fact
  means figuring out if they were declared in non-static context).
  • Loading branch information
@Ladicek
Ladicek committed Jun 8, 2023
1 parent 57cd32c commit 5f044c1
Show file tree
Hide file tree
Showing 2 changed files with 252 additions and 65 deletions.
149 changes: 84 additions & 65 deletions core/src/main/java/org/jboss/jandex/Indexer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -396,8 +396,7 @@ void returnConstantAnnoAttributes(byte[] attributes) {
private List<MethodInfo> methods;
private List<FieldInfo> fields;
private List<RecordComponentInfo> recordComponents;
private byte[][] debugParameterNames;
private byte[][] methodParameterNames;
private IdentityHashMap<MethodInfo, MethodParamList> methodParams;
private List<DotName> modulePackages;
private DotName moduleMainClass;

Expand Down Expand Up @@ -453,6 +452,8 @@ private void initClassFields() {
// in bytecode, record components are stored as class attributes,
// and if the attribute is missing, processRecordComponents isn't called at all
recordComponents = new ArrayList<RecordComponentInfo>();

methodParams = new IdentityHashMap<>();
}

private void processMethodInfo(DataInputStream data) throws IOException {
Expand All @@ -475,16 +476,17 @@ private void processMethodInfo(DataInputStream data) throws IOException {
if (parameters.length == 0 && Arrays.equals(Utils.INIT_METHOD_NAME, name)) {
currentClass.setHasNoArgsConstructor(true);
}
methodParameterNames = debugParameterNames = null;
MethodParamList parameterList = new MethodParamList(method);
methodParams.put(method, parameterList);
processAttributes(data, method);
method.setAnnotations(elementAnnotations);
elementAnnotations.clear();
parameterList.finish();

// Prefer method parameter names over debug info
if (methodParameterNames != null)
method.methodInternal().setParameterNames(methodParameterNames);
else if (debugParameterNames != null)
method.methodInternal().setParameterNames(debugParameterNames);
byte[][] parameterNames = parameterList.getNames();
if (parameterNames != null) {
method.methodInternal().setParameterNames(parameterNames);
}

methods.add(method);
}
Expand Down Expand Up @@ -782,30 +784,20 @@ private void processMethodParameters(DataInputStream data, MethodInfo target) th
// the one we just read (for extra safety, do NOT do this for compliant methods)
numParameters = target.parametersCount();
}
byte[][] parameterNames = numParameters > 0 ? new byte[numParameters][] : MethodInternal.EMPTY_PARAMETER_NAMES;
int filledParameters = 0;
for (int i = 0; i < numParameters; i++) {
int nameIndex = data.readUnsignedShort();
byte[] parameterName = nameIndex == 0 ? null : decodeUtf8EntryAsBytes(nameIndex);
int flags = data.readUnsignedShort();
// skip synthetic/mandated params to get the same param name index as annotations (which do not count them)
if ((flags & (Modifiers.SYNTHETIC | Modifiers.MANDATED)) != 0) {
continue;
}
boolean syntheticOrMandated = (flags & (Modifiers.SYNTHETIC | Modifiers.MANDATED)) != 0;

parameterNames[filledParameters++] = parameterName;
if (methodParams.containsKey(target)) { // should always be there, just being extra careful
methodParams.get(target).appendProper(parameterName, syntheticOrMandated);
}
}

byte[][] realParameterNames = filledParameters > 0 ? new byte[filledParameters][]
: MethodInternal.EMPTY_PARAMETER_NAMES;
if (filledParameters > 0)
System.arraycopy(parameterNames, 0, realParameterNames, 0, filledParameters);
this.methodParameterNames = realParameterNames;
}

private void processLocalVariableTable(DataInputStream data, MethodInfo target) throws IOException {
int numVariables = data.readUnsignedShort();
byte[][] variableNames = numVariables > 0 ? new byte[numVariables][] : MethodInternal.EMPTY_PARAMETER_NAMES;
int numParameters = 0;
for (int i = 0; i < numVariables; i++) {
int startPc = data.readUnsignedShort();
Expand All @@ -815,32 +807,38 @@ private void processLocalVariableTable(DataInputStream data, MethodInfo target)
int index = data.readUnsignedShort();

// parameters have startPc == 0
if (startPc != 0)
if (startPc != 0) {
continue;
}

byte[] parameterName = nameIndex == 0 ? null : decodeUtf8EntryAsBytes(nameIndex);

// ignore "this"
if (numParameters == 0 && parameterName != null && parameterName.length == 4
&& parameterName[0] == 0x74
&& parameterName[1] == 0x68
&& parameterName[2] == 0x69
&& parameterName[3] == 0x73)
&& parameterName[3] == 0x73) {
continue;
// ignore "this$*" that javac adds (not ECJ)
}

// treat "this$*" that javac adds (not ecj) as synthetic (or mandated)
boolean synthetic = false;
if (numParameters == 0 && parameterName != null && parameterName.length > 5
&& parameterName[0] == 0x74
&& parameterName[1] == 0x68
&& parameterName[2] == 0x69
&& parameterName[3] == 0x73
&& parameterName[4] == 0x24)
continue;
&& parameterName[4] == 0x24) {
synthetic = true;
}

if (methodParams.containsKey(target)) { // should always be there, just being extra careful
methodParams.get(target).appendDebug(parameterName, synthetic);
}

// here we rely on the parameters being in the right order
variableNames[numParameters++] = parameterName;
numParameters++;
}
byte[][] parameterNames = numParameters > 0 ? new byte[numParameters][] : MethodInternal.EMPTY_PARAMETER_NAMES;
if (numParameters > 0)
System.arraycopy(variableNames, 0, parameterNames, 0, numParameters);
this.debugParameterNames = parameterNames;
}

private void processEnclosingMethod(DataInputStream data, ClassInfo target) throws IOException {
Expand Down Expand Up @@ -984,9 +982,14 @@ private void adjustMethodParameters() {
}
alreadyProcessedMethods.put(method, null);

if (isInnerConstructor(method) && !signaturePresent.containsKey(method)) {
// inner class constructor without signature, list of parameter types is derived
// from the descriptor, which includes 1 mandated or synthetic parameter at the beginning
if (signaturePresent.containsKey(method)) {
// if the method has a signature, we derived the parameter list from it,
// and it never includes mandated or synthetic parameters
continue;
}

if (isInnerConstructor(method)) {
// inner class constructor, descriptor includes 1 mandated or synthetic parameter at the beginning
DotName enclosingClass = null;
if (method.declaringClass().enclosingClass() != null) {
enclosingClass = method.declaringClass().enclosingClass();
Expand All @@ -1000,10 +1003,8 @@ private void adjustMethodParameters() {
System.arraycopy(parameterTypes, 1, newParameterTypes, 0, parameterTypes.length - 1);
method.setParameters(intern(newParameterTypes));
}
} else if (isEnumConstructor(method) && !signaturePresent.containsKey(method)) {
// enum constructor without signature, list of parameter types is derived from the descriptor,
// which includes 2 synthetic parameters at the beginning -- let's remove those
// (javac typically emits the signature, while ecj does not)
} else if (isEnumConstructor(method)) {
// enum constructor, descriptor includes 2 synthetic parameters at the beginning
Type[] parameterTypes = method.methodInternal().parameterTypesArray();
if (parameterTypes.length >= 2
&& parameterTypes[0].kind() == Type.Kind.CLASS
Expand All @@ -1023,6 +1024,48 @@ private void adjustMethodParameters() {
}
}

private boolean isInnerConstructor(MethodInfo method) {
if (!method.isConstructor()) {
return false;
}

ClassInfo klass = method.declaringClass();

// there's no indication in the class file whether a local or anonymous class was declared in static context,
// so we use some heuristics here
if (klass.nestingType() == ClassInfo.NestingType.LOCAL
|| klass.nestingType() == ClassInfo.NestingType.ANONYMOUS) {

// synthetic or mandated parameter at the beginning of the parameter list is the enclosing instance,
// the constructor belongs to a "non-static" class
//
// this works for:
// - javac with `-g` or `-parameters`
// - javac since JDK 21 (see https://bugs.openjdk.org/browse/JDK-8292275)
// - ecj with `-parameters` (see also `processLocalVariableTable`)
MethodParamList parameters = methodParams.get(method);
if (parameters != null && parameters.firstIsEnclosingInstance()) {
return true;
}

// synthetic field named `this$*` is the enclosing instance,
// the constructor belongs to a "non-static" class
//
// this works for:
// - javac until JDK 18 (see https://bugs.openjdk.org/browse/JDK-8271623)
// - ecj
for (FieldInfo field : fields) {
if (Modifiers.isSynthetic(field.flags()) && field.name().startsWith("this$")) {
return true;
}
}

return false;
}

return klass.nestingType() == ClassInfo.NestingType.INNER && !Modifier.isStatic(klass.flags());
}

private static boolean isEnumConstructor(MethodInfo method) {
return method.declaringClass().isEnum() && method.isConstructor();
}
Expand Down Expand Up @@ -1102,29 +1145,6 @@ private static void setTypeParameters(AnnotationTarget target, Type[] typeParame

}

private boolean isInnerConstructor(MethodInfo method) {
if (!method.isConstructor()) {
return false;
}

ClassInfo klass = method.declaringClass();

// there's no indication in the class file whether a local or anonymous class was declared in static context,
// so we use a heuristic: a class was not declared in static context if it has a synthetic field named `this$N`
// (this seems to work both for javac and ecj)
if (klass.nestingType() == ClassInfo.NestingType.LOCAL
|| klass.nestingType() == ClassInfo.NestingType.ANONYMOUS) {
for (FieldInfo field : fields) {
if (Modifiers.isSynthetic(field.flags()) && field.name().startsWith("this$")) {
return true;
}
}
return false;
}

return klass.nestingType() == ClassInfo.NestingType.INNER && !Modifier.isStatic(klass.flags());
}

private void resolveTypeAnnotation(AnnotationTarget target, TypeAnnotationState typeAnnotationState) {
// Signature is erroneously omitted from bridge methods with generic type annotations
if (typeAnnotationState.genericsRequired && !signaturePresent.containsKey(target)) {
Expand Down Expand Up @@ -2510,8 +2530,7 @@ public ClassSummary indexWithSummary(InputStream stream) throws IOException {
methods = null;
fields = null;
recordComponents = null;
debugParameterNames = null;
methodParameterNames = null;
methodParams = null;
modulePackages = null;
moduleMainClass = null;
}
Expand Down
Loading

0 comments on commit 5f044c1

Please sign in to comment.