[Webkit-unassigned] [Bug 153981] [ES6] Arrow function syntax. Emit loading&putting this/super only if they are used in arrow function
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Feb 18 12:54:18 PST 2016
https://bugs.webkit.org/show_bug.cgi?id=153981
--- Comment #5 from GSkachkov <gskachkov at gmail.com> ---
Comment on attachment 271327
--> https://bugs.webkit.org/attachment.cgi?id=271327
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=271327&action=review
>> Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:132
>> + bool isInnerArrowFunctionUseNewTarget() { return m_arrowFunctionCodeFeatures & NewTargetArrowFunctionFeature; }
>
> I would name these like "doAnyInnerArrowFunctionsX"
> i.e:
> isInnerArrowFunctionUseArguments => doAnyInnerArrowFunctionsUseArguments
Renamed
>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:4051
>> +}
>
> Is "new.target" not allowed in eval?
Ups. I forget about eval. Fixed.
>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:648
>> + void emitPutThisToArrowFunctionContextScope(bool = false);
>
> Style: give this bool a name, like "shouldAlwaysPut" or something.
added
>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:724
>> + bool isArgumentsUsedInArrowFunction();
>
> nit: I would name these with "Inner" before "In"
> like:
> "isThisUsedInInnerArrowFunction"
Done
>> 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);
> ```
Done
>> 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"
Refactored to use popScope and fixed condition
>> 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.
Done
>> Source/JavaScriptCore/parser/ParserModes.h:164
>> +typedef uint16_t ArrowFunctionCodeFeatures;
>
> this can be uint8_t
Changed
>> Source/JavaScriptCore/parser/SourceProviderCacheItem.h:47
>> + unsigned innerArrowFunctionFeatures;
>
> Why not ArrowFunctionCodeFeatures instead of unsigned?
Fixed
>> Source/JavaScriptCore/parser/SourceProviderCacheItem.h:92
>> + unsigned innerArrowFunctionFeatures : 31;
>
> ditto.
Done
--
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160218/04743959/attachment.html>
More information about the webkit-unassigned
mailing list