Fix pp when passed a empty ruby2_keywords-flagged hash as array element #2966
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This causes problems because the hash is passed to a block not accepting keywords. Because the hash is empty and keyword flagged, it is removed before calling the block. This doesn't cause an ArgumentError because it is a block and not a lambda. Just like any other block not passed required arguments, arguments not passed are set to nil. Issues like this are a strong reason not to have ruby2_keywords by default. Fixes [Bug #16519] This backports 28d31ea and 0ea759e, but needed to be modified for 2.7 as 2.7 will perform empty keyword to positional hash conversion for required arguments, which will happen if "v" in the seplist method is empty when yielded.
eregon
reviewed
Mar 19, 2020
| yield(*v) | ||
| case v.last | ||
| when Hash | ||
| if Hash.ruby2_keywords_hash?(v.last) |
There was a problem hiding this comment.
Doesn't Hash.ruby2_keywords_hash? allow any object?
If so it could be simplified to:
if Hash.ruby2_keywords_hash?(v.last)
yield(*v, **{})
else
yield(*v)
endThere was a problem hiding this comment.
No, I tried that first.
There was a problem hiding this comment.
Mmh, that's really unfortunate, I'll file an issue for it.
There was a problem hiding this comment.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This causes problems because the hash is passed to a block not
accepting keywords. Because the hash is empty and keyword flagged,
it is removed before calling the block. This doesn't cause an
ArgumentError because it is a block and not a lambda. Just like
any other block not passed required arguments, arguments not
passed are set to nil.
Issues like this are a strong reason not to have ruby2_keywords
by default.
Fixes [Bug #16519]
This backports 28d31ea and
0ea759e, but needed to be modified
for 2.7 as 2.7 will perform empty keyword to positional hash
conversion for required arguments, which will happen if "v" in the
seplist method is empty when yielded.