Skip to content

Remove IronPython.Compiler.Ast.Arg #1175

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
slozier merged 4 commits into from
Apr 18, 2021
Merged

Remove IronPython.Compiler.Ast.Arg #1175

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
54b6611
Remove IronPython.Compiler.Ast.Arg
slozier Apr 17, 2021
c6ed751
Get rid of IList overloads
slozier Apr 17, 2021
79f7e0d
Fix starred argument
slozier Apr 18, 2021
3a5be2e
Fix test_ast
slozier Apr 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
188 changes: 96 additions & 92 deletions Src/IronPython/Compiler/Ast/CallExpression.cs
View file
Open in desktop

Large diffs are not rendered by default.

59 changes: 26 additions & 33 deletions Src/IronPython/Compiler/Ast/ClassDefinition.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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...

Expand All @@ -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<Arg>? bases, IReadOnlyList<Arg>? keywords, Statement? body = null) {
public ClassDefinition(string name, IReadOnlyList<Expression>? bases, IReadOnlyList<Keyword>? keywords, Statement? body = null) {
_name = name;
_bases = bases?.ToArray() ?? Array.Empty<Arg>();
_keywords = keywords?.ToArray() ?? Array.Empty<Arg>();
_bases = bases?.ToArray() ?? Array.Empty<Expression>();
Comment thread
slozier marked this conversation as resolved.
_keywords = keywords?.ToArray() ?? Array.Empty<Keyword>();
Body = body ?? EmptyStatement.PreCompiledInstance;
}

Expand All @@ -50,9 +50,9 @@ public ClassDefinition(string name, IReadOnlyList<Arg>? bases, IReadOnlyList<Arg

public override string Name => _name;

public IReadOnlyList<Arg> Bases => _bases;
public IReadOnlyList<Expression> Bases => _bases;

public IReadOnlyList<Arg> Keywords => _keywords;
public IReadOnlyList<Keyword> Keywords => _keywords;

public Statement Body { get; set; }

Expand Down Expand Up @@ -194,42 +194,35 @@ 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)
)
);

// Compare to: CallExpression.Reduce.__UnpackListHelper
static MSAst.Expression UnpackBasesHelper(IReadOnlyList<Arg> bases) {
if (bases.Count == 0) {
static MSAst.Expression UnpackBasesHelper(ReadOnlySpan<Expression> 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<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))));
}
}
foreach (var arg in bases) {
if (arg is StarredExpression) {
return Expression.Call(AstMethods.ListToTuple,
Expression.UnpackSequenceHelper<PythonList>(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<Arg> kwargs) {
static MSAst.Expression UnpackKeywordsHelper(MSAst.Expression context, IReadOnlyList<Keyword> kwargs) {
Comment thread
slozier marked this conversation as resolved.
if (kwargs.Count == 0) {
return AstUtils.Constant(null, typeof(PythonDictionary));
}
Expand All @@ -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))));
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions Src/IronPython/Compiler/Ast/Expression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ namespace IronPython.Compiler.Ast {
public abstract class Expression : Node {
internal static readonly Expression[] EmptyArray = Array.Empty<Expression>();

protected internal static MSAst.BlockExpression UnpackSequenceHelper<T>(IList<Expression> items, MethodInfo makeEmpty, MethodInfo append, MethodInfo extend) {
var expressions = new ReadOnlyCollectionBuilder<MSAst.Expression>(items.Count + 2);
protected internal static MSAst.BlockExpression UnpackSequenceHelper<T>(ReadOnlySpan<Expression> items, MethodInfo makeEmpty, MethodInfo append, MethodInfo extend) {
var expressions = new ReadOnlyCollectionBuilder<MSAst.Expression>(items.Length + 2);
var varExpr = Expression.Variable(typeof(T), "$coll");
expressions.Add(Expression.Assign(varExpr, Expression.Call(makeEmpty)));
foreach (var item in items) {
Expand Down
4 changes: 2 additions & 2 deletions Src/IronPython/Compiler/Ast/FlowChecker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion Src/IronPython/Compiler/Ast/IndexExpression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
20 changes: 3 additions & 17 deletions Src/IronPython/Compiler/Ast/Arg.cs → Src/IronPython/Compiler/Ast/Keyword.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.

Expand All @@ -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;
Comment thread
slozier marked this conversation as resolved.
}
Expand Down
4 changes: 2 additions & 2 deletions Src/IronPython/Compiler/Ast/ListExpression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@ public override MSAst.Expression Reduce() {
}

if (HasStarredExpression) {
return UnpackSequenceHelper<PythonList>(Items, AstMethods.MakeEmptyList, AstMethods.ListAppend, AstMethods.ListExtend);
return UnpackSequenceHelper<PythonList>(_items, AstMethods.MakeEmptyList, AstMethods.ListAppend, AstMethods.ListExtend);
}

return Call(
AstMethods.MakeListNoCopy, // method
NewArrayInit( // parameters
typeof(object),
ToObjectArray(Items)
ToObjectArray(_items)
)
);
}
Expand Down
6 changes: 3 additions & 3 deletions Src/IronPython/Compiler/Ast/Node.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,9 @@ internal virtual string GetDocumentation(Statement/*!*/ stmt) {

#region Transformation Helpers

internal static MSAst.Expression[] ToObjectArray(IList<Expression> expressions) {
MSAst.Expression[] to = new MSAst.Expression[expressions.Count];
for (int i = 0; i < expressions.Count; i++) {
internal static MSAst.Expression[] ToObjectArray(ReadOnlySpan<Expression> 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;
Expand Down
17 changes: 8 additions & 9 deletions Src/IronPython/Compiler/Ast/PythonNameBinder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
17 changes: 8 additions & 9 deletions Src/IronPython/Compiler/Ast/PythonWalker.Generated.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) { }
Expand Down Expand Up @@ -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) { }
Expand All @@ -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) { }
Expand Down Expand Up @@ -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) { }
Expand All @@ -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) { }
Expand Down
6 changes: 4 additions & 2 deletions Src/IronPython/Compiler/Ast/SequenceExpression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<Expression> Items => _items;
public IReadOnlyList<Expression> Items => _items;

internal Expression[] UnsafeItems => _items; // TODO: get rid of this
Copy link
Copy Markdown
Member

@BCSharp BCSharp Apr 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good luck with this! 😄 IReadOnlyCollection should have had CopyTo, just like ICollection has. I suppose ArrayUtils.Insert<T>(T item, IReadOnlyList<T> list) can also do if (list is T[] array) ... to maintain performance .

Copy link
Copy Markdown
Contributor Author

@slozier slozier Apr 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately ArrayUtils.Insert is in the DLR and I didn't want to rev it just for this. I guess as long as we don't start using UnsafeItems all over the place (I don't see why we would) this is fine.


protected bool HasStarredExpression => Items.OfType<StarredExpression>().Any();

Expand Down
4 changes: 2 additions & 2 deletions Src/IronPython/Compiler/Ast/SetExpression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@ public SetExpression(params Expression[] items) {
_items = items;
}

public IList<Expression> Items => _items;
public IReadOnlyList<Expression> Items => _items;

protected bool HasStarredExpression => Items.OfType<StarredExpression>().Any();

public override MSAst.Expression Reduce() {
if (HasStarredExpression) {
return UnpackSequenceHelper<SetCollection>(Items, AstMethods.MakeEmptySet, AstMethods.SetAdd, AstMethods.SetUpdate);
return UnpackSequenceHelper<SetCollection>(_items, AstMethods.MakeEmptySet, AstMethods.SetAdd, AstMethods.SetUpdate);
}

return Expression.Call(
Expand Down
Loading