Skip to content
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

Fix class declaration hoisting and typeof double-evaluation #1104

Merged
merged 1 commit into from
Mar 8, 2022
Merged

Fix class declaration hoisting and typeof double-evaluation #1104

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
15 changes: 15 additions & 0 deletions Jint.Tests.Test262/Language/Expressions/TypeOfTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
using Xunit;

namespace Jint.Tests.Test262.Language.Expressions
{
public class TypeOfTests : Test262Test
{
[Theory(DisplayName = "language\\expressions\\typeof")]
[MemberData(nameof(SourceFiles), "language\\expressions\\typeof", false)]
[MemberData(nameof(SourceFiles), "language\\expressions\\typeof", true, Skip = "Skipped")]
protected void TemplateLiteral(SourceFile sourceFile)
{
RunTestInternal(sourceFile);
}
}
}
7 changes: 7 additions & 0 deletions Jint.Tests/Runtime/EngineTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2809,6 +2809,13 @@ public void TypeofShouldEvaluateOnce()
Assert.Equal(1, result);
}

[Fact]
public void ClassDeclarationHoisting()
{
var ex = Assert.Throws<JavaScriptException>(() => _engine.Evaluate("typeof MyClass; class MyClass {}"));
Assert.Equal("Cannot access 'MyClass' before initialization", ex.Message);
}

[Fact]
public void ShouldObeyScriptLevelStrictModeInFunctions()
{
Expand Down
4 changes: 2 additions & 2 deletions Jint.Tests/Runtime/ModuleTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ public void ShouldAllowChaining()
Assert.Equal(-1, ns.Get("num").AsInteger());
}

