Handle keyword arguments in class definitions (#1173)

BCSharp · web-flow · commit c3f049950843 · 2021-04-18T08:02:43.000-04:00
* Handle keyword arguments in class definitions

* Add tests
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, PythonTuple, object, string, object>)PythonOps.MakeClass);
+        public static readonly MethodInfo MakeClass = GetMethod((Func<FunctionCode, Func<CodeContext, CodeContext>, CodeContext, string, PythonTuple, PythonDictionary, 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);
@@ -31,6 +31,7 @@ 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 DictMergeOne = GetMethod((Action<CodeContext, PythonDictionary, object, object>)PythonOps.DictMergeOne);
         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);
diff --git a/Src/IronPython/Compiler/Ast/CallExpression.cs b/Src/IronPython/Compiler/Ast/CallExpression.cs
@@ -156,6 +156,7 @@ static MSAst.Expression UnpackListHelper(IReadOnlyList<Arg> args, int firstListP
                 return Expression.Block(typeof(PythonList), new MSAst.ParameterExpression[] { varExpr }, expressions);
             }
 
+            // Compare to: ClassDefinition.Reduce.__UnpackKeywordsHelper
             static MSAst.Expression UnpackDictHelper(MSAst.Expression context, IReadOnlyList<Arg> kwargs, int numDict, int firstDictPos) {
                 Debug.Assert(kwargs.Count > 0);
                 Debug.Assert(0 < numDict && numDict <= kwargs.Count);
diff --git a/Src/IronPython/Compiler/Ast/ClassDefinition.cs b/Src/IronPython/Compiler/Ast/ClassDefinition.cs
@@ -42,7 +42,6 @@ public ClassDefinition(string name, IReadOnlyList<Arg>? bases, IReadOnlyList<Arg
             _bases = bases?.ToArray() ?? Array.Empty<Arg>();
             _keywords = keywords?.ToArray() ?? Array.Empty<Arg>();
             Body = body ?? EmptyStatement.PreCompiledInstance;
-            Metaclass = _keywords.Where(arg => arg.Name == "metaclass").Select(arg => arg.Expression).FirstOrDefault();
         }
 
         public SourceLocation Header => GlobalParent.IndexToLocation(HeaderIndex);
@@ -55,8 +54,6 @@ public ClassDefinition(string name, IReadOnlyList<Arg>? bases, IReadOnlyList<Arg
 
         public IReadOnlyList<Arg> Keywords => _keywords;
 
-        public Expression? Metaclass { get; }
-
         public Statement Body { get; set; }
 
         public IList<Expression>? Decorators { get; internal set; }
@@ -190,7 +187,7 @@ public override MSAst.Expression Reduce() {
                 Parent.LocalContext,
                 AstUtils.Constant(_name),
                 UnpackBasesHelper(_bases),
-                Metaclass is null ? AstUtils.Constant(null, typeof(object)) : AstUtils.Convert(Metaclass, typeof(object)),
+                UnpackKeywordsHelper(Parent.LocalContext, _keywords),
                 AstUtils.Constant(FindSelfNames())
             );
 
@@ -230,6 +227,26 @@ static MSAst.Expression UnpackBasesHelper(IReadOnlyList<Arg> bases) {
                     return Expression.Block(typeof(PythonTuple), new MSAst.ParameterExpression[] { varExpr }, expressions);
                 }
             }
+
+            // Compare to: CallExpression.Reduce.__UnpackDictHelper
+            static MSAst.Expression UnpackKeywordsHelper(MSAst.Expression context, IReadOnlyList<Arg> kwargs) {
+                if (kwargs.Count == 0) {
+                    return AstUtils.Constant(null, typeof(PythonDictionary));
+                }
+
+                var expressions = new ReadOnlyCollectionBuilder<MSAst.Expression>(kwargs.Count + 2);
+                var varExpr = Expression.Variable(typeof(PythonDictionary), "$dict");
+                expressions.Add(Expression.Assign(varExpr, Expression.Call(AstMethods.MakeEmptyDict)));
+                foreach (var arg in kwargs) {
+                    if (arg.ArgumentInfo.Kind == ArgumentType.Dictionary) {
+                        expressions.Add(Expression.Call(AstMethods.DictMerge, context, varExpr, AstUtils.Convert(arg.Expression, typeof(object))));
+                    } else {
+                        expressions.Add(Expression.Call(AstMethods.DictMergeOne, context, varExpr, AstUtils.Constant(arg.Name, typeof(object)), AstUtils.Convert(arg.Expression, typeof(object))));
+                    }
+                }
+                expressions.Add(varExpr);
+                return Expression.Block(typeof(PythonDictionary), new MSAst.ParameterExpression[] { varExpr }, expressions);
+            }
         }
 
         private Microsoft.Scripting.Ast.LightExpression<Func<CodeContext, CodeContext>> MakeClassBody() {
diff --git a/Src/IronPython/Runtime/Operations/PythonOps.cs b/Src/IronPython/Runtime/Operations/PythonOps.cs
@@ -1305,13 +1305,18 @@ public static void InitializeForFinalization(CodeContext/*!*/ context, object ne
             iwr.SetFinalizer(new WeakRefTracker(iwr, nif, nif));
         }
 
-        internal static object? CallPrepare(CodeContext/*!*/ context, PythonType meta, string name, PythonTuple bases, PythonDictionary dict) {
+        internal static object? CallPrepare(CodeContext/*!*/ context, PythonType meta, string name, PythonTuple bases, PythonDictionary? keywords, PythonDictionary dict) {
             object? classdict = dict;
 
             // if available, call the __prepare__ method to get the classdict (PEP 3115)
             if (meta.TryLookupSlot(context, "__prepare__", out PythonTypeSlot pts)) {
                 if (pts.TryGetValue(context, null, meta, out object value)) {
-                    classdict = PythonOps.CallWithContext(context, value, name, bases);
+                    if (keywords is null || keywords.Count == 0) {
+                        classdict = PythonOps.CallWithContext(context, value, name, bases);
+                    } else {
+                        var args = new object[] { name, bases };
+                        classdict = PythonCalls.CallWithKeywordArgs(context, value, args, keywords);
+                    }
                     // copy the contents of dict to the classdict
                     foreach (var pair in dict)
                         context.LanguageContext.SetIndex(classdict, pair.Key, pair.Value);
@@ -1321,10 +1326,15 @@ public static void InitializeForFinalization(CodeContext/*!*/ context, object ne
             return classdict;
         }
 
-        public static object MakeClass(FunctionCode funcCode, Func<CodeContext, CodeContext> body, CodeContext/*!*/ parentContext, string name, PythonTuple bases, object metaclass, string selfNames) {
+        public static object MakeClass(FunctionCode funcCode, Func<CodeContext, CodeContext> body, CodeContext/*!*/ parentContext, string name, PythonTuple bases, PythonDictionary? keywords, string selfNames) {
             Func<CodeContext, CodeContext> func = GetClassCode(parentContext, funcCode, body);
 
-            return MakeClass(parentContext, name, bases, metaclass, selfNames, func(parentContext).Dict);
+            object? metaclass = null;
+            if (keywords is not null && keywords.TryGetValueNoMissing("metaclass", out metaclass)) {
+                keywords.RemoveDirect("metaclass"); // keyword argument consumed
+            }
+
+            return MakeClass(parentContext, name, bases, metaclass, keywords, selfNames, func(parentContext).Dict);
         }
 
         private static Func<CodeContext, CodeContext> GetClassCode(CodeContext/*!*/ context, FunctionCode funcCode, Func<CodeContext, CodeContext> body) {
@@ -1342,7 +1352,9 @@ private static Func<CodeContext, CodeContext> GetClassCode(CodeContext/*!*/ cont
             }
         }
 
-        private static object MakeClass(CodeContext/*!*/ context, string name, PythonTuple bases, object metaclass, string selfNames, PythonDictionary vars) {
+        private static object MakeClass(CodeContext/*!*/ context, string name, PythonTuple bases, object? metaclass, PythonDictionary? keywords, string selfNames, PythonDictionary vars) {
+            Debug.Assert(metaclass is null || keywords is not null);
+
             foreach (object? dt in bases) {
                 if (dt is TypeGroup) {
                     object?[] newBases = new object[bases.Count];
@@ -1368,6 +1380,9 @@ private static object MakeClass(CodeContext/*!*/ context, string name, PythonTup
             }
 
             if (metaclass is null) {
+                if (keywords != null && keywords.Count > 0) {
+                    throw TypeError("type() takes no keyword arguments");
+                }
                 // this makes sure that object is a base
                 if (bases.Count == 0) {
                     bases = PythonTuple.MakeTuple(DynamicHelpers.GetPythonTypeFromType(typeof(object)));
@@ -1377,14 +1392,13 @@ private static object MakeClass(CodeContext/*!*/ context, string name, PythonTup
 
             object? classdict = vars;
 
-            if (metaclass is PythonType) {
-                classdict = CallPrepare(context, (PythonType)metaclass, name, bases, vars);
+            if (metaclass is PythonType metatype) {
+                classdict = CallPrepare(context, metatype, name, bases, keywords, vars);
             }
 
             // eg:
-            // def foo(*args): print args            
-            // __metaclass__ = foo
-            // class bar: pass
+            // def foo(*args): print(args)
+            // class bar(metaclass=foo): pass
             // calls our function...
             PythonContext pc = context.LanguageContext;
 
@@ -1394,7 +1408,8 @@ private static object MakeClass(CodeContext/*!*/ context, string name, PythonTup
                 metaclass,
                 name,
                 bases,
-                classdict
+                classdict,
+                keywords
             );
 
             if (obj is PythonType newType && newType.BaseTypes.Count == 0) {
@@ -1506,12 +1521,21 @@ public static void DictMerge(CodeContext context, PythonDictionary dict, object?
             // enumerate the keys getting their values
             IEnumerator enumerator = GetEnumerator(keys);
             while (enumerator.MoveNext()) {
-                object? o = enumerator.Current;
-                if (dict.ContainsKey(o) && (o is string || o is Extensible<string>)) {
-                    throw TypeError($"function got multiple values for keyword argument '{o}'");
-                }
-                dict[o] = PythonOps.GetIndex(context, item, o);
+                object? key = enumerator.Current;
+                object? value = PythonOps.GetIndex(context, item, key);
+                DictMergeOne(context, dict, key, value);
+            }
+        }
+
+        /// <summary>
+        /// Like DictMerge but for a single key/value element
+        /// </summary>
+        [EditorBrowsable(EditorBrowsableState.Never)]
+        public static void DictMergeOne(CodeContext context, PythonDictionary dict, object? key, object? value) {
+            if (dict.ContainsKey(key) && (key is string || key is Extensible<string>)) {
+                throw TypeError($"function got multiple values for keyword argument '{key}'");
             }
+            dict[key] = value;
         }
 
         /// <summary>
diff --git a/Src/IronPython/Runtime/PythonContext.cs b/Src/IronPython/Runtime/PythonContext.cs
@@ -70,7 +70,7 @@ public sealed partial class PythonContext : LanguageContext {
 
         private Dictionary<AttrKey, CallSite<Func<CallSite, object, object, object>>> _setAttrSites;
         private Dictionary<AttrKey, CallSite<Action<CallSite, object>>> _deleteAttrSites;
-        private CallSite<Func<CallSite, CodeContext, object, string, PythonTuple, object, object>> _metaClassSite;
+        private CallSite<Func<CallSite, CodeContext, object, string, PythonTuple, object, PythonDictionary, object>> _metaClassSite;
         private CallSite<Func<CallSite, CodeContext, object, string, object>> _writeSite;
         private CallSite<Func<CallSite, object, object, object>> _getIndexSite, _equalSite;
         private CallSite<Action<CallSite, object, object>> _delIndexSite;
@@ -2197,14 +2197,19 @@ internal void DeleteAttr(CodeContext/*!*/ context, object o, string name) {
             site.Target(site, o);
         }
 
-        internal CallSite<Func<CallSite, CodeContext, object, string, PythonTuple, object, object>> MetaClassCallSite {
+        internal CallSite<Func<CallSite, CodeContext, object, string, PythonTuple, object, PythonDictionary, object>> MetaClassCallSite {
             get {
                 if (_metaClassSite == null) {
                     Interlocked.CompareExchange(
                         ref _metaClassSite,
-                        CallSite<Func<CallSite, CodeContext, object, string, PythonTuple, object, object>>.Create(
+                        CallSite<Func<CallSite, CodeContext, object, string, PythonTuple, object, PythonDictionary, object>>.Create(
                             Invoke(
-                                new CallSignature(3)
+                                new CallSignature(
+                                    ArgumentType.Simple, // name
+                                    ArgumentType.Simple, // bases
+                                    ArgumentType.Simple, // classdict
+                                    ArgumentType.Dictionary // keywords
+                                )
                             )
                         ),
                         null
diff --git a/Src/IronPython/Runtime/Types/PythonType.cs b/Src/IronPython/Runtime/Types/PythonType.cs
@@ -267,7 +267,7 @@ internal static object __new__(CodeContext/*!*/ context, PythonType cls, string
             object type;
 
             if (meta != TypeCache.PythonType) {
-                object classdict = PythonOps.CallPrepare(context, meta, name, bases, dict);
+                object classdict = PythonOps.CallPrepare(context, meta, name, bases, null, dict);
 
                 if (meta != cls) {
                     // the user has a custom __new__ which picked the wrong meta class, call the correct metaclass
diff --git a/Tests/interop/net/type/test___clrtype.py b/Tests/interop/net/type/test___clrtype.py
@@ -835,7 +835,7 @@ class X(object, metaclass=MyType):
             self.fail("type.__new__ signature is wrong")
         except TypeError as e:
             self.assertEqual(str(e),
-                    "__new__() takes exactly 1 argument (4 given)")
+                    "__new__() takes exactly 1 non-keyword argument (4 given)")
         finally:
             self.assertEqual(called, False)
 
diff --git a/Tests/test_metaclass.py b/Tests/test_metaclass.py
diff --git a/WhatsNewInPython30.md b/WhatsNewInPython30.md

Original file line number	Diff line number	Diff line change
`@@ -156,6 +156,7 @@ static MSAst.Expression UnpackListHelper(IReadOnlyList<Arg> args, int firstListP`
`156`	`156`	`return Expression.Block(typeof(PythonList), new MSAst.ParameterExpression[] { varExpr }, expressions);`
`157`	`157`	`}`
`158`	`158`
	`159`	`+ // Compare to: ClassDefinition.Reduce.__UnpackKeywordsHelper`
`159`	`160`	`static MSAst.Expression UnpackDictHelper(MSAst.Expression context, IReadOnlyList<Arg> kwargs, int numDict, int firstDictPos) {`
`160`	`161`	`Debug.Assert(kwargs.Count > 0);`
`161`	`162`	`Debug.Assert(0 < numDict && numDict <= kwargs.Count);`