This is an automated email from the ASF dual-hosted git repository. garydgregory pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-bcel.git
commit 7fc6cd4fa73e6b1da0da1dfb163cc2cd988c873f Author: Gary Gregory <[email protected]> AuthorDate: Sun Oct 18 08:02:42 2026 -0400 [BCEL-379] Preserve boolean constants in BCELifier round trips Emit boolean literals so generated creators call FieldGen.setInitValue(boolean) and avoid the type mismatch caused by the int overload. Improve on PR #536 by preserving explicit false ConstantValue attributes, which the PR loses during reconstruction. This keeps regenerated fields usable as compile-time constants. Also allow false to replace a previous true value. Use the low bit for noncanonical integer encodings to match HotSpot behavior. Document the fixture's indirect use and generated creator workflow. Extend tests to cover initializer replacement and cancellation, noncanonical encodings, and preservation of boolean ConstantValue attributes. The PR's javap comparison alone does not detect missing constant attributes. --- src/changes/changes.xml | 1 + .../java/org/apache/bcel/generic/FieldGen.java | 6 +- src/main/java/org/apache/bcel/util/BCELifier.java | 8 ++- src/test/java/BooleanConstants.java | 37 ++++++++++++ .../java/org/apache/bcel/generic/FieldGenTest.java | 36 ++++++++++++ .../java/org/apache/bcel/util/BCELifierTest.java | 65 +++++++++++++++++++++- 6 files changed, 148 insertions(+), 5 deletions(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index a13b8681..64808f96 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -64,6 +64,7 @@ The <action> type attribute can be add,update,fix,remove. <action type="fix" dev="ggregory" due-to="Gary Gregory">Use modern Mockito on Java 11+ (#528).</action> <action type="fix" dev="ggregory" due-to="Max Kammerer, akashchamp, Gary Gregory" issue="BCEL-280">MethodGen.setMaxLocals() should account for LocalVariableTable entries (#529).</action> <action type="fix" dev="ggregory" due-to="Gary Gregory">Fix constant pool module access test on Java 27 (#534).</action> + <action issue="BCEL-379" type="fix" due-to="Gary Gregory, Aryan Ganjoo">BCELifier emits boolean literals for boolean ConstantValue fields so generated creator classes run. FieldGen preserves explicit false initial values (#536).</action> <!-- ADD --> <action type="add" dev="ggregory" due-to="Gary Gregory">Add and use ClassFormatException.ClassFormatException(String, Object...).</action> <!-- UPDATE --> diff --git a/src/main/java/org/apache/bcel/generic/FieldGen.java b/src/main/java/org/apache/bcel/generic/FieldGen.java index 0e01e591..ca8223fc 100644 --- a/src/main/java/org/apache/bcel/generic/FieldGen.java +++ b/src/main/java/org/apache/bcel/generic/FieldGen.java @@ -251,13 +251,13 @@ public class FieldGen extends FieldGenOrMethodGen { /** * Sets the initial value. * + * <p>Records an explicit constant value, including {@code false}. Use {@link #cancelInitValue()} to remove it.</p> + * * @param b The boolean value. */ public void setInitValue(final boolean b) { checkType(Type.BOOLEAN); - if (b) { - value = Integer.valueOf(1); - } + value = Integer.valueOf(b ? 1 : 0); } /** diff --git a/src/main/java/org/apache/bcel/util/BCELifier.java b/src/main/java/org/apache/bcel/util/BCELifier.java index 392f60bf..4872a11e 100644 --- a/src/main/java/org/apache/bcel/util/BCELifier.java +++ b/src/main/java/org/apache/bcel/util/BCELifier.java @@ -281,7 +281,13 @@ public class BCELifier extends org.apache.bcel.classfile.EmptyVisitor { if (field.getType() == Type.BYTE) { printWriter.print("(byte)"); } - printWriter.print(cv); + if (field.getType() == Type.BOOLEAN) { + // A boolean ConstantValue is stored as a CONSTANT_Integer, but FieldGen.setInitValue(int) rejects boolean fields. + // Match HotSpot initialization for noncanonical integer encodings by using the low bit. + printWriter.print((field.getConstantPool().getConstantInteger(cv.getConstantValueIndex()).getBytes() & 1) != 0); + } else { + printWriter.print(cv); + } if (field.getType() == Type.LONG) { printWriter.print("L"); } diff --git a/src/test/java/BooleanConstants.java b/src/test/java/BooleanConstants.java new file mode 100644 index 00000000..32bd9145 --- /dev/null +++ b/src/test/java/BooleanConstants.java @@ -0,0 +1,37 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +/** + * Provides compiled boolean constants for BCELifier round-trip tests. + * <p> + * Test Java code does not reference this class directly. The tests locate its compiled class file by name and pass it to {@link org.apache.bcel.util.BCELifier} + * to generate creator source code. That source is compiled and executed to recreate this class file and verify that its boolean constants are preserved. + * </p> + */ +public class BooleanConstants { + + /** + * A true constant. + */ + public static final boolean ON = true; + + /** + * A false constant. + */ + public static final boolean OFF = false; +} diff --git a/src/test/java/org/apache/bcel/generic/FieldGenTest.java b/src/test/java/org/apache/bcel/generic/FieldGenTest.java index fe79b9f9..dd4090b5 100644 --- a/src/test/java/org/apache/bcel/generic/FieldGenTest.java +++ b/src/test/java/org/apache/bcel/generic/FieldGenTest.java @@ -19,14 +19,20 @@ package org.apache.bcel.generic; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import org.apache.bcel.Const; import org.apache.bcel.classfile.ConstantLong; import org.apache.bcel.classfile.ConstantPool; import org.apache.bcel.classfile.Field; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; /** * Tests {@link FieldGen}. @@ -43,4 +49,34 @@ class FieldGenTest { assertFalse(FieldGen.getComparator().equals(obj, null)); assertFalse(FieldGen.getComparator().equals(null, obj)); } + + @ParameterizedTest + @ValueSource(booleans = { false, true }) + void testBooleanInitValue(final boolean value) { + final FieldGen fg = new FieldGen(Const.ACC_PUBLIC | Const.ACC_STATIC | Const.ACC_FINAL, Type.BOOLEAN, "VALUE", new ConstantPoolGen()); + fg.setInitValue(value); + final Field field = fg.getField(); + assertNotNull(field.getConstantValue()); + assertEquals(value ? 1 : 0, field.getConstantPool().getConstantInteger(field.getConstantValue().getConstantValueIndex()).getBytes()); + } + + @ParameterizedTest + @ValueSource(booleans = { false, true }) + void testBooleanInitValueReplacement(final boolean value) { + final FieldGen fg = new FieldGen(Const.ACC_PUBLIC | Const.ACC_STATIC | Const.ACC_FINAL, Type.BOOLEAN, "VALUE", new ConstantPoolGen()); + fg.setInitValue(!value); + fg.setInitValue(value); + final Field field = fg.getField(); + assertNotNull(field.getConstantValue()); + assertEquals(value ? 1 : 0, field.getConstantPool().getConstantInteger(field.getConstantValue().getConstantValueIndex()).getBytes()); + } + + @ParameterizedTest + @ValueSource(booleans = { false, true }) + void testCancelBooleanInitValue(final boolean value) { + final FieldGen fg = new FieldGen(Const.ACC_PUBLIC | Const.ACC_STATIC | Const.ACC_FINAL, Type.BOOLEAN, "VALUE", new ConstantPoolGen()); + fg.setInitValue(value); + fg.cancelInitValue(); + assertNull(fg.getField().getConstantValue()); + } } diff --git a/src/test/java/org/apache/bcel/util/BCELifierTest.java b/src/test/java/org/apache/bcel/util/BCELifierTest.java index 715b4016..40d13258 100644 --- a/src/test/java/org/apache/bcel/util/BCELifierTest.java +++ b/src/test/java/org/apache/bcel/util/BCELifierTest.java @@ -38,11 +38,15 @@ import java.util.regex.Pattern; import org.apache.bcel.AbstractTest; import org.apache.bcel.Const; import org.apache.bcel.HelloWorldCreator; +import org.apache.bcel.classfile.ClassParser; +import org.apache.bcel.classfile.ConstantValue; +import org.apache.bcel.classfile.Field; import org.apache.bcel.classfile.JavaClass; import org.apache.bcel.classfile.Utility; import org.apache.bcel.generic.BinaryOpCreator; import org.apache.bcel.generic.ClassGen; import org.apache.bcel.generic.ConstantPoolGen; +import org.apache.bcel.generic.FieldGen; import org.apache.bcel.generic.InstructionConst; import org.apache.bcel.generic.InstructionFactory; import org.apache.bcel.generic.InstructionList; @@ -54,6 +58,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.DisabledForJreRange; import org.junit.jupiter.api.condition.JRE; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.ValueSource; class BCELifierTest extends AbstractTest { @@ -177,6 +182,46 @@ class BCELifierTest extends AbstractTest { } } + @Test + void testBooleanConstantFieldInitValue() throws Exception { + // javac stores "static final boolean ON = true;" as a CONSTANT_Integer ConstantValue; the generated creator + // must call FieldGen.setInitValue(boolean), since setInitValue(int) throws for boolean fields. + final ClassGen cg = new ClassGen("Example", "java.lang.Object", "Example.java", Const.ACC_PUBLIC | Const.ACC_SUPER, new String[] {}); + final ConstantPoolGen cp = cg.getConstantPool(); + final FieldGen fg = new FieldGen(Const.ACC_PUBLIC | Const.ACC_STATIC | Const.ACC_FINAL, Type.BOOLEAN, "ON", cp); + fg.setInitValue(true); + cg.addField(fg.getField()); + + final ByteArrayOutputStream os = new ByteArrayOutputStream(); + new BCELifier(cg.getJavaClass(), os).start(); + final String source = new String(os.toByteArray(), StandardCharsets.UTF_8); + + assertTrue(source.contains("field.setInitValue(true);"), source); + assertFalse(source.contains("field.setInitValue(1);"), source); + + final ByteArrayOutputStream compiledClassOutput = new ByteArrayOutputStream(); + new BCELifier(BCELifier.getJavaClass("BooleanConstants"), compiledClassOutput).start(); + final String compiledClassSource = new String(compiledClassOutput.toByteArray(), StandardCharsets.UTF_8); + assertTrue(compiledClassSource.contains("field.setInitValue(true);"), compiledClassSource); + assertTrue(compiledClassSource.contains("field.setInitValue(false);"), compiledClassSource); + assertFalse(compiledClassSource.contains("field.setInitValue(0);"), compiledClassSource); + } + + @ParameterizedTest + @CsvSource({ "0, false", "1, true", "2, false", "3, true", "-2, false", "-1, true" }) + void testBooleanConstantFieldLowBit(final int value, final boolean expected) { + final ClassGen cg = new ClassGen("Example", "java.lang.Object", "Example.java", Const.ACC_PUBLIC | Const.ACC_SUPER, new String[] {}); + final ConstantPoolGen cp = cg.getConstantPool(); + final FieldGen fg = new FieldGen(Const.ACC_PUBLIC | Const.ACC_STATIC | Const.ACC_FINAL, Type.BOOLEAN, "VALUE", cp); + fg.addAttribute(new ConstantValue(cp.addUtf8("ConstantValue"), 2, cp.addInteger(value), cp.getConstantPool())); + cg.addField(fg.getField()); + + final ByteArrayOutputStream os = new ByteArrayOutputStream(); + new BCELifier(cg.getJavaClass(), os).start(); + final String source = new String(os.toByteArray(), StandardCharsets.UTF_8); + assertTrue(source.contains("field.setInitValue(" + expected + ");"), source); + } + @Test void testClassNameRejectedWhenNotJavaIdentifier() { // Class file names may contain characters the Java language forbids (JVMS 4.2.2 bans only . ; [ /). @@ -244,6 +289,23 @@ class BCELifierTest extends AbstractTest { final String javapOutput = exec(workDir, getAppJavaP(), "-p", "-c", inFile.getName()); // Finally compares the output of the roundtrip assertEquals(canonHashRef(javapOutInital), canonHashRef(javapOutput)); + // javap -p -c does not show ConstantValue attributes. Check boolean constants explicitly. + final JavaClass regenerated = new ClassParser(new File(workDir, javaClassName + JavaClass.EXTENSION).getPath()).parse(); + final Field[] originalFields = javaClass.getFields(); + final Field[] regeneratedFields = regenerated.getFields(); + assertEquals(originalFields.length, regeneratedFields.length); + for (int i = 0; i < originalFields.length; i++) { + final Field original = originalFields[i]; + if (original.getType() == Type.BOOLEAN && original.getConstantValue() != null) { + final Field actual = regeneratedFields[i]; + assertEquals(original.getName(), actual.getName()); + assertEquals(original.getSignature(), actual.getSignature()); + assertNotNull(actual.getConstantValue(), original.getName()); + final int expectedValue = original.getConstantPool().getConstantInteger(original.getConstantValue().getConstantValueIndex()).getBytes() & 1; + assertEquals(expectedValue, actual.getConstantPool().getConstantInteger(actual.getConstantValue().getConstantValueIndex()).getBytes(), + original.getName()); + } + } } @Test @@ -297,7 +359,8 @@ class BCELifierTest extends AbstractTest { "org.apache.commons.lang.builder.ToStringBuilder.class", "org.apache.commons.lang.SerializationUtils.class", "org.apache.commons.lang.ArrayUtils.class", - "target/test-classes/Java4Example.class" + "target/test-classes/Java4Example.class", + "target/test-classes/BooleanConstants.class" // @formatter:on }) void testJavapCompare(final String pathToClass) throws Exception {
