GeezerVison is showing github.com — @() (array-subexpression operator) should not be optimized away when a cast to an array type is involved · Issue #4280 · PowerShell/PowerShell
@() (array-subexpression operator) should not be optimized away when a cast to an array type is involved #4280DescriptionThis is a follow-up to the discussion involving Dongbo Wang (@daxian-dbw) and PetSerAl in #4257
Context:
-
PSv5.1 introduced a generally helpful optimization that optimizes @() away in cases where an array is directly constructed inside of it.
- I presume that the motivation for this optimization was the widespread, but redundant and inefficient practice of enclosing
,-constructed arrays ("array literals") in @(), such as in @(1, 2), where 1, 2 would not only suffice, but, in PSv5-, @() would actually (effectively) clone the array.
-
Unfortunately, this optimization is also applied when a cast to an array type (e.g, [int[]]) is involved, which can have unpleasant side effects - see below.
Not optimizing in this scenario makes for conceptually clearer, predictable behavior.
-
While a change would technically be a breaking one, it is hard to imagine anyone having relied on the optimized behavior with casts - a typical Bucket 3: Unlikely Grey Area change.
- Also note that when the optimization was introduced in PSv5.1, it was technically a breaking change too - though I suspect it wasn't announced as such or even at all.
Steps to reproduce
@([int[]] (1, 2)).GetType().Name
$arr = 1, 2; @([int[]] $arr).GetType().Name Expected behavior
Actual behavior
Note that versions up to and including v5 - before the optimization was introduced - behave as expected.
Why is the current behavior problematic?
The examples in this section are courtesy of PetSerAl.
All examples are undoubtedly edge cases, but the larger point is that this behavior should never have been introduced, and, given its "stealth status", this is an opportunity to get rid of it.
- It subverts the fundamental assumption that
@() always creates regular PowerShell arrays (System.Object[]):
> @([int[]] (1, 2))[-1] = 'foo' # BREAKS, because the array is [int[]], not [object[]]
Cannot convert value "foo" to type "System.Int32". ...
... PetSerAl points out that this fundamental assumption is even still reflected in other places in the source code: ArrayExpressionAst.StaticType (a @(...) expression is of type [System.Management.Automation.Language.ArrayExpressionAst]).
- It subverts the expectation that
@() always clones an array:
> $a = [int[]] (1, 2); $b = @([int[]] $a); [object]::ReferenceEquals($a, $b)
True # $a and $b unexpectedly reference the same array. Note that the accidental reference equality only happens if the source type and the cast type are identical if the cast type is either the same as the source type or a type that the source type is (directly or indirectly) derived from (covariance).
Thus, if the cast type is [object[]] and the source type is a reference type too, the problem always surfaces.
Environment data
PowerShell Core v6.0.0-beta.4 on macOS 10.12.5
PowerShell Core v6.0.0-beta.4 on Ubuntu 16.04.2 LTS
PowerShell Core v6.0.0-beta.4 on Microsoft Windows 10 Pro (64-bit; v10.0.15063)
Windows PowerShell v5.1.15063.413 on Microsoft Windows 10 Pro (64-bit; v10.0.15063) ActivityPetSerAl commented on Jul 18, 2017
Note that the accidental reference equality only happens if the source type and the cast type are identical.
It not necessary to be identical due to array covariance in .NET:
$a = [string[]]@()
$b = @([object[]]$a)
[object]::ReferenceEquals($a, $b) mklement0 commented on Jul 18, 2017 ContributorAuthor More actions PetSerAl: Thanks for the correction; I've fixed the original post. daxian-dbw commented on Jul 18, 2017 Moving the discussions about this issue from #4257. Starting from this comment
From Michael Klement (@mklement0):
Given the potentially problematic behavior of something like @([int[]] (1, 2)) (the only case where @() doesn't output an [object[]] array), is it worth creating a new issue for that?
It's hard to imagine that anyone would have relied on that behavior, and eliminating it makes for a more predictable environment. daxian-dbw commented on Jul 18, 2017 From Dongbo Wang (@daxian-dbw):
Here is the code that handles this special case. In my understanding, this happens only in one of the following two conditions:
- the expression is a
ConvertExpressionAst or a ParenExpression that wraps a ConvertExpressionAst, and the convert-to type is an array.
- the expression is an
ArrayLiteralAst or a ParenExpression that wraps an ArrayLiteralAst.
In the first case, since the conversion is explicitly specified, it's very unlikely the resulting array will again be used as object[]. In the second case, the resulting array from ArrayLiteralAst is already object[] so it doesn't matter. Plus, altering this behavior would be a breaking change. Given the above, I prefer to not make a behavior change. But feel free to notify the PowerShell-Committee if you want a further discussion. daxian-dbw commented on Jul 18, 2017 From Michael Klement (@mklement0):
I can't imagine much existing code breaking, but, conversely, I can't imagine many people running into a problem with the current behavior in real-world scenarios, so, personally, I'm happy to leave it at that; perhaps PetSerAl feels differently. daxian-dbw commented on Jul 18, 2017 daxian-dbw commented on Jul 18, 2017 From Dongbo Wang (@daxian-dbw):
Now $b is not copy of $a but is $a
Yes, $b and $a point to the same array in this case. But I don't think the array expression in powershell language is defined to always return a new array object.
Is not ArrayExpressionAst.StaticType should be adjusted to match current behavior?
Good catch. It is inconsistent and should be fixed. But IMHO, the fix should be to change ArrayExpressionAst.StaticType to always return System.Array (also a breaking change :)). StaticType is by definition not accurate, because sometimes the actual type can be known only at runtime, for example, BinaryExpressionAst.StaticType returns System.Object when it's -bor, and the actual type may vary at runtime. daxian-dbw commented on Jul 18, 2017 From PetSerAl:
Dongbo Wang (@daxian-dbw) But where I can see that definition of array expression? Before now I always expect it to return new array object each time. I myself use $b = @($a) (without cast although) as array copy operator, and I really do not like if it stop copying array at some point in the future. daxian-dbw commented on Jul 18, 2017 From Dongbo Wang (@daxian-dbw):
You can find the Windows PowerShell Langauge Specification Version 3.0 here. It hasn't been updated for a while, so new language features like DSC or PowerShell Class are not in it, but I believe the content in the specification should still apply to PowerShell 6.0. mklement0 commented on Jul 20, 2017 ContributorAuthor More actions Ilya (@iSazonov):
No, that (still) works as intended:
-
The first statement creates another variable pointing to the same array.
-
The second statement creates a new array, by enumerating the result of expression $a and packaging that as a new [object[]] array.
The latter behavior is in line with @()'s documented intent, and while the aspect of always creating a new array (effectively, a copy (shallow clone), if the input array is also [object[]]) is not explicitly stated, to me it is implied by the spec, has always worked that way, and people may have come to rely on this behavior, as evidenced by PetSerAl's habit of using @() as an array-cloning shortcut. iSazonov commented on Jul 20, 2017 Thanks for clarify.
Continue 😄
$a = 1..3
$b = @($a = 1..4;[object[]]$a)
[object]::ReferenceEquals($a, $b)
$a = 1..3
$b = @([object[]]$a)
[object]::ReferenceEquals($a, $b)
Reacted by Michael Klement iSazonov commented on Aug 3, 2017 iSazonov commented on Aug 3, 2017 mklement0 commented on Aug 3, 2017 ContributorAuthor More actions Ilya (@iSazonov): I think that a mention in the release notes should be sufficient, given that the original optimization was never documented and many people probably never noticed it. SteveL-MSFT commented on Aug 3, 2017 This has been a good discussion, but I need a summary here. Is there anything that the committee still needs to review? Since Dongbo Wang (@daxian-dbw) fixed some aspects of this discussion, I think the only thing left is a regression introduced by an optimization in 5.x? If so, then I think Documentation Needed is sufficient. daxian-dbw commented on Aug 4, 2017 The thing that committee needed to review was this:
With PSv5.1, @(...) returns an array of a specific type in some scenarios, for example @([int[]] (1, 2)).GetType().Name. However, according to PowerShell Language Specification Version 3.0, as quoted: "The result is the (possibly empty) unconstrained 1-dimensional array", @(...) should only return object[] array.
The PR #4296 addressed this, and Jason approved, so I assume the committee is fine with the change to make @(...) always return object[] array. If so, we can remove the 'Review Committee' label. mklement0 commented on Aug 4, 2017 ContributorAuthor More actions Just for future consideration, I'd like to address Jason Shirk (@lzybkr)'s comments on the linked PR:
I should add that the intent of @() is not to create a new array, but to ensure the result is an array, and not necessarily an object[].
While I agree re the overall intent of @(), I feel that it is better to not contravene two likely user expectations (which happen to be consistent with what the 3.0 spec says):
-
that the array returned is unconstrained, i.e., of type [object[]], i.e., a regular PS array. (Most PowerShell users probably don't even think about array-element typing and probably expect arrays to be able to hold any mix of data types)
-
that @() always returns a new array, without having to worry about reference equality (which is probably not even on the radar of most PowerShell users).
In other words, I think we could be open to changing what the language specification says to allow for better performance.
Passing strongly typed arrays through strikes me as a rare use case that I don't think warrants optimization, given that it would contravene the expectations above.
By contrast, optimizing the typical use case is well worth considering: passing the [object[]] output of a single command through (optimizing the @() away if the enclosed single command happens to return a [object[]] array already).
I can't speak to the technical feasibility, but here's an example where the @() could be optimized away (without contravening the assumptions above), which makes a noticeable difference with large output sets:
# Could be optimized to: Write-Output (1..10000000)
# Currently, the output is collected and a new array is constructed.
@(Write-Output (1..10000000)) iSazonov commented on Aug 6, 2017 mklement0 commented on Aug 6, 2017 ContributorAuthor More actions Ilya (@iSazonov):
No, that's one of the user expectations I do not want to contravene.
Conversely, if we commit to never optimizing @() away in this scenario, we can advertise that @() applied to a variable has a secondary use as a shallow-clone-any-array-or-collection-to-a-regular PS array operator (which it has been restored to with this PR).
What I was suggesting is: make @() return a new array except in the following 2 scenarios - and no others - in which @() can be optimized away:
-
already implemented: an array literal without a cast; e.g., @(1, 2, 3)
-
suggested improvement: a single command that outputs an [object[]] array already; e.g., @(Get-ChildItem -Recurse); of course, if a scalar is situationally returned, it still needs to be wrapped in an (invariably new) array - that is the primary purpose of @(), after all.
In both scenarios reference equality is a moot point, given that no variable referencing a preexisting array is involved. (I suppose in roundabout ways a command's output could hypothetically reference an array stored in a PS variable, but I don't think we need to worry about that).
The first optimization is really just a courtesy to improve the performance in cases where users mistakenly apply @() to (unconstrained) array literals, even though it is pointless.
The 2nd one is by far the most common use case for @(), if I were to guess, and many scripts could benefit from it. Again, I can't speak to technical feasibility / effort. iSazonov commented on Aug 7, 2017 Michael Klement (@mklement0) Thanks for clarify.
Above I see Bruce and Jason said "the intent of @() is not to create a new array". I expect that behavior.
We need conclusion from @powershell-committee for this point too. mklement0 commented on Aug 7, 2017 ContributorAuthor More actions Ilya (@iSazonov):
Indeed, array-cloning is not @()'s purpose, and maybe what I'm proposing is too convoluted (I like the conceptual simplicity of "make this an array if it isn't; otherwise use it as-is" - which guarantees neither cloning nor being unconstrained).
My thought was that since changing the always-create-a-new-array behavior would (a) break existing code and (b) applying @() to a variable is an exotic use case to begin with, why not turn it into a documented virtue that gives us a convenient, PS-idiomatic way to clone arrays?
However, my suggestion to optimize the @(<single-command>) use case is independent of the above, and strikes me as a worthwhile, given that it's probably the most common use of @() by far.
I should say that I've inferred that this use case is currently not optimized solely from timing commands - I haven't looked at the source code.
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|