RFR: 8034066: Incorrect alignment in the "Code" section for "-c -XDdetails" options

Chen Liang liach at openjdk.org
Fri Oct 25 02:58:04 UTC 2024


On Thu, 24 Oct 2024 16:31:20 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>> This PR includes changes to ensure `Code:` block indentation in `javap`for the `-verbose` case and non `-verbose` case is the same, which currently does not hold.
>> 
>> Current behaviour of `javap` differs with and without `-verbose` in the following way:
>> **Command**: `javap -c -XDdetails:stackMaps A.class`
>> 
>> Without `-verbose`
>> 
>> 
>> ...
>>   public void a();
>>     Code:
>>        0: iconst_0
>>        1: istore_1
>>     StackMap locals:  this int
>>     StackMap stack:
>> ...
>> 
>> 
>> With `-verbose`
>> 
>> 
>> ...
>>   public void a();
>>     descriptor: ()V
>>     flags: (0x0001) ACC_PUBLIC
>>     Code:
>>       stack=2, locals=2, args_size=1
>>          0: iconst_0
>>          1: istore_1
>>       StackMap locals:  this int
>>       StackMap stack:
>> ...
>> 
>> 
>> With `-verbose` all contents of the `Code:` section include an extra (2 space) indent, which is missing in the non `-verbose` case. This is because the `CodeWriter` is called via `CoderWriter.write(...)` in the `-verbose` case, which wraps the `Code:` block in `indent(+1);...indent(-1)`.
>> 
>> In the non-verbose case this call is circumvented and a simplified version of `CoderWriter.write(...)` is included directly in `ClassWriter.writeMethod`.
>> 
>> ---
>> 
>> Alternatively to keep the logic within `CodeWriter` with the goal of keeping the logic for `-verbose` and non `-verbose` in the same place one could add `CodeWriter.writeSimple(...)`.
>> 
>> 
>> void writeSimple(CodeAttribute attr) {
>>         println("Code:");
>>         indent(+1);
>>         writeInstrs(attr);
>>         writeExceptionTable(attr);
>>         indent(-1);
>>     }
>> 
>> 
>> ---
>> 
>> Note: Test setup is inspired by existing tests: [T6622232.java](https://github.com/openjdk/jdk/blob/master/test/langtools/tools/javap/T6622232.java) and [8244573](https://github.com/openjdk/jdk/blob/master/test/langtools/tools/javap/8244573)
>
> test/langtools/tools/javap/8034066/T8034066.java line 37:
> 
>> 35: import java.util.regex.Pattern;
>> 36: 
>> 37: public class T8034066 {
> 
> Please don't continue the old practice of naming tests after their bug ID. It obfuscates which test class does what (both in the source code and in the test logs). Suggestion for name: `TestStackMapDetailsIndent`

How about `CodeWriterIndentTest`? We prefer to use prefix modifiers, and this tests the indent for general code writer output.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/21685#discussion_r1815928810


More information about the compiler-dev mailing list