BytecodeGenerator should do this since it will have the necessary info to do so.
👍
Created attachment 258326 [details] patch
Attachment 258326 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:2635: Missing space before { [whitespace/braces] [5] Total errors found: 1 in 13 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 258350 [details] patch
Attachment 258350 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:2635: Missing space before { [whitespace/braces] [5] Total errors found: 1 in 13 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 258350 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=258350&action=review r=me, with a fixup below > Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:2840 > + m_topMostScope = addVar(); > + m_topMostScope->ref(); > + emitMove(m_topMostScope, scopeRegister()); Not new to this patch, but: Does our new lexical scoping have the behavior that we now have var registers and temporary registers interleaved with each other, such that I might have: [Var][Temp][Var][Temp] ? That's a little sad because all temporary registers prior to the last vars will "leak" in the sense that the function will never reuse them, thus bloating its stack frame size. Also, there's no need to ref m_topMostScope here because addVar() already refs. Also, if you did want to ref(), I would ask you to use RefPtr.
Comment on attachment 258350 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=258350&action=review >> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:2840 >> + emitMove(m_topMostScope, scopeRegister()); > > Not new to this patch, but: Does our new lexical scoping have the behavior that we now have var registers and temporary registers interleaved with each other, such that I might have: > > [Var][Temp][Var][Temp] > > ? > > That's a little sad because all temporary registers prior to the last vars will "leak" in the sense that the function will never reuse them, thus bloating its stack frame size. > > Also, there's no need to ref m_topMostScope here because addVar() already refs. > > Also, if you did want to ref(), I would ask you to use RefPtr. We don't do this. I believe we have an assert in addVar() that will prevent this from happening. It would also be wrong if we did this because it would break op_enter. The way we handle register reuse for block scope variables like "let" is that while we're generating code for the block's statements we'll reserve a non-temp register for the block scope variable. Once we're done generating code for that block scope, we will reclaim those block scoped registers. The code that does this is in pushLexicalScopeInternal and popLexicalScopeInternal and reclaimFreeRegisters. This doesn't mean that we won't have some block scoped variables that are live for the entire function. For example, functions with default parameter values will have all parameters be in a block scope that never frees its variables. (These registers will still be allocated after the "var" register locations). I'll remove the ref and land later.
Created attachment 258502 [details] patch for landing.
Attachment 258502 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:2635: Missing space before { [whitespace/braces] [5] Total errors found: 1 in 13 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 258502 [details] patch for landing. Clearing flags on attachment: 258502 Committed r188136: <http://trac.webkit.org/changeset/188136>
All reviewed patches have been landed. Closing bug.
Committed r188144: <http://trac.webkit.org/changeset/188144>
(In reply to comment #12) > Committed r188144: <http://trac.webkit.org/changeset/188144> Sorry, wrong bug. My changelog got messed up somehow.
> The way we handle register reuse for block scope variables like "let" is that > while we're generating code for the block's statements we'll reserve a > non-temp > register for the block scope variable. Once we're done generating code for > that block scope, we will reclaim those block scoped registers. The code that > does this is in pushLexicalScopeInternal and popLexicalScopeInternal and > reclaimFreeRegisters. Oh! I get it now.