Skip to content

Revert "WIP: Fix strict mode function declaration scoping in switch statements" #429

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rogeralsing merged 1 commit into from
Jan 6, 2026
Merged

Revert "WIP: Fix strict mode function declaration scoping in switch statements" #429

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/Asynkron.JsEngine/Ast/SwitchInstantiationPlan.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
namespace Asynkron.JsEngine.Ast;

internal sealed record SwitchInstantiationPlan(
bool IsStrict,
ImmutableArray<SwitchLexicalBinding> LexicalBindings,
ImmutableArray<SwitchFunctionBinding> FunctionBindings,
ImmutableArray<Symbol> ClassBindings);
12 changes: 9 additions & 3 deletions src/Asynkron.JsEngine/Ast/SwitchStatement.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,15 @@ SwitchInstantiationPlan IAstCacheable<SwitchInstantiationPlan>.GetOrCreateCache(
var functionBindings = ImmutableArray.CreateBuilder<SwitchFunctionBinding>();
var classBindings = ImmutableArray.CreateBuilder<Symbol>();

var isStrict = false;

foreach (var switchCase in self.Cases)
{
if (switchCase.Body.IsStrict)
{
isStrict = true;
}

foreach (var stmt in switchCase.Body.Statements)
{
if (stmt is VariableDeclaration
Expand All @@ -49,9 +56,7 @@ or VariableKind.AwaitUsing
{
var isAsyncOrGenerator = funcDecl.Function.IsAsync || funcDecl.Function.WasAsync ||
funcDecl.Function.IsGenerator;
functionBindings.Add(new SwitchFunctionBinding(
funcDecl.Name,
funcDecl.Function,
functionBindings.Add(new SwitchFunctionBinding(funcDecl.Name, funcDecl.Function,
!isAsyncOrGenerator));
continue;
}
Expand All @@ -64,6 +69,7 @@ or VariableKind.AwaitUsing
}

return new SwitchInstantiationPlan(
isStrict,
lexicalBindings.ToImmutable(),
functionBindings.ToImmutable(),
classBindings.ToImmutable());
Expand Down
11 changes: 4 additions & 7 deletions src/Asynkron.JsEngine/Ast/SwitchStatementExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,16 @@ private JsValue EvaluateSwitchJsValue(JsEnvironment environment,

var instantiationPlan = ((IAstCacheable<SwitchInstantiationPlan>)statement).GetOrCreateCache();

var isStrict = context.CurrentScope.IsStrict;
// Create a lexical environment for the entire switch block
// This environment is shared by all case clause bodies
var switchEnv = new JsEnvironment(environment, false, isStrict);
var switchEnv = new JsEnvironment(environment, false, instantiationPlan.IsStrict);

// Push a scope context for the switch block
var scopeMode = isStrict ? ScopeMode.Strict : ScopeMode.Sloppy;
var scopeMode = instantiationPlan.IsStrict ? ScopeMode.Strict : ScopeMode.Sloppy;
Comment on lines +30 to +33
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Jan 6, 2026

Choose a reason for hiding this comment

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

P2 Badge Avoid promoting strict mode to all switch cases

Because instantiationPlan.IsStrict is derived from any case body’s IsStrict, the single switch environment becomes strict for all cases as soon as one case starts with a "use strict" directive. In sloppy code this changes semantics for other cases (e.g., a non‑strict case assigning to a read‑only property or deleting a non‑configurable property will now throw), even if the strict case is never matched. Strictness should follow the outer scope or be applied per‑case, not promoted to the entire switch.

Useful? React with 👍 / 👎.

using var scopeHandle = context.PushScope(ScopeKind.Block, scopeMode);

// Hoist lexical declarations from all case bodies
SwitchStatement.InstantiateSwitchLexicalDeclarations(instantiationPlan, switchEnv, context, isStrict);
SwitchStatement.InstantiateSwitchLexicalDeclarations(instantiationPlan, switchEnv, context);

// V = undefined (spec step 1)
var completionValue = JsValue.Undefined;
Expand Down Expand Up @@ -109,7 +108,7 @@ private JsValue EvaluateSwitchJsValue(JsEnvironment environment,
}

private static void InstantiateSwitchLexicalDeclarations(SwitchInstantiationPlan plan, JsEnvironment switchEnv,
EvaluationContext context, bool isStrict)
EvaluationContext context)
{
foreach (var binding in plan.LexicalBindings)
{
Expand All @@ -129,8 +128,6 @@ private static void InstantiateSwitchLexicalDeclarations(SwitchInstantiationPlan
continue;
}

// In strict mode, block-scoped function declarations are initialized at switch entry
// (mirrors block evaluation behavior).
var functionValue = funcBinding.Function.CreateFunctionValue(switchEnv, context,
skipInternalNameBinding: true);
switchEnv.DefineJsValue(
Expand Down
Loading