[Fact]
[Fact(Skip = "TODO re-enable in module fix branch")]
public void ShouldAllowLoadingMoreThanOnce()
{
var called = 0;
Expand All @@ -215,7 +215,7 @@ public void ShouldAllowLoadingMoreThanOnce()

#if(NET6_0_OR_GREATER)

[Fact]
[Fact(Skip = "TODO re-enable in module fix branch")]
public void CanLoadModuleImportsFromFiles()
{
var engine = new Engine(options => options.EnableModules(GetBasePath()));
Expand Down
6 changes: 3 additions & 3 deletions Jint/Engine.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -835,7 +835,7 @@ private void GlobalDeclarationInstantiation(
ExceptionHelper.ThrowSyntaxError(realm, $"Identifier '{dn}' has already been declared");
}

if (d.Kind == VariableDeclarationKind.Const)
if (d.IsConstantDeclaration())
{
env.CreateImmutableBinding(dn, strict: true);
}
Expand Down Expand Up @@ -981,7 +981,7 @@ internal ArgumentsInstance FunctionDeclarationInstantiation(
for (var j = 0; j < d.BoundNames.Count; j++)
{
var dn = d.BoundNames[j];
if (d.Kind == VariableDeclarationKind.Const)
if (d.IsConstantDeclaration)
{
lexEnv.CreateImmutableBinding(dn, strict: true);
}
Expand Down Expand Up @@ -1143,7 +1143,7 @@ internal void EvalDeclarationInstantiation(
for (var j = 0; j < boundNames.Count; j++)
{
var dn = boundNames[j];
if (d.Kind == VariableDeclarationKind.Const)
if (d.IsConstantDeclaration())
{
lexEnvRec.CreateImmutableBinding(dn, strict: true);
}
Expand Down
17 changes: 16 additions & 1 deletion Jint/EsprimaExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,15 @@ or Nodes.ArrowParameterPlaceHolder
or Nodes.ClassExpression;
}

/// <summary>
/// https://tc39.es/ecma262/#sec-static-semantics-isconstantdeclaration
/// </summary>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal static bool IsConstantDeclaration(this Declaration d)
{
return d is VariableDeclaration { Kind: VariableDeclarationKind.Const };
Copy link
Owner

Choose a reason for hiding this comment

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

It hurts somewhere, but I can't locate it

Copy link
Owner

Choose a reason for hiding this comment

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

It's equivalent to return d is VariableDeclaration v && v.Kind == VariableDeclarationKind.Const; right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yup, takes some time to digest, but it's quite nice syntax, like is/are both these true - type and it's property values

}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal static bool HasName<T>(this T node) where T : Node
{
Expand Down Expand Up @@ -186,7 +195,8 @@ internal static void GetBoundNames(this Node? parameter, List<string> target)
parameter = restElement.Argument;
continue;
}
else if (parameter is ArrayPattern arrayPattern)

if (parameter is ArrayPattern arrayPattern)
{
ref readonly var arrayPatternElements = ref arrayPattern.Elements;
for (var i = 0; i < arrayPatternElements.Count; i++)
Expand Down Expand Up @@ -216,6 +226,11 @@ internal static void GetBoundNames(this Node? parameter, List<string> target)
parameter = assignmentPattern.Left;
continue;
}
else if (parameter is ClassDeclaration classDeclaration)
{
parameter = classDeclaration.Id;
continue;
}

break;
}
Expand Down
13 changes: 9 additions & 4 deletions Jint/HoistingScope.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@ internal readonly struct HoistingScope
internal readonly List<VariableDeclaration> _variablesDeclarations;
internal readonly List<Key> _varNames;

internal readonly List<VariableDeclaration> _lexicalDeclarations;
internal readonly List<Declaration> _lexicalDeclarations;
internal readonly List<string> _lexicalNames;

private HoistingScope(
List<FunctionDeclaration> functionDeclarations,
List<Key> varNames,
List<VariableDeclaration> variableDeclarations,
List<VariableDeclaration> lexicalDeclarations,
List<Declaration> lexicalDeclarations,
List<string> lexicalNames)
{
_functionDeclarations = functionDeclarations;
Expand Down Expand Up @@ -205,7 +205,7 @@ private sealed class ScriptWalker
internal List<Key> _varNames;

private readonly bool _collectLexicalNames;
internal List<VariableDeclaration> _lexicalDeclarations;
internal List<Declaration> _lexicalDeclarations;
internal List<string> _lexicalNames;

public ScriptWalker(bool strict, bool collectVarNames, bool collectLexicalNames)
Expand Down Expand Up @@ -248,7 +248,7 @@ public void Visit(Node node, Node parent)

if ((parent is null or Module) && variableDeclaration.Kind != VariableDeclarationKind.Var)
{
_lexicalDeclarations ??= new List<VariableDeclaration>();
_lexicalDeclarations ??= new List<Declaration>();
_lexicalDeclarations.Add(variableDeclaration);
if (_collectLexicalNames)
{
Expand All @@ -271,6 +271,11 @@ public void Visit(Node node, Node parent)
_functions ??= new List<FunctionDeclaration>();
_functions.Add((FunctionDeclaration)childNode);
}
else if (childNode.Type == Nodes.ClassDeclaration)
{
_lexicalDeclarations ??= new List<Declaration>();
_lexicalDeclarations.Add((Declaration) childNode);
}

if (childNode.Type != Nodes.FunctionDeclaration
&& childNode.Type != Nodes.ArrowFunctionExpression
Expand Down
44 changes: 33 additions & 11 deletions Jint/Runtime/Environments/JintEnvironment.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,37 +8,59 @@ namespace Jint.Runtime.Environments
{
internal static class JintEnvironment
{
internal static bool TryGetIdentifierEnvironmentWithBinding(
EnvironmentRecord env,
in EnvironmentRecord.BindingName name,
out EnvironmentRecord? record)
{
record = env;

var keyName = name.Key.Name;
if (env._outerEnv is null)
{
return env.HasBinding(keyName);
}

while (!ReferenceEquals(record, null))
{
if (record.HasBinding(keyName))
{
return true;
}

record = record._outerEnv;
}

return false;
}

internal static bool TryGetIdentifierEnvironmentWithBindingValue(
Engine engine,
EnvironmentRecord? lex,
EnvironmentRecord env,
in EnvironmentRecord.BindingName name,
bool strict,
out EnvironmentRecord? record,
out JsValue? value)
{
record = default;
record = env;
value = default;

if (ReferenceEquals(lex, engine.Realm.GlobalEnv)
&& lex.TryGetBinding(name, strict, out _, out value))
if (env._outerEnv is null)
{
record = lex;
return true;
return env.TryGetBinding(name, strict, out _, out value);
}

while (!ReferenceEquals(lex, null))
while (!ReferenceEquals(record, null))
{
if (lex.TryGetBinding(
if (record.TryGetBinding(
name,
strict,
out _,
out value))
{
record = lex;
return true;
}

lex = lex._outerEnv;
record = record._outerEnv;
}

return false;
Expand Down
7 changes: 2 additions & 5 deletions Jint/Runtime/Interpreter/Expressions/JintAssignmentExpression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,13 +390,10 @@ private ExpressionResult SetValue(EvaluationContext context)
var engine = context.Engine;
var env = engine.ExecutionContext.LexicalEnvironment;
var strict = StrictModeScope.IsStrictModeCode;
if (JintEnvironment.TryGetIdentifierEnvironmentWithBindingValue(
engine,
if (JintEnvironment.TryGetIdentifierEnvironmentWithBinding(
env,
left._expressionName,
strict,
out var environmentRecord,
out _))
out var environmentRecord))
{
if (strict && hasEvalOrArguments)
{
Expand Down
3 changes: 1 addition & 2 deletions Jint/Runtime/Interpreter/Expressions/JintIdentifierExpression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ protected override ExpressionResult EvaluateInternal(EvaluationContext context)
var engine = context.Engine;
var env = engine.ExecutionContext.LexicalEnvironment;
var strict = StrictModeScope.IsStrictModeCode;
var identifierEnvironment = JintEnvironment.TryGetIdentifierEnvironmentWithBindingValue(engine, env, _expressionName, strict, out var temp, out _)
var identifierEnvironment = JintEnvironment.TryGetIdentifierEnvironmentWithBinding(env, _expressionName, out var temp)
? temp
: JsValue.Undefined;

Expand All @@ -49,7 +49,6 @@ public override Completion GetValue(EvaluationContext context)
var env = engine.ExecutionContext.LexicalEnvironment;

if (JintEnvironment.TryGetIdentifierEnvironmentWithBindingValue(
engine,
env,
_expressionName,
strict,
Expand Down
1 change: 0 additions & 1 deletion Jint/Runtime/Interpreter/Expressions/JintMemberExpression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ protected override ExpressionResult EvaluateInternal(EvaluationContext context)
var strict = isStrictModeCode;
var env = engine.ExecutionContext.LexicalEnvironment;
JintEnvironment.TryGetIdentifierEnvironmentWithBindingValue(
engine,
env,
identifierExpression._expressionName,
strict,
Expand Down
1 change: 0 additions & 1 deletion Jint/Runtime/Interpreter/Expressions/JintUpdateExpression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ private JsValue UpdateIdentifier(EvaluationContext context)
var engine = context.Engine;
var env = engine.ExecutionContext.LexicalEnvironment;
if (JintEnvironment.TryGetIdentifierEnvironmentWithBindingValue(
engine,
env,
name,
strict,
Expand Down
4 changes: 2 additions & 2 deletions Jint/Runtime/Interpreter/JintFunctionDefinition.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ internal struct VariableValuePair

internal struct LexicalVariableDeclaration
{
public VariableDeclarationKind Kind;
public bool IsConstantDeclaration;
public List<string> BoundNames;
}
}
Expand Down Expand Up @@ -200,7 +200,7 @@ private State DoInitialize(FunctionInstance functionInstance)
d.GetBoundNames(boundNames);
declarations[i] = new State.LexicalVariableDeclaration
{
Kind = d.Kind,
IsConstantDeclaration = d.IsConstantDeclaration(),
BoundNames = boundNames
};
}
Expand Down
1 change: 0 additions & 1 deletion Jint/Runtime/Interpreter/Statements/JintClassDeclarationStatement.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ protected override Completion ExecuteInternal(EvaluationContext context)
var classBinding = _classDefinition._className;
if (classBinding != null)
{
env.CreateMutableBinding(classBinding);
env.InitializeBinding(classBinding, F);
}

Expand Down
2 changes: 1 addition & 1 deletion Jint/Runtime/Modules/JsModule.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -712,7 +712,7 @@ private void InitializeEnvironment()
for (var j = 0; j < boundNames.Count; j++)
{
var dn = boundNames[j];
if(d.Kind == VariableDeclarationKind.Const)
if(d.IsConstantDeclaration())
{
env.CreateImmutableBinding(dn, true);
}
Expand Down