<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - [ES6] Arrow function syntax. Emit loading&putting this/super only if they are used in arrow function"
href="https://bugs.webkit.org/show_bug.cgi?id=153981#c5">Comment # 5</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - [ES6] Arrow function syntax. Emit loading&putting this/super only if they are used in arrow function"
href="https://bugs.webkit.org/show_bug.cgi?id=153981">bug 153981</a>
from <span class="vcard"><a class="email" href="mailto:gskachkov@gmail.com" title="GSkachkov <gskachkov@gmail.com>"> <span class="fn">GSkachkov</span></a>
</span></b>
<pre>Comment on <span class="bz_obsolete"><a href="attachment.cgi?id=271327&action=diff" name="attach_271327" title="Patch">attachment 271327</a> <a href="attachment.cgi?id=271327&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=271327&action=review">https://bugs.webkit.org/attachment.cgi?id=271327&action=review</a>
<span class="quote">>> Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:132
>> + bool isInnerArrowFunctionUseNewTarget() { return m_arrowFunctionCodeFeatures & NewTargetArrowFunctionFeature; }
>
> I would name these like "doAnyInnerArrowFunctionsX"
> i.e:
> isInnerArrowFunctionUseArguments => doAnyInnerArrowFunctionsUseArguments</span >
Renamed
<span class="quote">>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:4051
>> +}
>
> Is "new.target" not allowed in eval?</span >
Ups. I forget about eval. Fixed.
<span class="quote">>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:648
>> + void emitPutThisToArrowFunctionContextScope(bool = false);
>
> Style: give this bool a name, like "shouldAlwaysPut" or something.</span >
added
<span class="quote">>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:724
>> + bool isArgumentsUsedInArrowFunction();
>
> nit: I would name these with "Inner" before "In"
> like:
> "isThisUsedInInnerArrowFunction"</span >
Done
<span class="quote">>> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:764
>> + generator.emitPutThisToArrowFunctionContextScope(true);
>
> Style: We usually write code like this by assigning the boolean as a local variable and passing the local variable to the function:
> ```
> bool forcePutToScope = true;
> generator.emitPutThisToArrowFunctionContextScope(forcePutToScope);
> ```</span >
Done
<span class="quote">>> Source/JavaScriptCore/parser/Parser.cpp:2067
>> + closestParentNonArrowFunctionNonLexicalScope()->setInnerArrowFunctionUseSuperCall();
>
> An alternative to repeatedly doing a scope search here is to just propagate this information
> when we pop the scope. That's usually how we propagate information upwards. Look at popScopeInternal.
> Also, instead of "functionBodyType != StandardFunctionBodyBlock", I think the code is easier to read
> with affirmative conditions, like "bodyType == ArrowExpr || bodyType == ArrowBlock"</span >
Refactored to use popScope and fixed condition
<span class="quote">>> Source/JavaScriptCore/parser/Parser.cpp:3644
>> + closestParentNonArrowFunctionNonLexicalScope()->setInnerArrowFunctionUseArgumetns();
>
> If you go with the popScopeInternalApproach, this can be determined then as well when we're already iterating over all used variables.</span >
Done
<span class="quote">>> Source/JavaScriptCore/parser/ParserModes.h:164
>> +typedef uint16_t ArrowFunctionCodeFeatures;
>
> this can be uint8_t</span >
Changed
<span class="quote">>> Source/JavaScriptCore/parser/SourceProviderCacheItem.h:47
>> + unsigned innerArrowFunctionFeatures;
>
> Why not ArrowFunctionCodeFeatures instead of unsigned?</span >
Fixed
<span class="quote">>> Source/JavaScriptCore/parser/SourceProviderCacheItem.h:92
>> + unsigned innerArrowFunctionFeatures : 31;
>
> ditto.</span >
Done</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>