[webkit-reviews] review granted: [Bug 127887] Make it possible to implement JS builtins in JS : [Attachment 223919] fix release builds

[bugzilla-daemon at webkit.org]

Michael Saboff <msaboff at apple.com> has granted Oliver Hunt <oliver at apple.com>'s
request for review:
Bug 127887: Make it possible to implement JS builtins in JS
https://bugs.webkit.org/show_bug.cgi?id=127887

Attachment 223919: fix release builds
https://bugs.webkit.org/attachment.cgi?id=223919&action=review

------- Additional Comments from Michael Saboff <msaboff at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=223919&action=review


r=me.  Get the EWS bots green before checkin.

> Source/JavaScriptCore/builtins/Array.prototype.js:46
> +	   if (!(i in array))
> +	       continue;
> +	   if (!callback. at call(thisArg, array[i], i, array))
> +	       return false;

Is there any concerns that the array will be mutated by the callback?  Is the
"i in array" check considered enough?

> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:548
> +	       

Extra space.

> Source/JavaScriptCore/generate-js-builtins:89
> +
> +
> +

One one blank line between function definitions.

> Source/JavaScriptCore/generate-js-builtins:127
> +	   
> +

One one blank line between function definitions.

> Source/JavaScriptCore/parser/Lexer.cpp:761
> +bool isSafeIdentifier(VM& vm, const Identifier* ident)

This should be called isSafeBuiltinIdentifier().

> Source/JavaScriptCore/runtime/CommonIdentifiers.h:237
> +	   

Extra space

> Source/JavaScriptCore/runtime/PropertySlot.h:47
> +    BuiltinOrFunction = Builtin | Function, // helper only used by static
hashtables

Indent the '=' the same as most other Attribute values (e.g. Accessor).