Add location to result from macro method interpretation #15213
+6
−1
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.
When calling a macro method, the result is typically a newly created AST node. So in most cases, the location of the AST node should be the location of the call. There are some exceptions though. Some methods don't create new nodes, they are just getters for pre-existing ones (for example:
Def#body
returns the AST node of the body).So if the returned node already has a location, we should not override it. Pre-existing nodes are typically created from parsing source code and thus should be expected to have a location.
I did not verify this for all cases, so this change has some potential for injecting a false location into an existing node becuase we're mutating the stored instance without cloning it. But I don't think there's a high risk for this, nor would this have a major effect. An incorrect location is worse than no location, but not much. And the fix for both is identical: make sure the correct location is used when the node is created.
This fixes the immediate bug in #15202, not the deeper issue with AST nodes missing location info being passed to LLVM.