Skip to content

Support backgrounding pipelines with ampersand #3360

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
lzybkr merged 5 commits into from
May 19, 2017
Merged

Support backgrounding pipelines with ampersand #3360

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
52b8460
Implements support for backgrounding pipelines with &. Putting & at t…
Feb 16, 2017
eaa69a1
Removed duplicate tests, minor cleanups
May 15, 2017
a5e49f2
Refactored background pipeline code into two methods, added redirecti…
May 17, 2017
0f21b5f
cleanup of background process code
May 18, 2017
4c67db1
renamed backgroundProcess to just background
May 19, 2017
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
182 changes: 98 additions & 84 deletions src/System.Management.Automation/engine/parser/Compiler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,8 @@ internal static class CachedReflectionInfo
typeof(PipelineOps).GetMethod(nameof(PipelineOps.FlushPipe), staticFlags);
internal static readonly MethodInfo PipelineOps_InvokePipeline =
typeof(PipelineOps).GetMethod(nameof(PipelineOps.InvokePipeline), staticFlags);
internal static readonly MethodInfo PipelineOps_InvokePipelineInBackground =
typeof(PipelineOps).GetMethod(nameof(PipelineOps.InvokePipelineInBackground), staticFlags);
internal static readonly MethodInfo PipelineOps_Nop =
typeof(PipelineOps).GetMethod(nameof(PipelineOps.Nop), staticFlags);
internal static readonly MethodInfo PipelineOps_PipelineResult =
Expand Down Expand Up @@ -1942,7 +1944,8 @@ private Expression CaptureStatementResultsHelper(
}

var pipelineAst = stmt as PipelineAst;
if (pipelineAst != null)
// If it's a pipeline that isn't being backgrounded, try to optimize expression
if (pipelineAst != null && ! pipelineAst.Background)
{
var expr = pipelineAst.GetPureExpression();
if (expr != null) { return Compile(expr); }
Expand Down Expand Up @@ -2983,104 +2986,115 @@ public object VisitPipeline(PipelineAst pipelineAst)
exprs.Add(UpdatePosition(pipelineAst));
}

