From 9b8b5c7d3be375c599552757fedfcab2d55ca0f5 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Thu, 28 Dec 2017 17:03:01 -0800 Subject: [PATCH 1/4] ListLiteral: Parser and Engine changes --- .../CommandCompletion/CompletionCompleters.cs | 7 +- .../engine/interpreter/LightCompiler.cs | 17 +++- .../engine/parser/AstVisitor.cs | 24 +++++- .../engine/parser/Compiler.cs | 18 +++-- .../engine/parser/ConstantValues.cs | 45 +++++++---- .../engine/parser/Parser.cs | 67 ++++++++++++++- .../engine/parser/PreOrderVisitor.cs | 13 ++- .../engine/parser/SafeValues.cs | 35 +++++++- .../engine/parser/TypeInferenceVisitor.cs | 7 +- .../engine/parser/VariableAnalysis.cs | 19 ++++- .../engine/parser/ast.cs | 81 +++++++++++++++++++ .../resources/ParserStrings.resx | 3 + 12 files changed, 301 insertions(+), 35 deletions(-) diff --git a/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs b/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs index 39a9860df95..ca94280b967 100644 --- a/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs +++ b/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs @@ -6702,7 +6702,7 @@ public int Compare(PSObject x, PSObject y) /// use is for intellisense where you don't want to run arbitrary code, but you do want to know the values /// of various expressions so you can get the members. /// - internal class SafeExprEvaluator : ICustomAstVisitor2 + internal class SafeExprEvaluator : ICustomAstVisitor3 { internal static bool TrySafeEval(ExpressionAst ast, ExecutionContext executionContext, out object value) { @@ -6862,6 +6862,11 @@ public object VisitArrayLiteral(ArrayLiteralAst arrayLiteralAst) return arrayLiteralAst.Elements.All(e => (bool)e.Accept(this)); } + public object VisitListLiteral(ListLiteralAst listExpressionAst) + { + return listExpressionAst.Elements.All(e => (bool)e.Accept(this)); + } + public object VisitHashtable(HashtableAst hashtableAst) { foreach (var keyValuePair in hashtableAst.KeyValuePairs) diff --git a/src/System.Management.Automation/engine/interpreter/LightCompiler.cs b/src/System.Management.Automation/engine/interpreter/LightCompiler.cs index 09236a3ec5d..12450c12945 100644 --- a/src/System.Management.Automation/engine/interpreter/LightCompiler.cs +++ b/src/System.Management.Automation/engine/interpreter/LightCompiler.cs @@ -1803,7 +1803,22 @@ private void CompileInvocationExpression(Expression expr) [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Usage", "CA1801:ReviewUnusedParameters", MessageId = "expr")] private void CompileListInitExpression(Expression expr) { - throw new System.NotImplementedException(); + var node = (ListInitExpression) expr; + CompileNewExpression(node.NewExpression); + if (node.Initializers.Count > 0) + { + LocalDefinition local = _locals.DefineLocal(Expression.Parameter(node.Type), _instructions.Count); + _instructions.EmitAssignLocal(local.Index); + + foreach (ElementInit eInit in node.Initializers) + { + foreach (Expression arg in eInit.Arguments) { Compile(arg); } + _instructions.EmitCall(eInit.AddMethod); + _instructions.EmitLoadLocal(local.Index); + } + + _locals.UndefineLocal(local, _instructions.Count); + } } [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Usage", "CA1801:ReviewUnusedParameters", MessageId = "expr")] diff --git a/src/System.Management.Automation/engine/parser/AstVisitor.cs b/src/System.Management.Automation/engine/parser/AstVisitor.cs index de4255de363..1a36e0bd743 100644 --- a/src/System.Management.Automation/engine/parser/AstVisitor.cs +++ b/src/System.Management.Automation/engine/parser/AstVisitor.cs @@ -175,8 +175,15 @@ public interface ICustomAstVisitor2 : ICustomAstVisitor object VisitDynamicKeywordStatement(DynamicKeywordStatementAst dynamicKeywordAst); } + /// + public interface ICustomAstVisitor3 : ICustomAstVisitor2 + { + /// + object VisitListLiteral(ListLiteralAst listLiteralAst); + } + #if DEBUG - class CheckAllParentsSet : AstVisitor2 + class CheckAllParentsSet : AstVisitor3 { internal CheckAllParentsSet(Ast root) { @@ -239,6 +246,7 @@ internal AstVisitAction CheckParent(Ast ast) public override AstVisitAction VisitInvokeMemberExpression(InvokeMemberExpressionAst ast) { return CheckParent(ast); } public override AstVisitAction VisitArrayExpression(ArrayExpressionAst ast) { return CheckParent(ast); } public override AstVisitAction VisitArrayLiteral(ArrayLiteralAst ast) { return CheckParent(ast); } + public override AstVisitAction VisitListLiteral(ListLiteralAst ast) { return CheckParent(ast); } public override AstVisitAction VisitHashtable(HashtableAst ast) { return CheckParent(ast); } public override AstVisitAction VisitScriptBlockExpression(ScriptBlockExpressionAst ast) { return CheckParent(ast); } public override AstVisitAction VisitParenExpression(ParenExpressionAst ast) { return CheckParent(ast); } @@ -258,7 +266,7 @@ internal AstVisitAction CheckParent(Ast ast) /// /// Check if contains type /// - class CheckTypeBuilder : AstVisitor2 + class CheckTypeBuilder : AstVisitor3 { public override AstVisitAction VisitTypeConstraint(TypeConstraintAst ast) { @@ -275,7 +283,7 @@ public override AstVisitAction VisitTypeConstraint(TypeConstraintAst ast) /// /// Searches an AST, using the evaluation function provided by either of the constructors /// - internal class AstSearcher : AstVisitor2 + internal class AstSearcher : AstVisitor3 { #region External interface @@ -408,6 +416,7 @@ protected AstVisitAction CheckScriptBlock(Ast ast) public override AstVisitAction VisitInvokeMemberExpression(InvokeMemberExpressionAst ast) { return Check(ast); } public override AstVisitAction VisitArrayExpression(ArrayExpressionAst ast) { return Check(ast); } public override AstVisitAction VisitArrayLiteral(ArrayLiteralAst ast) { return Check(ast); } + public override AstVisitAction VisitListLiteral(ListLiteralAst ast) { return Check(ast); } public override AstVisitAction VisitHashtable(HashtableAst ast) { return Check(ast); } public override AstVisitAction VisitScriptBlockExpression(ScriptBlockExpressionAst ast) { return CheckScriptBlock(ast); } public override AstVisitAction VisitParenExpression(ParenExpressionAst ast) { return Check(ast); } @@ -559,4 +568,13 @@ public abstract class DefaultCustomAstVisitor2 : DefaultCustomAstVisitor, ICusto /// public virtual object VisitFunctionMember(FunctionMemberAst functionMemberAst) { return null; } } + + /// + /// Default implementation of interface + /// + public abstract class DefaultCustomAstVisitor3 : DefaultCustomAstVisitor2, ICustomAstVisitor3 + { + /// + public virtual object VisitListLiteral(ListLiteralAst listExpressionAst) { return null; } + } } diff --git a/src/System.Management.Automation/engine/parser/Compiler.cs b/src/System.Management.Automation/engine/parser/Compiler.cs index 65b94436723..8adf79e02bc 100644 --- a/src/System.Management.Automation/engine/parser/Compiler.cs +++ b/src/System.Management.Automation/engine/parser/Compiler.cs @@ -37,6 +37,8 @@ internal static class CachedReflectionInfo typeof(List).GetConstructor(PSTypeExtensions.EmptyTypes); internal static readonly MethodInfo ObjectList_ToArray = typeof(List).GetMethod(nameof(List.ToArray), PSTypeExtensions.EmptyTypes); + internal static readonly MethodInfo ObjectList_Add = + typeof(List).GetMethod(nameof(List.Add)); internal static readonly MethodInfo ArrayOps_GetMDArrayValue = typeof(ArrayOps).GetMethod(nameof(ArrayOps.GetMDArrayValue), staticFlags); @@ -547,8 +549,8 @@ internal static class ExpressionCache internal static readonly Expression CurrentCultureIgnoreCaseComparer = Expression.Constant(StringComparer.CurrentCultureIgnoreCase, typeof(StringComparer)); internal static readonly Expression CatchAllType = Expression.Constant(typeof(ExceptionHandlingOps.CatchAll), typeof(Type)); internal static readonly Expression Empty = Expression.Empty(); - internal static Expression GetExecutionContextFromTLS = - Expression.Call(CachedReflectionInfo.LocalPipeline_GetExecutionContextFromTLS); + internal static readonly Expression GetExecutionContextFromTLS = Expression.Call(CachedReflectionInfo.LocalPipeline_GetExecutionContextFromTLS); + internal static readonly NewExpression NewEmptyObjectList = Expression.New(CachedReflectionInfo.ObjectList_ctor); internal static readonly Expression BoxedTrue = Expression.Field(null, typeof(Boxed).GetField("True", BindingFlags.Static | BindingFlags.NonPublic)); @@ -678,7 +680,7 @@ internal void PopTrapHandlers() } } - internal class Compiler : ICustomAstVisitor2 + internal class Compiler : ICustomAstVisitor3 { internal static readonly ParameterExpression _executionContextParameter; internal static readonly ParameterExpression _functionContext; @@ -1880,7 +1882,7 @@ private Expression CaptureAstResults( temps.Add(resultList); temps.Add(oldPipe); exprs.Add(Expression.Assign(oldPipe, s_getCurrentPipe)); - exprs.Add(Expression.Assign(resultList, Expression.New(CachedReflectionInfo.ObjectList_ctor))); + exprs.Add(Expression.Assign(resultList, ExpressionCache.NewEmptyObjectList)); exprs.Add(Expression.Assign(s_getCurrentPipe, Expression.New(CachedReflectionInfo.Pipe_ctor, resultList))); exprs.Add(Expression.Call(oldPipe, CachedReflectionInfo.Pipe_SetVariableListForTemporaryPipe, s_getCurrentPipe)); if (generateRedirectExprs != null) @@ -3185,7 +3187,7 @@ private Expression GetRedirectedExpression(CommandExpressionAst commandExpr, boo temps.Add(resultList); temps.Add(oldPipe); exprs.Add(Expression.Assign(oldPipe, s_getCurrentPipe)); - exprs.Add(Expression.Assign(resultList, Expression.New(CachedReflectionInfo.ObjectList_ctor))); + exprs.Add(Expression.Assign(resultList, ExpressionCache.NewEmptyObjectList)); exprs.Add(Expression.Assign(s_getCurrentPipe, Expression.New(CachedReflectionInfo.Pipe_ctor, resultList))); exprs.Add(Expression.Call(oldPipe, CachedReflectionInfo.Pipe_SetVariableListForTemporaryPipe, s_getCurrentPipe)); } @@ -5537,6 +5539,12 @@ public object VisitArrayLiteral(ArrayLiteralAst arrayLiteralAst) arrayLiteralAst.Elements.Select(elem => Compile(elem).Cast(typeof(object)))); } + public object VisitListLiteral(ListLiteralAst listLiteralAst) + { + return Expression.ListInit(ExpressionCache.NewEmptyObjectList, CachedReflectionInfo.ObjectList_Add, + listLiteralAst.Elements.Select(e => Compile(e).Cast(typeof(object)))); + } + private IEnumerable BuildHashtable(ReadOnlyCollection keyValuePairs, ParameterExpression temp, bool ordered) { yield return Expression.Assign(temp, diff --git a/src/System.Management.Automation/engine/parser/ConstantValues.cs b/src/System.Management.Automation/engine/parser/ConstantValues.cs index 8f6f0df41db..84ca573ef32 100644 --- a/src/System.Management.Automation/engine/parser/ConstantValues.cs +++ b/src/System.Management.Automation/engine/parser/ConstantValues.cs @@ -16,7 +16,7 @@ namespace System.Management.Automation.Language * There is a number of similarities between these two classes, and changes (fixes) in this code * may need to be reflected in that class and vice versa */ - internal class IsConstantValueVisitor : ICustomAstVisitor + internal class IsConstantValueVisitor : ICustomAstVisitor3 { public static bool IsConstant(Ast ast, out object constantValue, bool forAttribute = false, bool forRequires = false) { @@ -93,6 +93,17 @@ public static bool IsConstant(Ast ast, out object constantValue, bool forAttribu public object VisitAttributedExpression(AttributedExpressionAst attributedExpressionAst) { return false; } public object VisitBlockStatement(BlockStatementAst blockStatementAst) { return false; } public object VisitInvokeMemberExpression(InvokeMemberExpressionAst invokeMemberExpressionAst) { return false; } + public object VisitArrayExpression(ArrayExpressionAst arrayExpressionAst) { return false; } + public object VisitTypeDefinition(TypeDefinitionAst typeDefinitionAst) { return false; } + public object VisitPropertyMember(PropertyMemberAst propertyMemberAst) { return false; } + public object VisitFunctionMember(FunctionMemberAst functionMemberAst) { return false; } + public object VisitBaseCtorInvokeMemberExpression(BaseCtorInvokeMemberExpressionAst baseCtorInvokeMemberExpressionAst) { return false; } + public object VisitUsingStatement(UsingStatementAst usingStatement) { return false; } + public object VisitConfigurationDefinition(ConfigurationDefinitionAst configurationDefinitionAst) { return false; } + public object VisitDynamicKeywordStatement(DynamicKeywordStatementAst dynamicKeywordAst) { return false; } + + public object VisitConstantExpression(ConstantExpressionAst constantExpressionAst) { return true; } + public object VisitStringConstantExpression(StringConstantExpressionAst stringConstantExpressionAst) { return true; } public object VisitStatementBlock(StatementBlockAst statementBlockAst) { @@ -165,16 +176,6 @@ public object VisitConvertExpression(ConvertExpressionAst convertExpressionAst) return (bool)convertExpressionAst.Child.Accept(this); } - public object VisitConstantExpression(ConstantExpressionAst constantExpressionAst) - { - return true; - } - - public object VisitStringConstantExpression(StringConstantExpressionAst stringConstantExpressionAst) - { - return true; - } - public object VisitSubExpression(SubExpressionAst subExpressionAst) { return subExpressionAst.SubExpression.Accept(this); @@ -229,9 +230,11 @@ public object VisitMemberExpression(MemberExpressionAst memberExpressionAst) return (((FieldInfo)memberInfo[0]).Attributes & FieldAttributes.Literal) != 0; } - public object VisitArrayExpression(ArrayExpressionAst arrayExpressionAst) + public object VisitListLiteral(ListLiteralAst listLiteralAst) { - return false; + // A list literal is a constant when we're generating metadata, but when + // we're generating code, we need to create new arrays or we'd have an aliasing problem. + return (CheckingAttributeArgument || CheckingRequiresArgument) && listLiteralAst.Elements.All(e => (bool)e.Accept(this)); } public object VisitArrayLiteral(ArrayLiteralAst arrayLiteralAst) @@ -262,7 +265,7 @@ public object VisitParenExpression(ParenExpressionAst parenExpressionAst) } } - internal class ConstantValueVisitor : ICustomAstVisitor + internal class ConstantValueVisitor : ICustomAstVisitor3 { internal bool AttributeArgument { get; set; } internal bool RequiresArgument { get; set; } @@ -325,6 +328,14 @@ private static object CompileAndInvoke(Ast ast) public object VisitAttributedExpression(AttributedExpressionAst attributedExpressionAst) { return AutomationNull.Value; } public object VisitBlockStatement(BlockStatementAst blockStatementAst) { return AutomationNull.Value; } public object VisitInvokeMemberExpression(InvokeMemberExpressionAst invokeMemberExpressionAst) { return AutomationNull.Value; } + public object VisitArrayExpression(ArrayExpressionAst arrayExpressionAst) { return AutomationNull.Value; } + public object VisitTypeDefinition(TypeDefinitionAst typeDefinitionAst) { return AutomationNull.Value; } + public object VisitPropertyMember(PropertyMemberAst propertyMemberAst) { return AutomationNull.Value; } + public object VisitFunctionMember(FunctionMemberAst functionMemberAst) { return AutomationNull.Value; } + public object VisitBaseCtorInvokeMemberExpression(BaseCtorInvokeMemberExpressionAst baseCtorInvokeMemberExpressionAst) { return AutomationNull.Value; } + public object VisitUsingStatement(UsingStatementAst usingStatement) { return AutomationNull.Value; } + public object VisitConfigurationDefinition(ConfigurationDefinitionAst configurationDefinitionAst) { return AutomationNull.Value; } + public object VisitDynamicKeywordStatement(DynamicKeywordStatementAst dynamicKeywordAst) { return AutomationNull.Value; } public object VisitStatementBlock(StatementBlockAst statementBlockAst) { @@ -411,10 +422,10 @@ public object VisitMemberExpression(MemberExpressionAst memberExpressionAst) return ((FieldInfo)memberInfo[0]).GetValue(null); } - public object VisitArrayExpression(ArrayExpressionAst arrayExpressionAst) + public object VisitListLiteral(ListLiteralAst listLiteralAst) { - CheckIsConstant(arrayExpressionAst, "Caller to verify ast is constant"); - return arrayExpressionAst.SubExpression.Accept(this); + CheckIsConstant(listLiteralAst, "Caller to verify ast is constant"); + return listLiteralAst.Elements.Select(e => e.Accept(this)).ToList(); } public object VisitArrayLiteral(ArrayLiteralAst arrayLiteralAst) diff --git a/src/System.Management.Automation/engine/parser/Parser.cs b/src/System.Management.Automation/engine/parser/Parser.cs index b347cbd3e03..7440e98fe0e 100644 --- a/src/System.Management.Automation/engine/parser/Parser.cs +++ b/src/System.Management.Automation/engine/parser/Parser.cs @@ -1041,9 +1041,9 @@ private AttributeBaseAst AttributeRule() if (typeName == null) { // ErrorRecovery: Return null so we stop looking for attributes. + // Do not report error because it could be a list literal. Resync(lBracket); // TypeNameRule might have consumed some tokens - ReportIncompleteInput(After(lBracket), () => ParserStrings.MissingTypename); return null; } @@ -5987,6 +5987,68 @@ private ExpressionAst ErrorRecoveryParameterInExpression(ParameterToken paramTok new CommandParameterAst(paramToken.Extent, paramToken.ParameterName, null, paramToken.Extent)}); } + private ExpressionAst ListLiteralRule() + { + //G list-literal-expression + //G '[' new-lines:opt list-entries new-lines:opt ']' + //G list-entries + //G unary-expression + //G list-entries ',' new-lines:opt unary-expression + + var lBracket = NextLBracket(); + if (lBracket == null) { return null; } + + var listValues = new List(); + Token commaToken = null; + + do { + if (commaToken != null) { SkipToken(); } + V3SkipNewlines(); + + var expr = UnaryExpressionRule(); + if (expr == null) + { + // It's OK to have an empty list '[]' + if (commaToken == null) { break; } + + // ErrorRecovery: create an error expression for the ast and break. + ReportIncompleteInput(After(commaToken), () => ParserStrings.MissingExpressionAfterToken, commaToken.Text); + expr = new ErrorExpressionAst(commaToken.Extent); + listValues.Add(expr); + break; + } + listValues.Add(expr); + + commaToken = PeekToken(); + } while (commaToken.Kind == TokenKind.Comma); + + V3SkipNewlines(); + var rBracket = NextToken(); + + if (rBracket.Kind != TokenKind.RBracket) + { + // ErrorRecovery: Assume only the closing bracket is missing, continue as though it was present. + UngetToken(rBracket); + ReportIncompleteInput(rBracket.Extent, () => ParserStrings.MissingEndBracketInListExpression); + } + + IScriptExtent extent; + if (rBracket.Kind == TokenKind.RBracket) + { + extent = ExtentOf(lBracket, rBracket.Extent); + } + else if (listValues.Count > 0) + { + extent = ExtentOf(lBracket, listValues[listValues.Count-1].Extent); + } + else + { + extent = ExtentOf(lBracket, lBracket.Extent); + } + + return new ListLiteralAst(extent, listValues);; + } + private ExpressionAst ArrayLiteralRule() { //G array-literal-expression: @@ -6112,7 +6174,8 @@ private ExpressionAst UnaryExpressionRule() var attributes = AttributeListRule(true); if (attributes == null) { - return null; + var listExpr = ListLiteralRule(); + return listExpr; } AttributeBaseAst lastAttribute = attributes.Last(); diff --git a/src/System.Management.Automation/engine/parser/PreOrderVisitor.cs b/src/System.Management.Automation/engine/parser/PreOrderVisitor.cs index 62defcc9a58..23ab558b299 100644 --- a/src/System.Management.Automation/engine/parser/PreOrderVisitor.cs +++ b/src/System.Management.Automation/engine/parser/PreOrderVisitor.cs @@ -162,7 +162,7 @@ internal AstVisitAction CheckForPostAction(Ast ast, AstVisitAction action) } /// - /// AstVisitor for new Ast node types + /// AstVisitor for new Ast node types added in PSv5.0 /// public abstract class AstVisitor2 : AstVisitor { @@ -189,7 +189,16 @@ public abstract class AstVisitor2 : AstVisitor } /// - /// Implement this interface when you implement or when + /// AstVisitor for new Ast node types added in PSv6.1 + /// + public abstract class AstVisitor3 : AstVisitor2 + { + /// + public virtual AstVisitAction VisitListLiteral(ListLiteralAst listLiteralAst) { return AstVisitAction.Continue; } + } + + /// + /// Implement this interface when you implement , or when /// you want to do something after possibly visiting the children of the ast. /// public interface IAstPostVisitHandler diff --git a/src/System.Management.Automation/engine/parser/SafeValues.cs b/src/System.Management.Automation/engine/parser/SafeValues.cs index ed1924382de..ac3db88ecdb 100644 --- a/src/System.Management.Automation/engine/parser/SafeValues.cs +++ b/src/System.Management.Automation/engine/parser/SafeValues.cs @@ -3,6 +3,7 @@ --********************************************************************/ using System.Collections; +using System.Collections.Generic; using System.IO; using System.Linq; using System.Management.Automation.Internal; @@ -37,7 +38,7 @@ namespace System.Management.Automation.Language { - internal class IsSafeValueVisitor : ICustomAstVisitor + internal class IsSafeValueVisitor : ICustomAstVisitor3 { public static bool IsAstSafe(Ast ast, GetSafeValueVisitor.SafeValueContext safeValueContext) { @@ -99,6 +100,13 @@ internal IsSafeValueVisitor(GetSafeValueVisitor.SafeValueContext safeValueContex public object VisitAttributedExpression(AttributedExpressionAst attributedExpressionAst) { return false; } public object VisitBlockStatement(BlockStatementAst blockStatementAst) { return false; } public object VisitInvokeMemberExpression(InvokeMemberExpressionAst invokeMemberExpressionAst) { return false; } + public object VisitTypeDefinition(TypeDefinitionAst typeDefinitionAst) { return false; } + public object VisitPropertyMember(PropertyMemberAst propertyMemberAst) { return false; } + public object VisitFunctionMember(FunctionMemberAst functionMemberAst) { return false; } + public object VisitBaseCtorInvokeMemberExpression(BaseCtorInvokeMemberExpressionAst baseCtorInvokeMemberExpressionAst) { return false; } + public object VisitUsingStatement(UsingStatementAst usingStatement) { return false; } + public object VisitConfigurationDefinition(ConfigurationDefinitionAst configurationDefinitionAst) { return false; } + public object VisitDynamicKeywordStatement(DynamicKeywordStatementAst dynamicKeywordAst) { return false; } public object VisitIndexExpression(IndexExpressionAst indexExpressionAst) { @@ -258,6 +266,11 @@ public object VisitArrayLiteral(ArrayLiteralAst arrayLiteralAst) return isSafe; } + public object VisitListLiteral(ListLiteralAst listLiteralAst) + { + return listLiteralAst.Elements.All(e => (bool)e.Accept(this)); + } + public object VisitHashtable(HashtableAst hashtableAst) { if (hashtableAst.KeyValuePairs.Count > MaxHashtableKeyCount) @@ -285,7 +298,7 @@ public object VisitParenExpression(ParenExpressionAst parenExpressionAst) * except in the case of handling the unary operator * ExecutionContext is provided to ensure we can resolve variables */ - internal class GetSafeValueVisitor : ICustomAstVisitor + internal class GetSafeValueVisitor : ICustomAstVisitor3 { internal enum SafeValueContext { @@ -349,6 +362,13 @@ public static object GetSafeValue(Ast ast, ExecutionContext context, SafeValueCo public object VisitAttributedExpression(AttributedExpressionAst attributedExpressionAst) { throw PSTraceSource.NewArgumentException("ast"); } public object VisitBlockStatement(BlockStatementAst blockStatementAst) { throw PSTraceSource.NewArgumentException("ast"); } public object VisitInvokeMemberExpression(InvokeMemberExpressionAst invokeMemberExpressionAst) { throw PSTraceSource.NewArgumentException("ast"); } + public object VisitTypeDefinition(TypeDefinitionAst typeDefinitionAst) { throw PSTraceSource.NewArgumentException("ast"); } + public object VisitPropertyMember(PropertyMemberAst propertyMemberAst) { throw PSTraceSource.NewArgumentException("ast"); } + public object VisitFunctionMember(FunctionMemberAst functionMemberAst) { throw PSTraceSource.NewArgumentException("ast"); } + public object VisitBaseCtorInvokeMemberExpression(BaseCtorInvokeMemberExpressionAst baseCtorInvokeMemberExpressionAst) { throw PSTraceSource.NewArgumentException("ast"); } + public object VisitUsingStatement(UsingStatementAst usingStatement) { throw PSTraceSource.NewArgumentException("ast"); } + public object VisitConfigurationDefinition(ConfigurationDefinitionAst configurationDefinitionAst) { throw PSTraceSource.NewArgumentException("ast"); } + public object VisitDynamicKeywordStatement(DynamicKeywordStatementAst dynamicKeywordAst) { throw PSTraceSource.NewArgumentException("ast"); } // // This is similar to logic used deep in the engine for slicing something that can be sliced @@ -637,6 +657,17 @@ public object VisitArrayLiteral(ArrayLiteralAst arrayLiteralAst) return arrayElements.ToArray(); } + public object VisitListLiteral(ListLiteralAst listLiteralAst) + { + // A list literal is safe + List listElements = new List(); + foreach (var element in listLiteralAst.Elements) + { + listElements.Add(element.Accept(this)); + } + return listElements; + } + public object VisitHashtable(HashtableAst hashtableAst) { Hashtable hashtable = new Hashtable(StringComparer.CurrentCultureIgnoreCase); diff --git a/src/System.Management.Automation/engine/parser/TypeInferenceVisitor.cs b/src/System.Management.Automation/engine/parser/TypeInferenceVisitor.cs index 7369607174a..75a44d3bd7a 100644 --- a/src/System.Management.Automation/engine/parser/TypeInferenceVisitor.cs +++ b/src/System.Management.Automation/engine/parser/TypeInferenceVisitor.cs @@ -418,7 +418,7 @@ private static bool IsConstructor(object member) } } - internal class TypeInferenceVisitor : ICustomAstVisitor2 + internal class TypeInferenceVisitor : ICustomAstVisitor3 { private readonly TypeInferenceContext _context; @@ -1575,6 +1575,11 @@ object ICustomAstVisitor2.VisitDynamicKeywordStatement(DynamicKeywordStatementAs return dynamicKeywordAst.CommandElements[0].Accept(this); } + object ICustomAstVisitor3.VisitListLiteral(ListLiteralAst listLiteralAst) + { + return new[] { new PSTypeName(listLiteralAst.StaticType) }; + } + private static CommandBaseAst GetPreviousPipelineCommand(CommandAst commandAst) { var pipe = (PipelineAst)commandAst.Parent; diff --git a/src/System.Management.Automation/engine/parser/VariableAnalysis.cs b/src/System.Management.Automation/engine/parser/VariableAnalysis.cs index f89c3b83575..917a5eee304 100644 --- a/src/System.Management.Automation/engine/parser/VariableAnalysis.cs +++ b/src/System.Management.Automation/engine/parser/VariableAnalysis.cs @@ -311,7 +311,7 @@ private void NoteVariable(string variableName, int index, Type type, bool automa } } - internal class VariableAnalysis : ICustomAstVisitor + internal class VariableAnalysis : ICustomAstVisitor3 { // Tuple slots start at index 0. >= 0 means a variable is allocated in the tuple. -1 means we haven't // analyzed a specific use of a variable and don't know what slot it might be assigned to yet. @@ -1747,6 +1747,15 @@ public object VisitArrayLiteral(ArrayLiteralAst arrayLiteralAst) return null; } + public object VisitListLiteral(ListLiteralAst listLiteralAst) + { + foreach (var element in listLiteralAst.Elements) + { + element.Accept(this); + } + return null; + } + public object VisitHashtable(HashtableAst hashtableAst) { foreach (var pair in hashtableAst.KeyValuePairs) @@ -1798,5 +1807,13 @@ public object VisitBlockStatement(BlockStatementAst blockStatementAst) blockStatementAst.Body.Accept(this); return null; } + + public object VisitTypeDefinition(TypeDefinitionAst typeDefinitionAst) { return null; } + public object VisitPropertyMember(PropertyMemberAst propertyMemberAst) { return null; } + public object VisitFunctionMember(FunctionMemberAst functionMemberAst) { return null; } + public object VisitBaseCtorInvokeMemberExpression(BaseCtorInvokeMemberExpressionAst baseCtorInvokeMemberExpressionAst) { return null; } + public object VisitUsingStatement(UsingStatementAst usingStatement) { return null; } + public object VisitConfigurationDefinition(ConfigurationDefinitionAst configurationDefinitionAst) { return null; } + public object VisitDynamicKeywordStatement(DynamicKeywordStatementAst dynamicKeywordAst) { return null; } } } diff --git a/src/System.Management.Automation/engine/parser/ast.cs b/src/System.Management.Automation/engine/parser/ast.cs index 63dca76ca86..3a414690606 100644 --- a/src/System.Management.Automation/engine/parser/ast.cs +++ b/src/System.Management.Automation/engine/parser/ast.cs @@ -9270,6 +9270,87 @@ internal override AstVisitAction InternalVisit(AstVisitor visitor) #endregion Visitors } + /// + /// The ast that represents a list literal expression, e.g. [1,2,3]. + /// + public class ListLiteralAst : ExpressionAst + { + /// + /// Construct a list literal expression. + /// + /// The extent of all of the elements. + /// The collection of asts that represent the list literal. + /// + /// If is null. + /// + /// + /// If is null or is an empty collection. + /// + public ListLiteralAst(IScriptExtent extent, IList elements) + : base(extent) + { + if (elements == null) + { + throw PSTraceSource.NewArgumentNullException("elements"); + } + + this.Elements = elements.Any() ? new ReadOnlyCollection(elements) : Utils.EmptyReadOnlyCollection(); + SetParents(Elements); + } + + /// + /// The collection of asts of the elements of the list. + /// + public ReadOnlyCollection Elements { get; private set; } + + /// + /// Copy the ArrayLiteralAst instance + /// + public override Ast Copy() + { + var newElements = CopyElements(this.Elements); + return new ArrayLiteralAst(this.Extent, newElements); + } + + /// + /// The result of an is always typeof(List[object]). + /// + public override Type StaticType { get { return typeof(List); } } + + #region Visitors + + internal override object Accept(ICustomAstVisitor visitor) + { + var visitor3 = visitor as ICustomAstVisitor3; + return visitor3 != null ? visitor3.VisitListLiteral(this) : null; + } + + internal override AstVisitAction InternalVisit(AstVisitor visitor) + { + var action = AstVisitAction.Continue; + var visitor3 = visitor as AstVisitor3; + if (visitor3 != null) + { + action = visitor3.VisitListLiteral(this); + if (action == AstVisitAction.SkipChildren) + return visitor.CheckForPostAction(this, AstVisitAction.Continue); + } + + if (action == AstVisitAction.Continue) + { + for (int index = 0; index < Elements.Count; index++) + { + var element = Elements[index]; + action = element.InternalVisit(visitor); + if (action != AstVisitAction.Continue) break; + } + } + return visitor.CheckForPostAction(this, action); + } + + #endregion Visitors + } + /// /// The ast that represents an array literal expression, e.g. 1,2,3. An array expression, e.g. @(dir), /// is represented by . An array literal expression can be constructed from a single diff --git a/src/System.Management.Automation/resources/ParserStrings.resx b/src/System.Management.Automation/resources/ParserStrings.resx index 5f9b75345fe..118368f7871 100644 --- a/src/System.Management.Automation/resources/ParserStrings.resx +++ b/src/System.Management.Automation/resources/ParserStrings.resx @@ -508,6 +508,9 @@ The correct form is: foreach ($a in $b) {...} Missing closing ')' in subexpression. + + Missing closing ']' in list expression. + Missing '(' after '{0}' in if statement. From 3469218e53267bcc88af7b322e22e700e576587b Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Fri, 29 Dec 2017 11:58:24 -0800 Subject: [PATCH 2/4] [Feature] ListLiteral: when there is only one element, behave like @() but return a list --- .../engine/interpreter/ArrayOperations.cs | 36 +++++++++++ .../engine/interpreter/InstructionFactory.cs | 16 +++++ .../engine/interpreter/InstructionList.cs | 14 +++++ .../engine/interpreter/LightCompiler.cs | 18 ++++++ .../engine/parser/Compiler.cs | 46 +++++++++++++-- .../engine/runtime/Binding/Binders.cs | 59 +++++++++++++++++++ .../engine/runtime/Operations/MiscOps.cs | 8 ++- 7 files changed, 192 insertions(+), 5 deletions(-) diff --git a/src/System.Management.Automation/engine/interpreter/ArrayOperations.cs b/src/System.Management.Automation/engine/interpreter/ArrayOperations.cs index f85482522c5..ce0ddc65e80 100644 --- a/src/System.Management.Automation/engine/interpreter/ArrayOperations.cs +++ b/src/System.Management.Automation/engine/interpreter/ArrayOperations.cs @@ -13,6 +13,8 @@ * * ***************************************************************************/ +using System.Collections.Generic; + namespace System.Management.Automation.Interpreter { internal sealed class NewArrayInitInstruction : Instruction @@ -39,6 +41,40 @@ public override int Run(InterpretedFrame frame) } } + internal sealed class NewListInitInstruction : Instruction + { + private readonly int _elementCount; + + internal NewListInitInstruction(int elementCount) + { + _elementCount = elementCount; + } + + public override int ConsumedStack { get { return _elementCount; } } + public override int ProducedStack { get { return 1; } } + + public override int Run(InterpretedFrame frame) + { + List list; + if (_elementCount == 1) + { + list = new List() { (TElement)frame.Pop() }; + } + else + { + TElement[] array = new TElement[_elementCount]; + for (int i = _elementCount - 1; i >= 0; i--) + { + array[i] = (TElement)frame.Pop(); + } + list = new List(array); + } + + frame.Push(list); + return +1; + } + } + internal sealed class NewArrayInstruction : Instruction { internal NewArrayInstruction() { } diff --git a/src/System.Management.Automation/engine/interpreter/InstructionFactory.cs b/src/System.Management.Automation/engine/interpreter/InstructionFactory.cs index 98e8545277a..f370a85f3d3 100644 --- a/src/System.Management.Automation/engine/interpreter/InstructionFactory.cs +++ b/src/System.Management.Automation/engine/interpreter/InstructionFactory.cs @@ -61,6 +61,7 @@ internal static InstructionFactory GetFactory(Type type) protected internal abstract Instruction DefaultValue(); protected internal abstract Instruction NewArray(); protected internal abstract Instruction NewArrayInit(int elementCount); + protected internal abstract Instruction NewListInit(int elementCount); } internal sealed class InstructionFactory : InstructionFactory @@ -75,6 +76,7 @@ internal sealed class InstructionFactory : InstructionFactory private Instruction _newArray; private Instruction _typeAs; private Instruction[] _newArrayInit; + private Instruction[] _newListInit; // This number is somewhat arbitrary - trying to avoid some gc without keeping // objects (instructions) around that aren't used that often. private const int MaxArrayInitElementCountCache = 32; @@ -124,5 +126,19 @@ protected internal override Instruction NewArrayInit(int elementCount) } return new NewArrayInitInstruction(elementCount); } + + protected internal override Instruction NewListInit(int elementCount) + { + if (elementCount < MaxArrayInitElementCountCache) + { + if (_newListInit == null) + { + _newListInit = new Instruction[MaxArrayInitElementCountCache]; + } + + return _newListInit[elementCount] ?? (_newListInit[elementCount] = new NewListInitInstruction(elementCount)); + } + return new NewListInitInstruction(elementCount); + } } } diff --git a/src/System.Management.Automation/engine/interpreter/InstructionList.cs b/src/System.Management.Automation/engine/interpreter/InstructionList.cs index cddf8dab6d8..128a9a1d311 100644 --- a/src/System.Management.Automation/engine/interpreter/InstructionList.cs +++ b/src/System.Management.Automation/engine/interpreter/InstructionList.cs @@ -773,6 +773,20 @@ public void EmitNewArrayInit(Type elementType, int elementCount) } } + public void EmitNewListInit(Type elementType, int elementCount) + { + // To avoid lock contention in InstructionFactory.GetFactory, we special case the most common + // types of lists that the compiler creates. + if (elementType == typeof(object)) + { + Emit(InstructionFactory.Factory.NewListInit(elementCount)); + } + else + { + Emit(InstructionFactory.GetFactory(elementType).NewListInit(elementCount)); + } + } + #endregion #region Arithmetic Operations diff --git a/src/System.Management.Automation/engine/interpreter/LightCompiler.cs b/src/System.Management.Automation/engine/interpreter/LightCompiler.cs index 12450c12945..ec49202b594 100644 --- a/src/System.Management.Automation/engine/interpreter/LightCompiler.cs +++ b/src/System.Management.Automation/engine/interpreter/LightCompiler.cs @@ -1804,6 +1804,24 @@ private void CompileInvocationExpression(Expression expr) private void CompileListInitExpression(Expression expr) { var node = (ListInitExpression) expr; +#if !DEBUG + // Optimize the most common use -- create and initialize an object list + // - Use the default constructor of List + // - Initializer collection is not empty and the AddMethod is 'Add(object)' + if (node.NewExpression == System.Management.Automation.Language.ExpressionCache.NewEmptyObjectList && + node.Initializers.Count > 0 && + node.Initializers.TrueForAll(e => e.AddMethod == System.Management.Automation.Language.CachedReflectionInfo.ObjectList_Add)) + { + for (int index = 0; index < node.Initializers.Count; index++) + { + var arg = node.Initializers[index].Arguments[0]; + Compile(arg); + } + + _instructions.EmitNewListInit(typeof(object), node.Initializers.Count); + return; + } +#endif CompileNewExpression(node.NewExpression); if (node.Initializers.Count > 0) { diff --git a/src/System.Management.Automation/engine/parser/Compiler.cs b/src/System.Management.Automation/engine/parser/Compiler.cs index 8adf79e02bc..edc471327b1 100644 --- a/src/System.Management.Automation/engine/parser/Compiler.cs +++ b/src/System.Management.Automation/engine/parser/Compiler.cs @@ -33,8 +33,10 @@ internal static class CachedReflectionInfo internal const BindingFlags staticPublicFlags = BindingFlags.Static | BindingFlags.Public; internal const BindingFlags instancePublicFlags = BindingFlags.Instance | BindingFlags.Public; - internal static readonly ConstructorInfo ObjectList_ctor = + internal static readonly ConstructorInfo ObjectList_Default_ctor = typeof(List).GetConstructor(PSTypeExtensions.EmptyTypes); + internal static readonly ConstructorInfo ObjectList_IEnumerable_ctor = + typeof(List).GetConstructor(new Type[] { typeof(IEnumerable) }); internal static readonly MethodInfo ObjectList_ToArray = typeof(List).GetMethod(nameof(List.ToArray), PSTypeExtensions.EmptyTypes); internal static readonly MethodInfo ObjectList_Add = @@ -126,6 +128,8 @@ internal static class CachedReflectionInfo typeof(EnumerableOps).GetMethod(nameof(EnumerableOps.SlicingIndex), staticFlags); internal static readonly MethodInfo EnumerableOps_ToArray = typeof(EnumerableOps).GetMethod(nameof(EnumerableOps.ToArray), staticFlags); + internal static readonly MethodInfo EnumerableOps_ToList = + typeof(EnumerableOps).GetMethod(nameof(EnumerableOps.ToList), staticFlags); internal static readonly MethodInfo EnumerableOps_WriteEnumerableToPipe = typeof(EnumerableOps).GetMethod(nameof(EnumerableOps.WriteEnumerableToPipe), staticFlags); @@ -550,7 +554,7 @@ internal static class ExpressionCache internal static readonly Expression CatchAllType = Expression.Constant(typeof(ExceptionHandlingOps.CatchAll), typeof(Type)); internal static readonly Expression Empty = Expression.Empty(); internal static readonly Expression GetExecutionContextFromTLS = Expression.Call(CachedReflectionInfo.LocalPipeline_GetExecutionContextFromTLS); - internal static readonly NewExpression NewEmptyObjectList = Expression.New(CachedReflectionInfo.ObjectList_ctor); + internal static readonly NewExpression NewEmptyObjectList = Expression.New(CachedReflectionInfo.ObjectList_Default_ctor); internal static readonly Expression BoxedTrue = Expression.Field(null, typeof(Boxed).GetField("True", BindingFlags.Static | BindingFlags.NonPublic)); @@ -5541,8 +5545,42 @@ public object VisitArrayLiteral(ArrayLiteralAst arrayLiteralAst) public object VisitListLiteral(ListLiteralAst listLiteralAst) { - return Expression.ListInit(ExpressionCache.NewEmptyObjectList, CachedReflectionInfo.ObjectList_Add, - listLiteralAst.Elements.Select(e => Compile(e).Cast(typeof(object)))); + if (listLiteralAst.Elements.Count == 0) + { + return ExpressionCache.NewEmptyObjectList; + } + + if (listLiteralAst.Elements.Count == 1) + { + Expression values = Compile(listLiteralAst.Elements[0]); + + if (values.Type == typeof(List)) { return values; } + if (values.Type.IsPrimitive || values.Type == typeof(string)) + { + // Slight optimization - no need for a dynamic site. We could special case other + // types as well, but it's probably not worth it. + return Expression.ListInit(ExpressionCache.NewEmptyObjectList, CachedReflectionInfo.ObjectList_Add, values.Cast(typeof(object))); + } + if (values.Type == typeof(void)) + { + // A dynamic site can't take void - but a void value is just an empty list. + return ExpressionCache.NewEmptyObjectList; + } + return DynamicExpression.Dynamic(PSToObjectListBinder.Get(), typeof(List), values); + } + + List elementValues = new List(listLiteralAst.Elements.Count); + foreach (var element in listLiteralAst.Elements) + { + var eValue = Compile(element); + if (eValue.Type != typeof(void)) { elementValues.Add(eValue.Cast(typeof(object))); } + } + + if (elementValues.Count == 0) + { + return ExpressionCache.NewEmptyObjectList; + } + return Expression.ListInit(ExpressionCache.NewEmptyObjectList, CachedReflectionInfo.ObjectList_Add, elementValues); } private IEnumerable BuildHashtable(ReadOnlyCollection keyValuePairs, ParameterExpression temp, bool ordered) diff --git a/src/System.Management.Automation/engine/runtime/Binding/Binders.cs b/src/System.Management.Automation/engine/runtime/Binding/Binders.cs index 6de70506924..8f5c9d19fc3 100644 --- a/src/System.Management.Automation/engine/runtime/Binding/Binders.cs +++ b/src/System.Management.Automation/engine/runtime/Binding/Binders.cs @@ -812,6 +812,65 @@ public override DynamicMetaObject Bind(DynamicMetaObject target, DynamicMetaObje } } + /// + /// This binder is used for the list expression. + /// + internal class PSToObjectListBinder : DynamicMetaObjectBinder + { + private static readonly PSToObjectListBinder s_binder = new PSToObjectListBinder(); + + internal static PSToObjectListBinder Get() + { + return s_binder; + } + + private PSToObjectListBinder() + { + } + + public override string ToString() + { + return "ToObjectList"; + } + + public override Type ReturnType { get { return typeof(List); } } + + public override DynamicMetaObject Bind(DynamicMetaObject target, DynamicMetaObject[] args) + { + if (!target.HasValue) + { + return Defer(target, args); + } + + if (target.Value == AutomationNull.Value) + { + return new DynamicMetaObject(ExpressionCache.NewEmptyObjectList, + BindingRestrictions.GetInstanceRestriction(target.Expression, AutomationNull.Value)).WriteToDebugLog(this); + } + + var enumerable = PSEnumerableBinder.IsEnumerable(target); + if (enumerable == null) + { + return new DynamicMetaObject( + Expression.ListInit(ExpressionCache.NewEmptyObjectList, CachedReflectionInfo.ObjectList_Add, target.Expression.Cast(typeof(object))), + target.PSGetTypeRestriction()).WriteToDebugLog(this); + } + + var value = PSObject.Base(target.Value); + if (value is IEnumerable) + { + return new DynamicMetaObject( + Expression.New(CachedReflectionInfo.ObjectList_IEnumerable_ctor, PSEnumerableBinder.MaybeDebase(this, e => e.Cast(typeof(IEnumerable)), target)), + PSEnumerableBinder.GetRestrictions(target)).WriteToDebugLog(this); + } + + // It's enumerable, but not an IEnumerable. Call EnumerableOps.ToArray + return new DynamicMetaObject( + Expression.Call(CachedReflectionInfo.EnumerableOps_ToList, enumerable.Expression), + target.PSGetTypeRestriction()).WriteToDebugLog(this); + } + } + internal class PSPipeWriterBinder : DynamicMetaObjectBinder { private static readonly PSPipeWriterBinder s_binder = new PSPipeWriterBinder(); diff --git a/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs b/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs index df5fc5f837d..e4964e2f8fa 100644 --- a/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs +++ b/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs @@ -3381,13 +3381,19 @@ internal static void WriteEnumerableToPipe(IEnumerator enumerator, Pipe pipe, Ex } } - internal static object[] ToArray(IEnumerator enumerator) + internal static List ToList(IEnumerator enumerator) { var result = new List(); while (MoveNext(null, enumerator)) { result.Add(Current(enumerator)); } + return result; + } + + internal static object[] ToArray(IEnumerator enumerator) + { + var result = ToList(enumerator); return result.ToArray(); } From c327e5159bf311fe0e098d51cfea519c82573fd3 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Fri, 29 Dec 2017 16:06:21 -0800 Subject: [PATCH 3/4] Use 'endExtent' to track the end extent --- .../engine/parser/Parser.cs | 28 +++++++++---------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/src/System.Management.Automation/engine/parser/Parser.cs b/src/System.Management.Automation/engine/parser/Parser.cs index 7440e98fe0e..98cb3994dc1 100644 --- a/src/System.Management.Automation/engine/parser/Parser.cs +++ b/src/System.Management.Automation/engine/parser/Parser.cs @@ -5998,12 +5998,18 @@ private ExpressionAst ListLiteralRule() var lBracket = NextLBracket(); if (lBracket == null) { return null; } + IScriptExtent endExtent = lBracket.Extent; var listValues = new List(); Token commaToken = null; do { - if (commaToken != null) { SkipToken(); } - V3SkipNewlines(); + if (commaToken != null) + { + SkipToken(); + endExtent = commaToken.Extent; + } + + SkipNewlines(); var expr = UnaryExpressionRule(); if (expr == null) @@ -6017,12 +6023,14 @@ private ExpressionAst ListLiteralRule() listValues.Add(expr); break; } + + endExtent = expr.Extent; listValues.Add(expr); commaToken = PeekToken(); } while (commaToken.Kind == TokenKind.Comma); - V3SkipNewlines(); + SkipNewlines(); var rBracket = NextToken(); if (rBracket.Kind != TokenKind.RBracket) @@ -6031,22 +6039,12 @@ private ExpressionAst ListLiteralRule() UngetToken(rBracket); ReportIncompleteInput(rBracket.Extent, () => ParserStrings.MissingEndBracketInListExpression); } - - IScriptExtent extent; - if (rBracket.Kind == TokenKind.RBracket) - { - extent = ExtentOf(lBracket, rBracket.Extent); - } - else if (listValues.Count > 0) - { - extent = ExtentOf(lBracket, listValues[listValues.Count-1].Extent); - } else { - extent = ExtentOf(lBracket, lBracket.Extent); + endExtent = rBracket.Extent; } - return new ListLiteralAst(extent, listValues);; + return new ListLiteralAst(ExtentOf(lBracket, endExtent), listValues);; } private ExpressionAst ArrayLiteralRule() From b58f1b1bb322e474f7c43a6ad17bd53cdb278505 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Fri, 29 Dec 2017 16:30:08 -0800 Subject: [PATCH 4/4] Do not unwrap the only expression element value --- .../engine/parser/Compiler.cs | 27 +-------- .../engine/runtime/Binding/Binders.cs | 59 ------------------- .../engine/runtime/Operations/MiscOps.cs | 8 +-- 3 files changed, 3 insertions(+), 91 deletions(-) diff --git a/src/System.Management.Automation/engine/parser/Compiler.cs b/src/System.Management.Automation/engine/parser/Compiler.cs index edc471327b1..09f05da10c5 100644 --- a/src/System.Management.Automation/engine/parser/Compiler.cs +++ b/src/System.Management.Automation/engine/parser/Compiler.cs @@ -33,10 +33,8 @@ internal static class CachedReflectionInfo internal const BindingFlags staticPublicFlags = BindingFlags.Static | BindingFlags.Public; internal const BindingFlags instancePublicFlags = BindingFlags.Instance | BindingFlags.Public; - internal static readonly ConstructorInfo ObjectList_Default_ctor = + internal static readonly ConstructorInfo ObjectList_ctor = typeof(List).GetConstructor(PSTypeExtensions.EmptyTypes); - internal static readonly ConstructorInfo ObjectList_IEnumerable_ctor = - typeof(List).GetConstructor(new Type[] { typeof(IEnumerable) }); internal static readonly MethodInfo ObjectList_ToArray = typeof(List).GetMethod(nameof(List.ToArray), PSTypeExtensions.EmptyTypes); internal static readonly MethodInfo ObjectList_Add = @@ -128,8 +126,6 @@ internal static class CachedReflectionInfo typeof(EnumerableOps).GetMethod(nameof(EnumerableOps.SlicingIndex), staticFlags); internal static readonly MethodInfo EnumerableOps_ToArray = typeof(EnumerableOps).GetMethod(nameof(EnumerableOps.ToArray), staticFlags); - internal static readonly MethodInfo EnumerableOps_ToList = - typeof(EnumerableOps).GetMethod(nameof(EnumerableOps.ToList), staticFlags); internal static readonly MethodInfo EnumerableOps_WriteEnumerableToPipe = typeof(EnumerableOps).GetMethod(nameof(EnumerableOps.WriteEnumerableToPipe), staticFlags); @@ -554,7 +550,7 @@ internal static class ExpressionCache internal static readonly Expression CatchAllType = Expression.Constant(typeof(ExceptionHandlingOps.CatchAll), typeof(Type)); internal static readonly Expression Empty = Expression.Empty(); internal static readonly Expression GetExecutionContextFromTLS = Expression.Call(CachedReflectionInfo.LocalPipeline_GetExecutionContextFromTLS); - internal static readonly NewExpression NewEmptyObjectList = Expression.New(CachedReflectionInfo.ObjectList_Default_ctor); + internal static readonly NewExpression NewEmptyObjectList = Expression.New(CachedReflectionInfo.ObjectList_ctor); internal static readonly Expression BoxedTrue = Expression.Field(null, typeof(Boxed).GetField("True", BindingFlags.Static | BindingFlags.NonPublic)); @@ -5550,25 +5546,6 @@ public object VisitListLiteral(ListLiteralAst listLiteralAst) return ExpressionCache.NewEmptyObjectList; } - if (listLiteralAst.Elements.Count == 1) - { - Expression values = Compile(listLiteralAst.Elements[0]); - - if (values.Type == typeof(List)) { return values; } - if (values.Type.IsPrimitive || values.Type == typeof(string)) - { - // Slight optimization - no need for a dynamic site. We could special case other - // types as well, but it's probably not worth it. - return Expression.ListInit(ExpressionCache.NewEmptyObjectList, CachedReflectionInfo.ObjectList_Add, values.Cast(typeof(object))); - } - if (values.Type == typeof(void)) - { - // A dynamic site can't take void - but a void value is just an empty list. - return ExpressionCache.NewEmptyObjectList; - } - return DynamicExpression.Dynamic(PSToObjectListBinder.Get(), typeof(List), values); - } - List elementValues = new List(listLiteralAst.Elements.Count); foreach (var element in listLiteralAst.Elements) { diff --git a/src/System.Management.Automation/engine/runtime/Binding/Binders.cs b/src/System.Management.Automation/engine/runtime/Binding/Binders.cs index 8f5c9d19fc3..6de70506924 100644 --- a/src/System.Management.Automation/engine/runtime/Binding/Binders.cs +++ b/src/System.Management.Automation/engine/runtime/Binding/Binders.cs @@ -812,65 +812,6 @@ public override DynamicMetaObject Bind(DynamicMetaObject target, DynamicMetaObje } } - /// - /// This binder is used for the list expression. - /// - internal class PSToObjectListBinder : DynamicMetaObjectBinder - { - private static readonly PSToObjectListBinder s_binder = new PSToObjectListBinder(); - - internal static PSToObjectListBinder Get() - { - return s_binder; - } - - private PSToObjectListBinder() - { - } - - public override string ToString() - { - return "ToObjectList"; - } - - public override Type ReturnType { get { return typeof(List); } } - - public override DynamicMetaObject Bind(DynamicMetaObject target, DynamicMetaObject[] args) - { - if (!target.HasValue) - { - return Defer(target, args); - } - - if (target.Value == AutomationNull.Value) - { - return new DynamicMetaObject(ExpressionCache.NewEmptyObjectList, - BindingRestrictions.GetInstanceRestriction(target.Expression, AutomationNull.Value)).WriteToDebugLog(this); - } - - var enumerable = PSEnumerableBinder.IsEnumerable(target); - if (enumerable == null) - { - return new DynamicMetaObject( - Expression.ListInit(ExpressionCache.NewEmptyObjectList, CachedReflectionInfo.ObjectList_Add, target.Expression.Cast(typeof(object))), - target.PSGetTypeRestriction()).WriteToDebugLog(this); - } - - var value = PSObject.Base(target.Value); - if (value is IEnumerable) - { - return new DynamicMetaObject( - Expression.New(CachedReflectionInfo.ObjectList_IEnumerable_ctor, PSEnumerableBinder.MaybeDebase(this, e => e.Cast(typeof(IEnumerable)), target)), - PSEnumerableBinder.GetRestrictions(target)).WriteToDebugLog(this); - } - - // It's enumerable, but not an IEnumerable. Call EnumerableOps.ToArray - return new DynamicMetaObject( - Expression.Call(CachedReflectionInfo.EnumerableOps_ToList, enumerable.Expression), - target.PSGetTypeRestriction()).WriteToDebugLog(this); - } - } - internal class PSPipeWriterBinder : DynamicMetaObjectBinder { private static readonly PSPipeWriterBinder s_binder = new PSPipeWriterBinder(); diff --git a/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs b/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs index e4964e2f8fa..df5fc5f837d 100644 --- a/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs +++ b/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs @@ -3381,19 +3381,13 @@ internal static void WriteEnumerableToPipe(IEnumerator enumerator, Pipe pipe, Ex } } - internal static List ToList(IEnumerator enumerator) + internal static object[] ToArray(IEnumerator enumerator) { var result = new List(); while (MoveNext(null, enumerator)) { result.Add(Current(enumerator)); } - return result; - } - - internal static object[] ToArray(IEnumerator enumerator) - { - var result = ToList(enumerator); return result.ToArray(); }