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

NodePattern: Optimize by using placeholders for post processing #6909

Merged
merged 2 commits into from Apr 10, 2019

Conversation

marcandre
Copy link
Contributor

@marcandre marcandre commented Apr 8, 2019

The improved handling of ... had to use some post-processing of the cur_node (#6843)
The easiest way to provide other "multiple match" patterns (e.g. #6841) would involve similar post-processing. This PR paves the way by using post processing of the current node pattern everywhere.

This as other multiple benefits:

  • deals automatically and better with generated temporary variables
  • much less argument passing to the various compiling methods
  • factorizes the #{'.type' if seq_head} pattern that was everywhere.
  • makes debugging easier with simpler & nicer generated code

I had hoped to be able to minimize the number of node guards, but unions make it tricky.

For the record, here's an example of the code generated before & after.

### Pattern:
($_ {:a :b} nil?)

### Before:
(temp1 = node0; temp1 = temp1;temp1.is_a?(RuboCop::AST::Node) &&
(capture1 = temp1.type; true) &&
(temp2 = temp1.children[0]; temp2 = temp2;(temp2 == :a) || (temp2 == :b)) &&
(temp1.children[1].nil?) &&
(temp1.children.size == 2))

### After:
node0.is_a?(RuboCop::AST::Node) &&
(capture1 = node0.type; true) &&
((temp1 = node0.children[0]) == :a || temp1 == :b) &&
node0.children[1].nil? &&
node0.children.size == 2

@marcandre marcandre force-pushed the current branch 2 times, most recently from 73eb530 to 49faee1 Compare April 9, 2019 06:52
This as multiple benefits:
- allows more flexible handling of future multiple match patterns.
  Handling of '...' was already using a less flexible post-processing.
- deals automatically and better with generated temporary variables
- much less argument passing to the various compiling methods
- factorizes the `#{'.type' if seq_head}` pattern that was everywhere.
- makes debugging easier with simpler & nicer generated code
@bbatsov
Copy link
Collaborator

bbatsov commented Apr 10, 2019

Fantastic work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants