RFR 8134941: Implement ES6 template literal support

Hannes Wallnoefer hannes.wallnoefer at oracle.com
Thu Sep 24 14:44:57 UTC 2015


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







More information about the nashorn-dev mailing list