RFR: 8215371: Better nodes for concatenated string literals

[Brian Goetz]

It sounds like you're suggesting a multi-string-concat node, which would 
greedily parse a sequence of strings to be concatenated, and put a list 
of string-valued nodes into it, so that if we had:

     "foo" + "bar " + i + " baz"

we'd have

     StringConcatNode[ Literal["foo"],
                       Literal["bar "],
                       Apply(Integer.toString, Var[i]),
                       Literal(" baz")]

and then do the folding later?

Given that we're adding more sophisticated constant folding to the 
compiler in a separate pass, there's an obvious place to do such folding 
(and ideally, one such place.  Then, most of the time, gen just hands 
the contents of this node to indy.)  That seems a more honest solution 
than trying to balance the tree (especially as some of the concat nodes 
might have side-effects, such as ++x, and we'd have to be careful to 
manage the order of side effects.)

Or, wait for multi-line string literals, and its less of a problem...

On 12/14/2018 12:31 PM, Liam Miller-Cushon wrote:
> Hi,
>
> I've been thinking a bit about the discussion of string literals 
> in JDK-8182769, and I started a separate bug for that part: 
> https://bugs.openjdk.java.net/browse/JDK-8215371
>
> The issue is that long string concatenation chains are represented as 
> unbalanced binary expression trees, and it's easy to run out of stack 
> when processing those trees recursively. As a work-around, javac 
> currently supports folding concatenated string literals during 
> parsing, but that work-around reduces the fidelity of the AST and is 
> problematic for IDE-like tooling.
>
> A possible fix is to remove the early constant folding, and instead 
> balance the binary trees for concatentation: 
> http://cr.openjdk.java.net/~cushon/8215371/webrev.00/
>
> There's some discussion of the tradeoffs in the comments 
> on JDK-8182769. Other ideas include:
>
> * Refactoring code processing string literals to avoid recursion. This 
> removes the constraint on the shape on the AST, but might reduce 
> readability. The problem also affects non-javac users of the 
> com.sun.source APIs which we don't have the option of refactoring.
>
> * Introducing a new AST node to represent concatenated string literals 
> as a flat list. This would require more refactoring to javac, and an 
> addition to the com.sun.source APIs.
>
> What do you think? My prototype fix is kind of a work-around, but I 
> think it's a modest improvement over the status quo. If there are 
> ideas of a more principled solution I'm happy to investigate alternatives.
>
> Thanks,