Merge pull request #2339 from natalie-lang/fix-arg-default-eval

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}"

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'

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

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

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,

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)

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