RFR 8035271: Incorrect code attribute indentation

[B. Blaser]

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);
> +    }
> +}