Support starred arguments as class bases (#1170)

BCSharp · web-flow · commit f0b690ad5d21 · 2021-04-13T18:50:58.000-04:00
diff --git a/Src/IronPython/Compiler/Ast/AstMethods.cs b/Src/IronPython/Compiler/Ast/AstMethods.cs
@@ -20,7 +20,7 @@ internal static class AstMethods {
         public static readonly MethodInfo IsTrue = GetMethod((Func<object, bool>)PythonOps.IsTrue);
         public static readonly MethodInfo RaiseAssertionError = GetMethod((Action<CodeContext, object>)PythonOps.RaiseAssertionError);
         public static readonly MethodInfo RaiseAssertionErrorNoMessage = GetMethod((Action<CodeContext>)PythonOps.RaiseAssertionError);
-        public static readonly MethodInfo MakeClass = GetMethod((Func<FunctionCode, Func<CodeContext, CodeContext>, CodeContext, string, object[], object, string, object>)PythonOps.MakeClass);
+        public static readonly MethodInfo MakeClass = GetMethod((Func<FunctionCode, Func<CodeContext, CodeContext>, CodeContext, string, PythonTuple, object, string, object>)PythonOps.MakeClass);
         public static readonly MethodInfo PrintExpressionValue = GetMethod((Action<CodeContext, object>)PythonOps.PrintExpressionValue);
         public static readonly MethodInfo ImportWithNames = GetMethod((Func<CodeContext, string, string[], int, object>)PythonOps.ImportWithNames);
         public static readonly MethodInfo ImportFrom = GetMethod((Func<CodeContext, object, string, object>)PythonOps.ImportFrom);
@@ -45,6 +45,7 @@ internal static class AstMethods {
         public static readonly MethodInfo CheckException = GetMethod((Func<CodeContext, object, object, object>)PythonOps.CheckException);
         public static readonly MethodInfo SetCurrentException = GetMethod((Func<CodeContext, Exception, object>)PythonOps.SetCurrentException);
         public static readonly MethodInfo MakeTuple = GetMethod((Func<object[], PythonTuple>)PythonOps.MakeTuple);
+        public static readonly MethodInfo MakeEmptyTuple = GetMethod((Func<PythonTuple>)PythonOps.MakeEmptyTuple);
         public static readonly MethodInfo IsNot = GetMethod((Func<object, object, object>)PythonOps.IsNot);
         public static readonly MethodInfo Is = GetMethod((Func<object, object, object>)PythonOps.Is);
         public static readonly MethodInfo ImportTop = GetMethod((Func<CodeContext, string, int, object>)PythonOps.ImportTop);
diff --git a/Src/IronPython/Compiler/Ast/CallExpression.cs b/Src/IronPython/Compiler/Ast/CallExpression.cs
@@ -134,6 +134,7 @@ static void ScanArgs(IReadOnlyList<Arg> args, ArgumentType scanForType, out int
                 }
             }
 
+            // Compare to: ClassDefinition.Reduce.__UnpackBasesHelper
             static MSAst.Expression UnpackListHelper(IReadOnlyList<Arg> args, int firstListPos) {
                 Debug.Assert(args.Count > 0);
                 Debug.Assert(args[firstListPos].ArgumentInfo.Kind == ArgumentType.List);
diff --git a/Src/IronPython/Compiler/Ast/ClassDefinition.cs b/Src/IronPython/Compiler/Ast/ClassDefinition.cs
@@ -12,6 +12,7 @@
 using IronPython.Runtime;
 
 using Microsoft.Scripting;
+using Microsoft.Scripting.Actions;
 using Microsoft.Scripting.Utils;
 
 using AstUtils = Microsoft.Scripting.Ast.Utils;
@@ -23,7 +24,7 @@ namespace IronPython.Compiler.Ast {
 
     public class ClassDefinition : ScopeStatement {
         private readonly string _name;
-        private readonly Expression[] _bases;
+        private readonly Arg[] _bases;
         private readonly Arg[] _keywords;
 
         private LightLambdaExpression _dlrBody;       // the transformed body including all of our initialization, etc...
@@ -33,15 +34,12 @@ public class ClassDefinition : ScopeStatement {
         private static readonly MSAst.ParameterExpression _parentContextParam = Ast.Parameter(typeof(CodeContext), "$parentContext");
         private static readonly MSAst.Expression _tupleExpression = MSAst.Expression.Call(AstMethods.GetClosureTupleFromContext, _parentContextParam);
 
-        public ClassDefinition(string name, Expression[] bases, Arg[] keywords, Statement body = null) {
-            ContractUtils.RequiresNotNullItems(bases, nameof(bases));
-            ContractUtils.RequiresNotNullItems(keywords, nameof(keywords));
-
+        public ClassDefinition(string name, IReadOnlyList<Arg> bases, IReadOnlyList<Arg> keywords, Statement body = null) {
             _name = name;
-            _bases = bases;
-            _keywords = keywords;
+            _bases = bases?.ToArray() ?? Array.Empty<Arg>();
+            _keywords = keywords?.ToArray() ?? Array.Empty<Arg>();
             Body = body;
-            Metaclass = keywords.Where(arg => arg.Name == "metaclass").Select(arg => arg.Expression).FirstOrDefault();
+            Metaclass = _keywords.Where(arg => arg.Name == "metaclass").Select(arg => arg.Expression).FirstOrDefault();
         }
 
         public SourceLocation Header => GlobalParent.IndexToLocation(HeaderIndex);
@@ -50,7 +48,7 @@ public ClassDefinition(string name, Expression[] bases, Arg[] keywords, Statemen
 
         public override string Name => _name;
 
-        public IReadOnlyList<Expression> Bases => _bases;
+        public IReadOnlyList<Arg> Bases => _bases;
 
         public IReadOnlyList<Arg> Keywords => _keywords;
 
@@ -181,10 +179,7 @@ public override MSAst.Expression Reduce() {
                 lambda,
                 Parent.LocalContext,
                 AstUtils.Constant(_name),
-                Ast.NewArrayInit(
-                    typeof(object),
-                    ToObjectArray(_bases)
-                ),
+                UnpackBasesHelper(_bases),
                 Metaclass is null ? AstUtils.Constant(null, typeof(object)) : AstUtils.Convert(Metaclass, typeof(object)),
                 AstUtils.Constant(FindSelfNames())
             );
@@ -198,6 +193,33 @@ public override MSAst.Expression Reduce() {
                     GlobalParent.IndexToLocation(HeaderIndex)
                 )
             );
+
+            // Compare to: CallExpression.Reduce.__UnpackListHelper
+            static MSAst.Expression UnpackBasesHelper(IReadOnlyList<Arg> bases) {
+                if (bases.Count == 0) {
+                    return Expression.Call(AstMethods.MakeEmptyTuple);
+                } else if (bases.All(arg => arg.ArgumentInfo.Kind is ArgumentType.Simple)) {
+                    return Expression.Call(AstMethods.MakeTuple,
+                        Expression.NewArrayInit(
+                            typeof(object),
+                            ToObjectArray(bases.Select(arg => arg.Expression).ToList())
+                        )
+                    );
+                } else {
+                    var expressions = new ReadOnlyCollectionBuilder<MSAst.Expression>(bases.Count + 2);
+                    var varExpr = Expression.Variable(typeof(PythonList), "$coll");
+                    expressions.Add(Expression.Assign(varExpr, Expression.Call(AstMethods.MakeEmptyList)));
+                    foreach (var arg in bases) {
+                        if (arg.ArgumentInfo.Kind == ArgumentType.List) {
+                            expressions.Add(Expression.Call(AstMethods.ListExtend, varExpr, AstUtils.Convert(arg.Expression, typeof(object))));
+                        } else {
+                            expressions.Add(Expression.Call(AstMethods.ListAppend, varExpr, AstUtils.Convert(arg.Expression, typeof(object))));
+                        }
+                    }
+                    expressions.Add(Expression.Call(AstMethods.ListToTuple, varExpr));
+                    return Expression.Block(typeof(PythonTuple), new MSAst.ParameterExpression[] { varExpr }, expressions);
+                }
+            }
         }
 
         private Microsoft.Scripting.Ast.LightExpression<Func<CodeContext, CodeContext>> MakeClassBody() {
@@ -297,10 +319,11 @@ public override void Walk(PythonWalker walker) {
                         decorator.Walk(walker);
                     }
                 }
-                if (_bases != null) {
-                    foreach (Expression b in _bases) {
-                        b.Walk(walker);
-                    }
+                foreach (Arg b in _bases) {
+                    b.Walk(walker);
+                }
+                foreach (Arg b in _keywords) {
+                    b.Walk(walker);
                 }
                 Body?.Walk(walker);
             }
diff --git a/Src/IronPython/Compiler/Ast/FlowChecker.cs b/Src/IronPython/Compiler/Ast/FlowChecker.cs
@@ -328,7 +328,10 @@ public override bool Walk(ClassDefinition node) {
             } else {
                 // analyze the class definition itself (it is visited while analyzing parent scope):
                 Define(node.Name);
-                foreach (Expression e in node.Bases) {
+                foreach (Arg e in node.Bases) {
+                    e.Walk(this);
+                }
+                foreach (Arg e in node.Keywords) {
                     e.Walk(this);
                 }
                 return false;
diff --git a/Src/IronPython/Compiler/Ast/PythonNameBinder.cs b/Src/IronPython/Compiler/Ast/PythonNameBinder.cs
@@ -206,7 +206,7 @@ public override bool Walk(ClassDefinition node) {
             node.PythonVariable = DefineName(node.Name);
 
             // Base references are in the outer context
-            foreach (Expression b in node.Bases) b.Walk(this);
+            foreach (Arg b in node.Bases) b.Walk(this);
 
             foreach (Arg a in node.Keywords) a.Walk(this);
 
diff --git a/Src/IronPython/Compiler/Parser.cs b/Src/IronPython/Compiler/Parser.cs
@@ -1006,27 +1006,21 @@ private ClassDefinition ParseClassDef() {
             string name = ReadName();
             if (name == null) {
                 // no name, assume there's no class.
-                return new ClassDefinition(null, new Expression[0], new Arg[0], ErrorStmt());
+                return new ClassDefinition(null, null, null, ErrorStmt());
             }
 
-            var bases = new List<Expression>();
-            var keywords = new List<Arg>();
+            List<Arg> bases = null;
+            List<Arg> keywords = null;
             if (MaybeEat(TokenKind.LeftParenthesis)) {
-                foreach (var arg in FinishArgumentList(null)) {
-                    var info = arg.ArgumentInfo;
-                    if (info.Kind == Microsoft.Scripting.Actions.ArgumentType.Simple) {
-                        bases.Add(arg.Expression);
-                    } else if (info.Kind == Microsoft.Scripting.Actions.ArgumentType.Named) {
-                        keywords.Add(arg);
-                    }
-                }
+                IReadOnlyList<Arg> args = FinishArgumentList(null);
+                SplitAndValidateArguments(args, out bases, out keywords);
             }
             var mid = GetEnd();
 
             // Save private prefix
             string savedPrefix = SetPrivatePrefix(name);
 
-            var ret = new ClassDefinition(name, bases.ToArray(), keywords.ToArray());
+            var ret = new ClassDefinition(name, bases, keywords);
             PushClass(ret);
 
             // Parse the class body
@@ -1989,13 +1983,7 @@ private Expression AddTrailers(Expression ret, bool allowGeneratorExpression) {
 
                             NextToken();
                             IReadOnlyList<Arg> args = FinishArgListOrGenExpr();
-                            CallExpression call;
-                            if (args != null) {
-                                call = FinishCallExpr(ret, args);
-                            } else {
-                                call = new CallExpression(ret, null, null);
-                            }
-
+                            CallExpression call = FinishCallExpr(ret, args);
                             call.SetLoc(_globalParent, ret.StartIndex, GetEnd());
                             ret = call;
                             break;
@@ -2900,11 +2888,22 @@ private void PushFunction(FunctionDefinition function) {
         }
 
         private CallExpression FinishCallExpr(Expression target, IEnumerable<Arg> args) {
-            bool hasKeyword = false;
-            bool hasKeywordUnpacking = false;
             List<Arg> posargs = null;
             List<Arg> kwargs = null;
 
+            if (args is not null) {
+                SplitAndValidateArguments(args, out posargs, out kwargs); 
+            }
+
+            return new CallExpression(target, posargs, kwargs);
+        }
+
+        private void SplitAndValidateArguments(IEnumerable<Arg> args, out List<Arg> posargs, out List<Arg> kwargs) {
+            bool hasKeyword = false;
+            bool hasKeywordUnpacking = false;
+
+            posargs = kwargs = null;
+
             foreach (Arg arg in args) {
                 if (arg.Name == null) {
                     if (hasKeywordUnpacking) {
@@ -2930,8 +2929,6 @@ private CallExpression FinishCallExpr(Expression target, IEnumerable<Arg> args)
                     kwargs.Add(arg);
                 }
             }
-
-            return new CallExpression(target, posargs, kwargs);
         }
 
         #endregion
diff --git a/Src/IronPython/Modules/_ast.cs b/Src/IronPython/Modules/_ast.cs
@@ -1064,10 +1064,10 @@ internal override AstExpression Revert() {
                     newArgs.Add(new Arg(expr.Revert(ex)));
                 if (null != starargs)
                     newArgs.Add(new Arg("*", expr.Revert(starargs)));
-                if (null != kwargs)
-                    newKwargs.Add(new Arg("**", expr.Revert(kwargs)));
                 foreach (keyword kw in keywords)
                     newKwargs.Add(new Arg(kw.arg, expr.Revert(kw.value)));
+                if (null != kwargs)
+                    newKwargs.Add(new Arg("**", expr.Revert(kwargs)));
                 return new CallExpression(target, newArgs, newKwargs);
             }
 
@@ -1106,8 +1106,19 @@ internal ClassDef(ClassDefinition def)
                 : this() {
                 name = def.Name;
                 bases = new PythonList(def.Bases.Count);
-                foreach (AstExpression expr in def.Bases)
-                    bases.Add(Convert(expr));
+                foreach (Arg arg in def.Bases) {
+                    if (arg.Name == null)
+                        bases.Add(Convert(arg.Expression));
+                    else // name == "*"
+                        starargs = Convert(arg.Expression);
+                }
+                keywords = new PythonList(def.Keywords.Count);
+                foreach (Arg arg in def.Keywords) {
+                    if (arg.Name == "**")
+                        kwargs = Convert(arg.Expression);
+                    else // name is proper
+                        keywords.Add(new keyword(arg));
+                }
                 body = ConvertStatements(def.Body);
                 if (def.Decorators != null) {
                     decorator_list = new PythonList(def.Decorators.Count);
@@ -1116,18 +1127,20 @@ internal ClassDef(ClassDefinition def)
                 } else {
                     decorator_list = new PythonList(0);
                 }
-                if (def.Keywords != null) {
-                    keywords = new PythonList(def.Keywords.Count);
-                    foreach (Arg arg in def.Keywords)
-                        keywords.AddNoLock(new keyword(arg));
-                } else {
-                    keywords = new PythonList(0);
-                }
             }
 
             internal override Statement Revert() {
-                var newBases = expr.RevertExprs(bases);
-                var newKeywords = keywords.Cast<keyword>().Select(kw => new Arg(kw.arg, expr.Revert(kw.value))).ToArray();
+                List<Arg> newBases = new List<Arg>();
+                List<Arg> newKeywords = new List<Arg>();
+                foreach (expr ex in bases)
+                    newBases.Add(new Arg(expr.Revert(ex)));
+                if (null != starargs)
+                    newBases.Add(new Arg("*", expr.Revert(starargs)));
+                foreach (keyword kw in keywords)
+                    newKeywords.Add(new Arg(kw.arg, expr.Revert(kw.value)));
+                if (null != kwargs)
+                    newKeywords.Add(new Arg("**", expr.Revert(kwargs)));
+
                 ClassDefinition cd = new ClassDefinition(name, newBases, newKeywords, RevertStmts(body));
                 if (decorator_list.Count != 0)
                     cd.Decorators = expr.RevertExprs(decorator_list);
diff --git a/Src/IronPython/Runtime/Operations/PythonOps.cs b/Src/IronPython/Runtime/Operations/PythonOps.cs
@@ -1321,7 +1321,7 @@ public static void InitializeForFinalization(CodeContext/*!*/ context, object ne
             return classdict;
         }
 
-        public static object MakeClass(FunctionCode funcCode, Func<CodeContext, CodeContext> body, CodeContext/*!*/ parentContext, string name, object[] bases, object metaclass, string selfNames) {
+        public static object MakeClass(FunctionCode funcCode, Func<CodeContext, CodeContext> body, CodeContext/*!*/ parentContext, string name, PythonTuple bases, object metaclass, string selfNames) {
             Func<CodeContext, CodeContext> func = GetClassCode(parentContext, funcCode, body);
 
             return MakeClass(parentContext, name, bases, metaclass, selfNames, func(parentContext).Dict);
@@ -1342,11 +1342,11 @@ private static Func<CodeContext, CodeContext> GetClassCode(CodeContext/*!*/ cont
             }
         }
 
-        private static object MakeClass(CodeContext/*!*/ context, string name, object[] bases, object metaclass, string selfNames, PythonDictionary vars) {
-            foreach (object dt in bases) {
+        private static object MakeClass(CodeContext/*!*/ context, string name, PythonTuple bases, object metaclass, string selfNames, PythonDictionary vars) {
+            foreach (object? dt in bases) {
                 if (dt is TypeGroup) {
-                    object[] newBases = new object[bases.Length];
-                    for (int i = 0; i < bases.Length; i++) {
+                    object?[] newBases = new object[bases.Count];
+                    for (int i = 0; i < bases.Count; i++) {
                         if (bases[i] is TypeGroup tc) {
                             if (!tc.TryGetNonGenericType(out Type nonGenericType)) {
                                 throw PythonOps.TypeError("cannot derive from open generic types {0}", Repr(context, tc));
@@ -1356,7 +1356,7 @@ private static object MakeClass(CodeContext/*!*/ context, string name, object[]
                             newBases[i] = bases[i];
                         }
                     }
-                    bases = newBases;
+                    bases = PythonTuple.MakeTuple(newBases);
                     break;
                 } else if (dt is PythonType pt) {
                     if (pt.Equals(PythonType.GetPythonType(typeof(Enum))) || pt.Equals(PythonType.GetPythonType(typeof(Array)))
@@ -1367,20 +1367,18 @@ private static object MakeClass(CodeContext/*!*/ context, string name, object[]
                 }
             }
 
-            PythonTuple tupleBases = PythonTuple.MakeTuple(bases);
-
             if (metaclass is null) {
                 // this makes sure that object is a base
-                if (tupleBases.Count == 0) {
-                    tupleBases = PythonTuple.MakeTuple(DynamicHelpers.GetPythonTypeFromType(typeof(object)));
+                if (bases.Count == 0) {
+                    bases = PythonTuple.MakeTuple(DynamicHelpers.GetPythonTypeFromType(typeof(object)));
                 }
-                return PythonType.__new__(context, TypeCache.PythonType, name, tupleBases, vars, selfNames);
+                return PythonType.__new__(context, TypeCache.PythonType, name, bases, vars, selfNames);
             }
 
             object? classdict = vars;
 
             if (metaclass is PythonType) {
-                classdict = CallPrepare(context, (PythonType)metaclass, name, tupleBases, vars);
+                classdict = CallPrepare(context, (PythonType)metaclass, name, bases, vars);
             }
 
             // eg:
@@ -1395,7 +1393,7 @@ private static object MakeClass(CodeContext/*!*/ context, string name, object[]
                 context,
                 metaclass,
                 name,
-                tupleBases,
+                bases,
                 classdict
             );
 
@@ -1486,6 +1484,15 @@ public static PythonTuple MakeTupleFromSequence(object items) {
             return PythonTuple.Make(items);
         }
 
+        /// <summary>
+        /// Python runtime helper to create an instance of an empty Tuple
+        /// </summary>
+        [NoSideEffects]
+        [EditorBrowsable(EditorBrowsableState.Never)]
+        public static PythonTuple MakeEmptyTuple() {
+            return PythonTuple.MakeTuple();
+        }
+
         /// <summary>
         /// DICT_MERGE
         /// </summary>
diff --git a/Tests/test_class.py b/Tests/test_class.py

Original file line number	Diff line number	Diff line change
`@@ -134,6 +134,7 @@ static void ScanArgs(IReadOnlyList<Arg> args, ArgumentType scanForType, out int`
`134`	`134`	`}`
`135`	`135`	`}`
`136`	`136`
	`137`	`+ // Compare to: ClassDefinition.Reduce.__UnpackBasesHelper`
`137`	`138`	`static MSAst.Expression UnpackListHelper(IReadOnlyList<Arg> args, int firstListPos) {`
`138`	`139`	`Debug.Assert(args.Count > 0);`
`139`	`140`	`Debug.Assert(args[firstListPos].ArgumentInfo.Kind == ArgumentType.List);`