On Thu, 24 Oct 2024 16:31:20 GMT, Jorn Vernee <jver...@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
