From 30dd8548dbb306101fdfd9267a92a60cb302b2bc Mon Sep 17 00:00:00 2001 From: Tim Morgan Date: Sat, 16 Nov 2024 09:18:19 -0600 Subject: [PATCH] Don't evaluate a parameter's default value if unused --- lib/natalie/compiler/args.rb | 12 ++++- lib/natalie/compiler/instructions.rb | 5 +-- .../array_is_empty_instruction.rb | 29 ++++++++++++ .../array_pop_with_default_instruction.rb | 24 ---------- .../array_shift_with_default_instruction.rb | 32 -------------- .../hash_delete_with_default_instruction.rb | 43 ------------------ .../instructions/hash_has_key_instruction.rb | 44 +++++++++++++++++++ lib/natalie/compiler/instructions/meta.rb | 5 +-- lib/natalie/compiler/multiple_assignment.rb | 8 ---- test/natalie/argument_test.rb | 19 ++++++++ 10 files changed, 106 insertions(+), 115 deletions(-) create mode 100644 lib/natalie/compiler/instructions/array_is_empty_instruction.rb delete mode 100644 lib/natalie/compiler/instructions/array_pop_with_default_instruction.rb delete mode 100644 lib/natalie/compiler/instructions/array_shift_with_default_instruction.rb delete mode 100644 lib/natalie/compiler/instructions/hash_delete_with_default_instruction.rb create mode 100644 lib/natalie/compiler/instructions/hash_has_key_instruction.rb diff --git a/lib/natalie/compiler/args.rb b/lib/natalie/compiler/args.rb index 803eb3644..aee4fe002 100644 --- a/lib/natalie/compiler/args.rb +++ b/lib/natalie/compiler/args.rb @@ -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 @@ -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}" diff --git a/lib/natalie/compiler/instructions.rb b/lib/natalie/compiler/instructions.rb index b5404c0d5..ad99a358e 100644 --- a/lib/natalie/compiler/instructions.rb +++ b/lib/natalie/compiler/instructions.rb @@ -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' @@ -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' diff --git a/lib/natalie/compiler/instructions/array_is_empty_instruction.rb b/lib/natalie/compiler/instructions/array_is_empty_instruction.rb new file mode 100644 index 000000000..600da5fb2 --- /dev/null +++ b/lib/natalie/compiler/instructions/array_is_empty_instruction.rb @@ -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 diff --git a/lib/natalie/compiler/instructions/array_pop_with_default_instruction.rb b/lib/natalie/compiler/instructions/array_pop_with_default_instruction.rb deleted file mode 100644 index ed0fecfa6..000000000 --- a/lib/natalie/compiler/instructions/array_pop_with_default_instruction.rb +++ /dev/null @@ -1,24 +0,0 @@ -require_relative './base_instruction' - -module Natalie - class Compiler - class ArrayPopWithDefaultInstruction < BaseInstruction - def to_s - 'array_pop_with_default' - end - - def generate(transform) - default = transform.pop - ary = transform.memoize(:ary, transform.peek) - code = "(#{ary}->as_array()->is_empty() ? #{default} : #{ary}->as_array()->pop())" - transform.exec_and_push(:last_item_of_array, code) - end - - def execute(vm) - default = vm.pop - ary = vm.peek - vm.push(ary.empty? ? default : ary.pop) - end - end - end -end diff --git a/lib/natalie/compiler/instructions/array_shift_with_default_instruction.rb b/lib/natalie/compiler/instructions/array_shift_with_default_instruction.rb deleted file mode 100644 index 267d0f53d..000000000 --- a/lib/natalie/compiler/instructions/array_shift_with_default_instruction.rb +++ /dev/null @@ -1,32 +0,0 @@ -require_relative './base_instruction' - -module Natalie - class Compiler - class ArrayShiftWithDefaultInstruction < BaseInstruction - def to_s - 'array_shift_with_default' - end - - def generate(transform) - default = transform.pop - ary = transform.memoize(:ary, transform.peek) - code = "(#{ary}->as_array()->is_empty() ? #{default} : #{ary}->as_array()->shift())" - transform.exec_and_push(:first_item_of_array, code) - end - - def execute(vm) - default = vm.pop - ary = vm.peek - vm.push(ary.empty? ? default : ary.shift) - end - - def serialize(_) - [instruction_number].pack('C') - end - - def self.deserialize(_, _) - new - end - end - end -end diff --git a/lib/natalie/compiler/instructions/hash_delete_with_default_instruction.rb b/lib/natalie/compiler/instructions/hash_delete_with_default_instruction.rb deleted file mode 100644 index d94caeb74..000000000 --- a/lib/natalie/compiler/instructions/hash_delete_with_default_instruction.rb +++ /dev/null @@ -1,43 +0,0 @@ -require_relative './base_instruction' - -module Natalie - class Compiler - class HashDeleteWithDefaultInstruction < BaseInstruction - def initialize(name) - @name = name.to_sym - end - - def to_s - "hash_delete_with_default #{@name}" - end - - def generate(transform) - default = transform.pop - hash = transform.memoize(:hash, transform.peek) - name_sym = transform.intern(@name) - code = "(#{hash}->as_hash()->has_key(env, #{name_sym}) ? #{hash}->as_hash()->remove(env, #{name_sym}) : #{default})" - transform.exec_and_push(:value_removed_from_hash, code) - end - - def execute(vm) - default = vm.pop - hash = vm.peek - vm.push(hash.key?(@name) ? hash.delete(@name) : default) - end - - def serialize(rodata) - position = rodata.add(@name.to_s) - [ - instruction_number, - position, - ].pack('Cw') - end - - def self.deserialize(io, rodata) - position = io.read_ber_integer - name = rodata.get(position, convert: :to_sym) - new(name) - end - end - end -end diff --git a/lib/natalie/compiler/instructions/hash_has_key_instruction.rb b/lib/natalie/compiler/instructions/hash_has_key_instruction.rb new file mode 100644 index 000000000..42eb35937 --- /dev/null +++ b/lib/natalie/compiler/instructions/hash_has_key_instruction.rb @@ -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 diff --git a/lib/natalie/compiler/instructions/meta.rb b/lib/natalie/compiler/instructions/meta.rb index 34e0a739c..0074722a2 100644 --- a/lib/natalie/compiler/instructions/meta.rb +++ b/lib/natalie/compiler/instructions/meta.rb @@ -10,11 +10,10 @@ class Compiler AnonymousSplatGetInstruction, AnonymousSplatSetInstruction, ArrayConcatInstruction, + ArrayIsEmptyInstruction, ArrayPopInstruction, - ArrayPopWithDefaultInstruction, ArrayPushInstruction, ArrayShiftInstruction, - ArrayShiftWithDefaultInstruction, ArrayWrapInstruction, AutoloadConstInstruction, BreakInstruction, @@ -49,7 +48,7 @@ class Compiler GlobalVariableGetInstruction, GlobalVariableSetInstruction, HashDeleteInstruction, - HashDeleteWithDefaultInstruction, + HashHasKeyInstruction, HashMergeInstruction, HashPutInstruction, IfInstruction, diff --git a/lib/natalie/compiler/multiple_assignment.rb b/lib/natalie/compiler/multiple_assignment.rb index 0d2203c31..ea0ae0248 100644 --- a/lib/natalie/compiler/multiple_assignment.rb +++ b/lib/natalie/compiler/multiple_assignment.rb @@ -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) diff --git a/test/natalie/argument_test.rb b/test/natalie/argument_test.rb index 7caea371d..581d70353 100644 --- a/test/natalie/argument_test.rb +++ b/test/natalie/argument_test.rb @@ -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