Manage skipping instance variable IDs in one place

Merged: https://github.com/ruby/ruby/pull/12923

ext/-test-/marshal/internal_ivar/internal_ivar.c

@@ -1,13 +1,14 @@
 #include <ruby.h>
 
-static ID id_normal_ivar, id_internal_ivar, id_encoding_short;
+static ID id_normal_ivar, id_internal_ivar, id_encoding_short, id_encoding_long;
 
 static VALUE
-init(VALUE self, VALUE arg1, VALUE arg2, VALUE arg3)
+init(VALUE self, VALUE arg1, VALUE arg2, VALUE arg3, VALUE arg4)
 {
     rb_ivar_set(self, id_normal_ivar, arg1);
     rb_ivar_set(self, id_internal_ivar, arg2);
     rb_ivar_set(self, id_encoding_short, arg3);
+    rb_ivar_set(self, id_encoding_long, arg4);
     return self;
 }
 
@@ -29,6 +30,12 @@ get_encoding_short(VALUE self)
     return rb_attr_get(self, id_encoding_short);
 }
 
+static VALUE
+get_encoding_long(VALUE self)
+{
+    return rb_attr_get(self, id_encoding_long);
+}
+
 void
 Init_internal_ivar(void)
 {
@@ -38,8 +45,10 @@ Init_internal_ivar(void)
     id_normal_ivar = rb_intern_const("normal");
     id_internal_ivar = rb_intern_const("K");
     id_encoding_short = rb_intern_const("E");
-    rb_define_method(newclass, "initialize", init, 3);
+    id_encoding_long = rb_intern_const("encoding");
+    rb_define_method(newclass, "initialize", init, 4);
     rb_define_method(newclass, "normal", get_normal, 0);
     rb_define_method(newclass, "internal", get_internal, 0);
     rb_define_method(newclass, "encoding_short", get_encoding_short, 0);
+    rb_define_method(newclass, "encoding_long", get_encoding_long, 0);
 }

marshal.c

@@ -100,6 +100,7 @@ static ID s_dump, s_load, s_mdump, s_mload;
 static ID s_dump_data, s_load_data, s_alloc, s_call;
 static ID s_getbyte, s_read, s_write, s_binmode;
 static ID s_encoding_short, s_ruby2_keywords_flag;
+#define s_encoding_long rb_id_encoding()
 
 #define name_s_dump	"_dump"
 #define name_s_load	"_load"
@@ -114,6 +115,7 @@ static ID s_encoding_short, s_ruby2_keywords_flag;
 #define name_s_write	"write"
 #define name_s_binmode	"binmode"
 #define name_s_encoding_short "E"
+#define name_s_encoding_long "encoding"
 #define name_s_ruby2_keywords_flag "K"
 
 typedef struct {
@@ -581,13 +583,21 @@ rb_hash_ruby2_keywords(VALUE obj)
     RHASH(obj)->basic.flags |= RHASH_PASS_AS_KEYWORDS;
 }
 
-static inline bool
+/*
-to_be_skipped_id(const ID id)
+ * if instance variable name `id` is a special name to be skipped,
+ * returns the name of it.  otherwise it cannot be dumped (unnamed),
+ * returns `name` as-is.  returns NULL for ID that can be dumped.
+ */
+static inline const char *
+skipping_ivar_name(const ID id, const char *name)
 {
-    if (id == s_encoding_short) return true;
+#define IS_SKIPPED_IVAR(idname) \
-    if (id == s_ruby2_keywords_flag) return true;
+    ((id == idname) && (name = name_##idname, true))
-    if (id == rb_id_encoding()) return true;
+    if (IS_SKIPPED_IVAR(s_encoding_short)) return name;
-    return !rb_id2str(id);
+    if (IS_SKIPPED_IVAR(s_ruby2_keywords_flag)) return name;
+    if (IS_SKIPPED_IVAR(s_encoding_long)) return name;
+    if (!rb_id2str(id)) return name;
+    return NULL;
 }
 
 struct w_ivar_arg {
@@ -600,15 +610,12 @@ w_obj_each(ID id, VALUE value, st_data_t a)
 {
     struct w_ivar_arg *ivarg = (struct w_ivar_arg *)a;
     struct dump_call_arg *arg = ivarg->dump;
+    const char unnamed[] = "", *ivname = skipping_ivar_name(id, unnamed);
 
-    if (to_be_skipped_id(id)) {
+    if (ivname) {
-        if (id == s_encoding_short) {
+        if (ivname != unnamed) {
-            rb_warn("instance variable '"name_s_encoding_short"' on class %"PRIsVALUE" is not dumped",
+            rb_warn("instance variable '%s' on class %"PRIsVALUE" is not dumped",
-                    CLASS_OF(arg->obj));
+                    ivname, CLASS_OF(arg->obj));
-        }
-        if (id == s_ruby2_keywords_flag) {
-            rb_warn("instance variable '"name_s_ruby2_keywords_flag"' on class %"PRIsVALUE" is not dumped",
-                    CLASS_OF(arg->obj));
         }
         return ST_CONTINUE;
     }
@@ -621,7 +628,7 @@ w_obj_each(ID id, VALUE value, st_data_t a)
 static int
 obj_count_ivars(ID id, VALUE val, st_data_t a)
 {
-    if (!to_be_skipped_id(id) && UNLIKELY(!++*(st_index_t *)a)) {
+    if (!skipping_ivar_name(id, "") && UNLIKELY(!++*(st_index_t *)a)) {
         rb_raise(rb_eRuntimeError, "too many instance variables");
     }
     return ST_CONTINUE;

test/-ext-/marshal/test_internal_ivar.rb

@@ -7,11 +7,16 @@ module Bug end
 module Bug::Marshal
   class TestInternalIVar < Test::Unit::TestCase
     def test_marshal
-      v = InternalIVar.new("hello", "world", "bye")
+      v = InternalIVar.new("hello", "world", "bye", "hi")
       assert_equal("hello", v.normal)
       assert_equal("world", v.internal)
       assert_equal("bye", v.encoding_short)
-      dump = assert_warn(/instance variable 'E' on class \S+ is not dumped/) {
+      assert_equal("hi", v.encoding_long)
+      warnings = ->(s) {
+        w = s.scan(/instance variable '(.+?)' on class \S+ is not dumped/)
+        assert_equal(%w[E K encoding], w.flatten.sort)
+      }
+      dump = assert_warn(warnings) {
         ::Marshal.dump(v)
       }
       v = assert_nothing_raised {break ::Marshal.load(dump)}
@@ -19,6 +24,7 @@ module Bug::Marshal
       assert_equal("hello", v.normal)
       assert_nil(v.internal)
       assert_nil(v.encoding_short)
+      assert_nil(v.encoding_long)
     end
   end
 end