Merge master.

3 files changed, 64 insertions, 75 deletions

diff --git a/mrbgems/mruby-struct/mrblib/struct.rb b/mrbgems/mruby-struct/mrblib/struct.rb
index c5b5354be..7682ac033 100644
--- a/mrbgems/mruby-struct/mrblib/struct.rb
+++ b/mrbgems/mruby-struct/mrblib/struct.rb
@@ -46,7 +46,9 @@ if Object.const_defined?(:Struct)
       ary
     end
 
-    def _inspect
+    def _inspect(recur_list)
+      return "#<struct #{self.class}:...>" if recur_list[self.object_id]
+      recur_list[self.object_id] = true
       name = self.class.to_s
       if name[0] == "#"
         str = "#<struct "
@@ -55,7 +57,7 @@ if Object.const_defined?(:Struct)
       end
       buf = []
       self.each_pair do |k,v|
-        buf.push [k.to_s + "=" + v._inspect]
+        buf.push [k.to_s + "=" + v._inspect(recur_list)]
       end
       str + buf.join(", ") + ">"
     end
@@ -70,11 +72,7 @@ if Object.const_defined?(:Struct)
     # 15.2.18.4.10(x)
     #
     def inspect
-      begin
-        self._inspect
-      rescue SystemStackError
-        "#<struct #{self.class.to_s}:...>"
-      end
+      self._inspect({})
     end
 
     ##
diff --git a/mrbgems/mruby-struct/src/struct.c b/mrbgems/mruby-struct/src/struct.c
index c0ce71219..2d82c2466 100644
--- a/mrbgems/mruby-struct/src/struct.c
+++ b/mrbgems/mruby-struct/src/struct.c
@@ -66,8 +66,8 @@ struct_members(mrb_state *mrb, mrb_value s)
     }
     else {
       mrb_raisef(mrb, E_TYPE_ERROR,
-                 "struct size differs (%S required %S given)",
-                 mrb_fixnum_value(RARRAY_LEN(members)), mrb_fixnum_value(RSTRUCT_LEN(s)));
+                 "struct size differs (%i required %i given)",
+                 RARRAY_LEN(members), RSTRUCT_LEN(s));
     }
   }
   return members;
@@ -87,10 +87,7 @@ mrb_struct_s_members_m(mrb_state *mrb, mrb_value klass)
 static void
 mrb_struct_modify(mrb_state *mrb, mrb_value strct)
 {
-  if (MRB_FROZEN_P(mrb_basic_ptr(strct))) {
-    mrb_raise(mrb, E_FROZEN_ERROR, "can't modify frozen struct");
-  }
-
+  mrb_check_frozen(mrb, mrb_basic_ptr(strct));
   mrb_write_barrier(mrb, mrb_basic_ptr(strct));
 }
 
@@ -126,19 +123,29 @@ mrb_struct_ref(mrb_state *mrb, mrb_value obj)
 static mrb_sym
 mrb_id_attrset(mrb_state *mrb, mrb_sym id)
 {
+#define ONSTACK_ALLOC_MAX 32
+#define ONSTACK_STRLEN_MAX (ONSTACK_ALLOC_MAX - 1) /* '=' character */
+
   const char *name;
   char *buf;
   mrb_int len;
   mrb_sym mid;
+  char onstack[ONSTACK_ALLOC_MAX];
 
-  name = mrb_sym2name_len(mrb, id, &len);
-  buf = (char *)mrb_malloc(mrb, (size_t)len+2);
+  name = mrb_sym_name_len(mrb, id, &len);
+  if (len > ONSTACK_STRLEN_MAX) {
+    buf = (char *)mrb_malloc(mrb, (size_t)len+1);
+  }
+  else {
+    buf = onstack;
+  }
   memcpy(buf, name, (size_t)len);
   buf[len] = '=';
-  buf[len+1] = '\0';
 
   mid = mrb_intern(mrb, buf, len+1);
-  mrb_free(mrb, buf);
+  if (buf != onstack) {
+    mrb_free(mrb, buf);
+  }
   return mid;
 }
 
@@ -161,20 +168,6 @@ mrb_struct_set_m(mrb_state *mrb, mrb_value obj)
   return val;
 }
 
