Thanks Attila. You’re right about that double cast, but I already pushed the 
change with Marcus’ and Sundar’s reviews. I’ll include that clean-up in my next 
commit.

Hannes


> Am 07.10.2016 um 15:22 schrieb Attila Szegedi <szege...@gmail.com>:
> 
> And the patch is now much easier to review! Anyway, apologies for having you 
> make another roundtrip.
> 
> That said, I’d prefer if subexpressions 
> (LiteralNode.ArrayLiteralNode)literalNode and 
> literalNode.getElementExpressions() were assigned to some shortish-named 
> locals and used as such to improve the readability of this expression: 
> 
> +                if (((LiteralNode.ArrayLiteralNode)literalNode).hasSpread() 
> && ((LiteralNode.ArrayLiteralNode)literalNode).hasTrailingComma()) {
> +                    throw error("Rest element must be last", 
> literalNode.getElementExpressions().get(literalNode.getElementExpressions().size()
>  - 1).getToken());
> +                }
> 
> +1 otherwise. (I’m also fine if you decide to not make this change; it’s 
> admittedly no big deal.)
> 
> Attila.
> 
> 
>> On 07 Oct 2016, at 10:25, Hannes Wallnöfer <hannes.wallnoe...@oracle.com> 
>> wrote:
>> 
>> This change conflicts with Attila’s add-finals change in Parser.java. I 
>> uploaded a new webrev with the merged patch. I double-checked that it keeps 
>> the finals and that the code I added uses final too.
>> 
>> http://cr.openjdk.java.net/~hannesw/8167289/webrev.01/
>> 
>> Hannes
>> 
>> 
>>> Am 07.10.2016 um 05:27 schrieb Sundararajan Athijegannathan 
>>> <sundararajan.athijegannat...@oracle.com>:
>>> 
>>> +1
>>> 
>>> -Sundar
>>> 
>>> 
>>> On 10/6/2016 8:21 PM, Hannes Wallnöfer wrote:
>>>> Please review:
>>>> 
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8167289
>>>> Webrev: http://cr.openjdk.java.net/~hannesw/8167289/webrev.00/
>>>> 
>>>> These is mostly refactoring Andreas did in the Graal.js parser. It also 
>>>> contains minor ES6 parsing fixes. Tests for these will be included 
>>>> together with other additional ES6 tests I’m working on, but I wanted to 
>>>> do a separate commit for the backport, so this issue is labeled 
>>>> noreg-cleanup.
>>>> 
>>>> Thanks,
>>>> Hannes
>>> 
>> 
> 

Reply via email to