Unpack function arguments (#1160)

slozier · web-flow · commit ee070adc0b09 · 2021-04-04T15:32:50.000-04:00
* Unpack function arguments

* Fix test_grammar
diff --git a/Src/IronPython/Compiler/Ast/Arg.cs b/Src/IronPython/Compiler/Ast/Arg.cs
@@ -2,22 +2,20 @@
 // The .NET Foundation licenses this file to you under the Apache 2.0 License.
 // See the LICENSE file in the project root for more information.
 
-using MSAst = System.Linq.Expressions;
+#nullable enable
 
-using IronPython.Runtime;
-using Microsoft.Scripting;
 using Microsoft.Scripting.Actions;
 
 namespace IronPython.Compiler.Ast {
     public class Arg : Node {
         public Arg(Expression expression) : this(null, expression) { }
 
-        public Arg(string name, Expression expression) {
+        public Arg(string? name, Expression expression) {
             Name = name;
             Expression = expression;
         }
 
-        public string Name { get; }
+        public string? Name { get; }
 
         public Expression Expression { get; }
 
diff --git a/Src/IronPython/Compiler/Ast/AstMethods.cs b/Src/IronPython/Compiler/Ast/AstMethods.cs
@@ -80,6 +80,7 @@ internal static class AstMethods {
         public static readonly MethodInfo GeneratorCheckThrowableAndReturnSendValue = GetMethod((Func<object, object>)PythonOps.GeneratorCheckThrowableAndReturnSendValue);
 
         // methods matching Python opcodes
+        public static readonly MethodInfo DictMerge = GetMethod((Action<CodeContext, PythonDictionary, object>)PythonOps.DictMerge);
         public static readonly MethodInfo DictUpdate = GetMethod((Action<CodeContext, PythonDictionary, object>)PythonOps.DictUpdate);
         public static readonly MethodInfo ListAppend = GetMethod((Action<PythonList, object>)PythonOps.ListAppend);
         public static readonly MethodInfo ListExtend = GetMethod((Action<PythonList, object>)PythonOps.ListExtend);
diff --git a/Src/IronPython/Compiler/Ast/CallExpression.cs b/Src/IronPython/Compiler/Ast/CallExpression.cs
@@ -2,27 +2,29 @@
 // The .NET Foundation licenses this file to you under the Apache 2.0 License.
 // See the LICENSE file in the project root for more information.
 
-using MSAst = System.Linq.Expressions;
+#nullable enable
 
 using System;
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Linq;
 using System.Runtime.CompilerServices;
 
+using IronPython.Runtime;
+
 using Microsoft.Scripting.Actions;
-using Microsoft.Scripting.Utils;
 using Microsoft.Scripting.Interpreter;
-using Microsoft.Scripting.Runtime;
+using Microsoft.Scripting.Utils;
 
-using IronPython.Runtime;
-using IronPython.Runtime.Operations;
-using IronPython.Runtime.Types;
+using AstUtils = Microsoft.Scripting.Ast.Utils;
+using MSAst = System.Linq.Expressions;
 
 namespace IronPython.Compiler.Ast {
 
     public class CallExpression : Expression, IInstructionProvider {
         public CallExpression(Expression target, Arg[] args) {
+            // TODO: use two arrays (args/keywords) instead
+            if (args == null) throw new ArgumentNullException(nameof(args));
             Target = target;
             Args = args;
         }
@@ -36,55 +38,168 @@ public CallExpression(Expression target, Arg[] args) {
         public bool NeedsLocalsDictionary() {
             if (!(Target is NameExpression nameExpr)) return false;
 
-            if (Args.Length == 0) {
-                if (nameExpr.Name == "locals") return true;
-                if (nameExpr.Name == "vars") return true;
-                if (nameExpr.Name == "dir") return true;
-                return false;
-            } else if (Args.Length == 1 && (nameExpr.Name == "dir" || nameExpr.Name == "vars")) {
-                if (Args[0].Name == "*" || Args[0].Name == "**") {
-                    // could be splatting empty list or dict resulting in 0-param call which needs context
-                    return true;
-                }
-            } else if (Args.Length == 2 && (nameExpr.Name == "dir" || nameExpr.Name == "vars")) {
-                if (Args[0].Name == "*" && Args[1].Name == "**") {
-                    // could be splatting empty list and dict resulting in 0-param call which needs context
-                    return true;
-                }
-            } else {
-                if (nameExpr.Name == "eval" || nameExpr.Name == "exec") return true;
+            if (nameExpr.Name == "eval" || nameExpr.Name == "exec") return true;
+            if (nameExpr.Name == "dir" || nameExpr.Name == "vars" || nameExpr.Name == "locals") {
+                // could be splatting empty list or dict resulting in 0-param call which needs context
+                return Args.All(arg => arg.Name == "*" || arg.Name == "**");
             }
+
             return false;
         }
 
         public override MSAst.Expression Reduce() {
             Arg[] args = Args;
-            if(Args.Length == 0 && ImplicitArgs.Count > 0) {
+            if (Args.Length == 0 && ImplicitArgs.Count > 0) {
                 args = ImplicitArgs.ToArray();
             }
 
-            MSAst.Expression[] values = new MSAst.Expression[args.Length + 2];
-            Argument[] kinds = new Argument[args.Length];
+            SplitArgs(args, out var simpleArgs, out var listArgs, out var namedArgs, out var dictArgs, out var numDict);
+
+            Argument[] kinds = new Argument[simpleArgs.Length + Math.Min(listArgs.Length, 1) + namedArgs.Length + (dictArgs.Length - numDict) + Math.Min(numDict, 1)];
+            MSAst.Expression[] values = new MSAst.Expression[2 + kinds.Length];
 
             values[0] = Parent.LocalContext;
             values[1] = Target;
 
-            for (int i = 0; i < args.Length; i++) {
-                kinds[i] = args[i].GetArgumentInfo();
-                values[i + 2] = args[i].Expression;
+            int i = 0;
+
+            // add simple arguments
+            foreach (var arg in simpleArgs) {
+                kinds[i] = arg.GetArgumentInfo();
+                values[i + 2] = arg.Expression;
+                i++;
+            }
+
+            // unpack list arguments
+            if (listArgs.Length > 0) {
+                var arg = listArgs[0];
+                Debug.Assert(arg.GetArgumentInfo().Kind == ArgumentType.List);
+                kinds[i] = arg.GetArgumentInfo();
+                values[i + 2] = UnpackListHelper(listArgs);
+                i++;
+            }
+
+            // add named arguments
+            foreach (var arg in namedArgs) {
+                kinds[i] = arg.GetArgumentInfo();
+                values[i + 2] = arg.Expression;
+                i++;
+            }
+
+            // add named arguments specified after a dict unpack
+            if (dictArgs.Length != numDict) {
+                foreach (var arg in dictArgs) {
+                    var info = arg.GetArgumentInfo();
+                    if (info.Kind == ArgumentType.Named) {
+                        kinds[i] = info;
+                        values[i + 2] = arg.Expression;
+                        i++;
+                    }
+                }
+            }
+
+            // unpack dict arguments
+            if (dictArgs.Length > 0) {
+                var arg = dictArgs[0];
+                Debug.Assert(arg.GetArgumentInfo().Kind == ArgumentType.Dictionary);
+                kinds[i] = arg.GetArgumentInfo();
+                values[i + 2] = UnpackDictHelper(Parent.LocalContext, dictArgs);
             }
 
             return Parent.Invoke(
                 new CallSignature(kinds),
                 values
             );
+
+            static void SplitArgs(Arg[] args, out ReadOnlySpan<Arg> simpleArgs, out ReadOnlySpan<Arg> listArgs, out ReadOnlySpan<Arg> namedArgs, out ReadOnlySpan<Arg> dictArgs, out int numDict) {
+                if (args.Length == 0) {
+                    simpleArgs = default;
+                    listArgs = default;
+                    namedArgs = default;
+                    dictArgs = default;
+                    numDict = 0;
+                    return;
+                }
+
+                int idxSimple = args.Length;
+                int idxList = args.Length;
+                int idxNamed = args.Length;
+                int idxDict = args.Length;
+                numDict = 0;
+
+                // we want idxSimple <= idxList <= idxNamed <= idxDict
+                for (var i = args.Length - 1; i >= 0; i--) {
+                    var arg = args[i];
+                    var info = arg.GetArgumentInfo();
+                    switch (info.Kind) {
+                        case ArgumentType.Simple:
+                            idxSimple = i;
+                            break;
+                        case ArgumentType.List:
+                            idxList = i;
+                            break;
+                        case ArgumentType.Named:
+                            idxNamed = i;
+                            break;
+                        case ArgumentType.Dictionary:
+                            idxDict = i;
+                            numDict++;
+                            break;
+                        default:
+                            throw new InvalidOperationException();
+                    }
+                }
+                dictArgs = args.AsSpan(idxDict);
+                if (idxNamed > idxDict) idxNamed = idxDict;
+                namedArgs = args.AsSpan(idxNamed, idxDict - idxNamed);
+                if (idxList > idxNamed) idxList = idxNamed;
+                listArgs = args.AsSpan(idxList, idxNamed - idxList);
+                if (idxSimple > idxList) idxSimple = idxList;
+                simpleArgs = args.AsSpan(idxSimple, idxList - idxSimple);
+            }
+
+            static MSAst.Expression UnpackListHelper(ReadOnlySpan<Arg> args) {
+                Debug.Assert(args.Length > 0);
+                Debug.Assert(args[0].GetArgumentInfo().Kind == ArgumentType.List);
+                if (args.Length == 1) return args[0].Expression;
+
+                var expressions = new ReadOnlyCollectionBuilder<MSAst.Expression>(args.Length + 2);
+                var varExpr = Expression.Variable(typeof(PythonList), "$coll");
+                expressions.Add(Expression.Assign(varExpr, Expression.Call(AstMethods.MakeEmptyList)));
+                foreach (var arg in args) {
+                    if (arg.GetArgumentInfo().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(varExpr);
+                return Expression.Block(typeof(PythonList), new MSAst.ParameterExpression[] { varExpr }, expressions);
+            }
+
+            static MSAst.Expression UnpackDictHelper(MSAst.Expression context, ReadOnlySpan<Arg> args) {
+                Debug.Assert(args.Length > 0);
+                Debug.Assert(args[0].GetArgumentInfo().Kind == ArgumentType.Dictionary);
+                if (args.Length == 1) return args[0].Expression;
+
+                var expressions = new List<MSAst.Expression>(args.Length + 2);
+                var varExpr = Expression.Variable(typeof(PythonDictionary), "$dict");
+                expressions.Add(Expression.Assign(varExpr, Expression.Call(AstMethods.MakeEmptyDict)));
+                foreach (var arg in args) {
+                    if (arg.GetArgumentInfo().Kind == ArgumentType.Dictionary) {
+                        expressions.Add(Expression.Call(AstMethods.DictMerge, context, varExpr, arg.Expression));
+                    }
+                }
+                expressions.Add(varExpr);
+                return Expression.Block(typeof(PythonDictionary), new MSAst.ParameterExpression[] { varExpr }, expressions);
+            }
         }
 
         #region IInstructionProvider Members
 
         void IInstructionProvider.AddInstructions(LightCompiler compiler) {
             Arg[] args = Args;
-            if(args.Length == 0 && ImplicitArgs.Count > 0) {
+            if (args.Length == 0 && ImplicitArgs.Count > 0) {
                 args = ImplicitArgs.ToArray();
             }
 
@@ -158,27 +273,19 @@ void IInstructionProvider.AddInstructions(LightCompiler compiler) {
                     compiler.Instructions.Emit(new Invoke6Instruction(Parent.PyContext));
                     return;
 
-                // *** END GENERATED CODE ***
+                    // *** END GENERATED CODE ***
 
-                #endregion
+                    #endregion
             }
             compiler.Compile(Reduce());
         }
 
         #endregion
 
         private abstract class InvokeInstruction : Instruction {
-            public override int ProducedStack {
-                get {
-                    return 1;
-                }
-            }
+            public override int ProducedStack => 1;
 
-            public override string InstructionName {
-                get {
-                    return "Python Invoke" + (ConsumedStack - 1);
-                }
-            }
+            public override string InstructionName => "Python Invoke" + (ConsumedStack - 1);
         }
 
         #region Generated Python Call Expression Instructions
diff --git a/Src/IronPython/Compiler/Ast/DictionaryExpression.cs b/Src/IronPython/Compiler/Ast/DictionaryExpression.cs
@@ -22,6 +22,7 @@ public class DictionaryExpression : Expression, IInstructionProvider {
         private static readonly MSAst.Expression EmptyDictExpression = Expression.Call(AstMethods.MakeEmptyDict);
 
         public DictionaryExpression(params SliceExpression[] items) {
+            // TODO: use two arrays instead of SliceExpression
             foreach (var item in items) {
                 if (item.SliceStart is null) _hasNullKey = true;
                 if (item.SliceStop is null) throw PythonOps.ValueError("None disallowed in expression list");
diff --git a/Src/IronPython/Compiler/Ast/SetExpression.cs b/Src/IronPython/Compiler/Ast/SetExpression.cs
@@ -29,10 +29,6 @@ public SetExpression(params Expression[] items) {
         protected bool HasStarredExpression => Items.OfType<StarredExpression>().Any();
 
         public override MSAst.Expression Reduce() {
-            if (Items.Count == 0) {
-                return Expression.Call(AstMethods.MakeEmptySet);
-            }
-
             if (HasStarredExpression) {
                 return UnpackSequenceHelper<SetCollection>(Items, AstMethods.MakeEmptySet, AstMethods.SetAdd, AstMethods.SetUpdate);
             }
diff --git a/Src/IronPython/Compiler/Parser.cs b/Src/IronPython/Compiler/Parser.cs
@@ -2901,31 +2901,24 @@ private void PushFunction(FunctionDefinition function) {
         }
 
         private CallExpression FinishCallExpr(Expression target, params Arg[] args) {
-            bool hasArgsTuple = false;
-            bool hasKeywordDict = false;
-            int keywordCount = 0;
-            int extraArgs = 0;
+            bool hasKeyword = false;
+            bool hasKeywordUnpacking = false;
 
             foreach (Arg arg in args) {
                 if (arg.Name == null) {
-                    if (hasArgsTuple || hasKeywordDict || keywordCount > 0) {
-                        ReportSyntaxError(IronPython.Resources.NonKeywordAfterKeywordArg);
+                    if (hasKeywordUnpacking) {
+                        ReportSyntaxError(arg.StartIndex, arg.EndIndex, "positional argument follows keyword argument unpacking");
+                    } else if (hasKeyword) {
+                        ReportSyntaxError(arg.StartIndex, arg.EndIndex, "positional argument follows keyword argument");
                     }
                 } else if (arg.Name == "*") {
-                    if (hasArgsTuple || hasKeywordDict) {
-                        ReportSyntaxError(IronPython.Resources.OneListArgOnly);
+                    if (hasKeywordUnpacking) {
+                        ReportSyntaxError(arg.StartIndex, arg.EndIndex, "iterable argument unpacking follows keyword argument unpacking");
                     }
-                    hasArgsTuple = true; extraArgs++;
                 } else if (arg.Name == "**") {
-                    if (hasKeywordDict) {
-                        ReportSyntaxError(IronPython.Resources.OneKeywordArgOnly);
-                    }
-                    hasKeywordDict = true; extraArgs++;
+                    hasKeywordUnpacking = true;
                 } else {
-                    if (hasKeywordDict) {
-                        ReportSyntaxError(IronPython.Resources.KeywordOutOfSequence);
-                    }
-                    keywordCount++;
+                    hasKeyword = true;
                 }
             }
 
diff --git a/Src/IronPython/Modules/_ast.cs b/Src/IronPython/Modules/_ast.cs
@@ -1283,7 +1283,7 @@ internal Dict(DictionaryExpression expr)
                 keys = new PythonList(expr.Items.Count);
                 values = new PythonList(expr.Items.Count);
                 foreach (SliceExpression item in expr.Items) {
-                    keys.Add(Convert(item.SliceStart));
+                    keys.Add(item.SliceStart is null ? null : Convert(item.SliceStart));
                     values.Add(Convert(item.SliceStop));
                 }
             }
diff --git a/Src/IronPython/Runtime/Operations/PythonOps.cs b/Src/IronPython/Runtime/Operations/PythonOps.cs
@@ -1486,6 +1486,27 @@ public static PythonTuple MakeTupleFromSequence(object items) {
             return PythonTuple.Make(items);
         }
 
+        /// <summary>
+        /// DICT_MERGE
+        /// </summary>
+        [EditorBrowsable(EditorBrowsableState.Never)]
+        public static void DictMerge(CodeContext context, PythonDictionary dict, object? item) {
+            // call dict.keys()
+            if (!PythonTypeOps.TryInvokeUnaryOperator(context, item, "keys", out object keys)) {
+                throw TypeError($"'{PythonTypeOps.GetName(item)}' object is not a mapping");
+            }
+
+            // 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);
+            }
+        }
+
         /// <summary>
         /// DICT_UPDATE
         /// </summary>
diff --git a/Src/StdLib/Lib/test/test_grammar.py b/Src/StdLib/Lib/test/test_grammar.py
diff --git a/Src/StdLib/Lib/test/test_unpack_ex.py b/Src/StdLib/Lib/test/test_unpack_ex.py

Original file line number	Diff line number	Diff line change
`@@ -1283,7 +1283,7 @@ internal Dict(DictionaryExpression expr)`
`1283`	`1283`	`keys = new PythonList(expr.Items.Count);`
`1284`	`1284`	`values = new PythonList(expr.Items.Count);`
`1285`	`1285`	`foreach (SliceExpression item in expr.Items) {`
`1286`		`- keys.Add(Convert(item.SliceStart));`
	`1286`	`+ keys.Add(item.SliceStart is null ? null : Convert(item.SliceStart));`
`1287`	`1287`	`values.Add(Convert(item.SliceStop));`
`1288`	`1288`	`}`
`1289`	`1289`	`}`