Skip to content

Commit

Permalink
Don't evaluate a parameter's default value if unused
Browse files Browse the repository at this point in the history
  • Loading branch information
seven1m committed Nov 16, 2024
1 parent 252e409 commit 30dd854
Show file tree
Hide file tree
Showing 10 changed files with 106 additions and 115 deletions.
12 changes: 10 additions & 2 deletions lib/natalie/compiler/args.rb
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,12 @@ def transform_optional_arg(arg)
raise SyntaxError, "circular argument reference - #{name}"
end

@instructions << ArrayIsEmptyInstruction.new
@instructions << IfInstruction.new
@instructions << @pass.transform_expression(default_value, used: true)
@instructions << ArrayShiftWithDefaultInstruction.new
@instructions << ElseInstruction.new(:if)
@instructions << ArrayShiftInstruction.new
@instructions << EndInstruction.new(:if)
@instructions << variable_set(name)
end

Expand All @@ -183,8 +187,12 @@ def transform_keyword_arg(arg)
@instructions << HashDeleteInstruction.new(arg.name)
@instructions << variable_set(arg.name)
when Prism::OptionalKeywordParameterNode
@instructions << HashHasKeyInstruction.new(arg.name)
@instructions << IfInstruction.new
@instructions << HashDeleteInstruction.new(arg.name)
@instructions << ElseInstruction.new(:if)
@instructions << @pass.transform_expression(arg.value, used: true)
@instructions << HashDeleteWithDefaultInstruction.new(arg.name)
@instructions << EndInstruction.new(:if)
@instructions << variable_set(arg.name)
else
raise "unhandled node: #{arg.inspect}"
Expand Down
5 changes: 2 additions & 3 deletions lib/natalie/compiler/instructions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,10 @@
require_relative './instructions/anonymous_splat_get_instruction'
require_relative './instructions/anonymous_splat_set_instruction'
require_relative './instructions/array_concat_instruction'
require_relative './instructions/array_is_empty_instruction'
require_relative './instructions/array_pop_instruction'
require_relative './instructions/array_pop_with_default_instruction'
require_relative './instructions/array_push_instruction'
require_relative './instructions/array_shift_instruction'
require_relative './instructions/array_shift_with_default_instruction'
require_relative './instructions/array_wrap_instruction'
require_relative './instructions/autoload_const_instruction'
require_relative './instructions/break_instruction'
Expand Down Expand Up @@ -46,7 +45,7 @@
require_relative './instructions/global_variable_get_instruction'
require_relative './instructions/global_variable_set_instruction'
require_relative './instructions/hash_delete_instruction'
require_relative './instructions/hash_delete_with_default_instruction'
require_relative './instructions/hash_has_key_instruction'
require_relative './instructions/hash_merge_instruction'
require_relative './instructions/hash_put_instruction'
require_relative './instructions/if_instruction'
Expand Down
29 changes: 29 additions & 0 deletions lib/natalie/compiler/instructions/array_is_empty_instruction.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
require_relative './base_instruction'

module Natalie
class Compiler
class ArrayIsEmptyInstruction < BaseInstruction
def to_s
'array_is_empty'
end

def generate(transform)
ary = transform.peek
transform.exec_and_push(:is_empty, "bool_object(#{ary}->as_array()->is_empty())")
end

def execute(vm)
ary = vm.peek
vm.push(ary.empty?)
end

def serialize(_)
[instruction_number].pack('C')
end

def self.deserialize(_, _)
new
end
end
end
end

This file was deleted.

This file was deleted.

This file was deleted.

44 changes: 44 additions & 0 deletions lib/natalie/compiler/instructions/hash_has_key_instruction.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
require_relative './base_instruction'

module Natalie
class Compiler
class HashHasKeyInstruction < BaseInstruction
def initialize(key)
raise "expected symbol but got: #{key.inspect}" unless key.is_a?(Symbol)

@key = key
end

def to_s
"hash_has_key #{@key.inspect}"
end

def generate(transform)
hash = transform.peek
transform.exec_and_push(
:has_key,
"bool_object(#{hash}->as_hash()->has_key(env, #{@key.to_s.inspect}_s))"
)
end

def execute(vm)
hash = vm.peek
vm.push(hash.key?(@key))
end

def serialize(rodata)
position = rodata.add(@key.to_s)
[
instruction_number,
position,
].pack('Cw')
end

def self.deserialize(io, rodata)
position = io.read_ber_integer
key = rodata.get(position, convert: :to_sym)
new(key)
end
end
end
end
5 changes: 2 additions & 3 deletions lib/natalie/compiler/instructions/meta.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,10 @@ class Compiler
AnonymousSplatGetInstruction,
AnonymousSplatSetInstruction,
ArrayConcatInstruction,
ArrayIsEmptyInstruction,
ArrayPopInstruction,
ArrayPopWithDefaultInstruction,
ArrayPushInstruction,
ArrayShiftInstruction,
ArrayShiftWithDefaultInstruction,
ArrayWrapInstruction,
AutoloadConstInstruction,
BreakInstruction,
Expand Down Expand Up @@ -49,7 +48,7 @@ class Compiler
GlobalVariableGetInstruction,
GlobalVariableSetInstruction,
HashDeleteInstruction,
HashDeleteWithDefaultInstruction,
HashHasKeyInstruction,
HashMergeInstruction,
HashPutInstruction,
IfInstruction,
Expand Down
8 changes: 0 additions & 8 deletions lib/natalie/compiler/multiple_assignment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -160,14 +160,6 @@ def shift_or_pop_next_arg
end
end

def shift_or_pop_next_arg_with_default
if @consumer.from_side == :left
@instructions << ArrayShiftWithDefaultInstruction.new
else
@instructions << ArrayPopWithDefaultInstruction.new
end
end

# returns a pair of [name, prep_instruction]
# prep_instruction being the instruction(s) needed to get the owner of the constant
def constant_name(name)
Expand Down
19 changes: 19 additions & 0 deletions test/natalie/argument_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,3 +119,22 @@ def keyword_references_positional(a = 1, b: a)
keyword_references_positional(2, b: 3).should == [2, 3]
end
end

describe 'default values' do
def blow_up
raise 'should not be called'
end

def positional_arg_with_default_value_call(a = blow_up)
a
end

def keyword_arg_with_default_value_call(a: blow_up)
a
end

it 'does not evaluate argument default value unless needed' do
-> { positional_arg_with_default_value_call(1) }.should_not raise_error
-> { keyword_arg_with_default_value_call(a: 1) }.should_not raise_error
end
end

0 comments on commit 30dd854

Please sign in to comment.