RFR 8134941: Implement ES6 template literal support

[Michael Haupt]

Hi all,

excellent - I'll sponsor the change.

Best,

Michael

> Am 23.10.2015 um 11:28 schrieb Attila Szegedi <attila.szegedi at oracle.com>:
> 
> +1 from me as well. Another apology from me for having to wait even more for the second review.
> 
> Very nice work. I’m sometimes in two minds about RuntimeNode (especially the fact that it currently can’t receive primitive parameters), but I have to admit that in this particular case it made the integration of the feature into the runtime fairly easy; it was really the parser parts that were tricky.
> 
> Attila.
> 
>> On Sep 24, 2015, at 4:44 PM, Hannes Wallnoefer <hannes.wallnoefer at oracle.com> wrote:
>> 
>> Hi Andreas,
>> 
>> Thanks for the contribution, and sorry for the long time it took to get back to you.
>> 
>> I like the way you implemented this feature, the code looks very good.  Comments inlined below.
>> 
>> Am 2015-09-03 um 18:49 schrieb Andreas Woess:
>>> Template literals are always scanned as a whole, quote-to-quote (as with EditStringLexer). This turned out to be a problem with embedded functions in expressions and lazy/optimistic compilation on: Parser#skipFunctionBody would skip over the body and restart lexing at the RBRACE, forgetting about the embedding string context. I've worked around this in skipFunctionBody by skipping over to the RBRACE directly if it is already in the token stream (which it is because we've already scanned to the end quote).
>> 
>> It took me some time to figure this out. Maybe some more explanatory comments would be good. Does this also apply in other cases?
>> 
>>> 
>>> Outstanding correctness issues not dealt with:
>>> * 12.2.9.3 GetTemplateObject stipulates that the returned template object be cached and unique. I don't know why you'd want the spec to demand caching rather than allow it (functionally it does not make a difference, but you could observe it not being cached in a test).
>> 
>> Actually object identity can be observed by the tag function, and that was the reason for the committee to specify this. They chose caching of objects for performance reasons. The discussion can be found here:
>> 
>> https://github.com/rwaldron/tc39-notes/blob/master/es6/2014-11/nov-18.md#48-template-literal-call-site-object-caching
>> 
>> Even though it's a minor issue we should follow the spec here.
>> 
>>> * 12.2.9.5 Evaluation: string concatenation using + is slightly off-spec. There are two way to solve this: (a) wrap the expressions in a ToString UnaryExpression (or RuntimeCall) or (b) generate a call to concat with interleaved string and expression arguments.
>>> 
>> 
>> The difference is between ToPrimitive being called with Number hint vs. String hint. This should not make a difference for the vast share of objects (all except those having a custom valueOf function I think), but it's something we should also get right. I've started experimenting with the solutions you suggested, I think adding a ToString conversion wrapper of some kind would be best.
>> 
>> All in all I think your patch looks good. We can probably file the issues as separate bugs and fix them later. One thing I want to do is add some more tests. Although your test script covers a lot of stuff (for its size) I would like to add a bit to it.
>> 
>> Thanks,
>> Hannes


-- 

 <http://www.oracle.com/>
Dr. Michael Haupt | Principal Member of Technical Staff
Phone: +49 331 200 7277 | Fax: +49 331 200 7561
Oracle Java Platform Group | LangTools Team | Nashorn
Oracle Deutschland B.V. & Co. KG, Schiffbauergasse 14 | 14467 Potsdam, Germany
 <http://www.oracle.com/commitment>	Oracle is committed to developing practices and products that help protect the environment