Mercurial > octave

changeset 22471:667d353d1ab8

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Clean up MEX API prototypes to match Matlab. Return int, rather than void, from mxSetDimensions. Return const mwSize *, rather than unconst version, from mxGetDimensions. Re-order switch statements to have most probable cases first. * mexproto.h (mxGetDimensions): Change prototype to return const mwSize *. * mexproto.h (mxSetDimensions): Change prototype to return int. * mxarray.in.h: Expand comment on why Octave uses OCTAVE_IDX_TYPE for all size and pointer types. * mxarray.in.h (mxArray_base::set_dimensions): Return int rather than void. * mxarray.in.h (mxArray::set_dimensions): Return int rather than void. Use DO_MUTABLE_METHOD, not DO_VOID_MUTABLE_METHOD, due to prototype change. * mex.cc (mxArray_octave_value::set_dimensions): Return int rather than void. Always return 0, success. * mex.cc (mxArray_octave_value::get_classid): Re-order if/else if tree to put more probable cases first. * mex.cc (mxArray_octave_value::get_element_size): Reorder switch statement to have most probable cases first. * mex.cc (mxArray_matlab::set_dimensions): Return int rather than void. Return failure (1) if malloc does not succeed. * mex.cc (mxArray_matlab::get_class_name): Reorder switch statement to have most probable cases first. * mex.cc (mxArray_number::get_double): Reorder switch statement to have most probable cases first. * mex.cc (mxArray_number::as_octave_value): Reorder switch statement to have most probable cases first. * mex.cc (mxGetDimensions): Return const mwSize *. * mex.cc (mxSetDimensions): Return int rather than void. * mex.cc (mxGetChars): Return NULL if input mxArray is not of type mxChar (done for Matlab compatibility.).
author Rik <rik@octave.org>
date Mon, 12 Sep 2016 13:39:32 -0700
parents 9d4cb0cf9cd2
children 76f2b0436423
files libinterp/corefcn/mex.cc libinterp/corefcn/mexproto.h libinterp/corefcn/mxarray.in.h
diffstat 3 files changed, 160 insertions(+), 129 deletions(-) [+]
line wrap: on
line diff
--- a/libinterp/corefcn/mex.cc	Sun Sep 11 21:08:56 2016 -0700
+++ b/libinterp/corefcn/mex.cc	Mon Sep 12 13:39:32 2016 -0700
@@ -296,9 +296,11 @@
 
   void set_n (mwSize /*n*/) { request_mutation (); }
 
-  void set_dimensions (mwSize * /*dims_arg*/, mwSize /*ndims_arg*/)
+  int set_dimensions (mwSize * /*dims_arg*/, mwSize /*ndims_arg*/)
   {
     request_mutation ();
+
+    return 0; 
   }
 
   mwSize get_number_of_elements (void) const { return val.numel (); }
@@ -319,18 +321,20 @@
 
     std::string cn = val.class_name ();
 
-    if (cn == "cell")
+    if (cn == "double")
+      id = mxDOUBLE_CLASS;
+    else if (cn == "single")
+      id = mxSINGLE_CLASS;
+    else if (cn == "char")
+      id = mxCHAR_CLASS;
+    else if (cn == "logical")
+      id = mxLOGICAL_CLASS;
+    else if (cn == "cell")
       id = mxCELL_CLASS;
     else if (cn == "struct")
       id = mxSTRUCT_CLASS;
-    else if (cn == "logical")
-      id = mxLOGICAL_CLASS;
-    else if (cn == "char")
-      id = mxCHAR_CLASS;
-    else if (cn == "double")
-      id = mxDOUBLE_CLASS;
-    else if (cn == "single")
-      id = mxSINGLE_CLASS;
+    else if (cn == "function_handle")
+      id = mxFUNCTION_CLASS;
     else if (cn == "int8")
       id = mxINT8_CLASS;
     else if (cn == "uint8")
@@ -347,8 +351,6 @@
       id = mxINT64_CLASS;
     else if (cn == "uint64")
       id = mxUINT64_CLASS;
-    else if (cn == "function_handle")
-      id = mxFUNCTION_CLASS;
 
     return id;
   }
@@ -376,6 +378,7 @@
   // Not allowed.
   void set_cell (mwIndex /*idx*/, mxArray * /*val*/) { request_mutation (); }
 
+  // FIXME: For sparse arrays, this should return the first non-zero value.
   double get_scalar (void) const { return val.scalar_value (true); }
 
   void *get_data (void) const
@@ -491,7 +494,7 @@
 
   char *array_to_string (void) const
   {
-    // FIXME: this is suposed to handle multi-byte character strings.
+    // FIXME: this is supposed to handle multi-byte character strings.
 
     char *buf = 0;
 
@@ -532,12 +535,13 @@
 
     switch (id)
       {
+      case mxDOUBLE_CLASS: return sizeof (double);
+      case mxSINGLE_CLASS: return sizeof (float);
+      case mxCHAR_CLASS: return sizeof (mxChar);
+      case mxLOGICAL_CLASS: return sizeof (mxLogical);
       case mxCELL_CLASS: return sizeof (mxArray *);
       case mxSTRUCT_CLASS: return sizeof (mxArray *);
-      case mxLOGICAL_CLASS: return sizeof (mxLogical);
-      case mxCHAR_CLASS: return sizeof (mxChar);
-      case mxDOUBLE_CLASS: return sizeof (double);
-      case mxSINGLE_CLASS: return sizeof (float);
+      case mxFUNCTION_CLASS: return 0;
       case mxINT8_CLASS: return 1;
       case mxUINT8_CLASS: return 1;
       case mxINT16_CLASS: return 2;
@@ -546,7 +550,8 @@
       case mxUINT32_CLASS: return 4;
       case mxINT64_CLASS: return 8;
       case mxUINT64_CLASS: return 8;
-      case mxFUNCTION_CLASS: return 0;
+      // FIXME: user-defined objects need their own class ID.
+      //        What should they return, size of pointer?
       default: return 0;
       }
   }
@@ -743,16 +748,30 @@
 
   void set_n (mwSize n) { dims[1] = n; }
 
-  void set_dimensions (mwSize *dims_arg, mwSize ndims_arg)
+  int set_dimensions (mwSize *dims_arg, mwSize ndims_arg)
   {
     ndims = ndims_arg;
 
     mxFree (dims);
-    dims = (ndims > 0 ? static_cast<mwSize *>
-                          (mxArray::malloc (ndims * sizeof (mwSize)))
-                      : 0);
-    for (int i = 0; i < ndims; i++)
-      dims[i] = dims_arg[i];
+
+    if (ndims > 0)
+      {
+        dims
+          = static_cast<mwSize *> (mxArray::malloc (ndims * sizeof (mwSize)));
+        
+        if (dims == NULL)
+          return 1;
+
+        for (int i = 0; i < ndims; i++)
+          dims[i] = dims_arg[i];
+
+        return 0;
+      }
+    else
+      {
+        dims = 0;
+        return 0;
+      }
   }
 
   mwSize get_number_of_elements (void) const
@@ -778,12 +797,13 @@
   {
     switch (id)
       {
+      case mxDOUBLE_CLASS: return "double";
+      case mxSINGLE_CLASS: return "single";
+      case mxCHAR_CLASS: return "char";
+      case mxLOGICAL_CLASS: return "logical";
       case mxCELL_CLASS: return "cell";
       case mxSTRUCT_CLASS: return "struct";
-      case mxLOGICAL_CLASS: return "logical";
-      case mxCHAR_CLASS: return "char";
-      case mxDOUBLE_CLASS: return "double";
-      case mxSINGLE_CLASS: return "single";
+      case mxFUNCTION_CLASS: return "function_handle";
       case mxINT8_CLASS: return "int8";
       case mxUINT8_CLASS: return "uint8";
       case mxINT16_CLASS: return "int16";
@@ -792,7 +812,8 @@
       case mxUINT32_CLASS: return "uint32";
       case mxINT64_CLASS: return "int64";
       case mxUINT64_CLASS: return "uint64";
-      case mxFUNCTION_CLASS: return "function_handle";
+      case mxUNKNOWN_CLASS: return "unknown";
+      // FIXME: should return the classname of user-defined objects
       default: return "unknown";
       }
   }
@@ -939,6 +960,8 @@
       case mxINT64_CLASS: return 8;
       case mxUINT64_CLASS: return 8;
       case mxFUNCTION_CLASS: return 0;
+      // FIXME: user-defined objects need their own class ID.
+      //        What should they return, size of pointer?
       default: return 0;
       }
   }
@@ -1108,20 +1131,20 @@
 
     switch (get_class_id ())
       {
-      case mxLOGICAL_CLASS:
-        retval = *(static_cast<bool *> (pr));
+      case mxDOUBLE_CLASS:
+        retval = *(static_cast<double *> (pr));
+        break;
+
+      case mxSINGLE_CLASS:
+        retval = *(static_cast<float *> (pr));
         break;
 
       case mxCHAR_CLASS:
         retval = *(static_cast<mxChar *> (pr));
         break;
 
-      case mxSINGLE_CLASS:
-        retval = *(static_cast<float *> (pr));
-        break;
-
-      case mxDOUBLE_CLASS:
-        retval = *(static_cast<double *> (pr));
+      case mxLOGICAL_CLASS:
+        retval = *(static_cast<bool *> (pr));
         break;
 
       case mxINT8_CLASS:
@@ -1199,7 +1222,7 @@
 
   char *array_to_string (void) const
   {
-    // FIXME: this is suposed to handle multi-byte character strings.
+    // FIXME: this is supposed to handle multi-byte character strings.
 
     mwSize nel = get_number_of_elements ();
 
@@ -1226,24 +1249,36 @@
 
     switch (get_class_id ())
       {
-      case mxLOGICAL_CLASS:
-        retval = int_to_ov<mxLogical, boolNDArray, bool> (dv);
-        break;
-
-      case mxCHAR_CLASS:
+      case mxDOUBLE_CLASS:
         {
           mwSize nel = get_number_of_elements ();
 
-          mxChar *ppr = static_cast<mxChar *> (pr);
-
-          charNDArray val (dv);
-
-          char *ptr = val.fortran_vec ();
-
-          for (mwIndex i = 0; i < nel; i++)
-            ptr[i] = static_cast<char> (ppr[i]);
-
-          retval = val;
+          double *ppr = static_cast<double *> (pr);
+
+          if (pi)
+            {
+              ComplexNDArray val (dv);
+
+              Complex *ptr = val.fortran_vec ();
+
+              double *ppi = static_cast<double *> (pi);
+
+              for (mwIndex i = 0; i < nel; i++)
+                ptr[i] = Complex (ppr[i], ppi[i]);
+
+              retval = val;
+            }
+          else
+            {
+              NDArray val (dv);
+
+              double *ptr = val.fortran_vec ();
+
+              for (mwIndex i = 0; i < nel; i++)
+                ptr[i] = ppr[i];
+
+              retval = val;
+            }
         }
         break;
 
@@ -1280,39 +1315,27 @@
         }
         break;
 
-      case mxDOUBLE_CLASS:
+      case mxCHAR_CLASS:
         {
           mwSize nel = get_number_of_elements ();
 
-          double *ppr = static_cast<double *> (pr);
-
-          if (pi)
-            {
-              ComplexNDArray val (dv);
-
-              Complex *ptr = val.fortran_vec ();
-
-              double *ppi = static_cast<double *> (pi);
-
-              for (mwIndex i = 0; i < nel; i++)
-                ptr[i] = Complex (ppr[i], ppi[i]);
-
-              retval = val;
-            }
-          else
-            {
-              NDArray val (dv);
-
-              double *ptr = val.fortran_vec ();
-
-              for (mwIndex i = 0; i < nel; i++)
-                ptr[i] = ppr[i];
-
-              retval = val;
-            }
+          mxChar *ppr = static_cast<mxChar *> (pr);
+
+          charNDArray val (dv);
+
+          char *ptr = val.fortran_vec ();
+
+          for (mwIndex i = 0; i < nel; i++)
+            ptr[i] = static_cast<char> (ppr[i]);
+
+          retval = val;
         }
         break;
 
+      case mxLOGICAL_CLASS:
+        retval = int_to_ov<mxLogical, boolNDArray, bool> (dv);
+        break;
+
       case mxINT8_CLASS:
         retval = int_to_ov<int8_t, int8NDArray, octave_int8> (dv);
         break;
@@ -1459,30 +1482,6 @@
 
     switch (get_class_id ())
       {
-      case mxLOGICAL_CLASS:
-        {
-          bool *ppr = static_cast<bool *> (pr);
-
-          SparseBoolMatrix val (get_m (), get_n (),
-                                static_cast<octave_idx_type> (nzmax));
-
-          for (mwIndex i = 0; i < nzmax; i++)
-            {
-              val.xdata (i) = ppr[i];
-              val.xridx (i) = ir[i];
-            }
-
-          for (mwIndex i = 0; i < get_n () + 1; i++)
-            val.xcidx (i) = jc[i];
-
-          retval = val;
-        }
-        break;
-
-      case mxSINGLE_CLASS:
-        error ("single precision sparse data type not supported");
-        break;
-
       case mxDOUBLE_CLASS:
         {
           if (pi)
@@ -1525,6 +1524,30 @@
         }
         break;
 
+      case mxLOGICAL_CLASS:
+        {
+          bool *ppr = static_cast<bool *> (pr);
+
+          SparseBoolMatrix val (get_m (), get_n (),
+                                static_cast<octave_idx_type> (nzmax));
+
+          for (mwIndex i = 0; i < nzmax; i++)
+            {
+              val.xdata (i) = ppr[i];
+              val.xridx (i) = ir[i];
+            }
+
+          for (mwIndex i = 0; i < get_n () + 1; i++)
+            val.xcidx (i) = jc[i];
+
+          retval = val;
+        }
+        break;
+
+      case mxSINGLE_CLASS:
+        error ("single precision sparse data type not supported");
+        break;
+
       default:
         panic_impossible ();
       }
@@ -2759,7 +2782,7 @@
   return ptr->get_n ();
 }
 
-mwSize *
+const mwSize *
 mxGetDimensions (const mxArray *ptr)
 {
   return ptr->get_dimensions ();
@@ -2790,12 +2813,12 @@
   ptr->set_n (n);
 }
 
-void
+int
 mxSetDimensions (mxArray *ptr, const mwSize *dims, mwSize ndims)
 {
-  ptr->set_dimensions (static_cast<mwSize *>
-                       (maybe_unmark (const_cast<mwSize *> (dims))),
-                       ndims);
+  return (ptr->set_dimensions (static_cast<mwSize *>
+                               (maybe_unmark (const_cast<mwSize *> (dims))),
+                               ndims));
 }
 
 // Data extractors.
@@ -2811,6 +2834,8 @@
   return static_cast<double *> (ptr->get_imag_data ());
 }
 
+// FIXME: For sparse arrays, mxGetScalar should return the first non-zero
+// element, rather than just the first element.
 double
 mxGetScalar (const mxArray *ptr)
 {
@@ -2820,7 +2845,10 @@
 mxChar *
 mxGetChars (const mxArray *ptr)
 {
-  return static_cast<mxChar *> (ptr->get_data ());
+  if (mxIsChar (ptr))
+    return static_cast<mxChar *> (ptr->get_data ());
+  else
+    return NULL;
 }
 
 mxLogical *
--- a/libinterp/corefcn/mexproto.h	Sun Sep 11 21:08:56 2016 -0700
+++ b/libinterp/corefcn/mexproto.h	Mon Sep 12 13:39:32 2016 -0700
@@ -194,6 +194,7 @@
 extern OCTINTERP_API bool mxIsClass (const mxArray *ptr, const char *name);
 extern OCTINTERP_API bool mxIsComplex (const mxArray *ptr);
 extern OCTINTERP_API bool mxIsDouble (const mxArray *ptr);
+/* Matlab seems to have deprecated IsFunctionHandle, but it seems useful */
 extern OCTINTERP_API bool mxIsFunctionHandle (const mxArray *ptr);
 extern OCTINTERP_API bool mxIsInt16 (const mxArray *ptr);
 extern OCTINTERP_API bool mxIsInt32 (const mxArray *ptr);
@@ -225,15 +226,15 @@
 /* Dimension extractors.  */
 extern OCTINTERP_API size_t mxGetM (const mxArray *ptr);
 extern OCTINTERP_API size_t mxGetN (const mxArray *ptr);
-extern OCTINTERP_API mwSize *mxGetDimensions (const mxArray *ptr);
+extern OCTINTERP_API const mwSize *mxGetDimensions (const mxArray *ptr);
 extern OCTINTERP_API mwSize mxGetNumberOfDimensions (const mxArray *ptr);
 extern OCTINTERP_API size_t mxGetNumberOfElements (const mxArray *ptr);
 
 /* Dimension setters.  */
 extern OCTINTERP_API void mxSetM (mxArray *ptr, mwSize M);
 extern OCTINTERP_API void mxSetN (mxArray *ptr, mwSize N);
-extern OCTINTERP_API void mxSetDimensions (mxArray *ptr, const mwSize *dims,
-                                           mwSize ndims);
+extern OCTINTERP_API int mxSetDimensions (mxArray *ptr, const mwSize *dims,
+                                          mwSize ndims);
 
 /* Data extractors.  */
 extern OCTINTERP_API double *mxGetPi (const mxArray *ptr);
@@ -296,8 +297,9 @@
 extern OCTINTERP_API char *mxArrayToString (const mxArray *ptr);
 
 /* Miscellaneous.  */
-extern OCTINTERP_API mwIndex
-mxCalcSingleSubscript (const mxArray *ptr, mwSize nsubs, mwIndex *subs);
+extern OCTINTERP_API mwIndex mxCalcSingleSubscript (const mxArray *ptr,
+                                                    mwSize nsubs,
+                                                    mwIndex *subs);
 
 extern OCTINTERP_API size_t mxGetElementSize (const mxArray *ptr);
 
--- a/libinterp/corefcn/mxarray.in.h	Sun Sep 11 21:08:56 2016 -0700
+++ b/libinterp/corefcn/mxarray.in.h	Mon Sep 12 13:39:32 2016 -0700
@@ -51,13 +51,6 @@
 
 typedef enum
 {
-  mxREAL = 0,
-  mxCOMPLEX = 1
-}
-mxComplexity;
-
-typedef enum
-{
   mxUNKNOWN_CLASS = 0,
   mxCELL_CLASS,
   mxSTRUCT_CLASS,
@@ -78,16 +71,24 @@
 }
 mxClassID;
 
-typedef unsigned char mxLogical;
+typedef enum
+{
+  mxREAL = 0,
+  mxCOMPLEX = 1
+}
+mxComplexity;
 
 /* Matlab uses a wide char (uint16) internally, but Octave uses plain char. */
 /* typedef Uint16 mxChar; */
 typedef char mxChar;
 
+typedef unsigned char mxLogical;
+
 /*
- * FIXME: Mathworks says these should be size_t on 64-bit system and when
- * mex is used with the -largearraydims flag, but why do that?  Its better
- * to conform to the same indexing as the rest of Octave.
+ * FIXME: Mathworks says mwSize, mwIndex should be int generally.
+ * But on 64-bit systems, or when mex -largeArrayDims is used, it is size_t.
+ * mwSignedIndex is supposed to be ptrdiff_t.  All of this is confusing.
+ * Its better to conform to the same indexing as the rest of Octave.
  */
 typedef %OCTAVE_IDX_TYPE% mwSize;
 typedef %OCTAVE_IDX_TYPE% mwIndex;
@@ -208,7 +209,7 @@
 
   virtual void set_n (mwSize n) = 0;
 
-  virtual void set_dimensions (mwSize *dims_arg, mwSize ndims_arg) = 0;
+  virtual int set_dimensions (mwSize *dims_arg, mwSize ndims_arg) = 0;
 
   virtual mwSize get_number_of_elements (void) const = 0;
 
@@ -416,8 +417,8 @@
 
   void set_n (mwSize n) { DO_VOID_MUTABLE_METHOD (set_n (n)); }
 
-  void set_dimensions (mwSize *dims_arg, mwSize ndims_arg)
-  { DO_VOID_MUTABLE_METHOD (set_dimensions (dims_arg, ndims_arg)); }
+  int set_dimensions (mwSize *dims_arg, mwSize ndims_arg)
+  { DO_MUTABLE_METHOD (int, set_dimensions (dims_arg, ndims_arg)); }
 
   mwSize get_number_of_elements (void) const
   { return rep->get_number_of_elements (); }