Keep track of and preserve classdef properties order (bug #55961).

The order of properties in the classdef definition was lost, and they were
returned - e.g., when displayed with function 'properties' - in alphabetical
order. We now keep track of their original order and use this order in a (map)
structure which is de facto sorted as expected.

* cdef-class.h: Add new typedef "octave::property_key".
(cdef_class, cdef_class::cdef_class_rep): Change map holding the classdef
properties to use the new type as the key (instead of "std::string") for all
relevant member functions and properties.
(cdef_class_rep::find_properties_aux): Add new (helper) method.
* cdef-class.cc (cdef_class::cdef_class_rep::install_property): Set
m_property_rank_map.
(cdef_class::cdef_class_rep::get_properties,
cdef_class::cdef_class_rep::get_property_map): Change type of properties map.
(cdef_class::cdef_class_rep::find_properties,
cdef_class::cdef_class_rep::find_properties_aux): Fill properties map in
correct order.
* ov-classdef.cc: Adapt test for "properties" function with the correct order.
(octave_classdef::print_raw, Fproperties): Change type of properties map.
* bp-table.cc (user_code_provider::populate_function_cache): Change type of
properties map.
* scripts/miscellaneous/fieldnames.m, test/classdef/classdef.tst: Adapt test
for "properties" function with the correct order.

libinterp/octave-value/cdef-class.cc

@@ -385,14 +385,16 @@ void
 cdef_class::cdef_class_rep::install_property (const cdef_property& prop)
 {
   m_property_map[prop.get_name ()] = prop;
+  // Register the insertion rank of this property
+  m_property_rank_map[prop.get_name ()] = m_member_count;
 
   m_member_count++;
 }
 
 Cell
 cdef_class::cdef_class_rep::get_properties (int mode)
 {
-  std::map<std::string, cdef_property> props;
+  std::map<property_key, cdef_property> props;
 
   props = get_property_map (mode);
 
@@ -406,26 +408,43 @@ cdef_class::cdef_class_rep::get_properties (int mode)
   return c;
 }
 
-std::map<std::string, cdef_property>
+std::map<property_key, cdef_property>
 cdef_class::cdef_class_rep::get_property_map (int mode)
 {
-  std::map<std::string, cdef_property> props;
+  std::map<property_key, cdef_property> props;
 
   find_properties (props, mode);
 
   return props;
 }
 
 void
-cdef_class::cdef_class_rep::find_properties (std::map<std::string,
-    cdef_property>& props,
-    int mode)
+cdef_class::cdef_class_rep::find_properties
+  (std::map<property_key, cdef_property>& props, int mode)
 {
+  std::set<std::string> prop_names;
+
+  // The only reason we are introducing 'prop_names' is to keep
+  // track of property names and avoid returning duplicates.
+  // There is no easy way to do it based on 'props', which is going
+  // to use complex keys of type 'property_key'.
+  find_properties_aux (props, prop_names, mode);
+}
+
+void
+cdef_class::cdef_class_rep::find_properties_aux
+  (std::map<property_key, cdef_property>& props,
+   std::set<std::string>& prop_names, int mode)
+{
+  // 'offset' starts at 0 and is incremented whenever we move
+  // in the class ancestry list.
+  static unsigned int property_offset {0};
+
   for (const auto& it : m_property_map)
     {
       std::string nm = it.second.get_name ();
 
-      if (props.find (nm) == props.end ())
+      if (prop_names.find (nm) == prop_names.end ())
         {
           if (mode == property_inherited)
             {
@@ -436,10 +455,15 @@ cdef_class::cdef_class_rep::find_properties (std::map<std::string,
                 continue;
             }
 
-          props[nm] = it.second;
+          const property_key pk =
+            std::make_pair (property_offset + m_property_rank_map[nm], nm);
+          props[pk] = it.second;
+          prop_names.insert (nm);
         }
     }
 
+  property_offset += m_member_count;
+
   // Look into superclasses
 
   Cell super_classes = get ("SuperClasses").cell_value ();
@@ -448,11 +472,13 @@ cdef_class::cdef_class_rep::find_properties (std::map<std::string,
     {
       cdef_class cls = lookup_class (super_classes(i));
 
-      cls.get_rep ()->find_properties (props,
-                                       (mode == property_all
-                                        ? property_all
-                                        : property_inherited));
+      cls.get_rep ()->find_properties_aux (props, prop_names,
+                                           (mode == property_all
+                                            ? property_all
+                                            : property_inherited));
     }
+
+  property_offset = 0;
 }
 
 void

libinterp/octave-value/cdef-class.h

@@ -31,6 +31,7 @@
 #include <map>
 #include <set>
 #include <string>
+#include <utility>
 
 #include "oct-refcount.h"
 
@@ -48,6 +49,12 @@ OCTAVE_BEGIN_NAMESPACE(octave)
 class interpreter;
 class tree_classdef;
 
+// When we want to preserve the order in which properties were first defined,
+// we index them with a std::pair key made of:
+// - first, their rank in the property list;
+// - and second, their name.
+typedef std::pair<unsigned int, std::string> property_key;
+
 class OCTINTERP_API cdef_class : public cdef_meta_object
 {
 private:
@@ -97,7 +104,7 @@ class OCTINTERP_API cdef_class : public cdef_meta_object
 
     OCTINTERP_API Cell get_properties (int mode);
 
-    OCTINTERP_API std::map<std::string, cdef_property>
+    OCTINTERP_API std::map<property_key, cdef_property>
     get_property_map (int mode);
 
     OCTINTERP_API string_vector get_names ();
@@ -149,6 +156,7 @@ class OCTINTERP_API cdef_class : public cdef_meta_object
           m_member_count = 0;
           m_method_map.clear ();
           m_property_map.clear ();
+          m_property_rank_map.clear ();
         }
       else
         delete this;
@@ -169,9 +177,14 @@ class OCTINTERP_API cdef_class : public cdef_meta_object
     OCTINTERP_API void find_names (std::set<std::string>& names, bool all);
 
     OCTINTERP_API void
-    find_properties (std::map<std::string, cdef_property>& props,
+    find_properties (std::map<property_key, cdef_property>& props,
                      int mode = 0);
 
+    OCTINTERP_API void
+    find_properties_aux (std::map<property_key, cdef_property>& props,
+                         std::set<std::string>& prop_names,
+                         int mode = 0);
+
     OCTINTERP_API void
     find_methods (std::map<std::string, cdef_method>& meths,
                   bool only_inherited, bool include_ctor = false);
@@ -196,6 +209,7 @@ class OCTINTERP_API cdef_class : public cdef_meta_object
     // The properties defined by this class.
 
     std::map<std::string, cdef_property> m_property_map;
+    std::map<std::string, unsigned int> m_property_rank_map;
 
     // The number of members in this class (methods, properties...)
 
@@ -220,8 +234,8 @@ class OCTINTERP_API cdef_class : public cdef_meta_object
 
     typedef std::map<std::string, cdef_method>::iterator method_iterator;
     typedef std::map<std::string, cdef_method>::const_iterator method_const_iterator;
-    typedef std::map<std::string, cdef_property>::iterator property_iterator;
-    typedef std::map<std::string, cdef_property>::const_iterator property_const_iterator;
+    typedef std::map<property_key, cdef_property>::iterator property_iterator;
+    typedef std::map<property_key, cdef_property>::const_iterator property_const_iterator;
 
     cdef_class_rep (const cdef_class_rep& c) = default;
   };
@@ -290,7 +304,7 @@ class OCTINTERP_API cdef_class : public cdef_meta_object
     return get_rep ()->get_properties (mode);
   }
 
-  std::map<std::string, cdef_property>
+  std::map<property_key, cdef_property>
   get_property_map (int mode = property_normal)
   {
     return get_rep ()->get_property_map (mode);

libinterp/octave-value/cdef-object.cc

@@ -148,7 +148,7 @@ cdef_object::map_value () const
 
   if (cls.ok ())
     {
-      std::map<std::string, cdef_property> props;
+      std::map<property_key, cdef_property> props;
 
       props = cls.get_property_map (cdef_class::property_all);
 
@@ -164,14 +164,14 @@ cdef_object::map_value () const
               for (octave_idx_type i = 0; i < a_obj.numel (); i++)
                 cvalue (i) = prop_val.second.get_value (a_obj(i), false);
 
-              retval.setfield (prop_val.first, cvalue);
+              retval.setfield (prop_val.second.get_name (), cvalue);
             }
           else
             {
               Cell cvalue (dim_vector (1, 1),
                            prop_val.second.get_value (*this, false));
 
-              retval.setfield (prop_val.first, cvalue);
+              retval.setfield (prop_val.second.get_name (), cvalue);
             }
         }
     }

libinterp/octave-value/ov-classdef.cc

@@ -363,7 +363,7 @@ octave_classdef::print_raw (std::ostream& os, bool) const
 
       increment_indent_level ();
 
-      std::map<std::string, octave::cdef_property> property_map
+      std::map<octave::property_key, octave::cdef_property> property_map
         = cls.get_property_map ();
 
       std::size_t max_len = 0;
@@ -750,7 +750,7 @@ is public and if the @code{Hidden} attribute is false.
   if (! cls.ok ())
     error ("invalid class: %s", class_name.c_str ());
 
-  std::map<std::string, cdef_property> property_map =
+  std::map<octave::property_key, cdef_property> property_map =
     cls.get_property_map ();
 
   std::list<std::string> property_names;
@@ -771,7 +771,7 @@ is public and if the @code{Hidden} attribute is false.
       if (hid.bool_value ())
         continue;
 
-      property_names.push_back (pname_prop.first);
+      property_names.push_back (pname_prop.second.get_name ());
     }
 
   if (nargout > 0)