-static mrb_bool
-is_local_id(mrb_state *mrb, const char *name)
-{
-  if (!name) return FALSE;
-  return !ISUPPER(name[0]);
-}
-
-static mrb_bool
-is_const_id(mrb_state *mrb, const char *name)
-{
-  if (!name) return FALSE;
-  return ISUPPER(name[0]);
-}
-
 static void
 make_struct_define_accessors(mrb_state *mrb, mrb_value members, struct RClass *c)
 {
@@ -185,19 +178,15 @@ make_struct_define_accessors(mrb_state *mrb, mrb_value members, struct RClass *c
 
   for (i=0; i<len; i++) {
     mrb_sym id = mrb_symbol(ptr_members[i]);
-    const char *name = mrb_sym2name_len(mrb, id, NULL);
-
-    if (is_local_id(mrb, name) || is_const_id(mrb, name)) {
-      mrb_method_t m;
-      mrb_value at = mrb_fixnum_value(i);
-      struct RProc *aref = mrb_proc_new_cfunc_with_env(mrb, mrb_struct_ref, 1, &at);
-      struct RProc *aset = mrb_proc_new_cfunc_with_env(mrb, mrb_struct_set_m, 1, &at);
-      MRB_METHOD_FROM_PROC(m, aref);
-      mrb_define_method_raw(mrb, c, id, m);
-      MRB_METHOD_FROM_PROC(m, aset);
-      mrb_define_method_raw(mrb, c, mrb_id_attrset(mrb, id), m);
-      mrb_gc_arena_restore(mrb, ai);
-    }
+    mrb_method_t m;
+    mrb_value at = mrb_fixnum_value(i);
+    struct RProc *aref = mrb_proc_new_cfunc_with_env(mrb, mrb_struct_ref, 1, &at);
+    struct RProc *aset = mrb_proc_new_cfunc_with_env(mrb, mrb_struct_set_m, 1, &at);
+    MRB_METHOD_FROM_PROC(m, aref);
+    mrb_define_method_raw(mrb, c, id, m);
+    MRB_METHOD_FROM_PROC(m, aset);
+    mrb_define_method_raw(mrb, c, mrb_id_attrset(mrb, id), m);
+    mrb_gc_arena_restore(mrb, ai);
   }
 }
 
@@ -215,11 +204,11 @@ make_struct(mrb_state *mrb, mrb_value name, mrb_value members, struct RClass *kl
     /* old style: should we warn? */
     mrb_to_str(mrb, name);
     id = mrb_obj_to_sym(mrb, name);
-    if (!is_const_id(mrb, mrb_sym2name_len(mrb, id, NULL))) {
-      mrb_name_error(mrb, id, "identifier %S needs to be constant", name);
+    if (!mrb_const_name_p(mrb, RSTRING_PTR(name), RSTRING_LEN(name))) {
+      mrb_name_error(mrb, id, "identifier %v needs to be constant", name);
     }
     if (mrb_const_defined_at(mrb, mrb_obj_value(klass), id)) {
-      mrb_warn(mrb, "redefining constant Struct::%S", name);
+      mrb_warn(mrb, "redefining constant Struct::%v", name);
       mrb_const_remove(mrb, mrb_obj_value(klass), id);
     }
     c = mrb_define_class_under(mrb, klass, RSTRING_PTR(name), klass);
@@ -399,23 +388,22 @@ struct_aref_sym(mrb_state *mrb, mrb_value obj, mrb_sym id)
       return ptr[i];
     }
   }
-  mrb_name_error(mrb, id, "no member '%S' in struct", mrb_sym2str(mrb, id));
+  mrb_name_error(mrb, id, "no member '%n' in struct", id);
   return mrb_nil_value();       /* not reached */
 }
 
 static mrb_value
 struct_aref_int(mrb_state *mrb, mrb_value s, mrb_int i)
 {
-  if (i < 0) i = RSTRUCT_LEN(s) + i;
-  if (i < 0)
-      mrb_raisef(mrb, E_INDEX_ERROR,
-                 "offset %S too small for struct(size:%S)",
-                 mrb_fixnum_value(i), mrb_fixnum_value(RSTRUCT_LEN(s)));
-  if (RSTRUCT_LEN(s) <= i)
+  mrb_int idx = i < 0 ? RSTRUCT_LEN(s) + i : i;
+
+  if (idx < 0)
+    mrb_raisef(mrb, E_INDEX_ERROR,
+               "offset %i too small for struct(size:%i)", i, RSTRUCT_LEN(s));
+  if (RSTRUCT_LEN(s) <= idx)
     mrb_raisef(mrb, E_INDEX_ERROR,
-               "offset %S too large for struct(size:%S)",
-               mrb_fixnum_value(i), mrb_fixnum_value(RSTRUCT_LEN(s)));
-  return RSTRUCT_PTR(s)[i];
+               "offset %i too large for struct(size:%i)", i, RSTRUCT_LEN(s));
+  return RSTRUCT_PTR(s)[idx];
 }
 
 /* 15.2.18.4.2  */
@@ -447,7 +435,7 @@ mrb_struct_aref(mrb_state *mrb, mrb_value s)
     mrb_value sym = mrb_check_intern_str(mrb, idx);
 
     if (mrb_nil_p(sym)) {
-      mrb_name_error(mrb, mrb_intern_str(mrb, idx), "no member '%S' in struct", idx);
+      mrb_name_error(mrb, mrb_intern_str(mrb, idx), "no member '%v' in struct", idx);
     }
     idx = sym;
   }
@@ -475,7 +463,7 @@ mrb_struct_aset_sym(mrb_state *mrb, mrb_value s, mrb_sym id, mrb_value val)
       return val;
     }
   }
-  mrb_name_error(mrb, id, "no member '%S' in struct", mrb_sym2str(mrb, id));
+  mrb_name_error(mrb, id, "no member '%n' in struct", id);
   return val;                   /* not reach */
 }
 
@@ -514,7 +502,7 @@ mrb_struct_aset(mrb_state *mrb, mrb_value s)
     mrb_value sym = mrb_check_intern_str(mrb, idx);
 
     if (mrb_nil_p(sym)) {
-      mrb_name_error(mrb, mrb_intern_str(mrb, idx), "no member '%S' in struct", idx);
+      mrb_name_error(mrb, mrb_intern_str(mrb, idx), "no member '%v' in struct", idx);
     }
     idx = sym;
   }
@@ -526,13 +514,11 @@ mrb_struct_aset(mrb_state *mrb, mrb_value s)
   if (i < 0) i = RSTRUCT_LEN(s) + i;
   if (i < 0) {
     mrb_raisef(mrb, E_INDEX_ERROR,
-               "offset %S too small for struct(size:%S)",
-               mrb_fixnum_value(i), mrb_fixnum_value(RSTRUCT_LEN(s)));
+               "offset %i too small for struct(size:%i)", i, RSTRUCT_LEN(s));
   }
   if (RSTRUCT_LEN(s) <= i) {
     mrb_raisef(mrb, E_INDEX_ERROR,
-               "offset %S too large for struct(size:%S)",
-               mrb_fixnum_value(i), mrb_fixnum_value(RSTRUCT_LEN(s)));
+               "offset %i too large for struct(size:%i)", i, RSTRUCT_LEN(s));
   }
   mrb_struct_modify(mrb, s);
   return RSTRUCT_PTR(s)[i] = val;
diff --git a/mrbgems/mruby-struct/test/struct.rb b/mrbgems/mruby-struct/test/struct.rb
index c298fef9f..93930730b 100644
--- a/mrbgems/mruby-struct/test/struct.rb
+++ b/mrbgems/mruby-struct/test/struct.rb
@@ -116,9 +116,10 @@ assert('struct dup') do
 end
 
 assert('struct inspect') do
-  c = Struct.new(:m1, :m2, :m3, :m4, :m5)
-  cc = c.new(1,2,3,4,5)
-  assert_equal "#<struct m1=1, m2=2, m3=3, m4=4, m5=5>", cc.inspect
+  c = Struct.new(:m1, :m2, :m3, :m4, :m5, :recur)
+  cc = c.new(1,2,3,4,5,nil)
+  cc.recur = cc
+  assert_equal "#<struct m1=1, m2=2, m3=3, m4=4, m5=5, recur=#<struct #{cc.class}:...>>", cc.inspect
 end
 
 assert('Struct#length, Struct#size') do
@@ -152,14 +153,14 @@ assert("Struct#dig") do
   assert_equal 1, a.dig(1, 0)
 end
 
-assert("Struct.new removes existing constant") do
-  skip "redefining Struct with same name cause warnings"
-  begin
-    assert_not_equal Struct.new("Test", :a), Struct.new("Test", :a, :b)
-  ensure
-    Struct.remove_const :Test
-  end
-end
+# TODO: suppress redefining Struct warning during test
+# assert("Struct.new removes existing constant") do
+#   begin
+#     assert_not_equal Struct.new("Test", :a), Struct.new("Test", :a, :b)
+#   ensure
+#     Struct.remove_const :Test
+#   end
+# end
 
 assert("Struct#initialize_copy requires struct to be the same type") do
   begin
@@ -181,6 +182,10 @@ assert("Struct.new does not allow array") do
   end
 end
 
+assert("Struct.new does not allow invalid class name") do
+  assert_raise(NameError) { Struct.new("Test-", :a) }
+end
+
 assert("Struct.new generates subclass of Struct") do
   begin
     original_struct = Struct