From 0f18a73982fac6a7076ad58fbc874de405445ffe Mon Sep 17 00:00:00 2001 From: Srikanth Sankaran Date: Tue, 28 Nov 2023 16:06:14 +0530 Subject: [PATCH 1/2] Regression test that demonstrates current behavior --- .../tests/builder/IncrementalTests18.java | 129 ++++++++++++++++++ 1 file changed, 129 insertions(+) diff --git a/org.eclipse.jdt.core.tests.builder/src/org/eclipse/jdt/core/tests/builder/IncrementalTests18.java b/org.eclipse.jdt.core.tests.builder/src/org/eclipse/jdt/core/tests/builder/IncrementalTests18.java index 9f08bb6ea83..0db4ddd31c3 100644 --- a/org.eclipse.jdt.core.tests.builder/src/org/eclipse/jdt/core/tests/builder/IncrementalTests18.java +++ b/org.eclipse.jdt.core.tests.builder/src/org/eclipse/jdt/core/tests/builder/IncrementalTests18.java @@ -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; @@ -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; @@ -809,4 +814,128 @@ 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" + + "}"; + + 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); + + 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); + } + } } From 895d4406c0e0edb599d1ced9f74862732b011c43 Mon Sep 17 00:00:00 2001 From: Srikanth Sankaran Date: Wed, 29 Nov 2023 11:35:06 +0530 Subject: [PATCH 2/2] Extract signature computation into a method of its own; Ensure nested type references are tracked properly --- .../compiler/lookup/MethodBinding.java | 124 ++++++------------ .../tests/builder/IncrementalTests18.java | 18 +-- 2 files changed, 46 insertions(+), 96 deletions(-) diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/MethodBinding.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/MethodBinding.java index ee821a535c0..986da77de3b 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/MethodBinding.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/MethodBinding.java @@ -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; @@ -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) { @@ -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. @@ -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(); @@ -1495,3 +1456,4 @@ public boolean hasPolymorphicSignature(Scope scope) { return false; } } + diff --git a/org.eclipse.jdt.core.tests.builder/src/org/eclipse/jdt/core/tests/builder/IncrementalTests18.java b/org.eclipse.jdt.core.tests.builder/src/org/eclipse/jdt/core/tests/builder/IncrementalTests18.java index 0db4ddd31c3..49aec360d55 100644 --- a/org.eclipse.jdt.core.tests.builder/src/org/eclipse/jdt/core/tests/builder/IncrementalTests18.java +++ b/org.eclipse.jdt.core.tests.builder/src/org/eclipse/jdt/core/tests/builder/IncrementalTests18.java @@ -878,7 +878,9 @@ public void testGH1631() throws JavaModelException, IOException, ClassFormatExce "\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 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)) { @@ -920,20 +922,6 @@ public void testGH1631() throws JavaModelException, IOException, ClassFormatExce "\n", ClassFileBytesDisassembler.DETAILED); - 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); }