refactor valid instance variable name check

3 files changed, 45 insertions, 60 deletions

diff --git a/include/mruby/variable.h b/include/mruby/variable.h
index 40b43972c..8758114b9 100644
--- a/include/mruby/variable.h
+++ b/include/mruby/variable.h
@@ -39,6 +39,8 @@ MRB_API void mrb_const_set(mrb_state*, mrb_value, mrb_sym, mrb_value);
 MRB_API mrb_bool mrb_const_defined(mrb_state*, mrb_value, mrb_sym);
 MRB_API void mrb_const_remove(mrb_state*, mrb_value, mrb_sym);
 
+MRB_API mrb_bool mrb_iv_p(mrb_state *mrb, mrb_sym sym);
+MRB_API mrb_bool mrb_iv_check(mrb_state *mrb, mrb_sym sym);
 MRB_API mrb_value mrb_obj_iv_get(mrb_state *mrb, struct RObject *obj, mrb_sym sym);
 MRB_API void mrb_obj_iv_set(mrb_state *mrb, struct RObject *obj, mrb_sym sym, mrb_value v);
 MRB_API mrb_bool mrb_obj_iv_defined(mrb_state *mrb, struct RObject *obj, mrb_sym sym);
diff --git a/src/kernel.c b/src/kernel.c
index ed4c0e00a..23dee1da3 100644
--- a/src/kernel.c
+++ b/src/kernel.c
@@ -478,45 +478,6 @@ obj_is_instance_of(mrb_state *mrb, mrb_value self)
   return mrb_bool_value(instance_of_p);
 }
 
-static void
-valid_iv_name(mrb_state *mrb, mrb_sym iv_name_id, const char* s, mrb_int len)
-{
-  if (len < 2 || !(s[0] == '@' && s[1] != '@')) {
-    mrb_name_error(mrb, iv_name_id, "`%S' is not allowed as an instance variable name", mrb_sym2str(mrb, iv_name_id));
-  }
-}
-
-static void
-check_iv_name(mrb_state *mrb, mrb_sym iv_name_id)
-{
-  const char *s;
-  mrb_int len;
-
-  s = mrb_sym2name_len(mrb, iv_name_id, &len);
-  valid_iv_name(mrb, iv_name_id, s, len);
-}
-
-static mrb_sym
-get_valid_iv_sym(mrb_state *mrb, mrb_value iv_name)
-{
-  mrb_sym iv_name_id;
-
-  mrb_assert(mrb_symbol_p(iv_name) || mrb_string_p(iv_name));
-
-  if (mrb_string_p(iv_name)) {
-    char *p = RSTRING_PTR(iv_name);
-    mrb_int l = RSTRING_LEN(iv_name);
-    iv_name_id = mrb_intern(mrb, p, l);
-    valid_iv_name(mrb, iv_name_id, p, l);
-  }
-  else {
-    iv_name_id = mrb_symbol(iv_name);
-    check_iv_name(mrb, iv_name_id);
-  }
-
-  return iv_name_id;
-}
-
 /* 15.3.1.3.20 */
 /*
  *  call-seq:
@@ -538,15 +499,11 @@ get_valid_iv_sym(mrb_state *mrb, mrb_value iv_name)
 static mrb_value
 mrb_obj_ivar_defined(mrb_state *mrb, mrb_value self)
 {
-  mrb_sym mid;
-  mrb_value sym;
-  mrb_bool defined_p;
-
-  mrb_get_args(mrb, "o", &sym);
-  mid = get_valid_iv_sym(mrb, sym);
-  defined_p = mrb_obj_iv_defined(mrb, mrb_obj_ptr(self), mid);
+  mrb_sym sym;
 
-  return mrb_bool_value(defined_p);
+  mrb_get_args(mrb, "n", &sym);
+  mrb_iv_check(mrb, sym);
+  return mrb_bool_value(mrb_iv_defined(mrb, self, sym));
 }
 
 /* 15.3.1.3.21 */
@@ -572,13 +529,11 @@ mrb_obj_ivar_defined(mrb_state *mrb, mrb_value self)
 static mrb_value
 mrb_obj_ivar_get(mrb_state *mrb, mrb_value self)
 {
-  mrb_sym iv_name_id;
-  mrb_value iv_name;
+  mrb_sym iv_name;
 
-  mrb_get_args(mrb, "o", &iv_name);
-
-  iv_name_id = get_valid_iv_sym(mrb, iv_name);
-  return mrb_iv_get(mrb, self, iv_name_id);
+  mrb_get_args(mrb, "n", &iv_name);
+  mrb_iv_check(mrb, iv_name);
+  return mrb_iv_get(mrb, self, iv_name);
 }
 
 /* 15.3.1.3.22 */
@@ -604,13 +559,12 @@ mrb_obj_ivar_get(mrb_state *mrb, mrb_value self)
 static mrb_value
 mrb_obj_ivar_set(mrb_state *mrb, mrb_value self)
 {
-  mrb_sym iv_name_id;
-  mrb_value iv_name, val;
-
-  mrb_get_args(mrb, "oo", &iv_name, &val);
+  mrb_sym iv_name;
+  mrb_value val;
 
-  iv_name_id = get_valid_iv_sym(mrb, iv_name);
-  mrb_iv_set(mrb, self, iv_name_id, val);
+  mrb_get_args(mrb, "no", &iv_name, &val);
+  mrb_iv_check(mrb, iv_name);
+  mrb_iv_set(mrb, self, iv_name, val);
   return val;
 }
 
@@ -911,7 +865,7 @@ mrb_obj_remove_instance_variable(mrb_state *mrb, mrb_value self)
   mrb_value val;
 
   mrb_get_args(mrb, "n", &sym);
-  check_iv_name(mrb, sym);
+  mrb_iv_check(mrb, sym);
   val = mrb_iv_remove(mrb, self, sym);
   if (mrb_undef_p(val)) {
     mrb_name_error(mrb, sym, "instance variable %S not defined", mrb_sym2str(mrb, sym));
diff --git a/src/variable.c b/src/variable.c
index 6318d97fc..948bd1871 100644
--- a/src/variable.c
+++ b/src/variable.c
@@ -542,6 +542,35 @@ mrb_iv_defined(mrb_state *mrb, mrb_value obj, mrb_sym sym)
   return mrb_obj_iv_defined(mrb, mrb_obj_ptr(obj), sym);
 }
 
+MRB_API mrb_bool
+mrb_iv_p(mrb_state *mrb, mrb_sym iv_name)
+{
+  const char *s;
+  mrb_int i, len;
+  size_t j;
+  const char *invalid = "@$!? ";
+
+  s = mrb_sym2name_len(mrb, iv_name, &len);
+  if (len < 2) return FALSE;
+  if (s[0] != '@') return FALSE;
+  if (s[1] == '@') return FALSE;
+  for (i=1; i<len; i++) {
+    char c = s[i];
+    for (j=0; j<sizeof(invalid); j++) {
+      if (c == invalid[j]) return FALSE;
+    }
+  }
+  return TRUE;
+}
+
+MRB_API void
+mrb_iv_check(mrb_state *mrb, mrb_sym iv_name)
+{
+  if (!mrb_iv_p(mrb, iv_name)) {
+    mrb_name_error(mrb, iv_name, "`%S' is not allowed as an instance variable name", mrb_sym2str(mrb, iv_name));
+  }
+}
+
 MRB_API void
 mrb_iv_copy(mrb_state *mrb, mrb_value dest, mrb_value src)
 {