From 62a18e22d4b39966d85084d83034a0355646b2d6 Mon Sep 17 00:00:00 2001 From: Deukhoofd Date: Sat, 24 Nov 2018 12:42:54 +0100 Subject: [PATCH] Fixed Scoping issue --- Upsilon/BaseTypes/ScriptFunction.cs | 2 +- Upsilon/BaseTypes/ScriptTable.cs | 2 +- Upsilon/Binder/Binder.cs | 22 ++++---- Upsilon/Binder/BoundScope.cs | 43 +++++---------- .../BoundVariableAssignment.cs | 4 +- Upsilon/Evaluator/EvaluationScope.cs | 52 ++++--------------- Upsilon/Evaluator/Evaluator.cs | 28 +++++----- Upsilon/StandardLibraries/StaticScope.cs | 9 ++-- UpsilonTests/GeneralTests/ScopeTests.cs | 18 +++++++ 9 files changed, 76 insertions(+), 104 deletions(-) diff --git a/Upsilon/BaseTypes/ScriptFunction.cs b/Upsilon/BaseTypes/ScriptFunction.cs index 4056733..a44db2a 100644 --- a/Upsilon/BaseTypes/ScriptFunction.cs +++ b/Upsilon/BaseTypes/ScriptFunction.cs @@ -45,7 +45,7 @@ namespace Upsilon.BaseTypes { var parameterVariable = Parameters[i]; var parameterValue = variables[i]; - innerEvaluator.Scope.Set(parameterVariable, parameterValue, true); + innerEvaluator.Scope.CreateLocal(parameterVariable, parameterValue); } return innerEvaluator.EvaluateNode(Block); } diff --git a/Upsilon/BaseTypes/ScriptTable.cs b/Upsilon/BaseTypes/ScriptTable.cs index bf8cffe..15cbbe2 100644 --- a/Upsilon/BaseTypes/ScriptTable.cs +++ b/Upsilon/BaseTypes/ScriptTable.cs @@ -41,7 +41,7 @@ namespace Upsilon.BaseTypes public void Set(Diagnostics diagnostics, TextSpan span, ScriptType index, ScriptType value) { var s = index.ToString(); - EvaluationScope.Set(new VariableSymbol(s, value.Type, false), value, true); + EvaluationScope.CreateLocal(new VariableSymbol(s, value.Type, false), value); } public IEnumerator<(string Key, ScriptType value)> GetEnumerator() diff --git a/Upsilon/Binder/Binder.cs b/Upsilon/Binder/Binder.cs index a49db90..607533e 100644 --- a/Upsilon/Binder/Binder.cs +++ b/Upsilon/Binder/Binder.cs @@ -38,7 +38,7 @@ namespace Upsilon.Binder Scope = new BoundScope(Scope); foreach (var valueParameter in unboundFunctionStatement.Value.Parameters) { - Scope.SetVariable(valueParameter); + Scope.AssignToNearest(valueParameter); } unboundFunctionStatement.Value.Block = (BoundBlockStatement) BindBlockStatement(unboundFunctionStatement.Value.UnboundBlock); @@ -202,7 +202,7 @@ namespace Upsilon.Binder var functionVariable = function.Parameters[index]; var callingVariable = parameters[index]; functionVariable.Type = callingVariable.Type; - Scope.SetVariable(functionVariable); + Scope.DefineLocalVariable(functionVariable); } var unboundFunctionStatement = _unboundFunctions[function.Name]; @@ -253,9 +253,9 @@ namespace Upsilon.Binder } if (isLocal) - Scope.SetVariable(variable); + Scope.DefineLocalVariable(variable); else - Scope.SetGlobalVariable(variable); + Scope.AssignToNearest(variable); } else { @@ -299,7 +299,7 @@ namespace Upsilon.Binder var boundVariable = TryBindVariable(name, isLocal, boundExpression); if (boundVariable != null) { - return new BoundVariableAssignment(boundVariable, boundExpression); + return new BoundVariableAssignment(boundVariable, boundExpression, isLocal); } } @@ -369,7 +369,7 @@ namespace Upsilon.Binder { var vari = new VariableSymbol(identifierToken.Name, Type.Unknown, true); parameters.Add(vari); - innerScope.SetVariable(vari); + innerScope.DefineLocalVariable(vari); } if (parameters.Count == 0) @@ -408,7 +408,7 @@ namespace Upsilon.Binder { var vari = new VariableSymbol(identifierToken.Name, Type.Unknown, true); parameters.Add(vari); - innerScope.SetVariable(vari); + innerScope.DefineLocalVariable(vari); } @@ -417,9 +417,9 @@ namespace Upsilon.Binder variable = new FunctionVariableSymbol(name, Type.Function, isLocal, parameters.ToImmutable()); ((FunctionVariableSymbol) variable).IsBound = !(func is UnboundFunctionExpression); if (isLocal) - Scope.SetVariable(variable); + Scope.DefineLocalVariable(variable); else - Scope.SetGlobalVariable(variable); + Scope.AssignToNearest(variable); } else { @@ -528,7 +528,7 @@ namespace Upsilon.Binder var variableName = e.Identifier.Name; Scope = new BoundScope(Scope); var variable = new VariableSymbol(variableName, Type.Number, true); - Scope.SetVariable(variable); + Scope.DefineLocalVariable(variable); var boundStart = BindExpression(e.StartExpression); var boundStop = BindExpression(e.StopExpression); BoundExpression boundStep = null; @@ -546,7 +546,7 @@ namespace Upsilon.Binder foreach (var variableIdentifier in e.Variables) { var variable = new VariableSymbol(variableIdentifier.Name, Type.Unknown, true); - Scope.SetVariable(variable); + Scope.DefineLocalVariable(variable); array.Add(variable); } var boundEnumerableExpression = BindExpression(e.EnumerableExpression); diff --git a/Upsilon/Binder/BoundScope.cs b/Upsilon/Binder/BoundScope.cs index 4679356..ee7581f 100644 --- a/Upsilon/Binder/BoundScope.cs +++ b/Upsilon/Binder/BoundScope.cs @@ -7,20 +7,17 @@ namespace Upsilon.Binder public readonly BoundScope ParentScope; private BoundScope _readOnlyScope; public readonly Dictionary Variables; - internal readonly Dictionary LocalVariables; public BoundScope(BoundScope parentScope) { ParentScope = parentScope; Variables = new Dictionary(); - LocalVariables = new Dictionary(); } public BoundScope(Dictionary variables, BoundScope parentScope) { ParentScope = parentScope; Variables = variables; - LocalVariables = new Dictionary(); } public static BoundScope WithReadOnlyScope(BoundScope readOnlyScope) @@ -29,43 +26,29 @@ namespace Upsilon.Binder return scope; } - public void SetVariable(VariableSymbol var) + public void DefineLocalVariable(VariableSymbol var) { - if (var.Local) + Variables.Add(var.Name, var); + } + + public void AssignToNearest(VariableSymbol var) + { + if (Variables.ContainsKey(var.Name)) { - if (LocalVariables.ContainsKey(var.Name)) - LocalVariables[var.Name] = var; - else - LocalVariables.Add(var.Name, var); + + } + else if (ParentScope != null) + { + ParentScope.AssignToNearest(var); } else { - if (Variables.ContainsKey(var.Name)) - Variables[var.Name] = var; - else - Variables.Add(var.Name, var); + Variables.Add(var.Name, var); } } - public void SetGlobalVariable(VariableSymbol var) - { - if (ParentScope == null) - { - SetVariable(var); - } - else - { - ParentScope.SetGlobalVariable(var); - } - } - - public bool TryGetVariable(string key, bool allowUpperScopes, out VariableSymbol result) { - if (LocalVariables.TryGetValue(key, out result)) - { - return true; - } if (Variables.TryGetValue(key, out result)) { return true; diff --git a/Upsilon/Binder/BoundStatements/BoundVariableAssignment.cs b/Upsilon/Binder/BoundStatements/BoundVariableAssignment.cs index 1cb0f4c..d72840c 100644 --- a/Upsilon/Binder/BoundStatements/BoundVariableAssignment.cs +++ b/Upsilon/Binder/BoundStatements/BoundVariableAssignment.cs @@ -4,11 +4,13 @@ namespace Upsilon.Binder { public VariableSymbol Variable { get; } public BoundExpression BoundExpression { get; } + public bool IsLocalDefinition { get; } - public BoundVariableAssignment(VariableSymbol variable, BoundExpression boundExpression) + public BoundVariableAssignment(VariableSymbol variable, BoundExpression boundExpression, bool isLocalDefinition) { Variable = variable; BoundExpression = boundExpression; + IsLocalDefinition = isLocalDefinition; } public override BoundKind Kind => BoundKind.BoundAssignmentStatement; diff --git a/Upsilon/Evaluator/EvaluationScope.cs b/Upsilon/Evaluator/EvaluationScope.cs index 68cf529..4bcb508 100644 --- a/Upsilon/Evaluator/EvaluationScope.cs +++ b/Upsilon/Evaluator/EvaluationScope.cs @@ -10,20 +10,17 @@ namespace Upsilon.Evaluator private EvaluationScope _getOnlyParentScope; public readonly Dictionary Variables; - private readonly Dictionary _localVariables; internal EvaluationScope(EvaluationScope parentScope) { _parentScope = parentScope; Variables = new Dictionary(); - _localVariables = new Dictionary(); } internal EvaluationScope(Dictionary vars) { Variables = vars; - _localVariables = new Dictionary(); } internal static EvaluationScope CreateWithGetOnlyParent(EvaluationScope parent) @@ -32,55 +29,30 @@ namespace Upsilon.Evaluator return scope; } - public void Set(VariableSymbol symbol, ScriptType obj, bool createNew) + public void AssignToNearest(VariableSymbol symbol, ScriptType value) { - if (symbol.Local && createNew) + if (Variables.ContainsKey(symbol.Name)) { - if (_localVariables.ContainsKey(symbol.Name)) - { - _localVariables[symbol.Name] = obj; - } - else - { - _localVariables.Add(symbol.Name, obj); - } + Variables[symbol.Name] = value; + } + else if (_parentScope != null) + { + _parentScope.AssignToNearest(symbol, value); } else { - if (Variables.ContainsKey(symbol.Name)) - { - Variables[symbol.Name] = obj; - } - else if (_localVariables.ContainsKey(symbol.Name)) - { - _localVariables[symbol.Name] = obj; - } - else if (_parentScope != null && _parentScope.TryGet(symbol.Name, out _)) - { - _parentScope.Set(symbol, obj, false); - } - else - { - Variables.Add(symbol.Name, obj); - } - + Variables.Add(symbol.Name, value); } } - public void SetGlobal(VariableSymbol symbol, ScriptType obj) + public void CreateLocal(VariableSymbol symbol, ScriptType value) { - if (_parentScope != null) - _parentScope.SetGlobal(symbol, obj); - else - { - Set(symbol, obj, true); - } + Variables[symbol.Name] = value; } + public bool TryGet(VariableSymbol symbol, out ScriptType obj) { - if (_localVariables.TryGetValue(symbol.Name, out obj)) - return true; if (Variables.TryGetValue(symbol.Name, out obj)) return true; if (_parentScope != null) @@ -94,8 +66,6 @@ namespace Upsilon.Evaluator public bool TryGet(string variable, out ScriptType obj) { - if (_localVariables.TryGetValue(variable, out obj)) - return true; if (Variables.TryGetValue(variable, out obj)) return true; if (_parentScope != null) diff --git a/Upsilon/Evaluator/Evaluator.cs b/Upsilon/Evaluator/Evaluator.cs index f4c67e8..2649455 100644 --- a/Upsilon/Evaluator/Evaluator.cs +++ b/Upsilon/Evaluator/Evaluator.cs @@ -59,7 +59,7 @@ namespace Upsilon.Evaluator { var parameter = parameters[index]; var parameterName = function.Parameters[index]; - innerEvaluator.Scope.Set(parameterName, parameter, true); + innerEvaluator.Scope.CreateLocal(parameterName, parameter); } var result = innerEvaluator.EvaluateNode(function.Block); @@ -295,13 +295,13 @@ namespace Upsilon.Evaluator private void EvaluateAssignmentStatement(BoundVariableAssignment e) { var val = EvaluateExpression(e.BoundExpression); - if (e.Variable.Local) + if (e.IsLocalDefinition) { - Scope.Set(e.Variable, val, true); + Scope.CreateLocal(e.Variable, val); } else { - Scope.SetGlobal(e.Variable, val); + Scope.AssignToNearest(e.Variable, val); } _lastValue = val; } @@ -320,9 +320,9 @@ namespace Upsilon.Evaluator continue; var value = table.Get(_diagnostics, e.Span, new ScriptNumberLong(i + 1), Scope); if (variableSymbol.Local) - Scope.Set(variableSymbol, value, true); + Scope.CreateLocal(variableSymbol, value); else - Scope.SetGlobal(variableSymbol, value); + Scope.AssignToNearest(variableSymbol, value); } } else @@ -374,9 +374,9 @@ namespace Upsilon.Evaluator { var func = EvaluateBoundFunctionStatement(e.Func); if (e.Variable.Local) - Scope.Set(e.Variable, func, true); + Scope.CreateLocal(e.Variable, func); else - Scope.SetGlobal(e.Variable, func); + Scope.AssignToNearest(e.Variable, func); } private ScriptType EvaluateBoundFunctionStatement(BoundFunctionExpression boundFunctionExpression) @@ -434,8 +434,8 @@ namespace Upsilon.Evaluator { innerEvaluator.EvaluateStatement(boundStatement); if (innerEvaluator._lastValue != null) - tableScope.Set(new VariableSymbol(currentPos.ToString(), innerEvaluator._lastValue.Type, false), - innerEvaluator._lastValue, true); + tableScope.CreateLocal(new VariableSymbol(currentPos.ToString(), innerEvaluator._lastValue.Type, false), + innerEvaluator._lastValue); innerEvaluator._lastValue = null; break; } @@ -506,7 +506,7 @@ namespace Upsilon.Evaluator { var innerEvaluator = new Evaluator(_diagnostics, Scope); var startVal = (ScriptNumberLong)innerEvaluator.EvaluateExpression(e.BoundStart); - innerEvaluator.Scope.Set(e.Variable, startVal, true); + innerEvaluator.Scope.CreateLocal(e.Variable, startVal); var stopVal = (ScriptNumberLong)innerEvaluator.EvaluateExpression(e.BoundStop); long step = 1; if (e.BoundStep != null) @@ -546,18 +546,16 @@ namespace Upsilon.Evaluator using (var enumerator = iterable.GetEnumerator()) { - bool first = true; while (enumerator.MoveNext()) { var (key, value) = enumerator.Current; if (e.Variables[0].Name != "_") - innerEvaluator.Scope.Set(e.Variables[0], key.ToLuaType(), first); + innerEvaluator.Scope.CreateLocal(e.Variables[0], key.ToLuaType()); if (e.Variables[1].Name != "_") - innerEvaluator.Scope.Set(e.Variables[1], value, first); + innerEvaluator.Scope.CreateLocal(e.Variables[1], value); innerEvaluator.EvaluateBoundBlockStatement((BoundBlockStatement) e.Block); if (innerEvaluator.HasBroken) break; - first = false; } } } diff --git a/Upsilon/StandardLibraries/StaticScope.cs b/Upsilon/StandardLibraries/StaticScope.cs index fd0cc4e..b66e83e 100644 --- a/Upsilon/StandardLibraries/StaticScope.cs +++ b/Upsilon/StandardLibraries/StaticScope.cs @@ -45,8 +45,9 @@ namespace Upsilon.StandardLibraries var scope = new EvaluationScope(basicFunctions.ToDictionary(x => x.Key, x => (ScriptType)x.Value)); var boundScope = new BoundScope( - scope.Variables.ToDictionary(x => x.Key - , x => (VariableSymbol)new FunctionVariableSymbol(x.Key, x.Value.Type, false, ImmutableArray.Empty){IsBound = true}), + scope.Variables.ToDictionary(x => x.Key, + x => (VariableSymbol) new FunctionVariableSymbol(x.Key, x.Value.Type, false, + ImmutableArray.Empty) {IsBound = true}), null); return (scope, boundScope); } @@ -55,8 +56,8 @@ namespace Upsilon.StandardLibraries { var luaVariable = value.ToLuaType(); var varSymbol = new VariableSymbol(name, luaVariable.Type, false); - BoundScope.SetVariable(varSymbol); - Scope.Set(varSymbol, luaVariable, true); + BoundScope.AssignToNearest(varSymbol); + Scope.AssignToNearest(varSymbol, luaVariable); } } } \ No newline at end of file diff --git a/UpsilonTests/GeneralTests/ScopeTests.cs b/UpsilonTests/GeneralTests/ScopeTests.cs index f3aafd0..b308e2f 100644 --- a/UpsilonTests/GeneralTests/ScopeTests.cs +++ b/UpsilonTests/GeneralTests/ScopeTests.cs @@ -45,5 +45,23 @@ b = a Assert.Equal(100, a); } + [Fact] + public void InnerScopeDefinesToUpperLocalIfLogical() + { + const string input = @" +arr = {100, 56, 28} +local value = 0 +for key, val in arr do + value = value + val +end +return value +"; + var script = new Script(input, BoundScope, StaticScope); + Assert.Empty(script.Diagnostics.Messages); + var result = script.Evaluate(); + Assert.Empty(script.Diagnostics.Messages); + Assert.Equal(184, result); + } + } } \ No newline at end of file