Skip to content

Commit

Permalink
ECJ emits different inner class references for the same code (#1635)
Browse files Browse the repository at this point in the history 
* Extract signature computation into a method of its own; Ensure nested
type references are tracked properly
* Fixes #1631
  • Loading branch information
@srikanth-sankaran
srikanth-sankaran authored Nov 29, 2023
1 parent 2e12221 commit bbe1806
Show file tree
Hide file tree
Showing 2 changed files with 160 additions and 81 deletions.
124 changes: 43 additions & 81 deletions ...e.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/MethodBinding.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1071,8 +1071,11 @@ public char[] shortReadableName() {
public final char[] signature() /* (ILjava/lang/Thread;)Ljava/lang/Object; */ {
if (this.signature != null)
return this.signature;
return computeSignature(null);
}

StringBuilder buffer = new StringBuilder(this.parameters.length + 1 * 20);
public final char[] computeSignature(ClassFile classFile) {
StringBuilder buffer = new StringBuilder((this.parameters.length + 1) * 20);
buffer.append('(');

TypeBinding[] targetParameters = this.parameters;
Expand All @@ -1081,24 +1084,39 @@ public char[] shortReadableName() {
buffer.append(ConstantPool.JavaLangStringSignature);
buffer.append(TypeBinding.INT.signature());
}
boolean needSynthetics = isConstructor && this.declaringClass.isNestedType();
boolean needSynthetics = isConstructor
&& this.declaringClass.isNestedType()
&& !this.declaringClass.isStatic();
if (needSynthetics) {
// take into account the synthetic argument type signatures as well
ReferenceBinding[] syntheticArgumentTypes = this.declaringClass.syntheticEnclosingInstanceTypes();
if (syntheticArgumentTypes != null) {
for (int i = 0, count = syntheticArgumentTypes.length; i < count; i++) {
buffer.append(syntheticArgumentTypes[i].signature());
ReferenceBinding syntheticArgumentType = syntheticArgumentTypes[i];
if ((syntheticArgumentType.tagBits & TagBits.ContainsNestedTypeReferences) != 0) {
this.tagBits |= TagBits.ContainsNestedTypeReferences;
if (classFile != null)
Util.recordNestedType(classFile, syntheticArgumentType);
}
buffer.append(syntheticArgumentType.signature());
}
}

if ((this instanceof SyntheticMethodBinding) && (!this.declaringClass.isRecord())) {
if (this instanceof SyntheticMethodBinding) {
targetParameters = ((SyntheticMethodBinding)this).targetMethod.parameters;
}
}

if (targetParameters != Binding.NO_PARAMETERS) {
for (int i = 0; i < targetParameters.length; i++) {
buffer.append(targetParameters[i].signature());
for (int i = 0, max = targetParameters.length; i < max; i++) {
TypeBinding targetParameter = targetParameters[i];
TypeBinding leafTargetParameterType = targetParameter.leafComponentType();
if ((leafTargetParameterType.tagBits & TagBits.ContainsNestedTypeReferences) != 0) {
this.tagBits |= TagBits.ContainsNestedTypeReferences;
if (classFile != null)
Util.recordNestedType(classFile, leafTargetParameterType);
}
buffer.append(targetParameter.signature());
}
}
if (needSynthetics) {
Expand All @@ -1109,18 +1127,33 @@ public char[] shortReadableName() {
}
// move the extra padding arguments of the synthetic constructor invocation to the end
for (int i = targetParameters.length, extraLength = this.parameters.length; i < extraLength; i++) {
buffer.append(this.parameters[i].signature());
TypeBinding parameter = this.parameters[i];
TypeBinding leafParameterType = parameter.leafComponentType();
if ((leafParameterType.tagBits & TagBits.ContainsNestedTypeReferences) != 0) {
this.tagBits |= TagBits.ContainsNestedTypeReferences;
if (classFile != null)
Util.recordNestedType(classFile, leafParameterType);
}
buffer.append(parameter.signature());
}
}
buffer.append(')');
if (this.returnType != null)
if (this.returnType != null) {
TypeBinding ret = this.returnType.leafComponentType();
if ((ret.tagBits & TagBits.ContainsNestedTypeReferences) != 0) {
this.tagBits |= TagBits.ContainsNestedTypeReferences;
if (classFile != null)
Util.recordNestedType(classFile, ret);
}
buffer.append(this.returnType.signature());
}
int nameLength = buffer.length();
this.signature = new char[nameLength];
buffer.getChars(0, nameLength, this.signature, 0);

return this.signature;
}

/*
* This method is used to record references to nested types inside the method signature.
* This is the one that must be used during code generation.
Expand Down Expand Up @@ -1181,79 +1214,7 @@ public char[] signature(ClassFile classFile) {
return this.signature;
}

StringBuilder buffer = new StringBuilder((this.parameters.length + 1) * 20);
buffer.append('(');

TypeBinding[] targetParameters = this.parameters;
boolean isConstructor = isConstructor();
if (isConstructor && this.declaringClass.isEnum()) { // insert String name,int ordinal
buffer.append(ConstantPool.JavaLangStringSignature);
buffer.append(TypeBinding.INT.signature());
}
boolean needSynthetics = isConstructor
&& this.declaringClass.isNestedType()
&& !this.declaringClass.isStatic();
if (needSynthetics) {
// take into account the synthetic argument type signatures as well
ReferenceBinding[] syntheticArgumentTypes = this.declaringClass.syntheticEnclosingInstanceTypes();
if (syntheticArgumentTypes != null) {
for (int i = 0, count = syntheticArgumentTypes.length; i < count; i++) {
ReferenceBinding syntheticArgumentType = syntheticArgumentTypes[i];
if ((syntheticArgumentType.tagBits & TagBits.ContainsNestedTypeReferences) != 0) {
this.tagBits |= TagBits.ContainsNestedTypeReferences;
Util.recordNestedType(classFile, syntheticArgumentType);
}
buffer.append(syntheticArgumentType.signature());
}
}

if (this instanceof SyntheticMethodBinding) {
targetParameters = ((SyntheticMethodBinding)this).targetMethod.parameters;
}
}

if (targetParameters != Binding.NO_PARAMETERS) {
for (int i = 0, max = targetParameters.length; i < max; i++) {
TypeBinding targetParameter = targetParameters[i];
TypeBinding leafTargetParameterType = targetParameter.leafComponentType();
if ((leafTargetParameterType.tagBits & TagBits.ContainsNestedTypeReferences) != 0) {
this.tagBits |= TagBits.ContainsNestedTypeReferences;
Util.recordNestedType(classFile, leafTargetParameterType);
}
buffer.append(targetParameter.signature());
}
}
if (needSynthetics) {
SyntheticArgumentBinding[] syntheticOuterArguments = this.declaringClass.syntheticOuterLocalVariables();
int count = syntheticOuterArguments == null ? 0 : syntheticOuterArguments.length;
for (int i = 0; i < count; i++) {
buffer.append(syntheticOuterArguments[i].type.signature());
}
// move the extra padding arguments of the synthetic constructor invocation to the end
for (int i = targetParameters.length, extraLength = this.parameters.length; i < extraLength; i++) {
TypeBinding parameter = this.parameters[i];
TypeBinding leafParameterType = parameter.leafComponentType();
if ((leafParameterType.tagBits & TagBits.ContainsNestedTypeReferences) != 0) {
this.tagBits |= TagBits.ContainsNestedTypeReferences;
Util.recordNestedType(classFile, leafParameterType);
}
buffer.append(parameter.signature());
}
}
buffer.append(')');
if (this.returnType != null) {
TypeBinding ret = this.returnType.leafComponentType();
if ((ret.tagBits & TagBits.ContainsNestedTypeReferences) != 0) {
this.tagBits |= TagBits.ContainsNestedTypeReferences;
Util.recordNestedType(classFile, ret);
}
buffer.append(this.returnType.signature());
}
int nameLength = buffer.length();
this.signature = new char[nameLength];
buffer.getChars(0, nameLength, this.signature, 0);

return this.signature;
return computeSignature(classFile);
}
public final int sourceEnd() {
AbstractMethodDeclaration method = sourceMethod();
Expand Down Expand Up @@ -1495,3 +1456,4 @@ public boolean hasPolymorphicSignature(Scope scope) {
return false;
}
}

117 changes: 117 additions & 0 deletions ...pse.jdt.core.tests.builder/src/org/eclipse/jdt/core/tests/builder/IncrementalTests18.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
package org.eclipse.jdt.core.tests.builder;

import java.io.File;
import java.io.IOException;

import junit.framework.Test;

import org.eclipse.core.runtime.FileLocator;
Expand All @@ -24,8 +26,11 @@
import org.eclipse.jdt.core.IJavaProject;
import org.eclipse.jdt.core.JavaCore;
import org.eclipse.jdt.core.JavaModelException;
import org.eclipse.jdt.core.ToolFactory;
import org.eclipse.jdt.core.tests.util.AbstractCompilerTest;
import org.eclipse.jdt.core.tests.util.Util;
import org.eclipse.jdt.core.util.ClassFileBytesDisassembler;
import org.eclipse.jdt.core.util.ClassFormatException;
import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants;
import org.osgi.framework.Bundle;

Expand Down Expand Up @@ -809,4 +814,116 @@ public void testBug483744_remove() throws JavaModelException {
incrementalBuild(projectPath); // was throwing NPE
expectingNoProblems();
}

public void testGH1631() throws JavaModelException, IOException, ClassFormatException {
IPath projectPath = env.addProject("Project", "1.8");
env.addExternalJars(projectPath, Util.getJavaClassLibs());
env.setOutputFolder(projectPath, "bin");

env.addClass(projectPath, "", "AThreeWaySynchronizer",
"public class AThreeWaySynchronizer {\n" +
" \n" +
" public final BatchingLock.IFlushOperation flushOperation = (info, monitor) -> {\n" +
" if (info != null && !info.isEmpty()) {\n" +
" broadcastSyncChanges(info.getChangedResources());\n" +
" }\n" +
" };\n" +
"\n" +
" private void broadcastSyncChanges(final IResource[] resources) {\n" +
" \n" +
" }\n" +
"}\n");

env.addClass(projectPath, "", "BatchingLock",
"interface IResource {}\n" +
"interface IProgressMonitor {}\n" +
"\n" +
"public class BatchingLock { \n" +
"\n" +
" public static class ThreadInfo {\n" +
" \n" +
" public boolean isEmpty() {\n" +
" return true;\n" +
" }\n" +
" public IResource[] getChangedResources() {\n" +
" return null;\n" +
" } \n" +
" } \n" +
" \n" +
" public interface IFlushOperation {\n" +
" public void flush(ThreadInfo info, IProgressMonitor monitor);\n" +
" }\n" +
"}\n");

fullBuild(projectPath);
expectingNoProblems();

IPath classFilePath = env.getOutputLocation(projectPath).append("BatchingLock$IFlushOperation.class");

File classFile = env.getWorkspaceRootPath().append(classFilePath).toFile();
ClassFileBytesDisassembler disassembler = ToolFactory.createDefaultClassFileBytesDisassembler();
byte[] classFileBytes = org.eclipse.jdt.internal.compiler.util.Util.getFileByteContent(classFile);
String actualOutput =
disassembler.disassemble(
classFileBytes,
"\n",
ClassFileBytesDisassembler.DETAILED);

String expectedOutput =
"// Compiled from BatchingLock.java (version 1.8 : 52.0, no super bit)\n" +
"public abstract static interface BatchingLock$IFlushOperation {\n" +
" \n" +
" // Method descriptor #6 (LBatchingLock$ThreadInfo;LIProgressMonitor;)V\n" +
" public abstract void flush(BatchingLock.ThreadInfo arg0, IProgressMonitor arg1);\n" +
"\n" +
" Inner classes:\n" +
" [inner class info: #1 BatchingLock$IFlushOperation, outer class info: #10 BatchingLock\n" +
" inner name: #12 IFlushOperation, accessflags: 1545 public abstract static],\n" +
" [inner class info: #13 BatchingLock$ThreadInfo, outer class info: #10 BatchingLock\n" +
" inner name: #15 ThreadInfo, accessflags: 9 public static]\n" +
"}";

if (!actualOutput.equals(expectedOutput)) {
assertEquals("Wrong contents", expectedOutput, actualOutput);
}

// introduce a non-material change to trigger incremental build

env.addClass(projectPath, "", "BatchingLock",
"interface IResource {}\n" +
"interface IProgressMonitor {}\n" +
"\n" +
"\n" + // extra line feed
"public class BatchingLock { \n" +
"\n" +
" public static class ThreadInfo {\n" +
" \n" +
" public boolean isEmpty() {\n" +
" return true;\n" +
" }\n" +
" public IResource[] getChangedResources() {\n" +
" return null;\n" +
" } \n" +
" } \n" +
" \n" +
" public interface IFlushOperation {\n" +
" public void flush(ThreadInfo info, IProgressMonitor monitor);\n" +
" }\n" +
"}\n");


incrementalBuild(projectPath);
expectingNoProblems();

classFileBytes = org.eclipse.jdt.internal.compiler.util.Util.getFileByteContent(classFile);
actualOutput =
disassembler.disassemble(
classFileBytes,
"\n",
ClassFileBytesDisassembler.DETAILED);

if (!actualOutput.equals(expectedOutput)) {
assertEquals("Wrong contents", expectedOutput, actualOutput);
}
}
}

0 comments on commit bbe1806

Please sign in to comment.