Bug in javac Pretty: Wrong precedence in JCConditional trees

(from Matthew Dempsky on compiler-dev)

Hi Java compiler developers,

I noticed a bug in javac's JCTree pretty printer: it mishandles the precedence rules when pretty printing JCConditional trees. The "condition" subexpression should be pretty printed at precedence condPrec+1 instead of condPrec.

I've put together a short test program demonstrating this bug, included below. It constructs and pretty prints two different nested conditional trees, but they generate identical output.

Additionally, but less severely, the "true" subexpression can be printed at noPrec without introducing any ambiguity; e.g., "x ? y = 1 : z" will parse the same as "x ? (y = 1) : z" anyway, so it's unnecessary (but also not incorrect) for Pretty to print the latter.

I believe (but have not verified) that these can both be fixed by changing Pretty.visitConditional() to:

            printExpr(tree.cond, TreeInfo.condPrec + 1);
            print(" ? ");
            printExpr(tree.truepart);
            print(" : ");
            printExpr(tree.falsepart, TreeInfo.condPrec);

I discovered this bug in what I believe to be jdk7u40, but it looks like it's still present in jdk8 langtools at tip: http://hg.openjdk.java.net/jdk8/jdk8/langtools/file/tip/src/share/classes/com/sun/tools/javac/tree/Pretty.java

Thanks


Test program:

package prettybug;

import com.sun.tools.javac.file.JavacFileManager;
import com.sun.tools.javac.tree.Pretty;
import com.sun.tools.javac.tree.TreeMaker;
import com.sun.tools.javac.util.Context;

import java.io.PrintWriter;

public class Bug {
  public static void main(String[] args) throws Exception {
    Context ctx = new Context();
    JavacFileManager.preRegister(ctx);
    TreeMaker maker = TreeMaker.instance(ctx);

    PrintWriter writer = new PrintWriter(System.out);
    Pretty pretty = new Pretty(writer, true);

    writer.println("These two trees should print differently:");

    pretty.printExpr(
      maker.Conditional(
        maker.Conditional(
          maker.Literal(1),
          maker.Literal(2),
          maker.Literal(3)),
        maker.Literal(4),
        maker.Literal(5)));
    writer.println();

    pretty.printExpr(
      maker.Conditional(
        maker.Literal(1),
        maker.Literal(2),
        maker.Conditional(
          maker.Literal(3),
          maker.Literal(4),
          maker.Literal(5))));
    writer.println();

    writer.println();

    writer.println("This expression is unambiguous even without the parentheses:");

    pretty.printExpr(
      maker.Conditional(
        maker.Literal(1),
        maker.Assign(
          maker.Literal(2),
          maker.Literal(3)),
        maker.Literal(4)));
    writer.println();

    writer.flush();
  }
}