variable.c (generic_ivar_remove): return original value

This fixes a bug introduced in r50678
("variable.c: use indices for generic ivars")
and does not affect any released version of Ruby

* 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

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@52378 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

ChangeLog

@@ -1,3 +1,10 @@
+Fri Oct 30 11:36:33 2015  Eric Wong  <e@80x24.org>
+
+	* 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  <e@80x24.org>
 
 	* internal.h (rb_st_insert_id_and_value): update prototype

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

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;