Bug 169293 – Revert changes in bug#160417 about extending `null` not being a derived class

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Saam Barati]

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

[Radar WebKit Bug Importer]

<rdar://problem/32746950>

[GSkachkov]

Created attachment 312963 [details]
Patch

Patch

[Saam Barati]

Comment on attachment 312963 [details]
Patch

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

> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:-3765
> -            Ref<Label> superclassIsNullLabel = generator.newLabel();
> -            Ref<Label> done = generator.newLabel();
> -
> -            generator.emitJumpIfTrue(generator.emitUnaryOp(op_eq_null, tempRegister.get(), superclass.get()), superclassIsNullLabel.get());
>              generator.emitNewDefaultConstructor(constructor.get(), ConstructorKind::Extends, m_name, ecmaName(), m_classSource);
>              generator.emitLoad(tempRegister.get(), jsBoolean(true));
> -            generator.emitJump(done.get());
> -            generator.emitLabel(superclassIsNullLabel.get());
> -            generator.emitNewDefaultConstructor(constructor.get(), ConstructorKind::Base, m_name, ecmaName(), m_classSource);
> -            generator.emitLoad(tempRegister.get(), jsBoolean(false));
> -            generator.emitLabel(done.get());

Lets move back  to determining this property statically. There is no need for a putById here, since it's always true!  Also, things that get this property should also be able to determine its value statically.

[Saam Barati]

Comment on attachment 312963 [details]
Patch

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

>> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:-3765
>> -            generator.emitLabel(done.get());
> 
> Lets move back  to determining this property statically. There is no need for a putById here, since it's always true!  Also, things that get this property should also be able to determine its value statically.

Also, there is the case above you didn't change. Perhaps we don't have tests for this. Please add them if this is true.

[GSkachkov]

Created attachment 313228 [details]
Patch

Patch with fixes

[Saam Barati]

Comment on attachment 313228 [details]
Patch

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

> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:3745
> +        constructor = generator.emitNewDefaultConstructor(generator.finalDestination(dst), m_classHeritage ? ConstructorKind::Extends : ConstructorKind::Base, m_name, ecmaName(), m_classSource);

style nit: no braces around "else" since this is just one line now

[GSkachkov]

Issue is closed.
Patch landed by r218581 commit.