[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