Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove loop counter from Reserved in Painless AST #45298

Merged
merged 1 commit into from
Aug 8, 2019

Conversation

jdconrad
Copy link
Contributor

@jdconrad jdconrad commented Aug 7, 2019

This change adds a compiler pass to give each node the chance to store settings necessary for analysis and writing. This removes the need to pass this in a somewhat convoluted way through an additional class called Reserved, and also removes the need to have the Walker set values for settings on reserved. This is next step in decoupling the Painless grammar from the Painless AST.

Note the length of this review is long because most nodes require a bit of boilerplate code that cannot be avoided in the current design. Nodes to pay attention to are SSource, SFunction, ELambda, and ENewArrayFunctionRef.

@jdconrad jdconrad added :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >refactoring v8.0.0 v7.4.0 labels Aug 7, 2019
@jdconrad jdconrad requested a review from rjernst August 7, 2019 17:54
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jdconrad jdconrad merged commit c57f6be into elastic:master Aug 8, 2019
@jdconrad
Copy link
Contributor Author

jdconrad commented Aug 8, 2019

@rjernst Thanks for the review!

jdconrad added a commit that referenced this pull request Aug 8, 2019
This change adds a compiler pass to give each node the chance to store 
settings necessary for analysis and writing. This removes the need to pass 
this in a somewhat convoluted way through an additional class called 
Reserved, and also removes the need to have the Walker set values for 
settings on reserved. This is next step in decoupling the Painless grammar 
from the Painless AST.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >refactoring v7.4.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants