Complete implementation of nonlocal (#1054)

* Implement nonlocal

* Enable nonlocal tests from stdlib

* Unblock some nonlocal-dependent tests

* Disable failing tests

* Revert "Workaround for missing nonlocal implementation (#453)"

* test_shelve is not parallel-safe

* Implement correct error line number reporting for nonlocal

* Add extra tests for nonlocal

* Align error type and message with CPython for unassigned variable use

* Implement ast.Nonlocal

* Cleanup

Author: BCSharp

Src/IronPython/Compiler/Ast/AstMethods.cs

@@ -31,7 +31,8 @@ internal static class AstMethods {
         public static readonly MethodInfo ListAddForComprehension = GetMethod((Action<PythonList, object>)PythonOps.ListAddForComprehension);
         public static readonly MethodInfo SetAddForComprehension = GetMethod((Action<SetCollection, object>)PythonOps.SetAddForComprehension);
         public static readonly MethodInfo DictAddForComprehension = GetMethod((Action<PythonDictionary, object, object>)PythonOps.DictAddForComprehension);
-        public static readonly MethodInfo CheckUninitialized = GetMethod((Func<object, string, object>)PythonOps.CheckUninitialized);
+        public static readonly MethodInfo CheckUninitializedFree = GetMethod((Func<object, string, object>)PythonOps.CheckUninitializedFree);
+        public static readonly MethodInfo CheckUninitializedLocal = GetMethod((Func<object, string, object>)PythonOps.CheckUninitializedLocal);
         public static readonly MethodInfo PublishModule = GetMethod((Func<CodeContext, string, object>)PythonOps.PublishModule);
         public static readonly MethodInfo RemoveModule = GetMethod((Action<CodeContext, string, object>)PythonOps.RemoveModule);
         public static readonly MethodInfo ModuleStarted = GetMethod((Action<CodeContext, ModuleOptions>)PythonOps.ModuleStarted);

Src/IronPython/Compiler/Ast/ClassDefinition.cs

@@ -114,12 +114,15 @@ internal override PythonVariable BindReference(PythonNameBinder binder, PythonRe
                     return null;
                 }
 
-                return variable;
+                if (variable.Kind != VariableKind.Nonlocal) {
+                    return variable;
+                }
             }
 
             // Try to bind in outer scopes, if we have an unqualified exec we need to leave the
             // variables as free for the same reason that locals are accessed by name.
-            for (ScopeStatement parent = Parent; parent != null; parent = parent.Parent) {
+            bool stopAtGlobal = variable?.Kind == VariableKind.Nonlocal;
+            for (ScopeStatement parent = Parent; parent != null && !(stopAtGlobal && parent.IsGlobal); parent = parent.Parent) {
                 if (parent.TryBindOuter(this, reference, out variable)) {
                     return variable;
                 }

Src/IronPython/Compiler/Ast/Comprehension.cs

@@ -242,6 +242,7 @@ internal override PythonVariable BindReference(PythonNameBinder binder, PythonRe
                 if (variable.Kind == VariableKind.Global) {
                     AddReferencedGlobal(reference.Name);
                 }
+                Debug.Assert(variable.Kind != VariableKind.Nonlocal, "there should be no nonlocals in a comprehension");
                 return variable;
             }
 

Src/IronPython/Compiler/Ast/FlowChecker.cs

@@ -116,6 +116,7 @@ public override bool Walk(Parameter node) {
         }
     }
 
+    // TODO: probably obsolete, PythonVariable.Deleted does the job better (across scopes)
     internal class FlowDeleter : PythonWalkerNonRecursive {
         private readonly FlowChecker _fc;
 
@@ -192,7 +193,7 @@ private void SetInitialized(PythonVariable/*!*/ variable, bool value) {
             _bits.Set(variable.Index * 2 + 1, value);
         }
         private bool IsAssigned(PythonVariable/*!*/ variable) {
-            return _bits.Get(variable.Index * 2);
+            return _bits.Get(variable.Index * 2) && !variable.MaybeDeleted;
         }
 
         private bool IsInitialized(PythonVariable/*!*/ variable) {

Src/IronPython/Compiler/Ast/FunctionDefinition.cs

@@ -188,7 +188,7 @@ internal override FunctionAttributes Flags {
         internal override bool TryBindOuter(ScopeStatement from, PythonReference reference, out PythonVariable variable) {
             // Functions expose their locals to direct access
             ContainsNestedFreeVariables = true;
-            if (TryGetVariable(reference.Name, out variable)) {
+            if (TryGetVariable(reference.Name, out variable) && variable.Kind != VariableKind.Nonlocal) {
                 variable.AccessedInNestedScope = true;
 
                 if (variable.Kind == VariableKind.Local || variable.Kind == VariableKind.Parameter) {
@@ -215,11 +215,15 @@ internal override PythonVariable BindReference(PythonNameBinder binder, PythonRe
                 if (variable.Kind == VariableKind.Global) {
                     AddReferencedGlobal(reference.Name);
                 }
-                return variable;
+
+                if (variable.Kind != VariableKind.Nonlocal) {
+                    return variable;
+                }
             }
 
             // Try to bind in outer scopes
-            for (ScopeStatement parent = Parent; parent != null; parent = parent.Parent) {
+            bool stopAtGlobal = variable?.Kind == VariableKind.Nonlocal;
+            for (ScopeStatement parent = Parent; parent != null && !(stopAtGlobal && parent.IsGlobal); parent = parent.Parent) {
                 if (parent.TryBindOuter(this, reference, out variable)) {
                     return variable;
                 }

Src/IronPython/Compiler/Ast/NameExpression.cs

@@ -44,7 +44,9 @@ public override MSAst.Expression Reduce() {
 
             if (!Assigned && !(read is IPythonGlobalExpression)) {
                 read = Ast.Call(
-                    AstMethods.CheckUninitialized,
+                    Parent.IsFreeVariable(Reference.PythonVariable) ?
+                        AstMethods.CheckUninitializedFree :
+                        AstMethods.CheckUninitializedLocal,
                     read,
                     Ast.Constant(Name)
                 );

Src/IronPython/Compiler/Ast/PythonNameBinder.cs

@@ -32,6 +32,7 @@
  *     if (cond): del x        # illegal because x is a closure variable
  *     def g():
  *         print x
+ * TODO: the example above is no longer valid; also the description of the name binding algorithm does not match the code
  */
 
 namespace IronPython.Compiler.Ast {
@@ -69,6 +70,7 @@ public override bool Walk(Parameter node) {
             return false;
         }
 
+        // TODO: obsolete?
         private void WalkTuple(TupleExpression tuple) {
             tuple.Parent = _binder._currentScope;
             foreach (Expression innerNode in tuple.Items) {
@@ -190,7 +192,7 @@ internal PythonVariable DefineParameter(string name) {
 
         internal PythonVariable DefineDeleted(string name) {
             PythonVariable variable = _currentScope.EnsureVariable(name);
-            variable.Deleted = true;
+            variable.RegisterDeletion();
             return variable;
         }
 
@@ -691,6 +693,10 @@ public override bool Walk(GlobalStatement node) {
                         case VariableKind.Parameter:
                             ReportSyntaxError($"name '{n}' is parameter and global", node);
                             break;
+
+                        case VariableKind.Nonlocal:
+                            ReportSyntaxError($"name '{n}' is nonlocal and global", node);
+                            break;
                     }
                 }
 
@@ -734,21 +740,27 @@ public override bool Walk(NonlocalStatement node) {
                         case VariableKind.Global:
                             ReportSyntaxError($"name '{n}' is nonlocal and global", node);
                             break;
+
                         case VariableKind.Local:
                             ReportSyntaxError($"name '{n}' is assigned to before nonlocal declaration", node);
                             break;
+
                         case VariableKind.Parameter:
                             ReportSyntaxError($"name '{n}' is parameter and nonlocal", node);
                             break;
+
+                        case VariableKind.Nonlocal:
+                            // no conflict, name redeclared as nonlocal
+                            break;
                     }
                 }
 
                 // Check for the name being referenced previously
-                if (_currentScope.IsReferenced(n)) {
+                if (_currentScope.IsReferenced(n) && conflict is null) {
                     ReportSyntaxError($"name '{n}' is used prior to nonlocal declaration", node);
                 }
 
-                // TODO: do we need to do other stuff here?
+                _currentScope.EnsureNonlocalVariable(n, node);
             }
             return true;
         }

Src/IronPython/Compiler/Ast/PythonVariable.cs

@@ -16,15 +16,20 @@
 namespace IronPython.Compiler.Ast {
     using Ast = MSAst.Expression;
 
+    [DebuggerDisplay("{Kind} {Name} from {Scope.Name}")]
     public class PythonVariable {
 
         public PythonVariable(string name, VariableKind kind, ScopeStatement/*!*/ scope) {
             Assert.NotNull(scope);
+            Debug.Assert(kind != VariableKind.Nonlocal || !scope.IsGlobal);
             Name = name;
             Kind = kind;
             Scope = scope;
         }
 
+        /// <summary>
+        /// The name of the variable as used in Python code.
+        /// </summary>
         public string Name { get; }
 
         public bool IsGlobal {
@@ -33,28 +38,56 @@ public bool IsGlobal {
             }
         }
 
+        /// <summary>
+        /// The original scope in which the variable is defined.
+        /// </summary>
         public ScopeStatement Scope { get; }
 
-        public VariableKind Kind { get; set; }
+        public VariableKind Kind { get; set; } // TODO: make readonly
+
+        /// <summary>
+        /// The actual variable represented by this variable instance.
+        /// For reference variables this may be null if the reference is not yet resolved.
+        /// </summary>
+        public virtual PythonVariable LimitVariable => this;
+
+        /// <summary>
+        /// Gets a value indicating whether the variable gets deleted by a <c>del</c> statement in any scope.
+        /// </summary>
+        internal bool MaybeDeleted { get; private set; }
 
         /// <summary>
-        /// Gets or sets a value indicating whether the variable gets deleted.
+        /// Mark the variable as argument to a del statement in some scope.
         /// </summary>
-        internal bool Deleted { get; set; }
+        internal void RegisterDeletion() => MaybeDeleted = true;
 
         /// <summary>
         /// Gets the index used for tracking in the flow checker.
         /// </summary>
         internal int Index { get; set; }
 
         /// <summary>
-        /// True iff there is a path in control flow graph on which the variable is used before initialized (assigned or deleted).
+        /// True iff there is a path in the control flow graph of a single scope
+        /// on which the variable is used before explicitly initialized (assigned or deleted)
+        /// in that scope.
         /// </summary>
         public bool ReadBeforeInitialized { get; set; }
 
         /// <summary>
-        /// True iff the variable is referred to from the inner scope.
+        /// True iff the variable is referred to from an inner scope.
         /// </summary>
         public bool AccessedInNestedScope { get; set; }
     }
+
+    internal class PythonReferenceVariable : PythonVariable {
+
+        internal PythonReferenceVariable(PythonReference reference, ScopeStatement scope)
+            : base(reference.Name, VariableKind.Nonlocal, scope) {
+            Reference = reference;
+        }
+
+        internal PythonReference Reference { get; }
+
+        public override PythonVariable LimitVariable => Reference.PythonVariable?.LimitVariable;
+    }
 }

Src/IronPython/Compiler/Ast/ScopeStatement.cs

@@ -32,6 +32,7 @@ public abstract class ScopeStatement : Statement {
         private List<string> _globalVars;                               // global variables accessed from this scope
         private List<string> _cellVars;                                 // variables accessed from nested scopes
         private Dictionary<string, PythonReference> _references;        // names of all variables referenced, null after binding completes
+        private Dictionary<string, NonlocalStatement> _nonlocalVars;    // nonlocal variables declared in this scope, null after binding completes
 
         internal Dictionary<PythonVariable, MSAst.Expression> _variableMapping = new Dictionary<PythonVariable, MSAst.Expression>();
         private readonly DelayedFunctionCode _funcCodeExpr = new DelayedFunctionCode();             // expression that refers to the function code for this scope
@@ -184,12 +185,20 @@ internal virtual IList<string> GetVarNames() {
 
 
         internal void AddFreeVariable(PythonVariable variable, bool accessedInScope) {
+            Debug.Assert(variable?.Kind is VariableKind.Local or VariableKind.Parameter);
+
             if (_freeVars == null) {
                 _freeVars = new List<PythonVariable>();
             }
 
             if (!_freeVars.Contains(variable)) {
                 _freeVars.Add(variable);
+                if (TryGetVariable(variable.Name, out PythonVariable nonlocal) &&
+                    nonlocal.Kind is VariableKind.Nonlocal &&
+                    nonlocal.MaybeDeleted) {
+
+                    variable.RegisterDeletion();
+                }
             }
         }
 
@@ -331,6 +340,9 @@ internal virtual void Bind(PythonNameBinder binder) {
             if (_references != null) {
                 foreach (var reference in _references.Values) {
                     reference.PythonVariable = BindReference(binder, reference);
+                    if (reference.PythonVariable is null && TryGetNonlocalStatement(reference.Name, out NonlocalStatement node)) {
+                        binder.ReportSyntaxError($"no binding for nonlocal '{reference.Name}' found", node);
+                    }
                 }
             }
         }
@@ -350,10 +362,10 @@ internal virtual void FinishBind(PythonNameBinder binder) {
             if (FreeVariables != null && FreeVariables.Count > 0) {
                 LocalParentTuple = Ast.Parameter(Parent.GetClosureTupleType(), "$tuple");
 
-                foreach (var variable in _freeVars) {
-                    var parentClosure = Parent._closureVariables;
-                    Debug.Assert(parentClosure != null);
+                var parentClosure = Parent._closureVariables;
+                Debug.Assert(parentClosure != null);
 
+                foreach (var variable in FreeVariables) {
                     for (int i = 0; i < parentClosure.Length; i++) {
                         if (parentClosure[i].Variable == variable) {
                             _variableMapping[variable] = new ClosureExpression(variable, Ast.Property(LocalParentTuple, String.Format("Item{0:D3}", i)), null);
@@ -372,7 +384,10 @@ internal virtual void FinishBind(PythonNameBinder binder) {
             if (Variables != null) {
                 foreach (PythonVariable variable in Variables.Values) {
                     if (!HasClosureVariable(closureVariables, variable) &&
-                        !variable.IsGlobal && (variable.AccessedInNestedScope || ExposesLocalVariable(variable))) {
+                        !variable.IsGlobal &&
+                        variable.Kind != VariableKind.Nonlocal &&
+                        (variable.AccessedInNestedScope || ExposesLocalVariable(variable))) {
+
                         if (closureVariables == null) {
                             closureVariables = new List<ClosureInfo>();
                         }
@@ -397,6 +412,7 @@ internal virtual void FinishBind(PythonNameBinder binder) {
 
             // no longer needed
             _references = null;
+            _nonlocalVars = null;
         }
 
         private static bool HasClosureVariable(List<ClosureInfo> closureVariables, PythonVariable variable) {
@@ -440,6 +456,15 @@ internal bool IsReferenced(string name) {
             return _references != null && _references.TryGetValue(name, out reference);
         }
 
+        internal bool IsFreeVariable(PythonVariable variable) {
+            return FreeVariables?.Contains(variable?.LimitVariable) ?? false;
+        }
+
+        private bool TryGetNonlocalStatement(string name, out NonlocalStatement node) {
+            node = null;
+            return _nonlocalVars?.TryGetValue(name, out node) ?? false;
+        }
+
         internal PythonVariable/*!*/ CreateVariable(string name, VariableKind kind) {
             EnsureVariables();
             Debug.Assert(!Variables.ContainsKey(name));
@@ -456,6 +481,24 @@ internal bool IsReferenced(string name) {
             return variable;
         }
 
+        private PythonVariable CreateNonlocalVariable(string name) {
+            EnsureVariables();
+            Debug.Assert(!Variables.ContainsKey(name));
+            Debug.Assert(!IsReferenced(name));
+            PythonReference reference = Reference(name);
+            PythonVariable variable;
+            Variables[name] = variable = new PythonReferenceVariable(reference, this);
+            return variable;
+        }
+
+        internal void EnsureNonlocalVariable(string name, NonlocalStatement node) {
+           _nonlocalVars ??= new();
+            if (!_nonlocalVars.ContainsKey(name)) {
+                CreateNonlocalVariable(name);
+                _nonlocalVars[name] = node;
+            }
+        }
+
         internal PythonVariable DefineParameter(string name) {
             return CreateVariable(name, VariableKind.Parameter);
         }
@@ -656,18 +699,19 @@ private MSAst.Expression GetClosureCell(ClosureInfo variable) {
         }
 
         internal virtual MSAst.Expression GetVariableExpression(PythonVariable variable) {
+            Assert.NotNull(variable?.LimitVariable);
             if (variable.IsGlobal) {
                 return GlobalParent.ModuleVariables[variable];
             }
 
-            Debug.Assert(_variableMapping.ContainsKey(variable));
-            return _variableMapping[variable];
+            Debug.Assert(_variableMapping.ContainsKey(variable.LimitVariable));
+            return _variableMapping[variable.LimitVariable];
         }
 
         internal void CreateVariables(ReadOnlyCollectionBuilder<MSAst.ParameterExpression> locals, List<MSAst.Expression> init) {
             if (Variables != null) {
                 foreach (PythonVariable variable in Variables.Values) {
-                    if (variable.Kind != VariableKind.Global) {
+                    if (variable.Kind is VariableKind.Local or VariableKind.Parameter) {
                         if (GetVariableExpression(variable) is ClosureExpression closure) {
                             init.Add(closure.Create());
                             locals.Add((MSAst.ParameterExpression)closure.ClosureCell);