Refactor with RB_TYPE_P (#509)

* Refactoring by adding IsNMatrixType

* Use RB_TYPE_P instead of simple comparison with TYPE(obj)

* Remove unused macro

ext/nmatrix/data/data.h

@@ -121,18 +121,17 @@ namespace nm {
 
   template <typename Type>
   Complex<Type>& Complex<Type>::operator=(const RubyObject& other) {
-    switch(TYPE(other.rval)) {
-    case T_COMPLEX:
+    if (RB_TYPE_P(other.rval, T_COMPLEX)) {
       this->r = NUM2DBL(rb_funcall(other.rval, rb_intern("real"), 0));
       this->i = NUM2DBL(rb_funcall(other.rval, rb_intern("imag"), 0));
-      break;
-    case T_FLOAT:
-    case T_FIXNUM:
-    case T_BIGNUM:
+    }
+    else if (RB_TYPE_P(other.rval, T_FLOAT) ||
+             RB_TYPE_P(other.rval, T_FIXNUM) ||
+             RB_TYPE_P(other.rval, T_BIGNUM)) {
       this->r = NUM2DBL(other.rval);
       this->i = 0.0;
-      break;
-    default:
+    }
+    else {
       rb_raise(rb_eTypeError, "not sure how to convert this type of VALUE to a complex");
     }
     return *this;

ext/nmatrix/data/ruby_object.h

@@ -45,10 +45,7 @@
 /*
  * Macros
  */
-#define NM_RUBYVAL_IS_NUMERIC(val)                (FIXNUM_P(val) or (TYPE(val) == T_FLOAT) or (TYPE(val) == T_COMPLEX))
-#define NMATRIX_CHECK_TYPE(val) \
-  if (TYPE(val) != T_DATA || (RDATA(val)->dfree != (RUBY_DATA_FUNC)nm_delete && RDATA(val)->dfree != (RUBY_DATA_FUNC)nm_delete_ref)) \
-    rb_raise(rb_eTypeError, "Expected NMatrix on left-hand side of operation.");
+#define NM_RUBYVAL_IS_NUMERIC(val)                (FIXNUM_P(val) or RB_FLOAT_TYPE_P(val) or RB_TYPE_P(val, T_COMPLEX))
 
 /*
  * Classes and Functions

ext/nmatrix/math.cpp

@@ -998,7 +998,7 @@ static VALUE nm_clapack_getrs(VALUE self, VALUE order, VALUE trans, VALUE n, VAL
 
   // Allocate the C version of the pivot index array
   int* ipiv_;
-  if (TYPE(ipiv) != T_ARRAY) {
+  if (!RB_TYPE_P(ipiv, T_ARRAY)) {
     rb_raise(rb_eArgError, "ipiv must be of type Array");
   } else {
     ipiv_ = NM_ALLOCA_N(int, RARRAY_LEN(ipiv));
@@ -1050,7 +1050,7 @@ static VALUE nm_clapack_laswp(VALUE self, VALUE n, VALUE a, VALUE lda, VALUE k1,
 
   // Allocate the C version of the pivot index array
   int* ipiv_;
-  if (TYPE(ipiv) != T_ARRAY) {
+  if (!RB_TYPE_P(ipiv, T_ARRAY)) {
     rb_raise(rb_eArgError, "ipiv must be of type Array");
   } else {
     ipiv_ = NM_ALLOCA_N(int, RARRAY_LEN(ipiv));

ext/nmatrix/nmatrix.h

@@ -58,6 +58,28 @@
   #include "nm_memory.h"
 #endif
 
+#ifndef RB_BUILTIN_TYPE
+# define RB_BUILTIN_TYPE(obj) BUILTIN_TYPE(obj)
+#endif
+
+#ifndef RB_FLOAT_TYPE_P
+/* NOTE: assume flonum doesn't exist */
+# define RB_FLOAT_TYPE_P(obj) ( \
+    (!SPECIAL_CONST_P(obj) && BUILTIN_TYPE(obj) == T_FLOAT))
+#endif
+
+#ifndef RB_TYPE_P
+# define RB_TYPE_P(obj, type) ( \
+    ((type) == T_FIXNUM) ? FIXNUM_P(obj) : \
+    ((type) == T_TRUE) ? ((obj) == Qtrue) : \
+    ((type) == T_FALSE) ? ((obj) == Qfalse) : \
+    ((type) == T_NIL) ? ((obj) == Qnil) : \
+    ((type) == T_UNDEF) ? ((obj) == Qundef) : \
+    ((type) == T_SYMBOL) ? SYMBOL_P(obj) : \
+    ((type) == T_FLOAT) ? RB_FLOAT_TYPE_P(obj) : \
+    (!SPECIAL_CONST_P(obj) && BUILTIN_TYPE(obj) == (type)))
+#endif
+
 #ifndef FIX_CONST_VALUE_PTR
 # if defined(__fcc__) || defined(__fcc_version) || \
     defined(__FCC__) || defined(__FCC_VERSION)
@@ -381,7 +403,8 @@ NM_DEF_STRUCT_POST(NM_GC_HOLDER);       // };
 
 #define RB_FILE_EXISTS(fn)   (rb_funcall(rb_const_get(rb_cObject, rb_intern("File")), rb_intern("exists?"), 1, (fn)) == Qtrue)
 
-#define CheckNMatrixType(v)   if (TYPE(v) != T_DATA || (RDATA(v)->dfree != (RUBY_DATA_FUNC)nm_delete && RDATA(v)->dfree != (RUBY_DATA_FUNC)nm_delete_ref)) rb_raise(rb_eTypeError, "expected NMatrix on left-hand side of operation");
+#define IsNMatrixType(v)  (RB_TYPE_P(v, T_DATA) && (RDATA(v)->dfree == (RUBY_DATA_FUNC)nm_delete || RDATA(v)->dfree == (RUBY_DATA_FUNC)nm_delete_ref))
+#define CheckNMatrixType(v)   if (!IsNMatrixType(v)) rb_raise(rb_eTypeError, "expected NMatrix on left-hand side of operation");
 
 #define NM_IsNMatrix(obj) \
   (rb_obj_is_kind_of(obj, cNMatrix) == Qtrue)

ext/nmatrix/ruby_nmatrix.c

@@ -1202,7 +1202,7 @@ static VALUE nm_init_new_version(int argc, VALUE* argv, VALUE self) {
       init = rubyobj_to_cval(default_val_num, dtype);
     else if (NIL_P(initial_ary))
       init = NULL;
-    else if (TYPE(initial_ary) == T_ARRAY)
+    else if (RB_TYPE_P(initial_ary, T_ARRAY))
       init = RARRAY_LEN(initial_ary) == 1 ? rubyobj_to_cval(rb_ary_entry(initial_ary, 0), dtype) : NULL;
     else
       init = rubyobj_to_cval(initial_ary, dtype);
@@ -1219,8 +1219,8 @@ static VALUE nm_init_new_version(int argc, VALUE* argv, VALUE self) {
 
   if (!NIL_P(initial_ary)) {
 
-    if (TYPE(initial_ary) == T_ARRAY)   v_size = RARRAY_LEN(initial_ary);
-    else                                v_size = 1;
+    if (RB_TYPE_P(initial_ary, T_ARRAY)) v_size = RARRAY_LEN(initial_ary);
+    else                                 v_size = 1;
 
     v = interpret_initial_value(initial_ary, dtype);
 
@@ -1368,7 +1368,7 @@ static VALUE nm_init(int argc, VALUE* argv, VALUE nm) {
   nm::stype_t stype;
   size_t  offset = 0;
 
-  if (!SYMBOL_P(argv[0]) && TYPE(argv[0]) != T_STRING) {
+  if (!SYMBOL_P(argv[0]) && !RB_TYPE_P(argv[0], T_STRING)) {
     stype = nm::DENSE_STORE;
 
   } else {
@@ -1400,7 +1400,7 @@ static VALUE nm_init(int argc, VALUE* argv, VALUE nm) {
 
   size_t init_cap = 0, init_val_len = 0;
   void* init_val  = NULL;
-  if (!SYMBOL_P(argv[1+offset]) || TYPE(argv[1+offset]) == T_ARRAY) {
+  if (!SYMBOL_P(argv[1+offset]) || RB_TYPE_P(argv[1+offset], T_ARRAY)) {
     // Initial value provided (could also be initial capacity, if yale).
 
     if (stype == nm::YALE_STORE && NM_RUBYVAL_IS_NUMERIC(argv[1+offset])) {
@@ -1410,8 +1410,8 @@ static VALUE nm_init(int argc, VALUE* argv, VALUE nm) {
       // 4: initial value / dtype
       init_val = interpret_initial_value(argv[1+offset], dtype);
 
-      if (TYPE(argv[1+offset]) == T_ARRAY)   init_val_len = RARRAY_LEN(argv[1+offset]);
-      else                                  init_val_len = 1;
+      if (RB_TYPE_P(argv[1+offset], T_ARRAY)) init_val_len = RARRAY_LEN(argv[1+offset]);
+      else                                    init_val_len = 1;
     }
 
   } else {
@@ -2060,7 +2060,7 @@ static VALUE nm_multiply(VALUE left_v, VALUE right_v) {
     return matrix_multiply_scalar(left, right_v);
   }
 
-  else if (TYPE(right_v) == T_ARRAY) {
+  else if (RB_TYPE_P(right_v, T_ARRAY)) {
     NM_CONSERVATIVE(nm_unregister_value(&left_v));
     NM_CONSERVATIVE(nm_unregister_value(&right_v));
     rb_raise(rb_eNotImpError, "please convert array to nx1 or 1xn NMatrix first");
@@ -2339,7 +2339,7 @@ static VALUE elementwise_op(nm::ewop_t op, VALUE left_val, VALUE right_val) {
   CheckNMatrixType(left_val);
   UnwrapNMatrix(left_val, left);
 
-  if (TYPE(right_val) != T_DATA || (RDATA(right_val)->dfree != (RUBY_DATA_FUNC)nm_delete && RDATA(right_val)->dfree != (RUBY_DATA_FUNC)nm_delete_ref)) {
+  if (!IsNMatrixType(right_val)) {
     // This is a matrix-scalar element-wise operation.
     std::string sym;
     switch(left->stype) {
@@ -2416,7 +2416,7 @@ static VALUE noncom_elementwise_op(nm::noncom_ewop_t op, VALUE self, VALUE other
   CheckNMatrixType(self);
   UnwrapNMatrix(self, self_nm);
 
-  if (TYPE(other) != T_DATA || (RDATA(other)->dfree != (RUBY_DATA_FUNC)nm_delete && RDATA(other)->dfree != (RUBY_DATA_FUNC)nm_delete_ref)) {
+  if (!IsNMatrixType(other)) {
     // This is a matrix-scalar element-wise operation.
     std::string sym;
     switch(self_nm->stype) {
@@ -2562,23 +2562,20 @@ nm::dtype_t nm_dtype_min_fixnum(int64_t v) {
  */
 nm::dtype_t nm_dtype_min(VALUE v) {
 
-  switch(TYPE(v)) {
-  case T_FIXNUM:
+  if (RB_TYPE_P(v, T_FIXNUM))
     return nm_dtype_min_fixnum(FIX2LONG(v));
-  case T_BIGNUM:
+  else if (RB_TYPE_P(v, T_BIGNUM))
     return nm::INT64;
-  case T_FLOAT:
+  else if (RB_TYPE_P(v, T_FLOAT))
     return nm::FLOAT32;
-  case T_COMPLEX:
+  else if (RB_TYPE_P(v, T_COMPLEX))
     return nm::COMPLEX64;
-  case T_STRING:
+  else if (RB_TYPE_P(v, T_STRING))
     return RSTRING_LEN(v) == 1 ? nm::BYTE : nm::RUBYOBJ;
-  case T_TRUE:
-  case T_FALSE:
-  case T_NIL:
-  default:
+  else if (RB_TYPE_P(v, T_TRUE) || RB_TYPE_P(v, T_FALSE) || RB_TYPE_P(v, T_NIL))
+    return nm::RUBYOBJ;
+  else
     return nm::RUBYOBJ;
-  }
 }
 
 
@@ -2588,60 +2585,39 @@ nm::dtype_t nm_dtype_min(VALUE v) {
  * TODO: Probably needs some work for Bignum.
  */
 nm::dtype_t nm_dtype_guess(VALUE v) {
-  switch(TYPE(v)) {
-  case T_TRUE:
-  case T_FALSE:
-  case T_NIL:
+  if (RB_TYPE_P(v, T_TRUE) || RB_TYPE_P(v, T_FALSE) || RB_TYPE_P(v, T_NIL))
     return nm::RUBYOBJ;
-  case T_STRING:
+  else if (RB_TYPE_P(v, T_STRING))
     return RSTRING_LEN(v) == 1 ? nm::BYTE : nm::RUBYOBJ;
-
+  else if (RB_TYPE_P(v, T_FIXNUM))
 #if SIZEOF_INT == 8
-  case T_FIXNUM:
     return nm::INT64;
-
-#else
-# if SIZEOF_INT == 4
-  case T_FIXNUM:
+#elif SIZEOF_INT == 4
     return nm::INT32;
-
 #else
-  case T_FIXNUM:
     return nm::INT16;
-
-# endif
 #endif
-
-  case T_BIGNUM:
+  else if (RB_TYPE_P(v, T_BIGNUM))
     return nm::INT64;
-
 #if SIZEOF_FLOAT == 4
-  case T_COMPLEX:
+  else if (RB_TYPE_P(v, T_COMPLEX))
     return nm::COMPLEX128;
-
-  case T_FLOAT:
+  else if (RB_TYPE_P(v, T_FLOAT))
     return nm::FLOAT64;
-
-#else
-# if SIZEOF_FLOAT == 2
-  case T_COMPLEX:
+#elif SIZEOF_FLOAT == 2
+  else if (RB_TYPE_P(v, T_COMPLEX))
     return nm::COMPLEX64;
-
-  case T_FLOAT:
+  else if (RB_TYPE_P(v, T_FLOAT))
     return nm::FLOAT32;
-# endif
 #endif
-
-  case T_ARRAY:
+  else if (RB_TYPE_P(v, T_ARRAY))
     /*
      * May be passed for dense -- for now, just look at the first element.
      *
      * TODO: Look at entire array for most specific type.
      */
-
     return nm_dtype_guess(RARRAY_AREF(v, 0));
-
-  default:
+  else {
     RB_P(v);
     rb_raise(rb_eArgError, "Unable to guess a data type from provided parameters; data type must be specified manually.");
   }
@@ -2688,7 +2664,7 @@ static SLICE* get_slice(size_t dim, int argc, VALUE* arg, size_t* shape) {
       slice->single     = false;
       t++;
 
-    } else if (TYPE(arg[t]) == T_HASH) { // 3:5 notation (inclusive)
+    } else if (RB_TYPE_P(arg[t], T_HASH)) { // 3:5 notation (inclusive)
       VALUE begin_end   = rb_funcall(v, rb_intern("shift"), 0); // rb_hash_shift
       nm_register_value(&begin_end);
 
@@ -2776,7 +2752,7 @@ static nm::dtype_t interpret_dtype(int argc, VALUE* argv, nm::stype_t stype) {
   if (SYMBOL_P(argv[offset])) {
     return nm_dtype_from_rbsymbol(argv[offset]);
 
-  } else if (TYPE(argv[offset]) == T_STRING) {
+  } else if (RB_TYPE_P(argv[offset], T_STRING)) {
     return nm_dtype_from_rbstring(StringValue(argv[offset]));
 
   } else if (stype == nm::YALE_STORE) {
@@ -2796,7 +2772,7 @@ static void* interpret_initial_value(VALUE arg, nm::dtype_t dtype) {
   unsigned int index;
   void* init_val;
 
-  if (TYPE(arg) == T_ARRAY) {
+  if (RB_TYPE_P(arg, T_ARRAY)) {
     // Array
     init_val = NM_ALLOC_N(char, DTYPE_SIZES[dtype] * RARRAY_LEN(arg));
     NM_CHECK_ALLOC(init_val);
@@ -2823,7 +2799,7 @@ static size_t* interpret_shape(VALUE arg, size_t* dim) {
   NM_CONSERVATIVE(nm_register_value(&arg));
   size_t* shape;
 
-  if (TYPE(arg) == T_ARRAY) {
+  if (RB_TYPE_P(arg, T_ARRAY)) {
     *dim = RARRAY_LEN(arg);
     shape = NM_ALLOC_N(size_t, *dim);
 
@@ -2854,7 +2830,7 @@ static nm::stype_t interpret_stype(VALUE arg) {
   if (SYMBOL_P(arg)) {
     return nm_stype_from_rbsymbol(arg);
 
-  } else if (TYPE(arg) == T_STRING) {
+  } else if (RB_TYPE_P(arg, T_STRING)) {
     return nm_stype_from_rbstring(StringValue(arg));
 
   } else {

ext/nmatrix/storage/dense/dense.cpp

@@ -140,7 +140,7 @@ namespace nm { namespace dense_storage {
       v                = reinterpret_cast<D*>(t->elements);
       v_size           = nm_storage_count_max_elements(t);
 
-    } else if (TYPE(right) == T_ARRAY) {
+    } else if (RB_TYPE_P(right, T_ARRAY)) {
 
       v_size = RARRAY_LEN(right);
       v      = NM_ALLOC_N(D, v_size);
@@ -875,7 +875,7 @@ namespace nm {
  */
 std::pair<NMATRIX*,bool> interpret_arg_as_dense_nmatrix(VALUE right, nm::dtype_t dtype) {
   NM_CONSERVATIVE(nm_register_value(&right));
-  if (TYPE(right) == T_DATA && (RDATA(right)->dfree == (RUBY_DATA_FUNC)nm_delete || RDATA(right)->dfree == (RUBY_DATA_FUNC)nm_delete_ref)) {
+  if (IsNMatrixType(right)) {
     NMATRIX *r;
     if (NM_STYPE(right) != DENSE_STORE || NM_DTYPE(right) != dtype || NM_SRC(right) != NM_STORAGE(right)) {
       UnwrapNMatrix( right, r );
@@ -888,7 +888,7 @@ std::pair<NMATRIX*,bool> interpret_arg_as_dense_nmatrix(VALUE right, nm::dtype_t
       return std::make_pair(r, false);
     }
     // Do not set v_alloc = true for either of these. It is the responsibility of r/ldtype_r
-  } else if (TYPE(right) == T_DATA) {
+  } else if (RB_TYPE_P(right, T_DATA)) {
     NM_CONSERVATIVE(nm_unregister_value(&right));
     rb_raise(rb_eTypeError, "unrecognized type for slice assignment");
   }

ext/nmatrix/storage/list/list.cpp

@@ -551,7 +551,7 @@ void set(VALUE left, SLICE* slice, VALUE right) {
     v                = reinterpret_cast<D*>(t->elements);
     v_size           = nm_storage_count_max_elements(t);
 
-  } else if (TYPE(right) == T_ARRAY) {
+  } else if (RB_TYPE_P(right, T_ARRAY)) {
     nm_register_nmatrix(nm_and_free.first);
     v_size = RARRAY_LEN(right);
     v      = NM_ALLOC_N(D, v_size);
@@ -1016,7 +1016,7 @@ VALUE nm_list_map_merged_stored(VALUE left, VALUE right, VALUE init) {
   void* scalar_init = NULL;
 
   // right might be a scalar, in which case this is a scalar operation.
-  if (TYPE(right) != T_DATA || (RDATA(right)->dfree != (RUBY_DATA_FUNC)nm_delete && RDATA(right)->dfree != (RUBY_DATA_FUNC)nm_delete_ref)) {
+  if (!IsNMatrixType(right)) {
     nm::dtype_t r_dtype = Upcast[NM_DTYPE(left)][nm_dtype_min(right)];
     scalar_init         = rubyobj_to_cval(right, r_dtype); // make a copy of right
 

ext/nmatrix/storage/yale/class.h

@@ -376,7 +376,7 @@ class YaleStorage {
       v       = reinterpret_cast<D*>(s->elements);
       v_size  = nm_storage_count_max_elements(s);
 
-    } else if (TYPE(right) == T_ARRAY) {
+    } else if (RB_TYPE_P(right, T_ARRAY)) {
       v_size = RARRAY_LEN(right);
       v      = NM_ALLOC_N(D, v_size);
       if (dtype() == nm::RUBYOBJ) {

ext/nmatrix_lapacke/math_lapacke.cpp

@@ -656,7 +656,7 @@ static VALUE nm_lapacke_lapacke_getri(VALUE self, VALUE order, VALUE n, VALUE a,
 
   // Allocate the C version of the pivot index array
   int* ipiv_;
-  if (TYPE(ipiv) != T_ARRAY) {
+  if (!RB_TYPE_P(ipiv, T_ARRAY)) {
     rb_raise(rb_eArgError, "ipiv must be of type Array");
   } else {
     ipiv_ = NM_ALLOCA_N(int, RARRAY_LEN(ipiv));
@@ -748,7 +748,7 @@ static VALUE nm_lapacke_lapacke_getrs(VALUE self, VALUE order, VALUE trans, VALU
 
   // Allocate the C version of the pivot index array
   int* ipiv_;
-  if (TYPE(ipiv) != T_ARRAY) {
+  if (!RB_TYPE_P(ipiv, T_ARRAY)) {
     rb_raise(rb_eArgError, "ipiv must be of type Array");
   } else {
     ipiv_ = NM_ALLOCA_N(int, RARRAY_LEN(ipiv));