Merge pull request #2587 from herwinw/kernel_complex_string

Implement parser for Kernel.Complex with String argument

include/natalie/kernel_module.hpp

@@ -25,6 +25,7 @@ class KernelModule {
     static Value catch_method(Env *, Value = nullptr, Block * = nullptr);
     static Value Complex(Env *env, Value real, Value imaginary, Value exception);
     static Value Complex(Env *env, Value real, Value imaginary, bool exception = true);
+    static Value Complex(Env *env, StringObject *real, Value imaginary, bool exception);
     static Value cur_callee(Env *env);
     static Value cur_dir(Env *env);
     static Value exit(Env *env, Value status);

spec/core/kernel/Complex_spec.rb

@@ -73,19 +73,15 @@
       end
 
       it "raises ArgumentError for unrecognised Strings" do
-        NATFIXME 'raises ArgumentError for unrecognised Strings', exception: SpecFailedException do
-          -> {
-            Complex("ruby")
-          }.should raise_error(ArgumentError, 'invalid value for convert(): "ruby"')
-        end
+        -> {
+          Complex("ruby")
+        }.should raise_error(ArgumentError, 'invalid value for convert(): "ruby"')
       end
 
       it "raises ArgumentError for trailing garbage" do
-        NATFIXME 'raises ArgumentError for trailing garbage', exception: SpecFailedException do
-          -> {
-            Complex("79+4iruby")
-          }.should raise_error(ArgumentError, 'invalid value for convert(): "79+4iruby"')
-        end
+        -> {
+          Complex("79+4iruby")
+        }.should raise_error(ArgumentError, 'invalid value for convert(): "79+4iruby"')
       end
 
       it "does not understand Float::INFINITY" do
@@ -101,11 +97,9 @@
       end
 
       it "does not understand Float::NAN" do
-        NATFIXME 'does not understand Float::NAN', exception: SpecFailedException do
-          -> {
-            Complex("NaN")
-          }.should raise_error(ArgumentError, 'invalid value for convert(): "NaN"')
-        end
+        -> {
+          Complex("NaN")
+        }.should raise_error(ArgumentError, 'invalid value for convert(): "NaN"')
       end
 
       it "does not understand a sequence of _" do
@@ -117,11 +111,9 @@
       end
 
       it "does not allow null-byte" do
-        NATFIXME 'does not allow null-byte', exception: SpecFailedException do
-          -> {
-            Complex("1-2i\0")
-          }.should raise_error(ArgumentError, "string contains null byte")
-        end
+        -> {
+          Complex("1-2i\0")
+        }.should raise_error(ArgumentError, "string contains null byte")
       end
     end
 
@@ -135,28 +127,22 @@
       end
 
       it "returns nil for unrecognised Strings" do
-        NATFIXME 'returns nil for unrecognised Strings', exception: SpecFailedException do
-          Complex("ruby", exception: false).should == nil
-        end
+        Complex("ruby", exception: false).should == nil
       end
 
       it "returns nil when trailing garbage" do
-        NATFIXME 'returns nil when trailing garbage', exception: NoMethodError, message: "undefined method 'real' for nil" do
-          Complex("79+4iruby", exception: false).should == nil
-        end
+        Complex("79+4iruby", exception: false).should == nil
       end
 
       it "returns nil for Float::INFINITY" do
-        NATFIXME 'returns nil for Float::INFINITY', exception: NoMethodError, message: "undefined method 'real' for nil" do
+        NATFIXME 'returns nil for Float::INFINITY', exception: SpecFailedException do
           Complex("Infinity", exception: false).should == nil
           Complex("-Infinity", exception: false).should == nil
         end
       end
 
       it "returns nil for Float::NAN" do
-        NATFIXME 'returns nil for Float::NAN', exception: SpecFailedException do
-          Complex("NaN", exception: false).should == nil
-        end
+        Complex("NaN", exception: false).should == nil
       end
 
       it "returns nil when there is a sequence of _" do
@@ -166,9 +152,7 @@
       end
 
       it "returns nil when String contains null-byte" do
-        NATFIXME 'returns nil when String contains null-byte', exception: NoMethodError, message: "undefined method 'real' for nil" do
-          Complex("1-2i\0", exception: false).should == nil
-        end
+        Complex("1-2i\0", exception: false).should == nil
       end
     end
   end
@@ -291,7 +275,7 @@
 
     describe "and [anything, non-Numeric] argument" do
       it "swallows an error" do
-        NATFIXME '[anything, non-Numeric] argument', exception: SpecFailedException do
+        NATFIXME '[anything, non-Numeric] argument', exception: NoMethodError, message: "undefined method 'real' for nil" do
           Complex("a",  :sym, exception: false).should == nil
           Complex(:sym, :sym, exception: false).should == nil
           Complex(0,    :sym, exception: false).should == nil
@@ -301,7 +285,7 @@
 
     describe "and non-numeric String arguments" do
       it "swallows an error" do
-        NATFIXME 'non-numeric String arguments', exception: SpecFailedException do
+        NATFIXME 'non-numeric String arguments', exception: NoMethodError, message: "undefined method 'real' for nil" do
           Complex("a", "b", exception: false).should == nil
           Complex("a", 0, exception: false).should == nil
           Complex(0, "b", exception: false).should == nil

src/kernel_module.cpp

@@ -134,10 +134,8 @@ Value KernelModule::Complex(Env *env, Value real, Value imaginary, Value excepti
 }
 
 Value KernelModule::Complex(Env *env, Value real, Value imaginary, bool exception) {
-    if (real.is_string()) {
-        // NATFIXME: This is more restrictive than String#to_c, needs its own parser
-        return real->as_string()->send(env, "to_c"_s);
-    }
+    if (real.is_string())
+        return Complex(env, real->as_string(), imaginary, exception);
 
     if (real.is_complex() && imaginary == nullptr) {
         return real;
@@ -157,6 +155,161 @@ Value KernelModule::Complex(Env *env, Value real, Value imaginary, bool exceptio
         return nullptr;
 }
 
+Value KernelModule::Complex(Env *env, StringObject *real, Value imaginary, bool exception) {
+    auto error = [&]() -> Value {
+        if (exception)
+            env->raise("ArgumentError", "invalid value for convert(): \"{}\"", real->string());
+        return NilObject::the();
+    };
+    if (!real->is_ascii_only())
+        return error();
+    enum class State {
+        Start,
+        Real,
+        Imag,
+        Finished,
+        Fallback, // In case of an error, use String#to_c until we finalize this parser
+    };
+    enum class Type {
+        Undefined,
+        Integer,
+        Float,
+    };
+    auto state = State::Start;
+    auto real_type = Type::Undefined;
+    auto imag_type = Type::Undefined;
+    const char *real_start = nullptr;
+    const char *real_end = nullptr;
+    const char *imag_start = nullptr;
+    const char *imag_end = nullptr;
+    auto *curr_type = &real_type;
+    const char **curr_start = &real_start;
+    const char **curr_end = &real_end;
+    auto new_real = Value::integer(0);
+    auto new_imag = Value::integer(0);
+    for (const char *c = real->c_str(); c < real->c_str() + real->bytesize(); c++) {
+        if (*c == 0) {
+            if (exception)
+                env->raise("ArgumentError", "string contains null byte");
+            return NilObject::the();
+        }
+        switch (state) {
+        case State::Start:
+            if ((*c >= '0' && *c <= '9') || *c == '+' || *c == '-') {
+                *curr_start = *curr_end = c;
+                state = State::Real;
+                *curr_type = Type::Integer;
+            } else if (*c == 'i') {
+                new_imag = Value::integer(1);
+                state = State::Finished;
+            } else if (*c != ' ' && *c != '\t' && *c != '\r' && *c != '\n') {
+                return error();
+            }
+            break;
+        case State::Real:
+        case State::Imag:
+            if (*c >= '0' && *c <= '9') {
+                *curr_end = c;
+            } else if (*c == '_') {
+                // TODO: Skip single underscore, fix in String#to_c as well
+                state = State::Fallback;
+            } else if (*c == '.') {
+                if (*curr_type == Type::Integer) {
+                    *curr_type = Type::Float;
+                    *curr_end = c;
+                } else if (*curr_type == Type::Float) {
+                    error();
+                } else {
+                    state = State::Fallback;
+                }
+            } else if (*c == '/') {
+                // TODO: Parse fraction, fix in String#to_c as well
+                state = State::Fallback;
+            } else if (*c == 'e') {
+                // TODO: Parse scientific notation, fix in String#to_c as well
+                state = State::Fallback;
+            } else if (*c == '@') {
+                // TODO: Parse polar form, fix in String#to_c as well
+                state = State::Fallback;
+            } else if (*c == '+' || *c == '-') {
+                if (*curr_start && *curr_start == *curr_end && (**curr_end == '-' || **curr_end == '+'))
+                    return error();
+                if (state == State::Imag)
+                    return error();
+                curr_start = &imag_start;
+                curr_end = &imag_end;
+                *curr_start = *curr_end = c;
+                curr_type = &imag_type;
+                *curr_type = Type::Integer;
+                state = State::Imag;
+            } else if (*c == 'i') {
+                if (*curr_start && *curr_start == *curr_end && (**curr_end == '-' || **curr_end == '+')) {
+                    // Corner case: '-i' or '+i'
+                    new_imag = Value::integer(**curr_end == '-' ? -1 : 1);
+                    *curr_start = nullptr;
+                    *curr_end = nullptr;
+                }
+                if (state == State::Real) {
+                    imag_type = real_type;
+                    imag_start = real_start;
+                    imag_end = c - 1;
+                    real_type = Type::Undefined;
+                    real_start = nullptr;
+                    real_end = nullptr;
+                }
+                state = State::Finished;
+            } else if (*c == 'I' || *c == 'j' || *c == 'J') {
+                // TODO: Parse other imaginary markers, fix in String#to_c as well
+                state = State::Fallback;
+            } else {
+                return error();
+            }
+            break;
+        case State::Finished:
+            if (*c != ' ' && *c != '\t' && *c != '\r' && *c != '\n')
+                return error();
+        case State::Fallback:
+            break;
+        }
+    }
+
+    switch (state) {
+    case State::Fallback:
+        return real->send(env, "to_c"_s);
+    case State::Start:
+        return error();
+    default: {
+        if (real_start != nullptr && real_end != nullptr) {
+            auto tmp = new StringObject { real_start, static_cast<size_t>(real_end - real_start + 1) };
+            switch (real_type) {
+            case Type::Integer:
+                new_real = Integer(env, tmp);
+                break;
+            case Type::Float:
+                new_real = Float(env, tmp);
+                break;
+            case Type::Undefined:
+                return error();
+            }
+        }
+        if (imag_start != nullptr && imag_end != nullptr) {
+            auto tmp = new StringObject { imag_start, static_cast<size_t>(imag_end - imag_start + 1) };
+            switch (imag_type) {
+            case Type::Integer:
+                new_imag = Integer(env, tmp);
+                break;
+            case Type::Float:
+                new_imag = Float(env, tmp);
+                break;
+            case Type::Undefined:
+                return error();
+            }
+        }
+        return new ComplexObject { new_real, new_imag };
+    }
+    }
+}
+
 Value KernelModule::cur_callee(Env *env) {
     auto method = env->caller()->current_method();
     if (method)

test/natalie/kernel_complex_test.rb

@@ -0,0 +1,23 @@
+require_relative '../spec_helper'
+
+describe 'Kernel.Complex' do
+  it 'does not accept anything nun-numeric after the real integer part' do
+    -> { Complex('1a') }.should raise_error(ArgumentError, 'invalid value for convert(): "1a"')
+    -> { Complex('+1a') }.should raise_error(ArgumentError, 'invalid value for convert(): "+1a"')
+    -> { Complex('-1a') }.should raise_error(ArgumentError, 'invalid value for convert(): "-1a"')
+  end
+
+  it 'does not accept anything nun-numeric after the real float part' do
+    -> { Complex('1.0a') }.should raise_error(ArgumentError, 'invalid value for convert(): "1.0a"')
+    -> { Complex('+1.0a') }.should raise_error(ArgumentError, 'invalid value for convert(): "+1.0a"')
+    -> { Complex('-1.0a') }.should raise_error(ArgumentError, 'invalid value for convert(): "-1.0a"')
+    -> { Complex('1.a') }.should raise_error(ArgumentError, 'invalid value for convert(): "1.a"')
+    -> { Complex('+1.a') }.should raise_error(ArgumentError, 'invalid value for convert(): "+1.a"')
+    -> { Complex('-1.a') }.should raise_error(ArgumentError, 'invalid value for convert(): "-1.a"')
+  end
+
+  it 'does not accept two dots in the real part' do
+    -> { Complex('1.0.') }.should raise_error(ArgumentError, 'invalid value for convert(): "1.0."')
+    -> { Complex('1..0') }.should raise_error(ArgumentError, 'invalid value for convert(): "1..0"')
+  end
+end