@@ -789,9 +789,9 @@ is public and if the @code{Hidden} attribute is false.
 
 /*
 %!assert (properties ("inputParser"),
-%!        {"CaseSensitive"; "FunctionName"; "KeepUnmatched";
-%!         "Parameters"; "PartialMatching"; "Results";
-%!         "StructExpand"; "Unmatched"; "UsingDefaults"});
+%!        {"CaseSensitive"; "FunctionName"; "KeepUnmatched"; "PartialMatching";
+%!         "StructExpand"; "Parameters"; "Results"; "Unmatched";
+%!         "UsingDefaults"});
 */
 
 // FIXME: Need to implement the -full option.

libinterp/parse-tree/bp-table.cc

@@ -853,7 +853,7 @@ class user_code_provider
             }
 
           // Check get and set methods of properties
-          const std::map<std::string, cdef_property>& prop_map
+          const std::map<property_key, cdef_property>& prop_map
             = m_cls.get_property_map (cdef_class::property_all);
           for (const auto& prop : prop_map)
             {

scripts/miscellaneous/fieldnames.m

@@ -89,7 +89,7 @@
 %!test
 %! m = containers.Map ();
 %! f = fieldnames (m);
-%! assert (f, {"Count"; "KeyType"; "ValueType"; "map"; "numeric_keys"});
+%! assert (f, {"KeyType"; "ValueType"; "Count"; "map"; "numeric_keys"});
 
 ## Test old-style @class object
 %!test

test/classdef/classdef.tst

@@ -203,7 +203,7 @@
 
 %!test <*55766>
 %! x = class_bug55766 ();
-%! props = {"notahiddentestprop"; "publictestprop"; "testprop"};
+%! props = {"testprop"; "publictestprop"; "notahiddentestprop"};
 %! assert (properties (x), props);
 
 %!test <*60763>