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

Reject next, break and return with blocks. #3039

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@animalsiknow
Contributor

animalsiknow commented Nov 30, 2015

Statements such as next [] do; end cause a segfault on latest master. This is due to the fact that it is parsed as a block_call, yet call_with_blockcannot handle these nodes. yield with a block was already rejected, so I added a bunch of similar checks for next, break and return.

Reject next, break and return with blocks.
Statements such as `next [] do; end` cause a segfault on latest master.
This is due to the fact that it is parsed as a block_call, yet
call_with_block cannot handle these nodes. `yield` with a block was
already rejected, so I added a bunch of similar checks for `next`,
`break` and `return`.
@animalsiknow

This comment has been minimized.

Show comment
Hide comment
@animalsiknow

animalsiknow Nov 30, 2015

Contributor

It seem it rejects code such as

def a(n)
  n + yield(7)
end

def b
  return a 2 do |x|
      x * 10
  end
end

puts b

that MRI handles correctly.

Contributor

animalsiknow commented Nov 30, 2015

It seem it rejects code such as

def a(n)
  n + yield(7)
end

def b
  return a 2 do |x|
      x * 10
  end
end

puts b

that MRI handles correctly.

@clayton-shopify

This comment has been minimized.

Show comment
Hide comment
@clayton-shopify

clayton-shopify Nov 30, 2015

Contributor

MRI does reject a 2 do |x| following a yield (with "block given to yield") so maybe it's justified for mruby to do so after next, break and return even if it's inconsistent with MRI. It's better than crashing at least.

Contributor

clayton-shopify commented Nov 30, 2015

MRI does reject a 2 do |x| following a yield (with "block given to yield") so maybe it's justified for mruby to do so after next, break and return even if it's inconsistent with MRI. It's better than crashing at least.

@clayton-shopify

This comment has been minimized.

Show comment
Hide comment
@clayton-shopify

clayton-shopify Nov 30, 2015

Contributor

In case they are helpful for testing, some other statements that presently crash mruby:

next a 0 do ; end
break a 0 do ; end
return a 0 do ; end
next {} do ; end
a a+a next yield do ; end

Contributor

clayton-shopify commented Nov 30, 2015

In case they are helpful for testing, some other statements that presently crash mruby:

next a 0 do ; end
break a 0 do ; end
return a 0 do ; end
next {} do ; end
a a+a next yield do ; end

@matz matz closed this in c299651 Dec 1, 2015

@animalsiknow animalsiknow deleted the animalsiknow:reject-next-with-block branch Dec 1, 2015

@tsahara tsahara referenced this pull request Dec 8, 2015

Closed

make December 2015 Stable Release #155

68 of 69 tasks complete

tsahara added a commit to iij/mruby that referenced this pull request Dec 10, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment