[Webkit-unassigned] [Bug 147051] Add support for new.target.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=147051

Yusuke Suzuki <utatane.tea at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #257099|review?, commit-queue?      |review+, commit-queue+
              Flags|                            |

--- Comment #13 from Yusuke Suzuki <utatane.tea at gmail.com> ---
Comment on attachment 257099
  --> https://bugs.webkit.org/attachment.cgi?id=257099
Patch

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

r+

>>>>> Source/JavaScriptCore/parser/ASTBuilder.h:183
>>>>> +    ExpressionNode* newTargetExpr(const JSTokenLocation location)
>>>> 
>>>> small nit: We usually try to name things that create AST nodes "createX".
>>>> So here, "createNewTargetNode" or "createNewTargetExpression".
>>> 
>>> Is there a reason why thisExpr and superExpr don't follow this rule? I named my function "newTargetExpr" since new.target is similar to this but for constructors, so I was trying to emulate those functions.
>> 
>> Ah, I guess they are just overlooked ones. Could you rename them to createThisExpr, createSuperExpr etc. at the same time?
> 
> I'm not a huge fan of mixing new code with refactors, in case things get rolled out. I'll throw another patch up with that fix.

Make sense.

-- 
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/20150721/5fbf3657/attachment.html>