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.

Sign up for GitHub

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

ECJ emits different inner class references for the same code #1635

Merged
merged 2 commits into from
Nov 29, 2023
Merged

ECJ emits different inner class references for the same code #1635

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0f18a73
Regression test that demonstrates current behavior
srikanth-sankaran Nov 28, 2023
895d440
Extract signature computation into a method of its own; Ensure nested
srikanth-sankaran Nov 29, 2023
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
124 changes: 43 additions & 81 deletions ...e.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/MethodBinding.java
View file
Open in desktop
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
Open in desktop
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);
}
}
}