diff --git a/ChangeLog b/ChangeLog index 5d9a54951c..7c4ae76b95 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +Fri Oct 30 11:36:33 2015 Eric Wong + + * variable.c (generic_ivar_remove): adjust type, set valp + (rb_obj_remove_instance_variable): simplify call + * test/ruby/test_object.rb (test_remove_instance_variable): + expand for implementation details + Fri Oct 30 10:37:56 2015 Eric Wong * internal.h (rb_st_insert_id_and_value): update prototype diff --git a/test/ruby/test_object.rb b/test/ruby/test_object.rb index 5ba44c8699..b0e561a206 100644 --- a/test/ruby/test_object.rb +++ b/test/ruby/test_object.rb @@ -232,10 +232,22 @@ class TestObject < Test::Unit::TestCase end def test_remove_instance_variable - o = Object.new - o.instance_eval { @foo = :foo } - o.remove_instance_variable(:@foo) - assert_equal(false, o.instance_variable_defined?(:@foo)) + { 'T_OBJECT' => Object.new, + 'T_CLASS,T_MODULE' => Class.new(Object), + 'generic ivar' => '', + }.each do |desc, o| + assert_raises(NameError, "#{desc} iv removal raises before set") do + o.remove_instance_variable(:@foo) + end + o.instance_eval { @foo = :foo } + assert_equal(:foo, o.remove_instance_variable(:@foo), + "#{desc} iv removal returns original value") + assert_equal(false, o.instance_variable_defined?(:@foo), + "#{desc} iv removed succesfully") + assert_raises(NameError, "#{desc} iv removal raises after removal") do + o.remove_instance_variable(:@foo) + end + end end def test_convert_string diff --git a/variable.c b/variable.c index 42a628e086..5bb6722f79 100644 --- a/variable.c +++ b/variable.c @@ -1142,7 +1142,7 @@ generic_ivar_defined(VALUE obj, ID id) } static int -generic_ivar_remove(VALUE obj, ID id, st_data_t *valp) +generic_ivar_remove(VALUE obj, ID id, VALUE *valp) { struct gen_ivtbl *ivtbl; st_data_t key = (st_data_t)id; @@ -1155,6 +1155,7 @@ generic_ivar_remove(VALUE obj, ID id, st_data_t *valp) if ((long)index < ivtbl->numiv) { if (ivtbl->ivptr[index] != Qundef) { + *valp = ivtbl->ivptr[index]; ivtbl->ivptr[index] = Qundef; return 1; } @@ -1769,9 +1770,8 @@ rb_obj_remove_instance_variable(VALUE obj, VALUE name) break; default: if (FL_TEST(obj, FL_EXIVAR)) { - v = val; - if (generic_ivar_remove(obj, (st_data_t)id, &v)) { - return (VALUE)v; + if (generic_ivar_remove(obj, id, &val)) { + return val; } } break;