Remove expandarray/splatarray sequence peephole optimization #9727
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
newarray, duparray, concatarray, and splatarray always leave an array at the top of the stack. expandarray does not, it takes an array from the top of the stack as input, and leaves individual elements on the stack. I assume no Ruby code generates the expandarray/splatarray sequence, or it could break. The only use of expandarray outside the peephole optimizer is in the masgn code, and it does not appear to generate splatarray directly after expandarray.
The splatarray/splatarray peephole optimization is probably also wrong in the following case:
This instruction sequence should result in a duplicate of [1,2] at the top of the stack, but the peephole optimizer would remove the
splatarray true
, resulting in change that made [1,2] on top of the stack. I'm not sure Ruby code can generatesplatarray false
followed bysplatarray true
(I could get it to generate chains ofsplatarray true
), so maybe this has no effect.newarray, duparray, and concatarray all result in newly allocated arrays at the top of the stack, so they shouldn't have an issue with removing either
splatarray true
orsplatarray false
.