Merge pull request #241 from ruby-rice/dev

Add support for Ruby keywords. See #202.

rice/detail/MethodInfo.hpp

@@ -24,8 +24,20 @@ namespace Rice
       */
     void addArg(const Arg& arg);
 
+    /**
+      * Get argument by position
+      */
     Arg* arg(size_t pos);
 
+    /**
+      * Get argument by name
+      */
+    Arg* arg(std::string name);
+
+    int requiredArgCount();
+    int optionalArgCount();
+    void verifyArgCount(int argc);
+
     // Iterator support
     std::vector<Arg>::iterator begin();
     std::vector<Arg>::iterator end();

rice/detail/MethodInfo.ipp

@@ -40,6 +40,60 @@ namespace Rice
     this->args_.push_back(arg);
   }
 
+  inline int MethodInfo::requiredArgCount()
+  {
+    int result = 0;
+
+    for (const Arg& arg : this->args_)
+    {
+      if (!arg.hasDefaultValue())
+      {
+        result++;
+      }
+    }
+
+    return result;
+  }
+
+  inline int MethodInfo::optionalArgCount()
+  {
+    int result = 0;
+
+    for (const Arg& arg : this->args_)
+    {
+      if (arg.hasDefaultValue())
+      {
+        result++;
+      }
+    }
+
+    return result;
+  }
+
+  inline void MethodInfo::verifyArgCount(int argc)
+  {
+    int requiredArgCount = this->requiredArgCount();
+    int optionalArgCount = this->optionalArgCount();
+
+    if (argc < requiredArgCount || argc > requiredArgCount + optionalArgCount)
+    {
+      std::string message;
+
+      if (optionalArgCount > 0)
+      {
+        message = "wrong number of arguments (given " +
+          std::to_string(argc) + ", expected " +
+          std::to_string(requiredArgCount) + ".." + std::to_string(requiredArgCount + optionalArgCount) + ")";
+      }
+      else
+      {
+        message = "wrong number of arguments (given " +
+          std::to_string(argc) + ", expected " + std::to_string(requiredArgCount) + ")";
+      }
+      throw std::invalid_argument(message);
+    }
+  }
+
   inline std::string MethodInfo::formatString()
   {
     size_t required = 0;
@@ -72,6 +126,18 @@ namespace Rice
     }
   }
 
