[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