var pipeElements = pipelineAst.PipelineElements;
var firstCommandExpr = (pipeElements[0] as CommandExpressionAst);
if (firstCommandExpr != null && pipeElements.Count == 1)
if (pipelineAst.Background)
{
if (firstCommandExpr.Redirections.Count > 0)
{
exprs.Add(GetRedirectedExpression(firstCommandExpr, captureForInput: false));
}
else
{
exprs.Add(Compile(firstCommandExpr));
}
Expression invokeBackgroundPipe = Expression.Call(
CachedReflectionInfo.PipelineOps_InvokePipelineInBackground,
Expression.Constant(pipelineAst),
_functionContext);
exprs.Add(invokeBackgroundPipe);
}
else
{
Expression input;
int i, commandsInPipe;

if (firstCommandExpr != null)
var pipeElements = pipelineAst.PipelineElements;
var firstCommandExpr = (pipeElements[0] as CommandExpressionAst);
if (firstCommandExpr != null && pipeElements.Count == 1)
{
if (firstCommandExpr.Redirections.Count > 0)
{
input = GetRedirectedExpression(firstCommandExpr, captureForInput: true);
exprs.Add(GetRedirectedExpression(firstCommandExpr, captureForInput: false));
}
else
{
input = GetRangeEnumerator(firstCommandExpr.Expression) ??
Compile(firstCommandExpr.Expression);
exprs.Add(Compile(firstCommandExpr));
}
i = 1;
commandsInPipe = pipeElements.Count - 1;
}
else
{
// Compiled code normally never sees AutomationNull. We use that value
// here so that we can tell the difference b/w $null and no input when
// starting the pipeline, in other words, PipelineOps.InvokePipe will
// not pass this value to the pipe.

input = ExpressionCache.AutomationNullConstant;
i = 0;
commandsInPipe = pipeElements.Count;
}
Expression[] pipelineExprs = new Expression[commandsInPipe];
CommandBaseAst[] pipeElementAsts = new CommandBaseAst[commandsInPipe];
var commandRedirections = new object[commandsInPipe];

for (int j = 0; i < pipeElements.Count; ++i, ++j)
{
var pipeElement = pipeElements[i];
pipelineExprs[j] = Compile(pipeElement);

commandRedirections[j] = GetCommandRedirections(pipeElement);
pipeElementAsts[j] = pipeElement;
}

// The redirections are passed as a CommandRedirection[][] - one dimension for each command in the pipe,
// one dimension because each command may have multiple redirections. Here we create the array for
// each command in the pipe, either a compile time constant or created at runtime if necessary.
Expression redirectionExpr;
if (commandRedirections.Any(r => r is Expression))
{
// If any command redirections are non-constant, commandRedirections will have a Linq.Expression in it,
// in which case we must create the array at runtime
redirectionExpr =
Expression.NewArrayInit(typeof(CommandRedirection[]),
commandRedirections.Select(r => (r as Expression) ?? Expression.Constant(r, typeof(CommandRedirection[]))));
}
else if (commandRedirections.Any(r => r != null))
{
// There were redirections, but all were compile time constant, so build the array at compile time.
redirectionExpr =
Expression.Constant(commandRedirections.Map(r => r as CommandRedirection[]));
}
else
{
// No redirections.
redirectionExpr = ExpressionCache.NullCommandRedirections;
}

if (firstCommandExpr != null)
{
var inputTemp = Expression.Variable(input.Type);
temps.Add(inputTemp);
exprs.Add(Expression.Assign(inputTemp, input));
input = inputTemp;
Expression input;
int i, commandsInPipe;

if (firstCommandExpr != null)
{
if (firstCommandExpr.Redirections.Count > 0)
{
input = GetRedirectedExpression(firstCommandExpr, captureForInput: true);
}
else
{
input = GetRangeEnumerator(firstCommandExpr.Expression) ??
Compile(firstCommandExpr.Expression);
}
i = 1;
commandsInPipe = pipeElements.Count - 1;
}
else
{
// Compiled code normally never sees AutomationNull. We use that value
// here so that we can tell the difference b/w $null and no input when
// starting the pipeline, in other words, PipelineOps.InvokePipe will
// not pass this value to the pipe.

input = ExpressionCache.AutomationNullConstant;
i = 0;
commandsInPipe = pipeElements.Count;
}
Expression[] pipelineExprs = new Expression[commandsInPipe];
CommandBaseAst[] pipeElementAsts = new CommandBaseAst[commandsInPipe];
var commandRedirections = new object[commandsInPipe];

for (int j = 0; i < pipeElements.Count; ++i, ++j)
{
var pipeElement = pipeElements[i];
pipelineExprs[j] = Compile(pipeElement);

commandRedirections[j] = GetCommandRedirections(pipeElement);
pipeElementAsts[j] = pipeElement;
}

// The redirections are passed as a CommandRedirection[][] - one dimension for each command in the pipe,
// one dimension because each command may have multiple redirections. Here we create the array for
// each command in the pipe, either a compile time constant or created at runtime if necessary.
Expression redirectionExpr;
if (commandRedirections.Any(r => r is Expression))
{
// If any command redirections are non-constant, commandRedirections will have a Linq.Expression in it,
// in which case we must create the array at runtime
redirectionExpr =
Expression.NewArrayInit(typeof(CommandRedirection[]),
commandRedirections.Select(r => (r as Expression) ?? Expression.Constant(r, typeof(CommandRedirection[]))));
}
else if (commandRedirections.Any(r => r != null))
{
// There were redirections, but all were compile time constant, so build the array at compile time.
redirectionExpr =
Expression.Constant(commandRedirections.Map(r => r as CommandRedirection[]));
}
else
{
// No redirections.
redirectionExpr = ExpressionCache.NullCommandRedirections;
}

if (firstCommandExpr != null)
{
var inputTemp = Expression.Variable(input.Type);
temps.Add(inputTemp);
exprs.Add(Expression.Assign(inputTemp, input));
input = inputTemp;
}

Expression invokePipe = Expression.Call(
CachedReflectionInfo.PipelineOps_InvokePipeline,
input.Cast(typeof(object)),
firstCommandExpr != null ? ExpressionCache.FalseConstant : ExpressionCache.TrueConstant,
Expression.NewArrayInit(typeof(CommandParameterInternal[]), pipelineExprs),
Expression.Constant(pipeElementAsts),
redirectionExpr,
_functionContext);
exprs.Add(invokePipe);
}

Expression invokePipe = Expression.Call(
CachedReflectionInfo.PipelineOps_InvokePipeline,
input.Cast(typeof(object)),
firstCommandExpr != null ? ExpressionCache.FalseConstant : ExpressionCache.TrueConstant,
Expression.NewArrayInit(typeof(CommandParameterInternal[]), pipelineExprs),
Expression.Constant(pipeElementAsts),
redirectionExpr,
_functionContext);

exprs.Add(invokePipe);
}

