Remove implicit conversion using to_str method; fix #3854

We have added internal convenience method `__to_str` which
does string type check.

The issue #3854 was fixed but fundamental flaw of lack of stack
depth check along with fibers still remains. Use `MRB_GC_FIXED_ARENA`
for workaround.

include/mruby.h

@@ -854,10 +854,6 @@ typedef const char *mrb_args_format;
 /**
  * Retrieve arguments from mrb_state.
  *
- * When applicable, implicit conversions (such as `to_str`, `to_ary`, `to_hash`) are
- * applied to received arguments.
- * Used inside a function of mrb_func_t type.
- *
  * @param mrb The current MRuby state.
  * @param format [mrb_args_format] is a list of format specifiers
  * @param ... The passing variadic arguments must be a pointer of retrieving type.
@@ -1187,6 +1183,7 @@ MRB_API void mrb_gc_unregister(mrb_state *mrb, mrb_value obj);
 
 MRB_API mrb_value mrb_to_int(mrb_state *mrb, mrb_value val);
 #define mrb_int(mrb, val) mrb_fixnum(mrb_to_int(mrb, val))
+MRB_API mrb_value mrb_to_str(mrb_state *mrb, mrb_value val);
 MRB_API void mrb_check_type(mrb_state *mrb, mrb_value x, enum mrb_vtype t);
 
 typedef enum call_type {

include/mruby/string.h

@@ -311,8 +311,7 @@ MRB_API mrb_value mrb_str_substr(mrb_state *mrb, mrb_value str, mrb_int beg, mrb
  * @param [mrb_value] str Ruby string.
  * @return [mrb_value] A Ruby string.
  */
-MRB_API mrb_value mrb_string_type(mrb_state *mrb, mrb_value str);
-
+MRB_API mrb_value mrb_ensure_string_type(mrb_state *mrb, mrb_value str);
 MRB_API mrb_value mrb_check_string_type(mrb_state *mrb, mrb_value str);
 MRB_API mrb_value mrb_str_new_capa(mrb_state *mrb, size_t capa);
 MRB_API mrb_value mrb_str_buf_new(mrb_state *mrb, size_t capa);

mrbgems/mruby-io/src/file.c

@@ -115,7 +115,7 @@ mrb_file_s_unlink(mrb_state *mrb, mrb_value obj)
   mrb_get_args(mrb, "*", &argv, &argc);
   for (i = 0; i < argc; i++) {
     const char *utf8_path;
-    pathv = mrb_convert_type(mrb, argv[i], MRB_TT_STRING, "String", "to_str");
+    pathv = mrb_ensure_string_type(mrb, argv[i]);
     utf8_path = mrb_string_value_cstr(mrb, &pathv);
     path = mrb_locale_from_utf8(utf8_path, -1);
     if (UNLINK(path) < 0) {

mrbgems/mruby-kernel-ext/src/kernel.c

@@ -141,8 +141,7 @@ mrb_f_float(mrb_state *mrb, mrb_value self)
  *     String(arg)   -> string
  *
  *  Returns <i>arg</i> as an <code>String</code>.
- *
- *  First tries to call its <code>to_str</code> method, then its to_s method.
+ *  converted using <code>to_s</code> method.
  *
  *     String(self)        #=> "main"
  *     String(self.class)  #=> "Object"
@@ -154,10 +153,7 @@ mrb_f_string(mrb_state *mrb, mrb_value self)
   mrb_value arg, tmp;
 
   mrb_get_args(mrb, "o", &arg);
-  tmp = mrb_check_convert_type(mrb, arg, MRB_TT_STRING, "String", "to_str");
-  if (mrb_nil_p(tmp)) {
-    tmp = mrb_check_convert_type(mrb, arg, MRB_TT_STRING, "String", "to_s");
-  }
+  tmp = mrb_convert_type(mrb, arg, MRB_TT_STRING, "String", "to_s");
   return tmp;
 }
 

mrbgems/mruby-string-ext/mrblib/string.rb

@@ -12,8 +12,8 @@ class String
   #     String.try_convert(/re/)      #=> nil
   #
   def self.try_convert(obj)
-    if obj.respond_to?(:to_str)
-      obj.to_str
+    if self === obj
+      obj
     else
       nil
     end
@@ -142,7 +142,7 @@ class String
   #    "abcdef".casecmp("ABCDEF")    #=> 0
   #
   def casecmp(str)
-    self.downcase <=> str.to_str.downcase
+    self.downcase <=> str.__to_str.downcase
   rescue NoMethodError
     nil
   end

mrbgems/mruby-string-ext/src/string.c

@@ -163,7 +163,7 @@ mrb_str_concat_m(mrb_state *mrb, mrb_value self)
   if (mrb_fixnum_p(str))
     str = mrb_fixnum_chr(mrb, str);
   else
-    str = mrb_string_type(mrb, str);
+    str = mrb_ensure_string_type(mrb, str);
   mrb_str_concat(mrb, self, str);
   return self;
 }
@@ -191,7 +191,7 @@ mrb_str_start_with(mrb_state *mrb, mrb_value self)
   for (i = 0; i < argc; i++) {
     size_t len_l, len_r;
     int ai = mrb_gc_arena_save(mrb);
-    sub = mrb_string_type(mrb, argv[i]);
+    sub = mrb_ensure_string_type(mrb, argv[i]);
     mrb_gc_arena_restore(mrb, ai);
     len_l = RSTRING_LEN(self);
     len_r = RSTRING_LEN(sub);
@@ -220,7 +220,7 @@ mrb_str_end_with(mrb_state *mrb, mrb_value self)
   for (i = 0; i < argc; i++) {
     size_t len_l, len_r;
     int ai = mrb_gc_arena_save(mrb);
-    sub = mrb_string_type(mrb, argv[i]);
+    sub = mrb_ensure_string_type(mrb, argv[i]);
     mrb_gc_arena_restore(mrb, ai);
     len_l = RSTRING_LEN(self);
     len_r = RSTRING_LEN(sub);

mrbgems/mruby-string-ext/test/string.rb

@@ -114,12 +114,6 @@ assert('String#concat') do
   assert_equal "Hello World!", "Hello " << "World" << 33
   assert_equal "Hello World!", "Hello ".concat("World").concat(33)
 
-  o = Object.new
-  def o.to_str
-    "to_str"
-  end
-  assert_equal "hi to_str", "hi " << o
-
   assert_raise(TypeError) { "".concat(Object.new) }
 end
 
@@ -128,11 +122,6 @@ assert('String#casecmp') do
   assert_equal 0, "aBcDeF".casecmp("abcdef")
   assert_equal(-1, "abcdef".casecmp("abcdefg"))
   assert_equal 0, "abcdef".casecmp("ABCDEF")
-  o = Object.new
-  def o.to_str
-    "ABCDEF"
-  end
-  assert_equal 0, "abcdef".casecmp(o)
 end
 
 assert('String#count') do

mrblib/string.rb

@@ -12,7 +12,7 @@ class String
   def each_line(rs = "\n", &block)
     return to_enum(:each_line, rs, &block) unless block
     return block.call(self) if rs.nil?
-    rs = rs.to_str
+    rs = rs.__to_str
     offset = 0
     rs_len = rs.length
     this = dup
@@ -67,7 +67,7 @@ class String
       block = nil
     end
     if !replace.nil? || !block
-      replace = replace.to_str
+      replace = replace.__to_str
     end
     offset = 0
     result = []
@@ -129,12 +129,12 @@ class String
     end
 
     pattern, replace = *args
-    pattern = pattern.to_str
+    pattern = pattern.__to_str
     if args.length == 2 && block
       block = nil
     end
     unless block
-      replace = replace.to_str
+      replace = replace.__to_str
     end
     result = []
     this = dup
@@ -245,14 +245,13 @@ class String
   ##
   # ISO 15.2.10.5.3
   def =~(re)
-    raise TypeError, "type mismatch: String given" if re.respond_to? :to_str
     re =~ self
   end
 
   ##
   # ISO 15.2.10.5.27
   def match(re, &block)
-    if re.respond_to? :to_str
+    if String === re
       if Object.const_defined?(:Regexp)
         r = Regexp.new(re)
         r.match(self, &block)

src/class.c

@@ -504,10 +504,17 @@ check_type(mrb_state *mrb, mrb_value val, enum mrb_vtype t, const char *c, const
   return tmp;
 }
 
+#define CHECK_TYPE(mrb, val, t, c) do { \
+  if (mrb_type(val) != (t)) {\
+    mrb_raisef(mrb, E_TYPE_ERROR, "expected %S", mrb_str_new_lit(mrb, c));\
+  }\
+} while (0)
+
 static mrb_value
 to_str(mrb_state *mrb, mrb_value val)
 {
-  return check_type(mrb, val, MRB_TT_STRING, "String", "to_str");
+  CHECK_TYPE(mrb, val, MRB_TT_STRING, "String");
+  return val;
 }
 
 static mrb_value
@@ -1972,7 +1979,7 @@ mrb_mod_const_get(mrb_state *mrb, mrb_value mod)
   }
 
   /* const get with class path string */
-  path = mrb_string_type(mrb, path);
+  path = mrb_ensure_string_type(mrb, path);
   ptr = RSTRING_PTR(path);
   len = RSTRING_LEN(path);
   off = 0;

src/kernel.c

@@ -746,6 +746,7 @@ basic_obj_respond_to(mrb_state *mrb, mrb_value obj, mrb_sym id, int pub)
 {
   return mrb_respond_to(mrb, obj, id);
 }
+
 /* 15.3.1.3.43 */
 /*
  *  call-seq:
@@ -765,45 +766,16 @@ basic_obj_respond_to(mrb_state *mrb, mrb_value obj, mrb_sym id, int pub)
 static mrb_value
 obj_respond_to(mrb_state *mrb, mrb_value self)
 {
-  mrb_value mid;
   mrb_sym id, rtm_id;
-  mrb_bool priv = FALSE, respond_to_p = TRUE;
-
-  mrb_get_args(mrb, "o|b", &mid, &priv);
-
-  if (mrb_symbol_p(mid)) {
-    id = mrb_symbol(mid);
-  }
-  else {
-    mrb_value tmp;
-    if (mrb_string_p(mid)) {
-      tmp = mrb_check_intern_str(mrb, mid);
-    }
-    else {
-      tmp = mrb_check_string_type(mrb, mid);
-      if (mrb_nil_p(tmp)) {
-        tmp = mrb_inspect(mrb, mid);
-        mrb_raisef(mrb, E_TYPE_ERROR, "%S is not a symbol", tmp);
-      }
-      tmp = mrb_check_intern_str(mrb, tmp);
-    }
-    if (mrb_nil_p(tmp)) {
-      respond_to_p = FALSE;
-    }
-    else {
-      id = mrb_symbol(tmp);
-    }
-  }
-
-  if (respond_to_p) {
-    respond_to_p = basic_obj_respond_to(mrb, self, id, !priv);
-  }
+  mrb_bool priv = FALSE, respond_to_p;
 
+  mrb_get_args(mrb, "n|b", &id, &priv);
+  respond_to_p = basic_obj_respond_to(mrb, self, id, !priv);
   if (!respond_to_p) {
     rtm_id = mrb_intern_lit(mrb, "respond_to_missing?");
     if (basic_obj_respond_to(mrb, self, rtm_id, !priv)) {
       mrb_value args[2], v;
-      args[0] = mid;
+      args[0] = mrb_symbol_value(id);
       args[1] = mrb_bool_value(priv);
       v = mrb_funcall_argv(mrb, self, rtm_id, 2, args);
       return mrb_bool_value(mrb_bool(v));
@@ -873,6 +845,7 @@ mrb_init_kernel(mrb_state *mrb)
   mrb_define_method(mrb, krn, "to_s",                       mrb_any_to_s,                    MRB_ARGS_NONE());    /* 15.3.1.3.46 */
   mrb_define_method(mrb, krn, "__case_eqq",                 mrb_obj_ceqq,                    MRB_ARGS_REQ(1));    /* internal */
   mrb_define_method(mrb, krn, "__to_int",                   mrb_to_int,                      MRB_ARGS_NONE()); /* internal */
+  mrb_define_method(mrb, krn, "__to_str",                   mrb_to_str,                      MRB_ARGS_NONE()); /* internal */
 
   mrb_define_method(mrb, krn, "class_defined?",             mrb_krn_class_defined,           MRB_ARGS_REQ(1));
 

src/object.c

@@ -579,6 +579,33 @@ mrb_Float(mrb_state *mrb, mrb_value val)
 }
 #endif
 
+MRB_API mrb_value
+mrb_to_str(mrb_state *mrb, mrb_value val)
+{
+  if (!mrb_string_p(val)) {
+    mrb_value type = inspect_type(mrb, val);
+    mrb_raisef(mrb, E_TYPE_ERROR, "can't convert %S to String", type);
+  }
+  return val;
+}
+
+MRB_API mrb_value
+mrb_ensure_string_type(mrb_state *mrb, mrb_value str)
+{
+  if (!mrb_string_p(str)) {
+    mrb_raisef(mrb, E_TYPE_ERROR, "%S cannot be converted to String",
+               inspect_type(mrb, str));
+  }
+  return str;
+}
+
+MRB_API mrb_value
+mrb_check_string_type(mrb_state *mrb, mrb_value str)
+{
+  if (!mrb_string_p(str)) return mrb_nil_value();
+  return str;
+}
+
 MRB_API mrb_value
 mrb_inspect(mrb_state *mrb, mrb_value obj)
 {

src/string.c

@@ -956,15 +956,7 @@ str_eql(mrb_state *mrb, const mrb_value str1, const mrb_value str2)
 MRB_API mrb_bool
 mrb_str_equal(mrb_state *mrb, mrb_value str1, mrb_value str2)
 {
-  if (mrb_immediate_p(str2)) return FALSE;
-  if (!mrb_string_p(str2)) {
-    if (mrb_nil_p(str2)) return FALSE;
-    if (!mrb_respond_to(mrb, str2, mrb_intern_lit(mrb, "to_str"))) {
-      return FALSE;
-    }
-    str2 = mrb_funcall(mrb, str2, "to_str", 0);
-    return mrb_equal(mrb, str2, str1);
-  }
+  if (!mrb_string_p(str2)) return FALSE;
   return str_eql(mrb, str1, str2);
 }
 
@@ -992,14 +984,8 @@ mrb_str_equal_m(mrb_state *mrb, mrb_value str1)
 MRB_API mrb_value
 mrb_str_to_str(mrb_state *mrb, mrb_value str)
 {
-  mrb_value s;
-
   if (!mrb_string_p(str)) {
-    s = mrb_check_convert_type(mrb, str, MRB_TT_STRING, "String", "to_str");
-    if (mrb_nil_p(s)) {
-      s = mrb_convert_type(mrb, str, MRB_TT_STRING, "String", "to_s");
-    }
-    return s;
+    return mrb_convert_type(mrb, str, MRB_TT_STRING, "String", "to_s");
   }
   return str;
 }
@@ -1714,18 +1700,6 @@ mrb_ptr_to_str(mrb_state *mrb, void *p)
   return mrb_obj_value(p_str);
 }
 
-MRB_API mrb_value
-mrb_string_type(mrb_state *mrb, mrb_value str)
-{
-  return mrb_convert_type(mrb, str, MRB_TT_STRING, "String", "to_str");
-}
-
-MRB_API mrb_value
-mrb_check_string_type(mrb_state *mrb, mrb_value str)
-{
-  return mrb_check_convert_type(mrb, str, MRB_TT_STRING, "String", "to_str");
-}
-
 /* 15.2.10.5.30 */
 /*
  *  call-seq:
@@ -2379,7 +2353,6 @@ mrb_str_to_f(mrb_state *mrb, mrb_value self)
 /*
  *  call-seq:
  *     str.to_s     => str
- *     str.to_str   => str
  *
  *  Returns the receiver.
  */
@@ -2783,7 +2756,6 @@ mrb_init_string(mrb_state *mrb)
 #endif
   mrb_define_method(mrb, s, "to_i",            mrb_str_to_i,            MRB_ARGS_ANY());  /* 15.2.10.5.39 */
   mrb_define_method(mrb, s, "to_s",            mrb_str_to_s,            MRB_ARGS_NONE()); /* 15.2.10.5.40 */
-  mrb_define_method(mrb, s, "to_str",          mrb_str_to_s,            MRB_ARGS_NONE());
   mrb_define_method(mrb, s, "to_sym",          mrb_str_intern,          MRB_ARGS_NONE()); /* 15.2.10.5.41 */
   mrb_define_method(mrb, s, "upcase",          mrb_str_upcase,          MRB_ARGS_NONE()); /* 15.2.10.5.42 */
   mrb_define_method(mrb, s, "upcase!",         mrb_str_upcase_bang,     MRB_ARGS_NONE()); /* 15.2.10.5.43 */

test/t/string.rb

@@ -253,19 +253,6 @@ assert('String#chomp!', '15.2.10.5.10') do
   assert_equal 'abc', e
 end
 
-assert('String#chomp! uses the correct length') do
-  class A
-    def to_str
-      $s.replace("AA")
-      "A"
-    end
-  end
-
-  $s = "AAA"
-  $s.chomp!(A.new)
-  assert_equal $s, "A"
-end
-
 assert('String#chop', '15.2.10.5.11') do
   a = ''.chop
   b = 'abc'.chop