Skip to content

Commit

Permalink
Fix next inside while and until loops
Browse files Browse the repository at this point in the history
Fixes #1337
  • Loading branch information
seven1m committed Nov 4, 2023
1 parent a52b096 commit bdb5d72
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 8 deletions.
1 change: 1 addition & 0 deletions lib/natalie/compiler/instructions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
require_relative './instructions/class_variable_set_instruction'
require_relative './instructions/const_find_instruction'
require_relative './instructions/const_set_instruction'
require_relative './instructions/continue_instruction'
require_relative './instructions/create_array_instruction'
require_relative './instructions/create_hash_instruction'
require_relative './instructions/create_lambda_instruction'
Expand Down
21 changes: 21 additions & 0 deletions lib/natalie/compiler/instructions/continue_instruction.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
require_relative './base_instruction'

module Natalie
class Compiler
# This instruction is really just used for skipping one iteration in a `while` loop.
class ContinueInstruction < BaseInstruction
def to_s
'continue'
end

def generate(transform)
transform.exec('continue')
transform.push_nil
end

def execute(vm)
:continue
end
end
end
end
43 changes: 35 additions & 8 deletions lib/natalie/compiler/pass1.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ def initialize(ast, compiler_context:, macro_expander:)

# Our MacroExpander and our REPL need to know local variables.
@locals_stack = []

# `next` needs to know its enclosing scope type,
# e.g. block vs while loop, so we'll use a stack to keep track.
@next_context = []
end

INLINE_CPP_MACROS = %i[
Expand Down Expand Up @@ -72,7 +76,9 @@ def transform_expression(node, used:)
return transform_expression(node, used: used) unless node.is_a?(::Prism::Node)
@depth += 1 unless node.type == :statements_node
method = "transform_#{node.type}"
result = send(method, node, used: used)
result = track_scope(node) do
send(method, node, used: used)
end
@depth -= 1 unless node.type == :statements_node
Array(result).flatten
when Array
Expand Down Expand Up @@ -104,13 +110,6 @@ def transform_body(body, used:)
instructions
end

def retry_context(id)
@retry_context << id
yield
ensure
@retry_context.pop
end

private

# INDIVIDUAL NODES = = = = =
Expand Down Expand Up @@ -1752,6 +1751,10 @@ def transform_multi_write_node(node, used:)
end

def transform_next_node(node, used:)
if %i[while_node until_node].include?(@next_context.last)
return [ContinueInstruction.new]
end

[
transform_arguments_node_for_returnish(node.arguments, location: node.location),
NextInstruction.new
Expand Down Expand Up @@ -2129,6 +2132,30 @@ def transform_yield_node(node, used:)

# HELPERS = = = = = = = = = = = = =

def retry_context(id)
@retry_context << id
yield
ensure
@retry_context.pop
end

def next_context(node)
case node
when ::Prism::WhileNode, ::Prism::UntilNode, ::Prism::BlockNode, ::Prism::DefNode
@next_context << node.type
result = yield
@next_context.pop
result
else
yield
end
end

def track_scope(...)
# NOTE: we may have other contexts to track here later
next_context(...)
end

# returns a set of [name, is_private, prep_instruction]
# prep_instruction being the instruction(s) needed to get the owner of the constant
def constant_name(name)
Expand Down
28 changes: 28 additions & 0 deletions test/natalie/next_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,32 @@
end
result.should == [0, 0, 3]
end

it 'skips iteration in a while loop' do
# wrapper because next was breaking out of the ^ above enclosing block
wrapper = -> {
a = 3
while a.positive?
a -= 1
next if a == 1
end
a
}

wrapper.().should == 0
end

it 'skips iteration in an until loop' do
# wrapper because next was breaking out of the ^ above enclosing block
wrapper = -> {
a = 3
until a.negative?
a -= 1
next if a == 1
end
a
}

wrapper.().should == -1
end
end

0 comments on commit bdb5d72

Please sign in to comment.