Improve error message for uninitialized |this| in derived constructor
Created attachment 416882 [details] Patch
Comment on attachment 416882 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=416882&action=review We found that our bytecode generation is too generic, and emitting many bytecodes. This prevents us from performing inlining in https://bugs.webkit.org/show_bug.cgi?id=220204. So, can you change this without increasing bytecode size? > Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:2852 > +void BytecodeGenerator::emitTDZCheckForThisRegisterInDerivedConstructor() > +{ > + Ref<Label> thisIsNotInTDZ = newLabel(); > + emitJumpIfFalse(emitIsEmpty(newTemporary(), thisRegister()), thisIsNotInTDZ.get()); > + emitThrowReferenceError("'super()' must be called in derived constructor before accessing |this| or returning non-object."_s); > + emitLabel(thisIsNotInTDZ.get()); The problem is that,
Comment on attachment 416882 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=416882&action=review >> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:2852 >> + emitLabel(thisIsNotInTDZ.get()); > > The problem is that, The problem is that, previously, it was check_tdz. But now, it is more bytecodes. This will cause performance/memory-usage problem because it increases bytecode size. I think that, 1. Since this is super rare case in practice, we should not emit bytecodes much for that pattern. 2. I think we should switch to the different solution: we should throw appropriate error message ad-hocly when we throw an error. Like, when we failed with check_tdz, we should investigate what causes it by checking bytecode stream / source code etc. and throws appropriate error.
(In reply to Yusuke Suzuki from comment #3) > The problem is that, previously, it was check_tdz. But now, it is more > bytecodes. This will cause performance/memory-usage problem because it > increases bytecode size. Yes, there is an increase in bytecode size, but only for derived constructors, which are super rare.
(In reply to Alexey Shvayka from comment #4) > (In reply to Yusuke Suzuki from comment #3) > > The problem is that, previously, it was check_tdz. But now, it is more > > bytecodes. This will cause performance/memory-usage problem because it > > increases bytecode size. > > Yes, there is an increase in bytecode size, but only for derived > constructors, which are super rare. Please see BytecodeGenerator::ensureThis(): emitTDZCheckForThisRegisterInDerivedConstructor(), that emits additional bytecodes, is guarded by `if (constructorKind() == ConstructorKind::Extends || isDerivedConstructorContext())`.
(In reply to Alexey Shvayka from comment #5) > Please see BytecodeGenerator::ensureThis(): > emitTDZCheckForThisRegisterInDerivedConstructor(), that emits additional > bytecodes, is guarded by `if (constructorKind() == ConstructorKind::Extends > || isDerivedConstructorContext())`. Discussed this on Slack: we should avoid having super rare things, like TDZ errors, in bytecode, and use slow paths instead. Emitting many bytecodes for error checking can penalize CodeBlock heuristics, telling DFG / FTL that the function is too large.
Created attachment 416887 [details] Patch
Comment on attachment 416887 [details] Patch r=me
Committed r271120: <https://trac.webkit.org/changeset/271120> All reviewed patches have been landed. Closing bug and clearing flags on attachment 416887 [details].
<rdar://problem/72771016>