GROOVY-8778: Cast short-hand breaks for empty map#792
Closed
paulk-asert wants to merge 1 commit into
Closed
Conversation
daniellansun
reviewed
Sep 8, 2018
| } | ||
| if (mapEntryExpressionList.size() == 0) { | ||
| // expecting list of MapEntryExpressions later so use SpreadMap to smuggle empty MapExpression to later stages | ||
| right = new SpreadMapExpression(configureAST(new MapExpression(), ctx.namedPropertyArgs())); |
Contributor
There was a problem hiding this comment.
It's better to configureAST the instance of SpreadMapExpression too, or its node position will be missing.
Contributor
Author
There was a problem hiding this comment.
I had that originally but we throw that expression away in resolve visitor.
Contributor
There was a problem hiding this comment.
The implementation of parser should decouple with back end(i.e. not rely on the implementation of back end).
If the implementation of resolve visitor changes in the future, the expression is not thrown away, the node position is missing.
Contributor
Author
There was a problem hiding this comment.
Yes, you are right - if we ever do support getAt(Map) more generally, we would need to process that differently, so best it is correct to start with.
paulk-asert
force-pushed
the
groovy8778
branch
from
921fbda to
89a11db
Compare
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
No description provided.