Bug 169293

Summary: Revert changes in bug#160417 about extending `null` not being a derived class
Product: WebKit Reporter: Saam Barati <saam>
Component: JavaScriptCoreAssignee: GSkachkov <gskachkov>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, fpizlo, ggaren, gskachkov, jfbastien, keith_miller, mark.lam, msaboff, ticaiolima, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Saam Barati 2017-03-07 11:01:34 PST
See:
https://bugs.webkit.org/show_bug.cgi?id=160417
Comment 1 Radar WebKit Bug Importer 2017-06-13 13:21:16 PDT
<rdar://problem/32746950>
Comment 2 GSkachkov 2017-06-15 02:08:02 PDT
Created attachment 312963 [details]
Patch

Patch
Comment 3 Saam Barati 2017-06-15 13:14:11 PDT
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.
Comment 4 Saam Barati 2017-06-15 13:15:42 PDT
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.
Comment 5 GSkachkov 2017-06-18 09:19:41 PDT
Created attachment 313228 [details]
Patch

Patch with fixes
Comment 6 Saam Barati 2017-06-19 11:57:02 PDT
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
Comment 7 GSkachkov 2017-06-20 03:29:55 PDT
Issue is closed.
Patch landed by r218581 commit.