Skip to content

Commit

Permalink
Generate reference to classes holding constants used in the code (ecl…
Browse files Browse the repository at this point in the history
…ipse-jdt#2341)

fixes eclipse-jdt#1382

Emit a class constant into the constant pool for every class from which
a constant is used.

Touch bundles affected by this change
  • Loading branch information
stephan-herrmann committed Apr 15, 2024
1 parent a68092e commit 204b598
Show file tree
Hide file tree
Showing 7 changed files with 169 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,17 @@ private void generateCode(
codeStream.recordPositionsFrom(0, declaringType.sourceStart);
classFile.completeCodeAttributeForClinit(codeAttributeOffset, classScope);
}
// the following block must happen after constantPool.resetForClinit()
if (TypeDeclaration.kind(declaringType.modifiers) != TypeDeclaration.ENUM_DECL
&& fieldDeclarations != null) {
int constantFlags = ClassFileConstants.AccStatic | ClassFileConstants.AccFinal;
for (FieldDeclaration fieldDecl : fieldDeclarations) {
if ((fieldDecl.modifiers & constantFlags) == constantFlags
&& fieldDecl.initialization != null) {
NameReference.emitDeclaringClassOfConstant(fieldDecl.initialization, codeStream);
}
}
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2000, 2021 IBM Corporation and others.
* Copyright (c) 2000, 2024 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand All @@ -21,6 +21,9 @@

import java.util.function.Predicate;

import org.eclipse.jdt.internal.compiler.ASTVisitor;
import org.eclipse.jdt.internal.compiler.codegen.CodeStream;
import org.eclipse.jdt.internal.compiler.impl.Constant;
import org.eclipse.jdt.internal.compiler.lookup.*;
import org.eclipse.jdt.internal.compiler.problem.AbortMethod;

Expand All @@ -41,6 +44,40 @@ public NameReference() {
this.bits |= Binding.TYPE | Binding.VARIABLE; // restrictiveFlag
}

/**
* Creates a constant pool entry which is not needed by the VM but might help tools.
* See https://bugs.openjdk.org/browse/JDK-7153958
*/
public void emitDeclaringClassOfConstant(CodeStream codeStream) {
if (this.constant != Constant.NotAConstant && this.binding instanceof FieldBinding f) {
codeStream.constantPool.literalIndexForType(f.declaringClass);
}
}
/**
* Creates a constant pool entry for each constant reference within expr.
* This is not needed by the VM but might help tools.
* See https://bugs.openjdk.org/browse/JDK-7153958
*/
public static void emitDeclaringClassOfConstant(Expression expr, CodeStream codeStream) {
if (expr instanceof Literal)
return;
expr.traverse(
new ASTVisitor() {
@Override
public boolean visit(SingleNameReference nameReference, BlockScope scope) {
nameReference.emitDeclaringClassOfConstant(codeStream);
return false;
}
@Override
public boolean visit(QualifiedNameReference nameReference, BlockScope scope) {
nameReference.emitDeclaringClassOfConstant(codeStream);
return false;
}
},
(BlockScope) null
);
}

/**
* Use this method only when sure that the current reference is <strong>not</strong>
* a chain of several fields (QualifiedNameReference with more than one field).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,7 @@ public void generateAssignment(BlockScope currentScope, CodeStream codeStream, A

@Override
public void generateCode(BlockScope currentScope, CodeStream codeStream, boolean valueRequired) {
emitDeclaringClassOfConstant(codeStream);
int pc = codeStream.position;
if (this.constant != Constant.NotAConstant) {
if (valueRequired) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,7 @@ public void generateAssignment(BlockScope currentScope, CodeStream codeStream, A

@Override
public void generateCode(BlockScope currentScope, CodeStream codeStream, boolean valueRequired) {
emitDeclaringClassOfConstant(codeStream);
int pc = codeStream.position;
if (this.constant != Constant.NotAConstant) {
if (valueRequired) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2003, 2023 IBM Corporation and others.
* Copyright (c) 2003, 2024 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand All @@ -14,6 +14,7 @@
package org.eclipse.jdt.core.tests.compiler.regression;

import java.io.File;
import java.util.regex.Pattern;

import junit.framework.Test;

Expand Down Expand Up @@ -1638,6 +1639,118 @@ public void testGH1256() throws Exception {
},
"2345");
}
public void testGH1382_singleName() throws Exception {
if (this.complianceLevel < ClassFileConstants.JDK1_5)
return;
this.runConformTest(
new String[] {
"api/Constants.java",
"""
package api;
public class Constants {
public static final boolean B = false;
public interface C1 {
int I = 1;
}
public interface C2 {
long L = 2l;
}
public interface C3 {
String S = "string";
}
}
""",
"X.java",
"""
import static api.Constants.B;
import static api.Constants.C1.I;
import static api.Constants.C2.L;
import static api.Constants.C3.S;
public class X {
static final String STRING = S+"suffix";
void test() {
System.out.print(B);
System.out.print(I);
System.out.print(L);
}
}
""",
},
"");

File f = new File(OUTPUT_DIR + File.separator + "X.class");
byte[] classFileBytes = org.eclipse.jdt.internal.compiler.util.Util.getFileByteContent(f);
ClassFileBytesDisassembler disassembler = ToolFactory.createDefaultClassFileBytesDisassembler();
String result = disassembler.disassemble(classFileBytes, "\n", ClassFileBytesDisassembler.SYSTEM);
assertContainsClassConstant(result, "api/Constants");
assertContainsClassConstant(result, "api/Constants$C1");
assertContainsClassConstant(result, "api/Constants$C2");
assertContainsClassConstant(result, "api/Constants$C3");
}

public void testGH1382_qualifiedName() throws Exception {
if (this.complianceLevel < ClassFileConstants.JDK1_5)
return;
this.runConformTest(
new String[] {
"api/Constants.java",
"""
package api;
public class Constants {
public static final boolean B = false;
}
""",
"api/Constants1.java",
"""
package api;
public interface Constants1 {
int I = 1;
}
""",
"api/Constants2.java",
"""
package api;
public interface Constants2 {
long L = 2l;
}
""",
"api/Constants3.java",
"""
package api;
public interface Constants3 {
String S = "string";
}
""",
"X.java",
"""
public class X {
static final boolean BOOL = !api.Constants.B;
void test() {
System.out.print(api.Constants1.I);
System.out.print(api.Constants2.L);
System.out.print(api.Constants3.S);
}
}
""",
},
"");

File f = new File(OUTPUT_DIR + File.separator + "X.class");
byte[] classFileBytes = org.eclipse.jdt.internal.compiler.util.Util.getFileByteContent(f);
ClassFileBytesDisassembler disassembler = ToolFactory.createDefaultClassFileBytesDisassembler();
String result = disassembler.disassemble(classFileBytes, "\n", ClassFileBytesDisassembler.SYSTEM);
assertContainsClassConstant(result, "api/Constants");
assertContainsClassConstant(result, "api/Constants1");
assertContainsClassConstant(result, "api/Constants2");
assertContainsClassConstant(result, "api/Constants3");
}

void assertContainsClassConstant(String disassembled, String className) {
className = className.replace("/", "\\/").replace("$", "\\$");
Pattern pattern = Pattern.compile(".*constant #[0-9]+ class: #[0-9]+ "+className+".*", Pattern.DOTALL);
assertTrue("Should contain class constant for "+className, pattern.matcher(disassembled).matches());
}

public static Class testClass() {
return ConstantTest.class;
}
Expand Down
3 changes: 2 additions & 1 deletion org.eclipse.jdt.core.tests.model/forceQualifierUpdate.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@
Bug 489604 - should not override <timestampProvider>
https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/issues/1184
https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/issues/1659
https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/issues/1704
https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/issues/1704
https://github.com/eclipse-jdt/eclipse.jdt.core/pull/2341
3 changes: 2 additions & 1 deletion org.eclipse.jdt.core/forceQualifierUpdate.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ Bug 566471 - I20200828-0150 - Comparator Errors Found
Bug 573283 - don't include javax15api.jar in jdt binaries
https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/issues/1184
https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/issues/1659
https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/issues/1781
https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/issues/1781
https://github.com/eclipse-jdt/eclipse.jdt.core/pull/2341

0 comments on commit 204b598

Please sign in to comment.