Summary: | Revert changes in bug#160417 about extending `null` not being a derived class | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Saam Barati <saam> | ||||||
Component: | JavaScriptCore | Assignee: | 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
Saam Barati
2017-03-07 11:01:34 PST
Created attachment 312963 [details]
Patch
Patch
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 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. Created attachment 313228 [details]
Patch
Patch with fixes
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 |