Bug 183987 – [JSC] Remove repeated iteration of ElementNode

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Yusuke Suzuki]

[JSC] Remove repeated iteration of ElementNode

[Yusuke Suzuki]

Created attachment 336486 [details]
Patch

[Keith Miller]

Comment on attachment 336486 [details]
Patch

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

r=me with some nits.

> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:398
> +    auto newArray = [&generator] (RegisterID* dst, ElementNode* elements, unsigned length, bool hadVariableExpression) {

Nit: Why not just capture the args you're using?

> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:418
> +                if (!!node->elision()) {

Nit: You don't need the !!.

[Yusuke Suzuki]

Comment on attachment 336486 [details]
Patch

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

Thank you!

>> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:398
>> +    auto newArray = [&generator] (RegisterID* dst, ElementNode* elements, unsigned length, bool hadVariableExpression) {
> 
> Nit: Why not just capture the args you're using?

I want to separate the responsibility of ArrayNode::emitBytecode and this newArray function.
This newArray does not recognize how `hadVariableExpression` and `length` is computed. Passing it as an argument explicitly makes this separation clear.

>> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:418
>> +                if (!!node->elision()) {
> 
> Nit: You don't need the !!.

Dropped.

[Yusuke Suzuki]

Committed r229993: <https://trac.webkit.org/changeset/229993>

[Radar WebKit Bug Importer]

<rdar://problem/38903107>