Bug 143183 – [ES6] Implement tagged templates

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Yusuke Suzuki]

Implement tagged templates, like

String.raw`Hello ${world}`

[Yusuke Suzuki]

Created attachment 252767 [details]
Patch

[Oliver Hunt]

Comment on attachment 252767 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=252767&action=review

r-, but literally just for the tweaks mentioned above.

> Source/JavaScriptCore/parser/Parser.h:781
> +    template <class TreeBuilder> NEVER_INLINE typename TreeBuilder::TemplateLiteral parseTemplateLiteral(TreeBuilder&, bool shouldBuildRawStrings);

Can you replace this bool with a clearly named enum? In general we've shied away from opaque bool parameters as you can't necessarily tells what's going on -- especially in the earlier call 'parseTemplateLiteral(context, false)

Overall it looks good though.

> Source/JavaScriptCore/parser/SyntaxChecker.h:85
> +        TemplateStringResult, TemplateStringListResult, TemplateExpressionListResult, TemplateExpr, TaggedTemplateExpr

i'd split the line at this point

[Yusuke Suzuki]

Created attachment 253114 [details]
Patch

[Yusuke Suzuki]

Comment on attachment 252767 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=252767&action=review

Thank you for your review :D

>> Source/JavaScriptCore/parser/Parser.h:781
>> +    template <class TreeBuilder> NEVER_INLINE typename TreeBuilder::TemplateLiteral parseTemplateLiteral(TreeBuilder&, bool shouldBuildRawStrings);
> 
> Can you replace this bool with a clearly named enum? In general we've shied away from opaque bool parameters as you can't necessarily tells what's going on -- especially in the earlier call 'parseTemplateLiteral(context, false)
> 
> Overall it looks good though.

Make sense! OK, I've introduced enum class RawStringsBuildMode { BuildRawStrings, DontBuildRawStrings }.

>> Source/JavaScriptCore/parser/SyntaxChecker.h:85
>> +        TemplateStringResult, TemplateStringListResult, TemplateExpressionListResult, TemplateExpr, TaggedTemplateExpr
> 
> i'd split the line at this point

Splitted into 

TemplateStringResult, TemplateStringListResult,
TemplateExpressionListResult, TemplateExpr,
TaggedTemplateExpr

[WebKit Commit Bot]

Comment on attachment 253114 [details]
Patch

Clearing flags on attachment: 253114

Committed r184337: <http://trac.webkit.org/changeset/184337>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Csaba Osztrogonác]

(In reply to comment #5)
> Comment on attachment 253114 [details]
> Patch
> 
> Clearing flags on attachment: 253114
> 
> Committed r184337: <http://trac.webkit.org/changeset/184337>

It caused zillion failures on the debug bots. Don't 
you watch the bots after landing patches? You should.

[Yusuke Suzuki]

(In reply to comment #7)
> (In reply to comment #5)
> > Comment on attachment 253114 [details]
> > Patch
> > 
> > Clearing flags on attachment: 253114
> > 
> > Committed r184337: <http://trac.webkit.org/changeset/184337>
> 
> It caused zillion failures on the debug bots. Don't 
> you watch the bots after landing patches? You should.

Ah, currently investigating...... Sorry for troubling you.

[Yusuke Suzuki]

Opened https://bugs.webkit.org/show_bug.cgi?id=145013.

[Yusuke Suzuki]

debug builds become green.
Now investigating EFL builld failures in ARM builds.
https://build.webkit.org/builders/EFL%20Linux%20ARMv7%20Thumb2%20Release?numbuilds=25

[Yusuke Suzuki]

Opened for EFL ARM build failures. https://bugs.webkit.org/show_bug.cgi?id=145019

[Yusuke Suzuki]

Green comes back.