-
Notifications
You must be signed in to change notification settings - Fork 123
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.
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
Add using () statement for automatic disposing #368
base: master
Are you sure you want to change the base?
Conversation
Dave Wyatt wrote a using-object in 2014. I still use it heavily: https://davewyatt.wordpress.com/2014/04/11/using-object-powershell-version-of-cs-using-statement/ - but yes, it suffers from some of the challenges you mention and isn't as flexible. I would just ask that you don't break it. :-) |
+ Should it dispose the variable or actual instance captured before the body is run | ||
+ Support only `IDisposable` vs duck typing with `Dispose()` method | ||
+ What to do in case of an error in the dispose | ||
+ Future support for `using` declarations without the block/braces |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a fan of omitting the braces or otherwise not creating a block. There's very little in PS that allows you to omit the braces. I don't believe we should start here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea it's definitely not part of this RFC but it is something I brought up to consider as possible future work or something that should at least be considering in case we need to adjust the behaviour here to accomodate that in the future.
} | ||
``` | ||
|
||
In this scenario the `using ()` statement supports multiple `AssignmentStatementAst` statements allowing it to track multiple variables to dispose. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what would happen in the case of:
[int]$a = 1
using ( $a = Get-Process -id $PID) {
$a.Kill()
}
my assumption is that this is a runtime error, but wanted to be explicit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's part of the error handling question later in the RFC. But I would have this just error like usual and the using branch would just be ignored, similar to how this code runs
[int]$a = 1
try {
$a = Get-Process -id $PID
$a.Kill()
}
finally {
if ($a -is [IDisposable]) { $a.Dispose() }
}
Essentially the dispose block is just syntatic sugar to ensure that the var(s) inside the using block are automatically wrapped in a try/finally to dispose it if available.
<Statement list> | ||
} | ||
``` | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If a new scope is not created, it means that any variables created in the statement list would persist after the execution of the block. Is that right? If a new scope is created, then the usual variable reaping would occur, wouldn't it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be my assumption. I would have implemented it like an if/while/for/foreach/etc
where the variables are still in the same scope and persist beyond it.
|
||
+ `PipelineBaseAst[]` - Disposables | ||
+ `StatementBlockAst` - Body | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$a = <disposable>
using ($b = $a) {
statement list
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry not sure what you are asking here, wouldn't like be an AssignmentStatementAst
and $b
is the one to be disposed at the end of the statement list?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's more about expectation - in some cases (if it's a reference type) I assume that $b
gets disposed, but $a
would too which might lead to unmet expectations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this particular scenario is covered by https://github.com/PowerShell/PowerShell-RFC/blob/92b6a481b21f84dab7a08a490664b819c291074b/Draft-Accepted/RFCXXXX-Using-IDisposables.md#variable-or-instance. It's the same question as doing
$a = <disposable 1>
using ($a) {
$a = <disposable 2>
}
Should the object being disposed be 1 or 2, C# will dispose 1. I agree that in either scenario, and yours, It's a perfectly valid point to bring up but I'm not sure how we could really avoid such a thing except for making sure the documentation covers such scenarios.
+ `StatementBlockAst` - Body | ||
|
||
When visited the Ast will visit the `Disposables` to find any `AssignmentStatementAst` or `VariableExpressionAst` which it can use to keep track of variables to dispose. | ||
The compiler will then emit an Expression that wraps the `Body` with the below for each `Disposables` (in definition order): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are there remoting issues that will cause difficulties?
I see the utility of:
using ($s = new-psssession) {
invoke-command -session $s { <statement list> }
}
are there situations where something unexpected may occur?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see how this would affect it as Invoke-Command
is a blocking call and the session isn't a disposable object anyway. Even if it was I picture your above example to be the equivalent to the following pwsh code
try {
$s = new-pssession
invoke-command -session $s { <statement list> }
}
finally {
if ($s) { $s.Dispose() }
}
|
||
This RFC proposes adding an equivalent `using` statement in PowerShell that can do the same thing as C#. | ||
For example the same C# `StreamReader` example in PowerShell can now be: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there may be some collisions with implementations of a using
function (I have one in my profile), so I think this would qualify as a breaking change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there may be some collisions with implementations of a
using
function (I have one in my profile), so I think this would qualify as a breaking change.
using
is thankfully already a keyword, so you wouldn't be able to use a command named using
without & 'using'
otherwise you'd get a parse error. use
is a common alias/function for this, but should be unaffected
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea the fact that using
is already a keyword is another reason why I chose to use it over something like dispose
/clean
. People can't do using () {}
as a command without the call operator so it's a nice way to avoid a breaking change.
``` | ||
using ($<disposable>; ...) { | ||
<Statement list> | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps a reasonable alternative
using (<assignmentAst>)
statement-block
and allow a single assignment only, rather than a collection of assignments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's called out later on under Multiple Disposables. The reason why I'm initially in favour of allowing a collection is if you are wrapping multiple disposable blocks you would essentially need to indent each using which is annoying to do.
This proposal would be more useful if there was a way for the
|
@DerpMcDerp that’s what the clean block added in PowerShell 7.3 is great for! |
No description provided.