+  inline Arg* MethodInfo::arg(std::string name)
+  {
+    for (Arg& arg : this->args_)
+    {
+      if (arg.name == name)
+      {
+        return &arg;
+      }
+    }
+    return nullptr;
+  }
+
   inline std::vector<Arg>::iterator MethodInfo::begin()
   {
     return this->args_.begin();

rice/detail/NativeFunction.hpp

@@ -83,7 +83,7 @@ namespace Rice::detail
     To_Ruby<To_Ruby_T> createToRuby();
 
     // Convert Ruby argv pointer to Ruby values
-    std::vector<VALUE> getRubyValues(int argc, const VALUE* argv);
+    std::vector<VALUE> getRubyValues(int argc, const VALUE* argv, bool validate);
 
     template<typename Arg_T, int I>
     Arg_T getNativeValue(std::vector<VALUE>& values);

rice/detail/NativeFunction.ipp

@@ -112,24 +112,26 @@ namespace Rice::detail
     MethodInfo* methodInfo = this->methodInfo_.get();
     int index = 0;
 
+    std::vector<VALUE> rubyValues = this->getRubyValues(argc, argv, false);
+
     // Loop over each FromRuby instance
     for_each_tuple(this->fromRubys_,
       [&](auto& fromRuby)
       {
         Convertible convertible = Convertible::None;
 
-        Arg* arg = methodInfo->arg(index);
+        const Arg* arg = methodInfo->arg(index);
 
         // Is a VALUE being passed directly to C++ ?
-        if (arg->isValue() && index < argc)
+        if (arg->isValue() && index < rubyValues.size())
         {
           convertible = Convertible::Exact;
         }
         // If index is less than argc then check with FromRuby if the VALUE is convertible
         // to C++.
-        else if (index < argc)
+        else if (index < rubyValues.size())
         {
-          VALUE value = argv[index];
+          VALUE value = rubyValues[index];
           convertible = fromRuby.is_convertible(value);
         }
         // Last check if a default value has been set
@@ -150,39 +152,58 @@ namespace Rice::detail
   }
 
   template<typename Class_T, typename Function_T, bool IsMethod>
-  std::vector<VALUE> NativeFunction<Class_T, Function_T, IsMethod>::getRubyValues(int argc, const VALUE* argv)
+  std::vector<VALUE> NativeFunction<Class_T, Function_T, IsMethod>::getRubyValues(int argc, const VALUE* argv, bool validate)
   {
-    // Block handling. If we find a block and there is a missing argument error, then we assume the
-    // block is being used as a callback for C/C++. The easiest thing to do is keep the format string the 
-    // same but convert the block to a proc and add it to argv.
-    std::vector<VALUE> argvCopy(argv, argv + argc);
-    if (rb_block_given_p() && argc < arity)
+    std::vector<VALUE> result;
+
+    // Keyword handling
+    if (rb_keyword_given_p())
     {
-      argc++;
-      VALUE proc = rb_block_proc();
-      argvCopy.push_back(proc);
-    }
+      // Keywords are stored in the last element in a hash
+      int actualArgc = argc - 1;
 
-    // Setup a tuple for the leading rb_scan_args arguments
-    std::string scanFormat = this->methodInfo_->formatString();
-    std::tuple<int, VALUE*, const char*> rbScanArgs = std::forward_as_tuple(argc, argvCopy.data(), scanFormat.c_str());
+      VALUE value = argv[actualArgc];
+      Hash keywords(value);
 
-    // Create a vector to store the VALUEs that will be returned by rb_scan_args
-    std::vector<VALUE> rbScanValues(std::tuple_size_v<Arg_Ts>, Qnil);
+      result.resize(actualArgc + keywords.size());
 
-    // Convert the vector to an array so it can be concatenated to a tuple. As importantly
-    // fill it with pointers to rbScanValues
-    std::array<VALUE*, std::tuple_size_v<Arg_Ts>> rbScanValuePointers;
-    std::transform(rbScanValues.begin(), rbScanValues.end(), rbScanValuePointers.begin(),
-      [](VALUE& value)
+      // Copy over leading arguments
+      for (int i = 0; i < actualArgc; i++)
       {
-        return &value;
-      });
+        result[i] = argv[i];
+      }
 
-    // Combine the tuples and call rb_scan_args
-    std::apply(rb_scan_args, std::tuple_cat(rbScanArgs, rbScanValuePointers));
+      // Copy over keyword arguments
+      for (auto pair : keywords)
+      {
+        Symbol key(pair.first);
+        const Arg* arg = this->methodInfo_->arg(key.str());
+        if (!arg)
+        {
+          throw std::invalid_argument("Unknown keyword: " + key.str());
+        }
+        result[arg->position] = pair.second.value();
+      }
+    }
+    else
+    {
+      std::copy(argv, argv + argc, std::back_inserter(result));
+    }
+
+    // Block handling. If we find a block and the last parameter is missing then
+    // set it to the block
+    if (rb_block_given_p() && result.size() < std::tuple_size_v<Arg_Ts>)
+    {
+      VALUE proc = rb_block_proc();
+      result.push_back(proc);
+    }
 
-    return rbScanValues;
+    if (validate)
+    {
+      this->methodInfo_->verifyArgCount(result.size());
+    }
+
+    return result;
   }
 
   template<typename Class_T, typename Function_T, bool IsMethod>
@@ -196,13 +217,17 @@ namespace Rice::detail
        An alternative solution is updating From_Ruby#convert to become a templated function that specifies
        the return type. That works but requires a lot more code changes for this one case and is not 
        backwards compatible. */
+
+    // If the user did provide a value assume Qnil
+    VALUE value = I < values.size() ? values[I] : Qnil;
+
     if constexpr (is_pointer_pointer_v<Arg_T> && !std::is_convertible_v<remove_cv_recursive_t<Arg_T>, Arg_T>)
     {
-      return (Arg_T)std::get<I>(this->fromRubys_).convert(values[I]);
+      return (Arg_T)std::get<I>(this->fromRubys_).convert(value);
     }
     else
     {
-      return std::get<I>(this->fromRubys_).convert(values[I]);
+      return std::get<I>(this->fromRubys_).convert(value);
     }
   }
 
@@ -383,8 +408,8 @@ namespace Rice::detail
   template<typename Class_T, typename Function_T, bool IsMethod>
   VALUE NativeFunction<Class_T, Function_T, IsMethod>::operator()(int argc, const VALUE* argv, VALUE self)
   {
-    // Get the ruby values
-    std::vector<VALUE> rubyValues = this->getRubyValues(argc, argv);
+    // Get the ruby values and make sure we have the correct number
+    std::vector<VALUE> rubyValues = this->getRubyValues(argc, argv, true);
 
     auto indices = std::make_index_sequence<std::tuple_size_v<Arg_Ts>>{};
 

test/test_Module.cpp

@@ -177,14 +177,18 @@ TESTCASE(method_int_passed_no_args)
 {
   Module m(anonymous_module());
   m.define_method("foo", method_int);
+
   ASSERT_EXCEPTION_CHECK(
       Exception,
       m.module_eval("o = Object.new; o.extend(self); o.foo"),
-      ASSERT_EQUAL(
-          Object(rb_eArgError),
-          Object(CLASS_OF(ex.value()))
-          )
+      ASSERT_EQUAL(Object(rb_eArgError), Object(CLASS_OF(ex.value())))
       );
+
+  ASSERT_EXCEPTION_CHECK(
+    Exception,
+    m.module_eval("o = Object.new; o.extend(self); o.foo"),
+    ASSERT_EQUAL("wrong number of arguments (given 0, expected 1)", ex.what())
+  );
 }
 
 TESTCASE(define_singleton_method_int_foo)
@@ -260,22 +264,28 @@ TESTCASE(default_arguments_still_throws_argument_error)
   m.define_function("foo", &defaults_method_one, Arg("arg1"), Arg("arg2") = 3, Arg("arg3") = true);
 
   ASSERT_EXCEPTION_CHECK(
-      Exception,
-      m.module_eval("o = Object.new; o.extend(self); o.foo()"),
-      ASSERT_EQUAL(
-          Object(rb_eArgError),
-          Object(CLASS_OF(ex.value()))
-          )
-      );
+    Exception,
+    m.module_eval("o = Object.new; o.extend(self); o.foo()"),
+    ASSERT_EQUAL(Object(rb_eArgError), Object(CLASS_OF(ex.value())))
+  );
 
   ASSERT_EXCEPTION_CHECK(
-      Exception,
-      m.module_eval("o = Object.new; o.extend(self); o.foo(3, 4, false, 17)"),
-      ASSERT_EQUAL(
-          Object(rb_eArgError),
-          Object(CLASS_OF(ex.value()))
-          )
-      );
+    Exception,
+    m.module_eval("o = Object.new; o.extend(self); o.foo()"),
+    ASSERT_EQUAL("wrong number of arguments (given 0, expected 1..3)", ex.what())
+  );
+
+  ASSERT_EXCEPTION_CHECK(
+    Exception,
+    m.module_eval("o = Object.new; o.extend(self); o.foo(3, 4, false, 17)"),
+    ASSERT_EQUAL(Object(rb_eArgError), Object(CLASS_OF(ex.value())))
+  );
+
+  ASSERT_EXCEPTION_CHECK(
+    Exception,
+    m.module_eval("o = Object.new; o.extend(self); o.foo(3, 4, false, 17)"),
+    ASSERT_EQUAL("wrong number of arguments (given 4, expected 1..3)", ex.what())
+  );
 }
 
 namespace {