[bug #67845] classdef constructors do not support multiple arguments

Please use the bug tracker to post updates to a bug report.  The mailing list is intended as a read-only notification stream.  Info posted to this mailing list address won't appear in the tracker database where it is most useful.

Update of bug #67845 (group octave):

                Category:                    None => Classdef
                  Status:                    None => Patch Reviewed

    _______________________________________________________

Follow-up Comment #5:

Thanks for the patch. And sorry for the late review.

I haven't attempted to build with the patch yet.
Just a few comments after reading your patch:


-  cdef_object construct_object (const octave_value_list& args,
+  //! The proper method to call for object construction.
+  //!
+  //! @return The first index slot in the octave_value_list contains the
+  //! constructed classdef object. All the other slots contain (possible)
+  //! extra constructor return outputs.
+  octave_value_list construct_object (const octave_value_list& args,
+                                const int nargout = 1,
                                 const bool default_initialize = false)


The indentation of the arguments no longer match the opening parenthesis after
this change. Maybe, consider adding a line-break between "octave_value_list"
and "construct_object".


@@ -951,13 +963,21 @@
 
       initialize_object (obj);
 
-      if (! default_initialize)
-        run_constructor (obj, args);
+      retval = run_constructor (obj, args, nargout, default_initialize);
 
-      return obj;
+      /*
+      // Copy any extra constructor outputs into retval.
+      for (int i = 1; i < ctor_retval.length (); i++)
+        {
+          retval.resize (i + 1);
+          retval(i) = ctor_retval(i);
+        }
+
+      retval(0) = to_ov (obj);
+      */
     }
 
-  return cdef_object ();
+  return retval;
 }
 
 static octave_value


The commented block looks like a remainder from an earlier attempt to fix the
bug. Did you leave that in on purpose, or can it be removed?

You are using MATLAB compatible syntax for the classdef files (generic "end"
instead of the more specific variants). Is that on purpose?

In the .tst file, you are writing "previous test, but with handle classes"
repeatedly. That might be because I'm not a native speaker, but shouldn't that
use a singular form instead? I.e., "previous test, but with handle class"?


If I understand correctly, many of the changed lines are just indentation. Or
did I miss some actual change in the indented code block in
`cdef_class::cdef_class_rep::run_constructor`?
The overall idea of changing the output argument to a `octave_value_list` to
allow returning multiple output arguments looks like a good approach to me.
👍



    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?67845>

_______________________________________________
Message sent via Savannah
https://savannah.gnu.org/

-----BEGIN PGP SIGNATURE-----

iHUEABYIAB0WIQQk97aszIMMAvLLwm6qLAuaBUf3TgUCac57tQAKCRCqLAuaBUf3
Tt0oAQCZK0rmnBaVDjeKPt+fhlq1d0eRAvYiBdco/+201nKOgwD/YmcfttGbtiaM
jrr8ApzAKwQOkDoHE7x/4hj7HTXzgQY=
=jcZv
-----END PGP SIGNATURE-----