[PATCH] 8035271: Incorrect code attribute indentation

Jonathan Gibbons jonathan.gibbons at oracle.com
Thu Feb 2 17:41:27 UTC 2017


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

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); // Same as CodeWriter.write()
>                   codeWriter.writeInstrs(code);
>                   codeWriter.writeExceptionTable(code);
> +                indent(-1);
>               }
>
>               if (options.showLineAndLocalVariableTables) {
> +                indent(+1); // Same as CodeWriter.write()
>                   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,63 @@
> +/*
> + * @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 {" + LS +
> +            "    public void a() {}" + LS +
> +            "}";
> +
> +        String expected =
> +            "Compiled from \"A.java\"" + LS +
> +            "public class A {" + LS +
> +            "  public A();" + LS +
> +            "    Code:" + LS +
> +            "         0: aload_0" + LS +
> +            "         1: invokespecial #1                  " +
> +            "// Method java/lang/Object.\"<init>\":()V" + LS +
> +            "         4: return" + LS +
> +            "      LineNumberTable:" + LS +
> +            "        line 1: 0" + LS +
> +            "      LocalVariableTable:" + LS +
> +            "        Start  Length  Slot  Name   Signature" + LS +
> +            "            0       5     0  this   LA;" + LS +
> +            LS +
> +            "  public void a();" + LS +
> +            "    Code:" + LS +
> +            "         0: return" + LS +
> +            "      LineNumberTable:" + LS +
> +            "        line 2: 0" + LS +
> +            "      LocalVariableTable:" + LS +
> +            "        Start  Length  Slot  Name   Signature" + LS +
> +            "            0       1     0  this   LA;" + LS +
> +            "}" + 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);
> +    }
> +}



More information about the compiler-dev mailing list