Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Semantic: disallow usage of return/break/next inside ensure (#4486)
* Semantic: refactor control statement inside ensure detection

Not use Array for stack and add utility method `with_block_kind`, by
#4486 (comment).
Correct `return` statement error message to fit other `return` statement
messages.

* Remove URL from error messages

#4486 (comment)
  • Loading branch information
makenowjust authored and Martin Verzilli committed Sep 16, 2017
1 parent b14d8c7 commit 9965a9b
Show file tree
Hide file tree
Showing 2 changed files with 231 additions and 3 deletions.
178 changes: 178 additions & 0 deletions spec/compiler/semantic/exception_spec.cr
Expand Up @@ -476,4 +476,182 @@ describe "Semantic: exception" do
program = result.program
program.vars["foo"].type.should be(program.nilable program.int32)
end

it "can't return from ensure (#4470)" do
assert_error(%(
def foo
return 1
ensure
return 2
end
foo
), "can't return from ensure")
end

it "can't return from block inside ensure (#4470)" do
assert_error(%(
def once
yield
end
def foo
return 1
ensure
once do
return 2
end
end
foo
), "can't return from ensure")
end

it "can't return from while inside ensure (#4470)" do
assert_error(%(
def foo
return 1
ensure
while true
return 2
end
end
foo
), "can't return from ensure")
end

it "can't use break inside while inside ensure (#4470)" do
assert_error(%(
while true
begin
break
ensure
break
end
end
), "can't use break inside ensure")
end

it "can use break inside while inside ensure (#4470)" do
assert_type(%(
while true
begin
break
ensure
while true
break
end
end
end
)) { nil_type }
end

it "can't use break inside block inside ensure (#4470)" do
assert_error(%(
def loop
while true
yield
end
end
loop do
begin
break
ensure
break
end
end
), "can't use break inside ensure")
end

it "can use break inside block inside ensure (#4470)" do
assert_type(%(
def loop
while true
yield
end
end
loop do
begin
break
ensure
loop do
break
end
end
end
)) { nil_type }
end

it "can't use next inside while inside ensure (#4470)" do
assert_error(%(
while true
begin
break
ensure
next
end
end
), "can't use next inside ensure")
end

it "can't use next inside block inside ensure (#4470)" do
assert_error(%(
def loop
while true
yield
end
end
loop do
begin
break
ensure
next
end
end
), "can't use next inside ensure")
end

it "can use next inside while inside ensure (#4470)" do
assert_type(%(
while true
begin
break
ensure
a = 0
while a < 1
a = 1
next
end
end
end
)) { nil_type }
end

it "can use next inside block inside ensure (#4470)" do
assert_type(%(
def loop
while true
yield
end
end
def once
yield
end
loop do
begin
break
ensure
once do
next
end
end
end
)) { nil_type }
end
end
56 changes: 53 additions & 3 deletions src/compiler/crystal/semantic/main_visitor.cr
Expand Up @@ -67,6 +67,26 @@ module Crystal
property is_initialize : Bool
property exception_handler_vars : MetaVars? = nil

# It means the last block kind, that is one of `block`, `while` and
# `ensure`. It is used to detect `break` or `next` from `ensure`.
#
# ```
# begin
# # `last_block_kind == nil`
# ensure
# # `last_block_kind == :ensure`
# while true
# # `last_block_kind == :while`
# end
# loop do
# # `last_block_kind == :block`
# end
# # `last_block_kind == :ensure`
# end
# ```
property last_block_kind : Symbol?
property? inside_ensure : Bool = false

@unreachable = false
@is_initialize = false
@in_type_args = 0
Expand Down Expand Up @@ -1031,6 +1051,9 @@ module Crystal
block_visitor.path_lookup = path_lookup || current_type
block_visitor.block_nest = @block_nest

block_visitor.last_block_kind = :block
block_visitor.inside_ensure = inside_ensure?

node.body.accept block_visitor

@block_nest -= 1
Expand Down Expand Up @@ -1628,6 +1651,10 @@ module Crystal
end

def visit(node : Return)
if inside_ensure?
node.raise "can't return from ensure"
end

typed_def = @typed_def || node.raise("can't return from top level")

if typed_def.captured_block?
Expand Down Expand Up @@ -1985,7 +2012,10 @@ module Crystal
@block, old_block = nil, @block

@while_stack.push node
node.body.accept self

with_block_kind :while do
node.body.accept self
end

endless_while = node.cond.true_literal?
merge_while_vars node.cond, endless_while, before_cond_vars_copy, before_cond_vars, after_cond_vars, @vars, node.break_vars
Expand Down Expand Up @@ -2149,6 +2179,10 @@ module Crystal
end

def end_visit(node : Break)
if last_block_kind == :ensure
node.raise "can't use break inside ensure"
end

if block = @block
node.target = block.call.not_nil!

Expand All @@ -2174,6 +2208,10 @@ module Crystal
end

def end_visit(node : Next)
if last_block_kind == :ensure
node.raise "can't use next inside ensure"
end

if block = @block
node.target = block

Expand All @@ -2200,6 +2238,14 @@ module Crystal
@unreachable = true
end

def with_block_kind(kind)
old_block_kind, @last_block_kind = last_block_kind, kind
old_inside_ensure, @inside_ensure = @inside_ensure, @inside_ensure || kind == :ensure
yield
@last_block_kind = old_block_kind
@inside_ensure = old_inside_ensure
end

def visit(node : Primitive)
case node.name
when "binary"
Expand Down Expand Up @@ -2599,7 +2645,9 @@ module Crystal
merge_rescue_vars exception_handler_vars, all_rescue_vars

# And then accept the ensure part
node.ensure.try &.accept self
with_block_kind :ensure do
node.ensure.try &.accept self
end
end
end

Expand All @@ -2619,7 +2667,9 @@ module Crystal

before_ensure_vars = @vars.dup

node_ensure.accept self
with_block_kind :ensure do
node_ensure.accept self
end

@vars = after_handler_vars

Expand Down

0 comments on commit 9965a9b

Please sign in to comment.