# HG changeset patch
# User Imad-Eddine Srairi <imad@srairi.net>
# Date 1560168431 -7200
#      Mon Jun 10 14:07:11 2019 +0200
# Node ID 59f394f89f2839031168c2619aa4e541215d9d2f
# Parent  58bdc2d984818211ed409b1ef2d6def22892cfcb
Keep track of and preserve classdef properties order

The order of properties in the classdef definition was lost, and they
were returned -e.g. when displayed with function 'properties'- in the
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
(this was reported in bug #55961).

diff -r 58bdc2d98481 -r 59f394f89f28 libinterp/octave-value/cdef-class.cc
--- a/libinterp/octave-value/cdef-class.cc	Sat Jun 08 21:17:03 2019 -0700
+++ b/libinterp/octave-value/cdef-class.cc	Mon Jun 10 14:07:11 2019 +0200
@@ -390,6 +390,8 @@
   cdef_class::cdef_class_rep::install_property (const cdef_property& prop)
   {
     property_map[prop.get_name ()] = prop;
+    // Register the insertion rank of this property
+    property_rank_map[prop.get_name ()] = member_count;
 
     member_count++;
   }
@@ -397,7 +399,7 @@
   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);
 
@@ -411,10 +413,10 @@
     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);
 
@@ -422,17 +424,36 @@
   }
 
   void
-  cdef_class::cdef_class_rep::find_properties (std::map<std::string,
+  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)
+  {
     property_const_iterator it;
+    // 'offset' starts at 0 and is incremented whenever we move
+    // in the class ancestry list.
+    static unsigned int property_offset {0};
 
     for (it = property_map.begin (); it != property_map.end (); ++it)
       {
         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)
               {
@@ -443,10 +464,15 @@
                   continue;
               }
 
-            props[nm] = it->second;
+            const property_key pk =
+              std::make_tuple(property_offset + property_rank_map[nm], nm);
+            props[pk] = it->second;
+            prop_names.insert(nm);
           }
       }
 
+    property_offset += member_count;
+
     // Look into superclasses
 
     Cell super_classes = get ("SuperClasses").cell_value ();
@@ -455,11 +481,14 @@
       {
         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
@@ -1148,4 +1177,9 @@
 
     return p;
   }
+
+  std::string get_name(property_key pk)
+  {
+    return std::get<1>(pk);
+  }
 }
diff -r 58bdc2d98481 -r 59f394f89f28 libinterp/octave-value/cdef-class.h
--- a/libinterp/octave-value/cdef-class.h	Sat Jun 08 21:17:03 2019 -0700
+++ b/libinterp/octave-value/cdef-class.h	Mon Jun 10 14:07:11 2019 +0200
@@ -44,6 +44,13 @@
   class interpreter;
   class tree_classdef;
 
+  // When we want to preserve the order in which properties were first defined,
+  // we index them with a 2-tuple key made of:
+  // - first, their rank in the property list;
+  // - and second, their name.
+  typedef std::tuple<unsigned int, std::string> property_key;
+  std::string get_name(property_key pk);
+
   class
   cdef_class : public cdef_meta_object
   {
@@ -92,7 +99,7 @@
 
       Cell get_properties (int mode);
 
-      std::map<std::string, cdef_property> get_property_map (int mode);
+      std::map<property_key, cdef_property> get_property_map (int mode);
 
       string_vector get_names (void);
 
@@ -143,6 +150,7 @@
             member_count = 0;
             method_map.clear ();
             property_map.clear ();
+            property_rank_map.clear ();
           }
         else
           delete this;
@@ -158,8 +166,11 @@
 
       void find_names (std::set<std::string>& names, bool all);
 
-      void find_properties (std::map<std::string,cdef_property>& props,
+      void find_properties (std::map<property_key,cdef_property>& props,
                             int mode = 0);
+      void find_properties_aux (std::map<property_key,cdef_property>& props,
+                                std::set<std::string>& prop_names,
+                                int mode = 0);
 
       void find_methods (std::map<std::string, cdef_method>& meths,
                          bool only_inherited, bool include_ctor = false);
@@ -182,6 +193,7 @@
       // The properties defined by this class.
 
       std::map<std::string,cdef_property> property_map;
+      std::map<std::string, unsigned int> property_rank_map;
 
       // The number of members in this class (methods, properties...)
 
@@ -283,7 +295,7 @@
       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);
diff -r 58bdc2d98481 -r 59f394f89f28 libinterp/octave-value/cdef-object.cc
--- a/libinterp/octave-value/cdef-object.cc	Sat Jun 08 21:17:03 2019 -0700
+++ b/libinterp/octave-value/cdef-object.cc	Mon Jun 10 14:07:11 2019 +0200
@@ -145,7 +145,7 @@
 
     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);
 
@@ -161,14 +161,14 @@
                 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 (get_name(prop_val.first), cvalue);
               }
             else
               {
                 Cell cvalue (dim_vector (1, 1),
                              prop_val.second.get_value (*this, false));
 
-                retval.setfield (prop_val.first, cvalue);
+                retval.setfield (get_name(prop_val.first), cvalue);
               }
           }
       }
diff -r 58bdc2d98481 -r 59f394f89f28 libinterp/octave-value/ov-classdef.cc
--- a/libinterp/octave-value/ov-classdef.cc	Sat Jun 08 21:17:03 2019 -0700
+++ b/libinterp/octave-value/ov-classdef.cc	Mon Jun 10 14:07:11 2019 +0200
@@ -266,14 +266,14 @@
 
       increment_indent_level ();
 
-      std::map<std::string, octave::cdef_property> props;
+      std::map<octave::property_key, octave::cdef_property> props;
 
       props = cls.get_property_map ();
 
       size_t max_len = 0;
       for (const auto& nm_prop : props)
         {
-          const std::string& nm = nm_prop.first;
+          const std::string& nm = octave::get_name(nm_prop.first);
 
           size_t sz = nm.size ();
 
@@ -285,7 +285,7 @@
         {
           indent (os);
 
-          const std::string& nm = nm_prop.first;
+          const std::string& nm = octave::get_name(nm_prop.first);
 
           if (is_array)
             os << "  " << nm;
@@ -555,7 +555,7 @@
   if (! cls.ok ())
     error ("invalid class: %s", class_name.c_str ());
 
-  std::map<std::string, octave::cdef_property> property_map = cls.get_property_map ();
+  std::map<octave::property_key, octave::cdef_property> property_map = cls.get_property_map ();
 
   std::list<std::string> property_names;
 
