Bug 142527 – "this" should be in TDZ until super is called in the constructor of a derived class

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

[Ryosuke Niwa]

Implement TDZ as an op-code and use it in the class implementation.

[Ryosuke Niwa]

Created attachment 248323 [details]
Adds TDZ in LLint and Baseline JIT

[Ryosuke Niwa]

Created attachment 248324 [details]
Added a missing @skip

[Filip Pizlo]

Comment on attachment 248324 [details]
Added a missing @skip

You should add this opcode to the DFG and FTL as part of this patch.  It's not hard to do, and we should stop this bad habit of not adding new things to the optimizing JITs immediately.

[Ryosuke Niwa]

Okay, but I literally have no idea what I'm supposed to do in DFG and FTL.  I think I need to send this bug to someone in JSC team in that case...

[Radar WebKit Bug Importer]

<rdar://problem/20117192>

[Filip Pizlo]

Created attachment 248449 [details]
WIP including DFG

[Filip Pizlo]

Created attachment 248453 [details]
all done

[Ryosuke Niwa]

Created attachment 248496 [details]
Updated change log

[Ryosuke Niwa]

Created attachment 248497 [details]
Benchmark results

I've ran relevant benchmarks with this patch applied. It doesn't seem to be a regression or progression at r181424.

[Ryosuke Niwa]

Created attachment 248498 [details]
Benchmark results

Oops, the previous benchmark result was wrong.  I wasn't turning on ES6_CLASS_SYNTAX.  I've re-measured it and I still don't see any progression or regression.

[Ryosuke Niwa]

Created attachment 248499 [details]
Fixed 32-bit build

[Ryosuke Niwa]

Comment on attachment 248499 [details]
Fixed 32-bit build

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

> Source/JavaScriptCore/ChangeLog:63
> +        (JSC::DFG::SpeculativeJIT::compile): Speculative the operand to be not empty. OSR exit if the speculation fails.

Speculate*

> Source/JavaScriptCore/ChangeLog:70
> +        (JSC::FTL::LowerDFGToLLVM::compileCheckNotEmpty): OSR exit with "TDZFailure" if the operand is not empty.

if the operand *is* empty.

[Mark Hahnenberg]

Comment on attachment 248499 [details]
Fixed 32-bit build

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

> Source/JavaScriptCore/tests/stress/class-syntax-tdz.js:1
> +//@ skip

Why are we skipping? Does this not mean what I think it means?

[Ryosuke Niwa]

(In reply to comment #13)
> Comment on attachment 248499 [details]
> Fixed 32-bit build
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=248499&action=review
> 
> > Source/JavaScriptCore/tests/stress/class-syntax-tdz.js:1
> > +//@ skip
> 
> Why are we skipping? Does this not mean what I think it means?

ES6 class syntax is disabled by default.

[Mark Hahnenberg]

Comment on attachment 248499 [details]
Fixed 32-bit build

LGTM.

[WebKit Commit Bot]

Comment on attachment 248499 [details]
Fixed 32-bit build

Clearing flags on attachment: 248499

Committed r181466: <http://trac.webkit.org/changeset/181466>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Joseph Pecoraro]

Comment on attachment 248499 [details]
Fixed 32-bit build

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

> Source/JavaScriptCore/tests/stress/class-syntax-tdz.js:26
> +        throw "Exception not thrown for an unitialized this at iteration " + i;

Typo: "unitialized" => "uninitialized"! But doesn't really matter. Nice patch!