[webkit-reviews] review granted: [Bug 214525] Remove OpIsObjectOrNull from ClassExprNode::emitBytecode() : [Attachment 404924] Patch

[bugzilla-daemon at webkit.org]

Keith Miller <keith_miller at apple.com> has granted Alexey Shvayka
<shvaikalesh at gmail.com>'s request for review:
Bug 214525: Remove OpIsObjectOrNull from ClassExprNode::emitBytecode()
https://bugs.webkit.org/show_bug.cgi?id=214525

Attachment 404924: Patch

https://bugs.webkit.org/attachment.cgi?id=404924&action=review




--- Comment #7 from Keith Miller <keith_miller at apple.com> ---
Comment on attachment 404924
  --> https://bugs.webkit.org/attachment.cgi?id=404924
Patch

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

r=me with some code relocation.

> Source/JavaScriptCore/ChangeLog:12
> +	   1. Replaces OpIsObjectOrNull in ClassExprNode::emitBytecode() with
emitIsObject() +
> +	      emitIsNull(), fixing `document.all` as `superclass.prototype`
case with DFG/FTL.
> +	      Also, tweaks error message to reflect that `null` is allowed.

Can you describe what the change in behavior is? I assume it's that we say
document.all can't be the superclass of a class expression?

> Source/JavaScriptCore/ChangeLog:14
> +	   2. Renames is_object_or_null bytecode op to typeof_is_object, fixing
confusing

Nit: fixing confusing => fixing the confusing.

> Source/JavaScriptCore/bytecode/BytecodeList.rb:336
> -	   :is_object_or_null,
> +	   :typeof_is_object,

Can you put this next to typeof_is_undefined.

> Source/JavaScriptCore/bytecode/BytecodeUseDef.cpp:191
> -    USES(OpIsObjectOrNull, operand)
> +    USES(OpTypeofIsObject, operand)

ditto on location.

> Source/JavaScriptCore/bytecode/BytecodeUseDef.cpp:498
> -    DEFS(OpIsObjectOrNull, dst)
> +    DEFS(OpTypeofIsObject, dst)

ditto on  location.

> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:1463
> -    case IsObjectOrNull:
> +    case TypeOfIsObject:

Ditto on location

> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:1499
> -	       case IsObjectOrNull:
> +	       case TypeOfIsObject:

Ditto on location

> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:1695
> -	   case IsObjectOrNull:
> +	   case TypeOfIsObject:

Ditto on location I'm not going to comment on each of them for brevity but can
you migrate all the various places.