return Expression.Block(temps, exprs);
Expand Down
17 changes: 9 additions & 8 deletions src/System.Management.Automation/engine/parser/Parser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2451,7 +2451,7 @@ private StatementAst SwitchStatementRule(LabelToken labelToken, Token switchToke
{
endErrorStatement = fileNameExpr.Extent;
condition = new PipelineAst(fileNameExpr.Extent,
new CommandExpressionAst(fileNameExpr.Extent, fileNameExpr, null));
new CommandExpressionAst(fileNameExpr.Extent, fileNameExpr, null), background: false);

if (!specifiedFlags.ContainsKey("file"))
{
Expand Down Expand Up @@ -5182,6 +5182,7 @@ private PipelineBaseAst PipelineRule()

Token pipeToken = null;
bool scanning = true;
bool background = false;
while (scanning)
{
CommandBaseAst commandAst;
Expand Down Expand Up @@ -5293,6 +5294,11 @@ private PipelineBaseAst PipelineRule()
case TokenKind.EndOfInput:
scanning = false;
continue;
case TokenKind.Ampersand:
SkipToken();
scanning = false;
background = true;
break;
case TokenKind.Pipe:
SkipToken();
SkipNewlines();
Expand Down Expand Up @@ -5328,7 +5334,7 @@ private PipelineBaseAst PipelineRule()
return null;
}

return new PipelineAst(ExtentOf(startExtent, pipelineElements[pipelineElements.Count - 1]), pipelineElements);
return new PipelineAst(ExtentOf(startExtent, pipelineElements[pipelineElements.Count - 1]), pipelineElements, background);
}

private RedirectionAst RedirectionRule(RedirectionToken redirectionToken, RedirectionAst[] redirections, ref IScriptExtent extent)
Expand Down Expand Up @@ -5672,16 +5678,11 @@ internal Ast CommandRule(bool forDynamicKeyword)
case TokenKind.Semi:
case TokenKind.AndAnd:
case TokenKind.OrOr:
case TokenKind.Ampersand:
Copy link
Collaborator

@JamesWTruher JamesWTruher Apr 5, 2017

Choose a reason for hiding this comment

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

TokenKind [](start = 29, length = 9)

what do the following emit?
$A = "bar" & "foo"
or
$A = $("bar" & echo "foo")
and we should probably have tests for them

Copy link
Collaborator Author

@BrucePay BrucePay Apr 7, 2017

Choose a reason for hiding this comment

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

& is a statement separator like in ksh so in your examples, you'd get an array of two objects: the job object and "foo".

UngetToken(token);
scanning = false;
continue;

case TokenKind.Ampersand:
// ErrorRecovery: just ignore the token.
endExtent = token.Extent;
ReportError(token.Extent, () => ParserStrings.AmpersandNotAllowed);
Copy link
Contributor

@lzybkr lzybkr Apr 2, 2017

Choose a reason for hiding this comment

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

You can remove the resource string - this is the only reference.

Copy link
Collaborator Author

@BrucePay BrucePay Apr 18, 2017

Choose a reason for hiding this comment

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

Done.

break;

case TokenKind.MinusMinus:
endExtent = token.Extent;
// Add the first -- as a parameter, which is then ignored when constructing the command processor unless it's a native
Expand Down
Loading