RFR 8035271: Incorrect code attribute indentation

[Jonathan Gibbons]

Now would be a good time to make progress on this issue.

-- Jon


On 7/1/17 1:58 AM, B. Blaser wrote:
> Jon,
>
> Is this the right time to consider pushing the following patch to JDK 10?
> Or should I update the JBS issue to "in progress/fix understood" not
> to forget it?
>
> Thanks,
> Bernard
>
> On 6 February 2017 at 15:02, B. Blaser <bsrbnd at gmail.com> wrote:
>> Jonathan,
>>
>> 2017-02-02 18:41 GMT+01:00 Jonathan Gibbons <jonathan.gibbons at oracle.com>:
>>> Bernard,
>>>
>>> In other tests with significant amounts of golden text, we have found it
>>> more
>>> readable to use \n in the golden strings, and use a single .replaceAll
>>> on the string to change the \n to the platform newline character. This
>>> avoids having to use + LS + at the end of each line.
>>>
>>> I will look at your changeset in full context, but comments like
>>> "// Same as CodeWriter.write()" may be meaningful now, but may be less
>>> relevant a year or two down the road, prompting questions like "why does
>>> this need to be stated?".
>>>
>>> Separately, I note that the window of opportunity for fixes like this in JDK
>>> 9
>>> is rapidly closing, and at some point, we will have to push such fixes to
>>> the
>>> JDK 10 repos that were recently opened.
>>>
>>> -- Jon
>> Thanks for your answer, I agree with your comments.
>> Please, find next the updated patch.
>>
>> Bernard
>>
>>> On 02/02/2017 09:05 AM, B. Blaser wrote:
>>>> Hi,
>>>>
>>>> Next is a small fix to preserve the same code attribute indentation
>>>> for all javap options (issue 8035271), along with a test.
>>>>
>>>> Bernard
>> diff --git a/src/jdk.jdeps/share/classes/com/sun/tools/javap/ClassWriter.java
>> b/src/jdk.jdeps/share/classes/com/sun/tools/javap/ClassWriter.java
>> --- a/src/jdk.jdeps/share/classes/com/sun/tools/javap/ClassWriter.java
>> +++ b/src/jdk.jdeps/share/classes/com/sun/tools/javap/ClassWriter.java
>> @@ -571,13 +571,17 @@
>>           } else if (code != null) {
>>               if (options.showDisassembled) {
>>                   println("Code:");
>> +                indent(+1);
>>                   codeWriter.writeInstrs(code);
>>                   codeWriter.writeExceptionTable(code);
>> +                indent(-1);
>>               }
>>
>>               if (options.showLineAndLocalVariableTables) {
>> +                indent(+1);
>>                   attrWriter.write(code,
>> code.attributes.get(Attribute.LineNumberTable), constant_pool);
>>                   attrWriter.write(code,
>> code.attributes.get(Attribute.LocalVariableTable), constant_pool);
>> +                indent(-1);
>>               }
>>           }
>>
>> diff --git a/test/tools/javap/CodeAttributeIndentation.java
>> b/test/tools/javap/CodeAttributeIndentation.java
>> new file mode 100644
>> --- /dev/null
>> +++ b/test/tools/javap/CodeAttributeIndentation.java
>> @@ -0,0 +1,65 @@
>> +/*
>> + * @test
>> + * @bug 8035271
>> + * @summary Same code attribute indentation for all options.
>> + * @library /tools/lib
>> + * @modules jdk.compiler/com.sun.tools.javac.api
>> + *          jdk.compiler/com.sun.tools.javac.main
>> + *          jdk.jdeps/com.sun.tools.javap
>> + * @build toolbox.ToolBox toolbox.JavacTask toolbox.JavapTask
>> + * @run main CodeAttributeIndentation
>> + */
>> +
>> +import toolbox.ToolBox;
>> +import toolbox.JavacTask;
>> +import toolbox.JavapTask;
>> +import toolbox.Task.OutputKind;
>> +
>> +public class CodeAttributeIndentation {
>> +    public static void main(String... args) {
>> +        String LS = System.getProperty("line.separator");
>> +
>> +        String src = (
>> +            "public class A {\n" +
>> +            "    public void a() {}\n" +
>> +            "}"
>> +            ).replaceAll("\n", LS);
>> +
>> +        String expected = (
>> +            "Compiled from \"A.java\"\n" +
>> +            "public class A {\n" +
>> +            "  public A();\n" +
>> +            "    Code:\n" +
>> +            "         0: aload_0\n" +
>> +            "         1: invokespecial #1                  " +
>> +            "// Method java/lang/Object.\"<init>\":()V\n" +
>> +            "         4: return\n" +
>> +            "      LineNumberTable:\n" +
>> +            "        line 1: 0\n" +
>> +            "      LocalVariableTable:\n" +
>> +            "        Start  Length  Slot  Name   Signature\n" +
>> +            "            0       5     0  this   LA;\n" +
>> +            "\n" +
>> +            "  public void a();\n" +
>> +            "    Code:\n" +
>> +            "         0: return\n" +
>> +            "      LineNumberTable:\n" +
>> +            "        line 2: 0\n" +
>> +            "      LocalVariableTable:\n" +
>> +            "        Start  Length  Slot  Name   Signature\n" +
>> +            "            0       1     0  this   LA;\n" +
>> +            "}\n"
>> +            ).replaceAll("\n", LS);
>> +
>> +        ToolBox tb = new ToolBox();
>> +
>> +        new JavacTask(tb).options("-g").sources(src).run();
>> +
>> +        String actual = new JavapTask(tb).options("-c",
>> "-l").classes("A.class")
>> +                .run().getOutput(OutputKind.DIRECT);
>> +
>> +        if (!actual.equals(expected))
>> +            throw new AssertionError("Incorrect code attribute indentation: " +
>> +                    LS + actual);
>> +    }
>> +}