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

Fix break bug in for/foreach loops. #20146

Merged
merged 3 commits into from
Aug 25, 2016
Merged

Fix break bug in for/foreach loops. #20146

merged 3 commits into from
Aug 25, 2016

Conversation

jdconrad
Copy link
Contributor

Break does not currently work in for and foreach loops since the ASM Label wasn't being appropriately passed to child nodes. This is fixed.

@clintongormley -- Thanks for finding this one!

@@ -272,4 +272,34 @@ public void testArrayLoopWithoutCounter() {
null, true
));
}

public void testForWithBreak() {
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe have a test with more than one break in it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding this now.

@jdconrad
Copy link
Contributor Author

@s1monw Updated the tests to reflect multiple break usage.

@s1monw
Copy link
Contributor

s1monw commented Aug 25, 2016

LGTM

@jdconrad
Copy link
Contributor Author

@s1monw Thanks for the review!

@jdconrad jdconrad merged commit 139d3f9 into elastic:master Aug 25, 2016
@jdconrad jdconrad deleted the break branch September 6, 2016 16:44
@clintongormley clintongormley added :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache and removed :Plugin Lang Painless labels Feb 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache v5.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants