Do not treat struct as return argument type for saveobj differently (bug #45833).

* libinterp/octave-value/ov-classdef.cc (octave_classdef::saveobj): Set
indicator for a different type of the return argument of an overloaded "saveobj"
method anytime the type and class does not match the original type and class of
the object that is to be saved.
(octave_classdef::loadobj): Adjust for the change in octave_classdef::saveobj.
Emit more specific warnings for (currently) unsupported scenarios.
* test/classdef-load-save/classdef-load-save.tst: Adjust test for saveobj
returning a structure to the actually expected behavior. Add test for saveobj
returning an object (of the correct class).
* test/classdef-load-save/saveobj_obj_class.m: Add definition for class that
returns an object of the correct class from the overloaded "saveobj" method and
that does not overload "loadobj".
* test/classdef-load-save/saveobj_struct_class.m: Rename from "saveobj_class.m".
* test/classdef-load-save/module.mk: Add new and renamed files to dist tarball.

Author: mmuetzel

libinterp/octave-value/ov-classdef.cc

@@ -104,11 +104,12 @@ octave_classdef::saveobj (std::vector<bool>& is_new)
                   warning ("save: saveobj method does not return a value");
                   return m;
                 }
-              else if (! (retval.isobject () || retval.isstruct ()))
+              else if (! retval.is_classdef_object ()
+                       || retval.class_name () != class_name ())
                 {
-                  // If retval is not a struct or an object, we put the value
-                  // in a map and set the 'custom_saveobj_ret_type' flag, which
-                  // has to be encoded in the file metadata.
+                  // If retval is not an object of the matching class, we put
+                  // the value in a map and set the 'custom_saveobj_ret_type'
+                  // flag, which has to be encoded in the file metadata.
                   // It's the caller's responsibility to check the flag.
                   std::get<octave_map> (m[n]).setfield ("any", retval);
                   std::get<bool> (m[n]) = true;
@@ -171,17 +172,25 @@ octave_classdef::loadobj (std::vector<std::tuple<octave_map, uint32_t, bool>>& m
                 }
 
               // FIXME: A loadobj method can return any type. If the return
-              //        type is not a classdef object, then the loaded object
-              //        must be replaced by whatever the return type and
-              //        contents are.
-              if (! ov.isobject ())
+              //        type is not a classdef object of the correct class,
+              //        then the loaded object must be replaced by whatever the
+              //        return type and contents are.
+              if (! ov.is_classdef_object ())
                 {
                   std::string type = ov.type_name ();
-                  warning ("load: loadobj method return non-classdef return type '%s'. "
-                           "This is currently not supported.",
+                  warning ("load: loadobj method does not return correct type "
+                           "'%s'. This is currently not supported.",
                            type.c_str ());
                   return;
                 }
+              else if (ov.class_name () != class_name ())
+                {
+                  std::string class_nm = ov.class_name ();
+                  warning ("load: loadobj method does not return classdef object "
+                           "of correct class '%s'. This is currently not supported.",
+                           class_nm.c_str ());
+                  return;
+                }
 
               if (in_obj_cache)
                 // Copy the results from the loadobj methods to the object in
@@ -192,12 +201,42 @@ octave_classdef::loadobj (std::vector<std::tuple<octave_map, uint32_t, bool>>& m
             }
           else
             {
-              // If custom saveobj is implemented, then a variable named 'any'
-              // is meant to be passed to loadobj, but if loadobj is not
-              // implemented, it should not fill in any property 'any' in the
-              // class.
               if (std::get<bool> (m[n]))
-                return;
+                {
+                  // If saveobj is overloaded by this classdef and it returned
+                  // anything other than a classdef object of the correct
+                  // class, then a variable named 'any' is meant to be passed
+                  // to loadobj, but if loadobj is not overloaded, it should
+                  // not fill in any property 'any' in the loaded object.
+                  if (! prop_map.isfield ("any") || prop_map.numel () != 1)
+                    {
+                      warning ("load: expected scalar value for custom type when loading object");
+                      return;
+                    }
+
+                  // FIXME: What should be done here?
+                  octave_value any_val = (prop_map.getfield ("any"))(0);
+                  std::string type = any_val.type_name ();
+                  if (type != type_name ())
+                    {
+                      warning ("load: cannot restore value of object that was saved as a different type (%s)",
+                               type.c_str ());
+                      return;
+                    }
+
+                  std::string cls_nm = any_val.class_name ();
+                  if (cls_nm != class_name ())
+                    {
+                      warning ("load: cannot restore value of object that was saved as a different class (%s)",
+                               cls_nm.c_str ());
+                      return;
+                    }
+
+                  // If the value in the "any" field has the correct type and
+                  // class, we can load it like it were saved "normally".
+                  // FIXME: Can this ever happen?
+                  prop_map = any_val.classdef_object_value ()->map_value (false, true);
+                }
 
               octave::cdef_object new_object;
               if (in_obj_cache)

test/classdef-load-save/classdef-load-save.tst

@@ -110,16 +110,33 @@
 %!   delete (savefile);
 %! end_unwind_protect
 
+## No constructor, ConstructOnLoad = false, saveobj returns an object, no loadobj
+%!test
+%! obj = saveobj_obj_class ();
+%! obj.a = 1;
+%! obj.b = 3;
+%! savefile = tempname ();
+%! save ('-text', savefile, 'obj');
+%! unwind_protect
+%!   clear obj;
+%!   load (savefile);
+%!   assert(obj.a, 2);
+%!   assert(obj.b, 3);
+%!   assert(obj.c, []);
+%! unwind_protect_cleanup
+%!   delete (savefile);
+%! end_unwind_protect
+
 ## No constructor, ConstructOnLoad = false, saveobj returns a struct, no loadobj
 %!test
-%! obj = saveobj_class ();
+%! obj = saveobj_struct_class ();
 %! obj.a = 1;
 %! savefile = tempname ();
 %! save ('-text', savefile, 'obj');
 %! unwind_protect
 %!   clear obj;
 %!   load (savefile);
-%!   assert(obj.a, 1);
+%!   assert(obj.a, []);
 %!   assert(obj.b, []);
 %! unwind_protect_cleanup
 %!   delete (savefile);

test/classdef-load-save/module.mk

@@ -8,7 +8,8 @@ classdef_load_save_TEST_FILES = \
   %reldir%/regular_class_construct_on_load.m \
   %reldir%/regular_class_with_constructor.m \
   %reldir%/regular_handle_class.m \
-  %reldir%/saveobj_class.m \
+  %reldir%/saveobj_obj_class.m \
+  %reldir%/saveobj_struct_class.m \
   %reldir%/transient_property_class.m
 
 TEST_FILES += $(classdef_load_save_TEST_FILES)

test/classdef-load-save/saveobj_obj_class.m

@@ -0,0 +1,7 @@
+classdef saveobj_obj_class < regular_class
+  methods
+    function obj = saveobj (obj)
+      obj.a = 2;
+    endfunction
+  endmethods
+endclassdef

test/classdef-load-save/saveobj_class.m renamed to test/classdef-load-save/saveobj_struct_class.m

@@ -1,4 +1,4 @@
-classdef saveobj_class < regular_class
+classdef saveobj_struct_class < regular_class
   methods
     function s = saveobj (obj)
       s.a = obj.a;