Skip to content

Conversation

@kwonjeomsim
Copy link
Contributor

This PR removes some bytecodes that generate function parameters which are not used during execution.
I add this feature in release mode only because there are some test failed in debug mode.

@clover2123
Copy link
Contributor

@kwonjeomsim
Why did you disable this update in debug mode?
FYI) debug mode and Debugger are different
Debugger enabled by ESCARGOT_DEBUGGER option is one of features that escargot supports.
Using Debugger, developers could debug (investigate) their JS code like other JS debugger (Ex chrome developer mode)

@kwonjeomsim kwonjeomsim force-pushed the optimize branch 15 times, most recently from 718bb14 to dd6bde3 Compare October 13, 2025 16:03
@kwonjeomsim
Copy link
Contributor Author

kwonjeomsim commented Oct 14, 2025

@kwonjeomsim Why did you disable this update in debug mode? FYI) debug mode and Debugger are different Debugger enabled by ESCARGOT_DEBUGGER option is one of features that escargot supports. Using Debugger, developers could debug (investigate) their JS code like other JS debugger (Ex chrome developer mode)

@clover2123
I fixed commit so this will disabled in debugger mode instead of debug mode. Please check again!

continue;
}

if (name == "eval" || name == "arguments") {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The added if (name == "eval" || name == "arguments") replaces the original check that used scopeCtx->m_hasEval and scopeCtx->m_allowArguments. This causes the function to mark parameters as used for the names "eval" or "arguments" even when the current scope does not actually allow eval or arguments, leading to incorrect parameter usage tracking. Restore the original flag-based check or add a guard that verifies scopeCtx->m_hasEval or scopeCtx->m_allowArguments before marking the parameter as used.

if (!contains) {
this->currentBlockContext->m_usingNames.push_back(name);
#ifndef ESCARGOT_DEBUGGER
ASTScopeContext* scope = this->currentScopeContext;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace the while loop with a for loop to make the traversal of the scope chain more idiomatic and reduce boilerplate. The for form for (auto* scope = this->currentScopeContext; scope; scope = scope->m_parent) expresses the same logic in a single line, improving readability. This change keeps the semantics identical and avoids the need for a separate assignment statement. It also aligns with common C++ style for simple pointer iteration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants