Refactor to_str2

include/natalie/env.hpp

@@ -81,15 +81,7 @@ class Env : public Cell {
     [[noreturn]] void raise_name_error(StringObject *name, String);
     [[noreturn]] void raise_not_comparable_error(Value lhs, Value rhs);
 
-    // Old error message style, e.g.:
-    // - no implicit conversion from nil to string
-    // - no implicit conversion of Integer into String
-    [[noreturn]] void raise_type_error(Value obj, const char *expected);
-
-    // New error message style, e.g.:
-    // - no implicit conversion of nil into String
-    // - no implicit conversion of Integer into String
-    [[noreturn]] void raise_type_error2(Value obj, const char *expected);
+    [[noreturn]] void raise_type_error(Value obj, const char *expected, bool = false);
 
     template <typename... Args>
     [[noreturn]] void raise_name_error(SymbolObject *name, const char *format, Args... args) {

include/natalie/value.hpp

@@ -94,15 +94,7 @@ class Value {
     ClassObject *singleton_class() const;
     ClassObject *singleton_class(Env *);
 
-    // Old error message style, e.g.:
-    // - no implicit conversion from nil to string
-    // - no implicit conversion of Integer into String
-    StringObject *to_str(Env *env);
-
-    // New error message style, e.g.:
-    // - no implicit conversion of nil into String
-    // - no implicit conversion of Integer into String
-    StringObject *to_str2(Env *env);
+    StringObject *to_str(Env *env, bool = false);
 
     Integer integer() const;
     Integer integer_or_raise(Env *) const;

lib/cgi/escape.rb

@@ -50,7 +50,7 @@ def unescape(string, encoding = @@accept_charset)
   #      # => "%27Stop%21%27%20said%20Fred"
   def escapeURIComponent(string)
     # NATALIE ruby converts this in a c extension
-    __inline__ 'string_var = string_var.to_str2(env)'
+    __inline__ 'string_var = string_var.to_str(env, /*mri_variant_error_msg*/ true)'
     # END NATALIE
     encoding = string.encoding
     buffer = string.b
@@ -66,7 +66,7 @@ def escapeURIComponent(string)
   #      # => "'Stop!'+said Fred"
   def unescapeURIComponent(string, encoding = @@accept_charset)
     # NATALIE ruby converts this in a c extension
-    __inline__ 'string_var = string_var.to_str2(env)'
+    __inline__ 'string_var = string_var.to_str(env, /*mri_variant_error_msg*/ true)'
     # END NATALIE
     str = string.b
     str.gsub!(/((?:%[0-9a-fA-F]{2})+)/) do |m|

src/env.cpp

@@ -223,27 +223,25 @@ void Env::raise_not_comparable_error(Value lhs, Value rhs) {
     raise("ArgumentError", std::move(message));
 }
 
-void Env::raise_type_error(const Value obj, const char *expected) {
+// The origin of the mri_variant_error_msg has been lost in the mists of time...
+// But spec depends on them
+void Env::raise_type_error(const Value obj, const char *expected, bool mri_variant_error_msg) {
     if (obj.is_nil()) {
-        auto lowercase_expected = StringObject::create(expected);
-        lowercase_expected->downcase_in_place(this);
-        raise("TypeError", "no implicit conversion from nil to {}", lowercase_expected->string());
-    } else {
-        raise("TypeError", "no implicit conversion of {} into {}", obj.klass()->inspect_module(), expected);
+        if (mri_variant_error_msg) {
+            raise("TypeError", "no implicit conversion of nil into {}", expected);
+        } else {
+            auto lowercase_expected = StringObject::create(expected);
+            lowercase_expected->downcase_in_place(this);
+            raise("TypeError", "no implicit conversion from nil to {}", lowercase_expected->string());
+        }
     }
-}
-
-// This is the same as Env::raise_type_error, but with more consistent error messages.
-// We need both methods because CRuby is inconsistent. :-(
-void Env::raise_type_error2(const Value obj, const char *expected) {
-    if (obj.is_nil())
-        raise("TypeError", "no implicit conversion of nil into {}", expected);
-    else if (obj.is_true())
-        raise("TypeError", "no implicit conversion of true into {}", expected);
-    else if (obj.is_false())
-        raise("TypeError", "no implicit conversion of false into {}", expected);
-    else
-        raise("TypeError", "no implicit conversion of {} into {}", obj.klass()->inspect_module(), expected);
+    if (mri_variant_error_msg) {
+        if (obj.is_true())
+            raise("TypeError", "no implicit conversion of true into {}", expected);
+        if (obj.is_false())
+            raise("TypeError", "no implicit conversion of false into {}", expected);
+    }
+    raise("TypeError", "no implicit conversion of {} into {}", obj.klass()->inspect_module(), expected);
 }
 
 bool Env::has_catch(Value value) const {

src/string_object.cpp

@@ -756,7 +756,7 @@ Value StringObject::byteindex(Env *env, Value needle_obj, Optional<Value> offset
     if (needle_obj.is_regexp())
         return byteindex_regexp_needle(env, this, needle_obj.as_regexp(), offset);
 
-    auto needle = needle_obj.to_str2(env);
+    auto needle = needle_obj.to_str(env, /*mri_variant_error_msg*/ true);
     return byteindex_string_needle(env, this, needle, offset);
 }
 
@@ -773,7 +773,7 @@ Value StringObject::byterindex(Env *env, Value needle_obj, Optional<Value> offse
     if (needle_obj.is_regexp())
         return byteindex_regexp_needle(env, this, needle_obj.as_regexp(), offset, true);
 
-    auto needle = needle_obj.to_str2(env);
+    auto needle = needle_obj.to_str(env, /*mri_variant_error_msg*/ true);
     return byteindex_string_needle(env, this, needle, offset, true);
 }
 

src/value.cpp

@@ -92,35 +92,15 @@ ClassObject *Value::singleton_class(Env *env) {
     return Object::singleton_class(env, *this);
 }
 
-StringObject *Value::to_str(Env *env) {
+// The origin of the mri_variant_error_msg has been lost in the mists of time...
+// But spec depends on them
+StringObject *Value::to_str(Env *env, bool mri_variant_error_msg) {
     if (is_string())
         return as_string();
 
     auto to_str = "to_str"_s;
     if (!respond_to(env, to_str))
-        env->raise_type_error(*this, "String");
-
-    auto result = send(env, to_str);
-
-    if (result.is_string())
-        return result.as_string();
-
-    env->raise(
-        "TypeError", "can't convert {} to String ({}#to_str gives {})",
-        klass()->inspect_module(),
-        klass()->inspect_module(),
-        result.klass()->inspect_module());
-}
-
-// This is just like Value::to_str, but it raises more consistent error messages.
-// We still need the old error messages because CRuby is inconsistent. :-(
-StringObject *Value::to_str2(Env *env) {
-    if (is_string())
-        return as_string();
-
-    auto to_str = "to_str"_s;
-    if (!respond_to(env, to_str))
-        env->raise_type_error2(*this, "String");
+        env->raise_type_error(*this, "String", mri_variant_error_msg);
 
     auto result = send(env, to_str);