From 54b66114e0761c58e7b102bdeb4f76835c3822a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Lozier?= Date: Sat, 17 Apr 2021 15:02:57 -0400 Subject: [PATCH 1/4] Remove IronPython.Compiler.Ast.Arg --- Src/IronPython/Compiler/Ast/CallExpression.cs | 188 +++++++++--------- .../Compiler/Ast/ClassDefinition.cs | 59 +++--- Src/IronPython/Compiler/Ast/Expression.cs | 15 ++ Src/IronPython/Compiler/Ast/FlowChecker.cs | 4 +- .../Compiler/Ast/{Arg.cs => Keyword.cs} | 20 +- Src/IronPython/Compiler/Ast/Node.cs | 8 + .../Compiler/Ast/PythonNameBinder.cs | 17 +- .../Compiler/Ast/PythonWalker.Generated.cs | 17 +- .../Compiler/Ast/StarredExpression.cs | 15 ++ Src/IronPython/Compiler/Parser.cs | 110 +++++----- Src/IronPython/Modules/_ast.cs | 56 ++---- Src/Scripts/generate_calls.py | 2 +- 12 files changed, 256 insertions(+), 255 deletions(-) rename Src/IronPython/Compiler/Ast/{Arg.cs => Keyword.cs} (50%) diff --git a/Src/IronPython/Compiler/Ast/CallExpression.cs b/Src/IronPython/Compiler/Ast/CallExpression.cs index f1600518d..c0e2f901d 100644 --- a/Src/IronPython/Compiler/Ast/CallExpression.cs +++ b/Src/IronPython/Compiler/Ast/CallExpression.cs @@ -22,20 +22,25 @@ namespace IronPython.Compiler.Ast { public class CallExpression : Expression, IInstructionProvider { - public CallExpression(Expression target, IReadOnlyList? args, IReadOnlyList? kwargs) { + private readonly Expression[] _args; + private readonly Keyword[] _kwargs; + private Expression[]? _implicitArgs; + + public CallExpression(Expression target, IReadOnlyList? args, IReadOnlyList? kwargs) { Target = target; - Args = args?.ToArray() ?? Array.Empty(); - Kwargs = kwargs?.ToArray() ?? Array.Empty(); + _args = args?.ToArray() ?? Array.Empty(); + _kwargs = kwargs?.ToArray() ?? Array.Empty(); } public Expression Target { get; } - public IReadOnlyList Args { get; } + public IReadOnlyList Args => _args; - public IReadOnlyList Kwargs { get; } + public IReadOnlyList Kwargs => _kwargs; - internal IList ImplicitArgs => _implicitArgs ??= new List(); - private List? _implicitArgs; + internal void SetImplicitArgs(params Expression[] args) { + _implicitArgs = args; + } public bool NeedsLocalsDictionary() { if (!(Target is NameExpression nameExpr)) return false; @@ -43,25 +48,30 @@ public bool NeedsLocalsDictionary() { 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 == "*") && Kwargs.All(arg => arg.Name == "**"); + return Args.All(arg => arg is StarredExpression) && Kwargs.All(arg => arg.Name is null); } return false; } + private static readonly Argument _listTypeArgument = new(ArgumentType.List); + private static readonly Argument _dictTypeArgument = new(ArgumentType.Dictionary); + public override MSAst.Expression Reduce() { - IReadOnlyList args = Args; - if (Args.Count == 0 && _implicitArgs != null && _implicitArgs.Count > 0) { + ReadOnlySpan args = _args; + ReadOnlySpan kwargs = _kwargs; + + if (args.Length == 0 && _implicitArgs != null) { args = _implicitArgs; } // count splatted list args and find the lowest index of a list argument, if any - ScanArgs(args, ArgumentType.List, out var numList, out int firstListPos); - Debug.Assert(numList == 0 || firstListPos < args.Count); + ScanArgs(args, out var numList, out int firstListPos); + Debug.Assert(numList == 0 || firstListPos < args.Length); // count splatted dictionary args and find the lowest index of a dict argument, if any - ScanArgs(Kwargs, ArgumentType.Dictionary, out var numDict, out int firstDictPos); - Debug.Assert(numDict == 0 || firstDictPos < Kwargs.Count); + ScanKwargs(kwargs, out var numDict, out int firstDictPos); + Debug.Assert(numDict == 0 || firstDictPos < kwargs.Length); // all list arguments and all simple arguments after the first list will be collated into a single list for the actual call // all dictionary arguments will be merged into a single dictionary for the actual call @@ -77,28 +87,25 @@ public override MSAst.Expression Reduce() { if (firstListPos > 0) { foreach (var arg in args) { if (i == firstListPos) break; - Debug.Assert(arg.ArgumentInfo.Kind == ArgumentType.Simple); - kinds[i] = arg.ArgumentInfo; - values[i + 2] = arg.Expression; + Debug.Assert(arg is not StarredExpression); + kinds[i] = Argument.Simple; + values[i + 2] = arg; i++; } } // unpack list arguments if (numList > 0) { - var arg = args[firstListPos]; - Debug.Assert(arg.ArgumentInfo.Kind == ArgumentType.List); - kinds[i] = arg.ArgumentInfo; - values[i + 2] = UnpackListHelper(args, firstListPos); + kinds[i] = _listTypeArgument; + values[i + 2] = UnpackListHelper(args.Slice(firstListPos)); i++; } // add named arguments if (Kwargs.Count != numDict) { foreach (var arg in Kwargs) { - var info = arg.ArgumentInfo; - if (info.Kind == ArgumentType.Named) { - kinds[i] = info; + if (arg.Name is not null) { + kinds[i] = new Argument(arg.Name); values[i + 2] = arg.Expression; i++; } @@ -107,10 +114,8 @@ public override MSAst.Expression Reduce() { // unpack dict arguments if (numDict > 0) { - var arg = Kwargs[firstDictPos]; - Debug.Assert(arg.ArgumentInfo.Kind == ArgumentType.Dictionary); - kinds[i] = arg.ArgumentInfo; - values[i + 2] = UnpackDictHelper(Parent.LocalContext, Kwargs, numDict, firstDictPos); + kinds[i] = _dictTypeArgument; + values[i + 2] = UnpackDictHelper(Parent.LocalContext, kwargs.Slice(firstDictPos), numDict); } return Parent.Invoke( @@ -118,58 +123,59 @@ public override MSAst.Expression Reduce() { values ); - static void ScanArgs(IReadOnlyList args, ArgumentType scanForType, out int numArgs, out int firstArgPos) { + static void ScanArgs(ReadOnlySpan args, out int numArgs, out int firstArgPos) { numArgs = 0; - firstArgPos = args.Count; + firstArgPos = args.Length; - if (args.Count == 0) return; + if (args.Length == 0) return; - for (var i = args.Count - 1; i >= 0; i--) { + for (var i = args.Length - 1; i >= 0; i--) { var arg = args[i]; - var info = arg.ArgumentInfo; - if (info.Kind == scanForType) { + if (arg is StarredExpression) { firstArgPos = i; numArgs++; } } } - // Compare to: ClassDefinition.Reduce.__UnpackBasesHelper - static MSAst.Expression UnpackListHelper(IReadOnlyList args, int firstListPos) { - Debug.Assert(args.Count > 0); - Debug.Assert(args[firstListPos].ArgumentInfo.Kind == ArgumentType.List); + static void ScanKwargs(ReadOnlySpan kwargs, out int numArgs, out int firstArgPos) { + numArgs = 0; + firstArgPos = kwargs.Length; - if (args.Count - firstListPos == 1) return args[firstListPos].Expression; + if (kwargs.Length == 0) return; - var expressions = new ReadOnlyCollectionBuilder(args.Count - firstListPos + 2); - var varExpr = Expression.Variable(typeof(PythonList), "$coll"); - expressions.Add(Expression.Assign(varExpr, Expression.Call(AstMethods.MakeEmptyList))); - for (int i = firstListPos; i < args.Count; i++) { - var arg = args[i]; - 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)))); + for (var i = kwargs.Length - 1; i >= 0; i--) { + var kwarg = kwargs[i]; + if (kwarg.Name is null) { + firstArgPos = i; + numArgs++; } } - expressions.Add(varExpr); - return Expression.Block(typeof(PythonList), new MSAst.ParameterExpression[] { varExpr }, expressions); } - // Compare to: ClassDefinition.Reduce.__UnpackKeywordsHelper - static MSAst.Expression UnpackDictHelper(MSAst.Expression context, IReadOnlyList kwargs, int numDict, int firstDictPos) { - Debug.Assert(kwargs.Count > 0); - Debug.Assert(0 < numDict && numDict <= kwargs.Count); - Debug.Assert(kwargs[firstDictPos].ArgumentInfo.Kind == ArgumentType.Dictionary); + // Compare to: ClassDefinition.Reduce.__UnpackBasesHelper + static MSAst.Expression UnpackListHelper(ReadOnlySpan args) { + Debug.Assert(args.Length > 0); + Debug.Assert(args[0] is StarredExpression); + + if (args.Length == 1) return ((StarredExpression)args[0]).Value; + + return UnpackSequenceHelper(args, AstMethods.MakeEmptyList, AstMethods.ListAppend, AstMethods.ListExtend); + } - if (numDict == 1) return kwargs[firstDictPos].Expression; + // Compare to: CallExpression.Reduce.__UnpackKeywordsHelper + static MSAst.Expression UnpackDictHelper(MSAst.Expression context, ReadOnlySpan kwargs, int numDict) { + Debug.Assert(kwargs.Length > 0); + Debug.Assert(0 < numDict && numDict <= kwargs.Length); + Debug.Assert(kwargs[0].Name is null); + + if (numDict == 1) return kwargs[0].Expression; var expressions = new ReadOnlyCollectionBuilder(numDict + 2); var varExpr = Expression.Variable(typeof(PythonDictionary), "$dict"); expressions.Add(Expression.Assign(varExpr, Expression.Call(AstMethods.MakeEmptyDict))); - for (int i = firstDictPos; i < kwargs.Count; i++) { - var arg = kwargs[i]; - if (arg.ArgumentInfo.Kind == ArgumentType.Dictionary) { + foreach (var arg in kwargs) { + if (arg.Name is null) { expressions.Add(Expression.Call(AstMethods.DictMerge, context, varExpr, arg.Expression)); } } @@ -181,8 +187,8 @@ static MSAst.Expression UnpackDictHelper(MSAst.Expression context, IReadOnlyList #region IInstructionProvider Members void IInstructionProvider.AddInstructions(LightCompiler compiler) { - IReadOnlyList args = Args; - if (args.Count == 0 && _implicitArgs != null && _implicitArgs.Count > 0) { + ReadOnlySpan args = _args; + if (args.Length == 0 && _implicitArgs != null) { args = _implicitArgs; } @@ -191,14 +197,14 @@ void IInstructionProvider.AddInstructions(LightCompiler compiler) { return; } - for (int i = 0; i < args.Count; i++) { - if (!args[i].ArgumentInfo.IsSimple) { + for (int i = 0; i < args.Length; i++) { + if (args[i] is StarredExpression) { compiler.Compile(Reduce()); return; } } - switch (args.Count) { + switch (args.Length) { #region Generated Python Call Expression Instruction Switch // *** BEGIN GENERATED CODE *** @@ -212,52 +218,52 @@ void IInstructionProvider.AddInstructions(LightCompiler compiler) { case 1: compiler.Compile(Parent.LocalContext); compiler.Compile(Target); - compiler.Compile(args[0].Expression); + compiler.Compile(args[0]); compiler.Instructions.Emit(new Invoke1Instruction(Parent.PyContext)); return; case 2: compiler.Compile(Parent.LocalContext); compiler.Compile(Target); - compiler.Compile(args[0].Expression); - compiler.Compile(args[1].Expression); + compiler.Compile(args[0]); + compiler.Compile(args[1]); compiler.Instructions.Emit(new Invoke2Instruction(Parent.PyContext)); return; case 3: compiler.Compile(Parent.LocalContext); compiler.Compile(Target); - compiler.Compile(args[0].Expression); - compiler.Compile(args[1].Expression); - compiler.Compile(args[2].Expression); + compiler.Compile(args[0]); + compiler.Compile(args[1]); + compiler.Compile(args[2]); compiler.Instructions.Emit(new Invoke3Instruction(Parent.PyContext)); return; case 4: compiler.Compile(Parent.LocalContext); compiler.Compile(Target); - compiler.Compile(args[0].Expression); - compiler.Compile(args[1].Expression); - compiler.Compile(args[2].Expression); - compiler.Compile(args[3].Expression); + compiler.Compile(args[0]); + compiler.Compile(args[1]); + compiler.Compile(args[2]); + compiler.Compile(args[3]); compiler.Instructions.Emit(new Invoke4Instruction(Parent.PyContext)); return; case 5: compiler.Compile(Parent.LocalContext); compiler.Compile(Target); - compiler.Compile(args[0].Expression); - compiler.Compile(args[1].Expression); - compiler.Compile(args[2].Expression); - compiler.Compile(args[3].Expression); - compiler.Compile(args[4].Expression); + compiler.Compile(args[0]); + compiler.Compile(args[1]); + compiler.Compile(args[2]); + compiler.Compile(args[3]); + compiler.Compile(args[4]); compiler.Instructions.Emit(new Invoke5Instruction(Parent.PyContext)); return; case 6: compiler.Compile(Parent.LocalContext); compiler.Compile(Target); - compiler.Compile(args[0].Expression); - compiler.Compile(args[1].Expression); - compiler.Compile(args[2].Expression); - compiler.Compile(args[3].Expression); - compiler.Compile(args[4].Expression); - compiler.Compile(args[5].Expression); + compiler.Compile(args[0]); + compiler.Compile(args[1]); + compiler.Compile(args[2]); + compiler.Compile(args[3]); + compiler.Compile(args[4]); + compiler.Compile(args[5]); compiler.Instructions.Emit(new Invoke6Instruction(Parent.PyContext)); return; @@ -424,17 +430,15 @@ public override int Run(InterpretedFrame frame) { public override void Walk(PythonWalker walker) { if (walker.Walk(this)) { Target?.Walk(walker); - if (Args != null) { - foreach (Arg arg in Args) { + if (_implicitArgs is not null) { + foreach (var arg in _implicitArgs) { arg.Walk(walker); } } - if (_implicitArgs != null && _implicitArgs.Count > 0) { - foreach (Arg arg in ImplicitArgs) { - arg.Walk(walker); - } + foreach (var arg in _args) { + arg.Walk(walker); } - foreach (Arg arg in Kwargs) { + foreach (var arg in _kwargs) { arg.Walk(walker); } } diff --git a/Src/IronPython/Compiler/Ast/ClassDefinition.cs b/Src/IronPython/Compiler/Ast/ClassDefinition.cs index 7a57d9826..c2f1cf3ce 100644 --- a/Src/IronPython/Compiler/Ast/ClassDefinition.cs +++ b/Src/IronPython/Compiler/Ast/ClassDefinition.cs @@ -27,8 +27,8 @@ namespace IronPython.Compiler.Ast { public class ClassDefinition : ScopeStatement { private readonly string _name; - private readonly Arg[] _bases; - private readonly Arg[] _keywords; + private readonly Expression[] _bases; + private readonly Keyword[] _keywords; private LightLambdaExpression? _dlrBody; // the transformed body including all of our initialization, etc... @@ -37,10 +37,10 @@ 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, IReadOnlyList? bases, IReadOnlyList? keywords, Statement? body = null) { + public ClassDefinition(string name, IReadOnlyList? bases, IReadOnlyList? keywords, Statement? body = null) { _name = name; - _bases = bases?.ToArray() ?? Array.Empty(); - _keywords = keywords?.ToArray() ?? Array.Empty(); + _bases = bases?.ToArray() ?? Array.Empty(); + _keywords = keywords?.ToArray() ?? Array.Empty(); Body = body ?? EmptyStatement.PreCompiledInstance; } @@ -50,9 +50,9 @@ public ClassDefinition(string name, IReadOnlyList? bases, IReadOnlyList _name; - public IReadOnlyList Bases => _bases; + public IReadOnlyList Bases => _bases; - public IReadOnlyList Keywords => _keywords; + public IReadOnlyList Keywords => _keywords; public Statement Body { get; set; } @@ -194,7 +194,7 @@ public override MSAst.Expression Reduce() { classDef = AddDecorators(classDef, Decorators); return GlobalParent.AddDebugInfoAndVoid( - AssignValue(Parent.GetVariableExpression(PythonVariable!), classDef), + AssignValue(Parent.GetVariableExpression(PythonVariable!), classDef), new SourceSpan( GlobalParent.IndexToLocation(StartIndex), GlobalParent.IndexToLocation(HeaderIndex) @@ -202,34 +202,27 @@ public override MSAst.Expression Reduce() { ); // Compare to: CallExpression.Reduce.__UnpackListHelper - static MSAst.Expression UnpackBasesHelper(IReadOnlyList bases) { - if (bases.Count == 0) { + static MSAst.Expression UnpackBasesHelper(ReadOnlySpan bases) { + if (bases.Length == 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(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)))); - } + } + foreach (var arg in bases) { + if (arg is StarredExpression) { + return Expression.Call(AstMethods.ListToTuple, + Expression.UnpackSequenceHelper(bases, AstMethods.MakeEmptyList, AstMethods.ListAppend, AstMethods.ListExtend) + ); } - expressions.Add(Expression.Call(AstMethods.ListToTuple, varExpr)); - return Expression.Block(typeof(PythonTuple), new MSAst.ParameterExpression[] { varExpr }, expressions); } + return Expression.Call(AstMethods.MakeTuple, + Expression.NewArrayInit( + typeof(object), + ToObjectArray(bases) + ) + ); } // Compare to: CallExpression.Reduce.__UnpackDictHelper - static MSAst.Expression UnpackKeywordsHelper(MSAst.Expression context, IReadOnlyList kwargs) { + static MSAst.Expression UnpackKeywordsHelper(MSAst.Expression context, IReadOnlyList kwargs) { if (kwargs.Count == 0) { return AstUtils.Constant(null, typeof(PythonDictionary)); } @@ -238,7 +231,7 @@ static MSAst.Expression UnpackKeywordsHelper(MSAst.Expression context, IReadOnly 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) { + if (arg.Name is null) { 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)))); @@ -346,10 +339,10 @@ public override void Walk(PythonWalker walker) { decorator.Walk(walker); } } - foreach (Arg b in _bases) { + foreach (var b in _bases) { b.Walk(walker); } - foreach (Arg b in _keywords) { + foreach (var b in _keywords) { b.Walk(walker); } Body.Walk(walker); diff --git a/Src/IronPython/Compiler/Ast/Expression.cs b/Src/IronPython/Compiler/Ast/Expression.cs index 868fdba04..6c11a6006 100644 --- a/Src/IronPython/Compiler/Ast/Expression.cs +++ b/Src/IronPython/Compiler/Ast/Expression.cs @@ -36,6 +36,21 @@ protected internal static MSAst.BlockExpression UnpackSequenceHelper(IList(ReadOnlySpan items, MethodInfo makeEmpty, MethodInfo append, MethodInfo extend) { + var expressions = new ReadOnlyCollectionBuilder(items.Length + 2); + var varExpr = Expression.Variable(typeof(T), "$coll"); + expressions.Add(Expression.Assign(varExpr, Expression.Call(makeEmpty))); + foreach (var item in items) { + if (item is StarredExpression starredExpression) { + expressions.Add(Expression.Call(extend, varExpr, AstUtils.Convert(starredExpression.Value, typeof(object)))); + } else { + expressions.Add(Expression.Call(append, varExpr, AstUtils.Convert(item, typeof(object)))); + } + } + expressions.Add(varExpr); + return Expression.Block(typeof(T), new MSAst.ParameterExpression[] { varExpr }, expressions); + } + internal virtual MSAst.Expression TransformSet(SourceSpan span, MSAst.Expression right, PythonOperationKind op) { // unreachable, CheckAssign prevents us from calling this at parse time. Debug.Assert(false); diff --git a/Src/IronPython/Compiler/Ast/FlowChecker.cs b/Src/IronPython/Compiler/Ast/FlowChecker.cs index a90af5bcb..8b3acf40f 100644 --- a/Src/IronPython/Compiler/Ast/FlowChecker.cs +++ b/Src/IronPython/Compiler/Ast/FlowChecker.cs @@ -328,10 +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 (Arg e in node.Bases) { + foreach (var e in node.Bases) { e.Walk(this); } - foreach (Arg e in node.Keywords) { + foreach (var e in node.Keywords) { e.Walk(this); } return false; diff --git a/Src/IronPython/Compiler/Ast/Arg.cs b/Src/IronPython/Compiler/Ast/Keyword.cs similarity index 50% rename from Src/IronPython/Compiler/Ast/Arg.cs rename to Src/IronPython/Compiler/Ast/Keyword.cs index 0b8bb27a0..17deb3a91 100644 --- a/Src/IronPython/Compiler/Ast/Arg.cs +++ b/Src/IronPython/Compiler/Ast/Keyword.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // 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. @@ -7,30 +7,16 @@ using Microsoft.Scripting.Actions; namespace IronPython.Compiler.Ast { - public class Arg : Node { - private static readonly Argument _listTypeArgument = new(ArgumentType.List); - private static readonly Argument _dictTypeArgument = new(ArgumentType.Dictionary); - - public Arg(Expression expression) : this(null, expression) { } - - public Arg(string? name, Expression expression) { + public class Keyword : Node { + public Keyword(string? name, Expression expression) { Name = name; Expression = expression; - - ArgumentInfo = name switch { - null => Argument.Simple, - "*" => _listTypeArgument, - "**" => _dictTypeArgument, - _ => new Argument(name) - }; } public string? Name { get; } public Expression Expression { get; } - internal Argument ArgumentInfo { get; } - public override string ToString() { return base.ToString() + ":" + Name; } diff --git a/Src/IronPython/Compiler/Ast/Node.cs b/Src/IronPython/Compiler/Ast/Node.cs index 31d4978ce..8c5ce6752 100644 --- a/Src/IronPython/Compiler/Ast/Node.cs +++ b/Src/IronPython/Compiler/Ast/Node.cs @@ -172,6 +172,14 @@ internal static MSAst.Expression[] ToObjectArray(IList expressions) return to; } + internal static MSAst.Expression[] ToObjectArray(ReadOnlySpan expressions) { + MSAst.Expression[] to = new MSAst.Expression[expressions.Length]; + for (int i = 0; i < expressions.Length; i++) { + to[i] = AstUtils.Convert(expressions[i], typeof(object)); + } + return to; + } + internal static MSAst.Expression TransformOrConstantNull(Expression expression, Type/*!*/ type) { if (expression == null) { return AstUtils.Constant(null, type); diff --git a/Src/IronPython/Compiler/Ast/PythonNameBinder.cs b/Src/IronPython/Compiler/Ast/PythonNameBinder.cs index 6bd8ddce3..84122c4ba 100644 --- a/Src/IronPython/Compiler/Ast/PythonNameBinder.cs +++ b/Src/IronPython/Compiler/Ast/PythonNameBinder.cs @@ -206,9 +206,9 @@ public override bool Walk(ClassDefinition node) { node.PythonVariable = DefineName(node.Name); // Base references are in the outer context - foreach (Arg b in node.Bases) b.Walk(this); + foreach (var b in node.Bases) b.Walk(this); - foreach (Arg a in node.Keywords) a.Walk(this); + foreach (var a in node.Keywords) a.Walk(this); // process the decorators in the outer context if (node.Decorators != null) { @@ -333,11 +333,6 @@ public override bool Walk(AndExpression node) { node.Parent = _currentScope; return base.Walk(node); } - // Arg - public override bool Walk(Arg node) { - node.Parent = _currentScope; - return base.Walk(node); - } // AssertStatement public override bool Walk(AssertStatement node) { node.Parent = _currentScope; @@ -413,6 +408,11 @@ public override bool Walk(IndexExpression node) { node.Parent = _currentScope; return base.Walk(node); } + // Keyword + public override bool Walk(Keyword node) { + node.Parent = _currentScope; + return base.Walk(node); + } // LambdaExpression public override bool Walk(LambdaExpression node) { node.Parent = _currentScope; @@ -836,8 +836,7 @@ public override bool Walk(CallExpression node) { if (node.Target is NameExpression nameExpr && nameExpr.Name == "super" && _currentScope is FunctionDefinition func) { _currentScope.Reference("__class__"); if (node.Args.Count == 0 && node.Kwargs.Count == 0 && func.ParameterNames.Length > 0) { - node.ImplicitArgs.Add(new Arg(new NameExpression("__class__"))); - node.ImplicitArgs.Add(new Arg(new NameExpression(func.ParameterNames[0]))); + node.SetImplicitArgs(new NameExpression("__class__"), new NameExpression(func.ParameterNames[0])); } } return base.Walk(node); diff --git a/Src/IronPython/Compiler/Ast/PythonWalker.Generated.cs b/Src/IronPython/Compiler/Ast/PythonWalker.Generated.cs index 88c801435..6c0a5afa6 100644 --- a/Src/IronPython/Compiler/Ast/PythonWalker.Generated.cs +++ b/Src/IronPython/Compiler/Ast/PythonWalker.Generated.cs @@ -72,7 +72,6 @@ public virtual void PostWalk(ListExpression node) { } public virtual bool Walk(MemberExpression node) { return true; } public virtual void PostWalk(MemberExpression node) { } - // NameExpression public virtual bool Walk(NameExpression node) { return true; } public virtual void PostWalk(NameExpression node) { } @@ -213,10 +212,6 @@ public virtual void PostWalk(WhileStatement node) { } public virtual bool Walk(WithStatement node) { return true; } public virtual void PostWalk(WithStatement node) { } - // Arg - public virtual bool Walk(Arg node) { return true; } - public virtual void PostWalk(Arg node) { } - // ComprehensionFor public virtual bool Walk(ComprehensionFor node) { return true; } public virtual void PostWalk(ComprehensionFor node) { } @@ -233,6 +228,10 @@ public virtual void PostWalk(DottedName node) { } public virtual bool Walk(IfStatementTest node) { return true; } public virtual void PostWalk(IfStatementTest node) { } + // Keyword + public virtual bool Walk(Keyword node) { return true; } + public virtual void PostWalk(Keyword node) { } + // ModuleName public virtual bool Walk(ModuleName node) { return true; } public virtual void PostWalk(ModuleName node) { } @@ -460,10 +459,6 @@ public override void PostWalk(WhileStatement node) { } public override bool Walk(WithStatement node) { return false; } public override void PostWalk(WithStatement node) { } - // Arg - public override bool Walk(Arg node) { return false; } - public override void PostWalk(Arg node) { } - // ComprehensionFor public override bool Walk(ComprehensionFor node) { return false; } public override void PostWalk(ComprehensionFor node) { } @@ -480,6 +475,10 @@ public override void PostWalk(DottedName node) { } public override bool Walk(IfStatementTest node) { return false; } public override void PostWalk(IfStatementTest node) { } + // Keyword + public override bool Walk(Keyword node) { return false; } + public override void PostWalk(Keyword node) { } + // ModuleName public override bool Walk(ModuleName node) { return false; } public override void PostWalk(ModuleName node) { } diff --git a/Src/IronPython/Compiler/Ast/StarredExpression.cs b/Src/IronPython/Compiler/Ast/StarredExpression.cs index 6339113b5..ab6c23a3b 100644 --- a/Src/IronPython/Compiler/Ast/StarredExpression.cs +++ b/Src/IronPython/Compiler/Ast/StarredExpression.cs @@ -67,6 +67,21 @@ public override bool Walk(AssignmentStatement node) { return false; } + public override bool Walk(CallExpression node) { + node.Target?.Walk(this); + foreach (var item in node.Args) { + if (item is StarredExpression starred) { + starred.Value.Walk(this); + } else { + item.Walk(this); + } + } + foreach (var arg in node.Kwargs) { + arg.Walk(this); + }; + return false; + } + public override bool Walk(ForStatement node) { WalkAssignmentTarget(node.Left); node.List?.Walk(this); diff --git a/Src/IronPython/Compiler/Parser.cs b/Src/IronPython/Compiler/Parser.cs index 96fa95a88..7f023e4eb 100644 --- a/Src/IronPython/Compiler/Parser.cs +++ b/Src/IronPython/Compiler/Parser.cs @@ -1009,10 +1009,10 @@ private ClassDefinition ParseClassDef() { return new ClassDefinition(string.Empty, null, null, ErrorStmt()); } - List bases = null; - List keywords = null; + List bases = null; + List keywords = null; if (MaybeEat(TokenKind.LeftParenthesis)) { - IReadOnlyList args = FinishArgumentList(null); + IReadOnlyList args = FinishArgumentList(null); SplitAndValidateArguments(args, out bases, out keywords); } var mid = GetEnd(); @@ -1058,7 +1058,7 @@ private List ParseDecorators() { if (MaybeEat(TokenKind.LeftParenthesis)) { ParserSink?.StartParameters(GetSourceSpan()); - IReadOnlyList args = FinishArgumentList(null); + IReadOnlyList args = FinishArgumentList(null); decorator = FinishCallExpr(decorator, args); } decorator.SetLoc(_globalParent, start, GetEnd()); @@ -1982,7 +1982,7 @@ private Expression AddTrailers(Expression ret, bool allowGeneratorExpression) { if (!allowGeneratorExpression) return ret; NextToken(); - IReadOnlyList args = FinishArgListOrGenExpr(); + IReadOnlyList args = FinishArgListOrGenExpr(); CallExpression call = FinishCallExpr(ret, args); call.SetLoc(_globalParent, ret.StartIndex, GetEnd()); ret = call; @@ -2120,8 +2120,8 @@ private List ParseExprList(out bool trailingComma) { // expression "=" expression rest_of_arguments // expression "for" gen_expr_rest // - private IReadOnlyList FinishArgListOrGenExpr() { - Arg a = null; + private IReadOnlyList FinishArgListOrGenExpr() { + Node a = null; ParserSink?.StartParameters(GetSourceSpan()); @@ -2135,20 +2135,14 @@ private IReadOnlyList FinishArgListOrGenExpr() { if (MaybeEat(TokenKind.Assign)) { // Keyword argument a = FinishKeywordArgument(e); - - if (a == null) { // Error recovery - a = new Arg(e); - a.SetLoc(_globalParent, e.StartIndex, GetEnd()); - } } else if (PeekToken(Tokens.KeywordForToken)) { // Generator expression - a = new Arg(ParseGeneratorExpression(e)); + a = ParseGeneratorExpression(e); Eat(TokenKind.RightParenthesis); a.SetLoc(_globalParent, start, GetEnd()); ParserSink?.EndParameters(GetSourceSpan()); - return new Arg[1] { a }; // Generator expression is the argument + return new Node[1] { a }; // Generator expression is the argument } else { - a = new Arg(e); - a.SetLoc(_globalParent, e.StartIndex, e.EndIndex); + a = e; } // Was this all? @@ -2159,31 +2153,31 @@ private IReadOnlyList FinishArgListOrGenExpr() { Eat(TokenKind.RightParenthesis); a.SetLoc(_globalParent, start, GetEnd()); ParserSink?.EndParameters(GetSourceSpan()); - return new Arg[1] { a }; + return new Node[1] { a }; } } return FinishArgumentList(a); } - private Arg FinishKeywordArgument(Expression t) { + private Keyword FinishKeywordArgument(Expression t) { if (t is NameExpression n) { Expression val = ParseTest(); - Arg arg = new Arg(n.Name, val); + var arg = new Keyword(n.Name, val); arg.SetLoc(_globalParent, n.StartIndex, val.EndIndex); return arg; } else { ReportSyntaxError(IronPython.Resources.ExpectedName); - Arg arg = new Arg(null, t); + var arg = new Keyword(null, t); arg.SetLoc(_globalParent, t.StartIndex, t.EndIndex); return arg; } } - private void CheckUniqueArgument(List names, Arg arg) { + private void CheckUniqueArgument(List names, Keyword arg) { if (arg != null && arg.Name != null) { for (int i = 0; i < names.Count; i++) { - if (names[i].Name == arg.Name) { + if (names[i] is Keyword k && k.Name == arg.Name) { ReportSyntaxError(IronPython.Resources.DuplicateKeywordArg); } } @@ -2192,9 +2186,9 @@ private void CheckUniqueArgument(List names, Arg arg) { //arglist: (argument ',')* (argument [',']| '*' expression [',' '**' expression] | '**' expression) //argument: [expression '='] expression # Really [keyword '='] expression - private IReadOnlyList FinishArgumentList(Arg first) { + private IReadOnlyList FinishArgumentList(Node first) { const TokenKind terminator = TokenKind.RightParenthesis; - List l = new List(); + List l = new List(); if (first != null) { l.Add(first); @@ -2206,20 +2200,23 @@ private IReadOnlyList FinishArgumentList(Arg first) { break; } var start = GetStart(); - Arg a; - if (MaybeEat(TokenKind.Multiply)) { - Expression t = ParseTest(); - a = new Arg("*", t); + Node a; + if (PeekToken(TokenKind.Multiply)) { + var s = GetStart(); + NextToken(); + a = new StarredExpression(ParseTest()); + a.SetLoc(_globalParent, s, GetEnd()); } else if (MaybeEat(TokenKind.Power)) { Expression t = ParseTest(); - a = new Arg("**", t); + a = new Keyword(null, t); } else { Expression e = ParseTest(); if (MaybeEat(TokenKind.Assign)) { - a = FinishKeywordArgument(e); - CheckUniqueArgument(l, a); + var k = FinishKeywordArgument(e); + a = k; + CheckUniqueArgument(l, k); } else { - a = new Arg(e); + a = e; } } a.SetLoc(_globalParent, start, GetEnd()); @@ -2887,9 +2884,9 @@ private void PushFunction(FunctionDefinition function) { _functions.Push(function); } - private CallExpression FinishCallExpr(Expression target, IEnumerable args) { - List posargs = null; - List kwargs = null; + private CallExpression FinishCallExpr(Expression target, IEnumerable args) { + List posargs = null; + List kwargs = null; if (args is not null) { SplitAndValidateArguments(args, out posargs, out kwargs); @@ -2898,35 +2895,38 @@ private CallExpression FinishCallExpr(Expression target, IEnumerable args) return new CallExpression(target, posargs, kwargs); } - private void SplitAndValidateArguments(IEnumerable args, out List posargs, out List kwargs) { + private void SplitAndValidateArguments(IEnumerable args, out List posargs, out List kwargs) { bool hasKeyword = false; bool hasKeywordUnpacking = false; - posargs = kwargs = null; + posargs = null; + kwargs = null; - foreach (Arg arg in args) { - if (arg.Name == null) { - 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"); + foreach (Node arg in args) { + if (arg is Keyword keyword) { + if (keyword.Name is null) { + hasKeywordUnpacking = true; + } else { + hasKeyword = true; } - posargs ??= new List(); - posargs.Add(arg); - } else if (arg.Name == "*") { + kwargs ??= new List(); + kwargs.Add(keyword); + } else if (arg is StarredExpression starredExpression) { if (hasKeywordUnpacking) { ReportSyntaxError(arg.StartIndex, arg.EndIndex, "iterable argument unpacking follows keyword argument unpacking"); } - posargs ??= new List(); - posargs.Add(arg); - } else if (arg.Name == "**") { - hasKeywordUnpacking = true; - kwargs ??= new List(); - kwargs.Add(arg); + posargs ??= new List(); + posargs.Add(starredExpression); } else { - hasKeyword = true; - kwargs ??= new List(); - kwargs.Add(arg); + var expr = (Expression)arg; + 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"); + } + posargs ??= new List(); + posargs.Add(expr); + } } } diff --git a/Src/IronPython/Modules/_ast.cs b/Src/IronPython/Modules/_ast.cs index 2ddf95151..cd63d6d62 100755 --- a/Src/IronPython/Modules/_ast.cs +++ b/Src/IronPython/Modules/_ast.cs @@ -650,7 +650,7 @@ public keyword(string arg, expr value) this.value = value; } - internal keyword(IronPython.Compiler.Ast.Arg arg) + internal keyword(IronPython.Compiler.Ast.Keyword arg) : this() { this.arg = arg.Name; value = Convert(arg.Expression); @@ -1037,34 +1037,25 @@ public Call(expr func, PythonList args, PythonList keywords, [Optional] int? lin internal Call(CallExpression call) : this() { args = new PythonList(call.Args.Count); - keywords = new PythonList(); - func = Convert(call.Target); - foreach (Arg arg in call.Args) { - if (arg.Name == "*") { - var loc = arg.Start; - args.Add(new Starred(Convert(arg.Expression), Load.Instance, loc.Line, loc.Column)); - } else { - args.Add(Convert(arg.Expression)); - } + foreach (var arg in call.Args) { + args.Add(Convert(arg)); } - foreach (Arg arg in call.Kwargs) { - keywords.Add(new keyword(arg.Name == "**" ? null : arg.Name, Convert(arg.Expression))); + keywords = new PythonList(call.Kwargs.Count); + foreach (var arg in call.Kwargs) { + keywords.Add(new keyword(arg)); } + func = Convert(call.Target); } internal override AstExpression Revert() { AstExpression target = expr.Revert(func); - List newArgs = new List(); - List newKwargs = new List(); + List newArgs = new(); + List newKwargs = new(); foreach (expr ex in args) { - if (ex is Starred starred) { - newArgs.Add(new Arg("*", starred.value?.Revert())); - } else { - newArgs.Add(new Arg(ex?.Revert())); - } + newArgs.Add(ex.Revert()); } foreach (keyword kw in keywords) { - newKwargs.Add(new Arg(kw.arg is null ? "**" : kw.arg, kw.value?.Revert())); + newKwargs.Add(new Keyword(kw.arg, kw.value.Revert())); } return new CallExpression(target, newArgs, newKwargs); @@ -1098,17 +1089,12 @@ internal ClassDef(ClassDefinition def) : this() { name = def.Name; bases = new PythonList(def.Bases.Count); - foreach (Arg arg in def.Bases) { - if (arg.Name == "*") { - var loc = arg.Start; - bases.Add(new Starred(Convert(arg.Expression), Load.Instance, loc.Line, loc.Column)); - } else { - bases.Add(Convert(arg.Expression)); - } + foreach (var arg in def.Bases) { + bases.Add(Convert(arg)); } keywords = new PythonList(def.Keywords.Count); - foreach (Arg arg in def.Keywords) { - keywords.Add(new keyword(arg.Name == "**" ? null : arg.Name, Convert(arg.Expression))); + foreach (var arg in def.Keywords) { + keywords.Add(new keyword(arg)); } body = ConvertStatements(def.Body); if (def.Decorators != null) { @@ -1121,17 +1107,13 @@ internal ClassDef(ClassDefinition def) } internal override Statement Revert() { - List newBases = new List(); - List newKeywords = new List(); + List newBases = new(); + List newKeywords = new(); foreach (expr ex in bases) { - if (ex is Starred starred) { - newBases.Add(new Arg("*", starred.value?.Revert())); - } else { - newBases.Add(new Arg(ex?.Revert())); - } + newBases.Add(ex.Revert()); } foreach (keyword kw in keywords) { - newKeywords.Add(new Arg(kw.arg is null ? "**" : kw.arg, kw.value?.Revert())); + newKeywords.Add(new Keyword(kw.arg, kw.value.Revert())); } ClassDefinition cd = new ClassDefinition(name, newBases, newKeywords, RevertStmts(body)); diff --git a/Src/Scripts/generate_calls.py b/Src/Scripts/generate_calls.py index 8e2b4292e..3864b17d3 100644 --- a/Src/Scripts/generate_calls.py +++ b/Src/Scripts/generate_calls.py @@ -488,7 +488,7 @@ def gen_call_expression_instruction_switch(cw): cw.write('compiler.Compile(Parent.LocalContext);') cw.write('compiler.Compile(Target);') for j in range(i): - cw.write('compiler.Compile(args[%d].Expression);' % j) + cw.write('compiler.Compile(args[%d]);' % j) cw.write('compiler.Instructions.Emit(new Invoke%dInstruction(Parent.PyContext));' % i) cw.write('return;') cw.dedent() From c6ed75158e99cfc0984778908d31083032685c86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Lozier?= Date: Sat, 17 Apr 2021 16:36:06 -0400 Subject: [PATCH 2/4] Get rid of IList overloads --- Src/IronPython/Compiler/Ast/Expression.cs | 15 --------------- Src/IronPython/Compiler/Ast/IndexExpression.cs | 2 +- Src/IronPython/Compiler/Ast/ListExpression.cs | 4 ++-- Src/IronPython/Compiler/Ast/Node.cs | 8 -------- Src/IronPython/Compiler/Ast/SequenceExpression.cs | 6 ++++-- Src/IronPython/Compiler/Ast/SetExpression.cs | 4 ++-- Src/IronPython/Compiler/Ast/StarredExpression.cs | 2 +- Src/IronPython/Compiler/Ast/TupleExpression.cs | 6 +++--- 8 files changed, 13 insertions(+), 34 deletions(-) diff --git a/Src/IronPython/Compiler/Ast/Expression.cs b/Src/IronPython/Compiler/Ast/Expression.cs index 6c11a6006..be558130e 100644 --- a/Src/IronPython/Compiler/Ast/Expression.cs +++ b/Src/IronPython/Compiler/Ast/Expression.cs @@ -21,21 +21,6 @@ namespace IronPython.Compiler.Ast { public abstract class Expression : Node { internal static readonly Expression[] EmptyArray = Array.Empty(); - protected internal static MSAst.BlockExpression UnpackSequenceHelper(IList items, MethodInfo makeEmpty, MethodInfo append, MethodInfo extend) { - var expressions = new ReadOnlyCollectionBuilder(items.Count + 2); - var varExpr = Expression.Variable(typeof(T), "$coll"); - expressions.Add(Expression.Assign(varExpr, Expression.Call(makeEmpty))); - foreach (var item in items) { - if (item is StarredExpression starredExpression) { - expressions.Add(Expression.Call(extend, varExpr, AstUtils.Convert(starredExpression.Value, typeof(object)))); - } else { - expressions.Add(Expression.Call(append, varExpr, AstUtils.Convert(item, typeof(object)))); - } - } - expressions.Add(varExpr); - return Expression.Block(typeof(T), new MSAst.ParameterExpression[] { varExpr }, expressions); - } - protected internal static MSAst.BlockExpression UnpackSequenceHelper(ReadOnlySpan items, MethodInfo makeEmpty, MethodInfo append, MethodInfo extend) { var expressions = new ReadOnlyCollectionBuilder(items.Length + 2); var varExpr = Expression.Variable(typeof(T), "$coll"); diff --git a/Src/IronPython/Compiler/Ast/IndexExpression.cs b/Src/IronPython/Compiler/Ast/IndexExpression.cs index ce87364c0..fdc830ea7 100644 --- a/Src/IronPython/Compiler/Ast/IndexExpression.cs +++ b/Src/IronPython/Compiler/Ast/IndexExpression.cs @@ -32,7 +32,7 @@ public override MSAst.Expression Reduce() { private MSAst.Expression[] GetActionArgumentsForGetOrDelete() { if (Index is TupleExpression te && te.IsExpandable) { - return ArrayUtils.Insert(Target, te.Items); + return ArrayUtils.Insert(Target, te.UnsafeItems); } if (Index is SliceExpression se) { diff --git a/Src/IronPython/Compiler/Ast/ListExpression.cs b/Src/IronPython/Compiler/Ast/ListExpression.cs index dabf39f90..d275c198c 100644 --- a/Src/IronPython/Compiler/Ast/ListExpression.cs +++ b/Src/IronPython/Compiler/Ast/ListExpression.cs @@ -20,14 +20,14 @@ public override MSAst.Expression Reduce() { } if (HasStarredExpression) { - return UnpackSequenceHelper(Items, AstMethods.MakeEmptyList, AstMethods.ListAppend, AstMethods.ListExtend); + return UnpackSequenceHelper(_items, AstMethods.MakeEmptyList, AstMethods.ListAppend, AstMethods.ListExtend); } return Call( AstMethods.MakeListNoCopy, // method NewArrayInit( // parameters typeof(object), - ToObjectArray(Items) + ToObjectArray(_items) ) ); } diff --git a/Src/IronPython/Compiler/Ast/Node.cs b/Src/IronPython/Compiler/Ast/Node.cs index 8c5ce6752..46a1135c1 100644 --- a/Src/IronPython/Compiler/Ast/Node.cs +++ b/Src/IronPython/Compiler/Ast/Node.cs @@ -164,14 +164,6 @@ internal virtual string GetDocumentation(Statement/*!*/ stmt) { #region Transformation Helpers - internal static MSAst.Expression[] ToObjectArray(IList expressions) { - MSAst.Expression[] to = new MSAst.Expression[expressions.Count]; - for (int i = 0; i < expressions.Count; i++) { - to[i] = AstUtils.Convert(expressions[i], typeof(object)); - } - return to; - } - internal static MSAst.Expression[] ToObjectArray(ReadOnlySpan expressions) { MSAst.Expression[] to = new MSAst.Expression[expressions.Length]; for (int i = 0; i < expressions.Length; i++) { diff --git a/Src/IronPython/Compiler/Ast/SequenceExpression.cs b/Src/IronPython/Compiler/Ast/SequenceExpression.cs index 0310d2673..1acdc4013 100644 --- a/Src/IronPython/Compiler/Ast/SequenceExpression.cs +++ b/Src/IronPython/Compiler/Ast/SequenceExpression.cs @@ -18,13 +18,15 @@ namespace IronPython.Compiler.Ast { public abstract class SequenceExpression : Expression { - private readonly Expression[] _items; + protected readonly Expression[] _items; protected SequenceExpression(Expression[] items) { _items = items; } - public IList Items => _items; + public IReadOnlyList Items => _items; + + internal Expression[] UnsafeItems => _items; // TODO: get rid of this protected bool HasStarredExpression => Items.OfType().Any(); diff --git a/Src/IronPython/Compiler/Ast/SetExpression.cs b/Src/IronPython/Compiler/Ast/SetExpression.cs index 4d9a337ce..80506faf9 100644 --- a/Src/IronPython/Compiler/Ast/SetExpression.cs +++ b/Src/IronPython/Compiler/Ast/SetExpression.cs @@ -24,13 +24,13 @@ public SetExpression(params Expression[] items) { _items = items; } - public IList Items => _items; + public IReadOnlyList Items => _items; protected bool HasStarredExpression => Items.OfType().Any(); public override MSAst.Expression Reduce() { if (HasStarredExpression) { - return UnpackSequenceHelper(Items, AstMethods.MakeEmptySet, AstMethods.SetAdd, AstMethods.SetUpdate); + return UnpackSequenceHelper(_items, AstMethods.MakeEmptySet, AstMethods.SetAdd, AstMethods.SetUpdate); } return Expression.Call( diff --git a/Src/IronPython/Compiler/Ast/StarredExpression.cs b/Src/IronPython/Compiler/Ast/StarredExpression.cs index ab6c23a3b..e8f5f4ef8 100644 --- a/Src/IronPython/Compiler/Ast/StarredExpression.cs +++ b/Src/IronPython/Compiler/Ast/StarredExpression.cs @@ -119,7 +119,7 @@ private void WalkAssignmentTarget(Expression expr) { } } - private bool WalkItems(IList items) { + private bool WalkItems(IReadOnlyList items) { foreach (var item in items) { if (item is StarredExpression starred) { starred.Value.Walk(this); diff --git a/Src/IronPython/Compiler/Ast/TupleExpression.cs b/Src/IronPython/Compiler/Ast/TupleExpression.cs index 92209e73f..eeda8c3da 100644 --- a/Src/IronPython/Compiler/Ast/TupleExpression.cs +++ b/Src/IronPython/Compiler/Ast/TupleExpression.cs @@ -35,7 +35,7 @@ public override MSAst.Expression Reduce() { if (IsExpandable) { return Expression.NewArrayInit( typeof(object), - ToObjectArray(Items) + ToObjectArray(_items) ); } @@ -47,14 +47,14 @@ public override MSAst.Expression Reduce() { } if (HasStarredExpression) { - return Expression.Call(AstMethods.ListToTuple, UnpackSequenceHelper(Items, AstMethods.MakeEmptyList, AstMethods.ListAppend, AstMethods.ListExtend)); + return Expression.Call(AstMethods.ListToTuple, UnpackSequenceHelper(_items, AstMethods.MakeEmptyList, AstMethods.ListAppend, AstMethods.ListExtend)); } return Expression.Call( AstMethods.MakeTuple, Expression.NewArrayInit( typeof(object), - ToObjectArray(Items) + ToObjectArray(_items) ) ); } From 79f7e0d390efd0c1edec000d98f13d77ccc31600 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Lozier?= Date: Sat, 17 Apr 2021 20:49:35 -0400 Subject: [PATCH 3/4] Fix starred argument --- Src/IronPython/Compiler/Ast/CallExpression.cs | 2 +- .../Compiler/Ast/ClassDefinition.cs | 2 +- .../Compiler/Ast/StarredExpression.cs | 26 ++++++++++++++++--- Src/IronPython/Compiler/Parser.cs | 16 +++++------- 4 files changed, 31 insertions(+), 15 deletions(-) diff --git a/Src/IronPython/Compiler/Ast/CallExpression.cs b/Src/IronPython/Compiler/Ast/CallExpression.cs index c0e2f901d..0de31757f 100644 --- a/Src/IronPython/Compiler/Ast/CallExpression.cs +++ b/Src/IronPython/Compiler/Ast/CallExpression.cs @@ -160,7 +160,7 @@ static MSAst.Expression UnpackListHelper(ReadOnlySpan args) { if (args.Length == 1) return ((StarredExpression)args[0]).Value; - return UnpackSequenceHelper(args, AstMethods.MakeEmptyList, AstMethods.ListAppend, AstMethods.ListExtend); + return UnpackSequenceHelper(args, AstMethods.MakeEmptyList, AstMethods.ListAppend, AstMethods.ListExtend); } // Compare to: CallExpression.Reduce.__UnpackKeywordsHelper diff --git a/Src/IronPython/Compiler/Ast/ClassDefinition.cs b/Src/IronPython/Compiler/Ast/ClassDefinition.cs index c2f1cf3ce..8fc04bc37 100644 --- a/Src/IronPython/Compiler/Ast/ClassDefinition.cs +++ b/Src/IronPython/Compiler/Ast/ClassDefinition.cs @@ -209,7 +209,7 @@ static MSAst.Expression UnpackBasesHelper(ReadOnlySpan bases) { foreach (var arg in bases) { if (arg is StarredExpression) { return Expression.Call(AstMethods.ListToTuple, - Expression.UnpackSequenceHelper(bases, AstMethods.MakeEmptyList, AstMethods.ListAppend, AstMethods.ListExtend) + Expression.UnpackSequenceHelper(bases, AstMethods.MakeEmptyList, AstMethods.ListAppend, AstMethods.ListExtend) ); } } diff --git a/Src/IronPython/Compiler/Ast/StarredExpression.cs b/Src/IronPython/Compiler/Ast/StarredExpression.cs index e8f5f4ef8..de08431ee 100644 --- a/Src/IronPython/Compiler/Ast/StarredExpression.cs +++ b/Src/IronPython/Compiler/Ast/StarredExpression.cs @@ -69,11 +69,11 @@ public override bool Walk(AssignmentStatement node) { public override bool Walk(CallExpression node) { node.Target?.Walk(this); - foreach (var item in node.Args) { - if (item is StarredExpression starred) { + foreach (var arg in node.Args) { + if (arg is StarredExpression starred) { starred.Value.Walk(this); } else { - item.Walk(this); + arg.Walk(this); } } foreach (var arg in node.Kwargs) { @@ -82,6 +82,26 @@ public override bool Walk(CallExpression node) { return false; } + public override bool Walk(ClassDefinition node) { + if (node.Decorators != null) { + foreach (Expression decorator in node.Decorators) { + decorator.Walk(this); + } + } + foreach (var b in node.Bases) { + if (b is StarredExpression starred) { + starred.Value.Walk(this); + } else { + b.Walk(this); + } + } + foreach (var b in node.Keywords) { + b.Walk(this); + } + node.Body.Walk(this); + return false; + } + public override bool Walk(ForStatement node) { WalkAssignmentTarget(node.Left); node.List?.Walk(this); diff --git a/Src/IronPython/Compiler/Parser.cs b/Src/IronPython/Compiler/Parser.cs index 7f023e4eb..5444ac33e 100644 --- a/Src/IronPython/Compiler/Parser.cs +++ b/Src/IronPython/Compiler/Parser.cs @@ -2175,7 +2175,7 @@ private Keyword FinishKeywordArgument(Expression t) { } private void CheckUniqueArgument(List names, Keyword arg) { - if (arg != null && arg.Name != null) { + if (arg.Name != null) { for (int i = 0; i < names.Count; i++) { if (names[i] is Keyword k && k.Name == arg.Name) { ReportSyntaxError(IronPython.Resources.DuplicateKeywordArg); @@ -2201,20 +2201,16 @@ private IReadOnlyList FinishArgumentList(Node first) { } var start = GetStart(); Node a; - if (PeekToken(TokenKind.Multiply)) { - var s = GetStart(); - NextToken(); + if (MaybeEat(TokenKind.Multiply)) { a = new StarredExpression(ParseTest()); - a.SetLoc(_globalParent, s, GetEnd()); + a.SetLoc(_globalParent, start, GetEnd()); } else if (MaybeEat(TokenKind.Power)) { - Expression t = ParseTest(); - a = new Keyword(null, t); + a = new Keyword(null, ParseTest()); } else { Expression e = ParseTest(); if (MaybeEat(TokenKind.Assign)) { - var k = FinishKeywordArgument(e); - a = k; - CheckUniqueArgument(l, k); + a = FinishKeywordArgument(e); + CheckUniqueArgument(l, (Keyword)a); } else { a = e; } From 3a5be2ea4bf4e2bafce160dbe09da06d0f307661 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Lozier?= Date: Sun, 18 Apr 2021 10:31:18 -0400 Subject: [PATCH 4/4] Fix test_ast --- Src/IronPython/Compiler/Parser.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Src/IronPython/Compiler/Parser.cs b/Src/IronPython/Compiler/Parser.cs index 5444ac33e..80fe4322b 100644 --- a/Src/IronPython/Compiler/Parser.cs +++ b/Src/IronPython/Compiler/Parser.cs @@ -2151,7 +2151,6 @@ private IReadOnlyList FinishArgListOrGenExpr() { ParserSink?.NextParameter(GetSourceSpan()); } else { Eat(TokenKind.RightParenthesis); - a.SetLoc(_globalParent, start, GetEnd()); ParserSink?.EndParameters(GetSourceSpan()); return new Node[1] { a }; } @@ -2199,15 +2198,17 @@ private IReadOnlyList FinishArgumentList(Node first) { if (MaybeEat(terminator)) { break; } - var start = GetStart(); Node a; if (MaybeEat(TokenKind.Multiply)) { + var start = GetStart(); a = new StarredExpression(ParseTest()); a.SetLoc(_globalParent, start, GetEnd()); } else if (MaybeEat(TokenKind.Power)) { - a = new Keyword(null, ParseTest()); + var e = ParseTest(); + a = new Keyword(null, e); + a.SetLoc(_globalParent, e.StartIndex, e.EndIndex); } else { - Expression e = ParseTest(); + var e = ParseTest(); if (MaybeEat(TokenKind.Assign)) { a = FinishKeywordArgument(e); CheckUniqueArgument(l, (Keyword)a); @@ -2215,7 +2216,6 @@ private IReadOnlyList FinishArgumentList(Node first) { a = e; } } - a.SetLoc(_globalParent, start, GetEnd()); l.Add(a); if (MaybeEat(TokenKind.Comma)) { ParserSink?.NextParameter(GetSourceSpan());