Revert "Re-enforce ordering"

This reverts commit b22fcf70cd77b4cb1bb44a63a9e1d1afe90fe65b.

eeschema/lib_field.cpp

@@ -232,14 +232,13 @@ void LIB_FIELD::Copy( LIB_FIELD* aTarget ) const
 
 int LIB_FIELD::compare( const SCH_ITEM& aOther, int aCompareFlags ) const
 {
+    wxASSERT( aOther.Type() == LIB_FIELD_T );
 
     int retv = SCH_ITEM::compare( aOther, aCompareFlags );
 
     if( retv )
         return retv;
 
-    wxASSERT( aOther.Type() == LIB_FIELD_T );
-
     const LIB_FIELD* tmp = ( LIB_FIELD* ) &aOther;
 
     // Equality test will vary depending whether or not the field is mandatory.  Otherwise,

eeschema/lib_pin.cpp

@@ -995,13 +995,13 @@ EDA_ITEM* LIB_PIN::Clone() const
 
 int LIB_PIN::compare( const SCH_ITEM& aOther, int aCompareFlags ) const
 {
+    wxASSERT( aOther.Type() == LIB_PIN_T );
+
     int retv = SCH_ITEM::compare( aOther, aCompareFlags );
 
     if( retv )
         return retv;
 
-    wxASSERT( aOther.Type() == LIB_PIN_T );
-
     const LIB_PIN* tmp = (LIB_PIN*) &aOther;
 
     // When comparing units, we do not compare the part numbers.  If everything else is

eeschema/lib_text.cpp

@@ -78,13 +78,13 @@ EDA_ITEM* LIB_TEXT::Clone() const
 
 int LIB_TEXT::compare( const SCH_ITEM& aOther, int aCompareFlags ) const
 {
+    wxASSERT( aOther.Type() == LIB_TEXT_T );
+
     int retv = SCH_ITEM::compare( aOther, aCompareFlags );
 
     if( retv )
         return retv;
 
-    wxASSERT( aOther.Type() == LIB_TEXT_T );
-
     const LIB_TEXT* tmp = ( LIB_TEXT* ) &aOther;
 
     int result = GetText().CmpNoCase( tmp->GetText() );

eeschema/lib_textbox.cpp

@@ -190,13 +190,13 @@ VECTOR2I LIB_TEXTBOX::GetDrawPos() const
 
 int LIB_TEXTBOX::compare( const SCH_ITEM& aOther, int aCompareFlags ) const
 {
+    wxASSERT( aOther.Type() == LIB_TEXTBOX_T );
+
     int retv = SCH_ITEM::compare( aOther, aCompareFlags );
 
     if( retv )
         return retv;
 
-    wxASSERT( aOther.Type() == LIB_TEXTBOX_T );
-
     const LIB_TEXTBOX* tmp = static_cast<const LIB_TEXTBOX*>( &aOther );
 
     int result = GetText().CmpNoCase( tmp->GetText() );

eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr.cpp

@@ -789,29 +789,7 @@ void SCH_IO_KICAD_SEXPR::saveSymbol( SCH_SYMBOL* aSymbol, const SCHEMATIC& aSche
             field.SetText( value );
     }
 
-    std::vector<SCH_PIN*> pins;
-    pins.reserve( aSymbol->GetRawPins().size() );
-
     for( const std::unique_ptr<SCH_PIN>& pin : aSymbol->GetRawPins() )
-        pins.push_back( pin.get() );
-
-    std::sort( pins.begin(), pins.end(), []( const SCH_PIN* a, const SCH_PIN* b )
-    {
-        int cmp = StrNumCmp( a->GetName(), b->GetName() );
-
-        if( cmp != 0 )
-            return cmp < 0;
-
-        cmp = StrNumCmp( a->GetNumber(), b->GetNumber() );
-
-        if( cmp != 0 )
-            return cmp < 0;
-
-        // The UUID is used as a tie breaker to ensure a stable sort.
-        return a->m_Uuid < b->m_Uuid;
-    } );
-
-    for( SCH_PIN* pin : pins )
     {
         if( pin->GetAlt().IsEmpty() )
         {
@@ -837,21 +815,10 @@ void SCH_IO_KICAD_SEXPR::saveSymbol( SCH_SYMBOL* aSymbol, const SCHEMATIC& aSche
         SCH_SHEET_LIST fullHierarchy = aSchematic.GetSheets();
         bool project_open = false;
 
-        std::vector<SCH_SYMBOL_INSTANCE> orderedInstances = aSymbol->GetInstances();
+        for( size_t i = 0; i < aSymbol->GetInstances().size(); i++ )
-        std::sort( orderedInstances.begin(), orderedInstances.end(), []( const SCH_SYMBOL_INSTANCE& a, const SCH_SYMBOL_INSTANCE& b )
         {
-            if( a.m_ProjectName != b.m_ProjectName )
-                return a.m_ProjectName < b.m_ProjectName;
-
-            return a.m_Path < b.m_Path;
-        } );
-
-        for( size_t ii = 0; ii < orderedInstances.size(); ++ii )
-        {
-            SCH_SYMBOL_INSTANCE& inst = orderedInstances[ii];
-
             // Zero length KIID_PATH objects are not valid and will cause a crash below.
-            wxCHECK2( inst.m_Path.size(), continue );
+            wxCHECK2( aSymbol->GetInstances()[i].m_Path.size(), continue );
 
             // If the instance data is part of this design but no longer has an associated sheet
             // path, don't save it.  This prevents large amounts of orphaned instance data for the
@@ -859,11 +826,11 @@ void SCH_IO_KICAD_SEXPR::saveSymbol( SCH_SYMBOL* aSymbol, const SCHEMATIC& aSche
             //
             // Keep all instance data when copying to the clipboard.  It may be needed on paste.
             if( !aForClipboard
-              && ( inst.m_Path[0] == rootSheetUuid )
+              && ( aSymbol->GetInstances()[i].m_Path[0] == rootSheetUuid )
-              && !fullHierarchy.GetSheetPathByKIIDPath( inst.m_Path ) )
+              && !fullHierarchy.GetSheetPathByKIIDPath( aSymbol->GetInstances()[i].m_Path ) )
             {
-                if( project_open && ( ( &inst == &orderedInstances.back() )
+                if( project_open && ( ( i + 1 == aSymbol->GetInstances().size() )
-                  || lastProjectUuid != orderedInstances[ii + 1].m_Path[0] ) )
+                  || lastProjectUuid != aSymbol->GetInstances()[i+1].m_Path[0] ) )
                 {
                     m_out->Print( aNestLevel + 2, ")\n" );  // Closes `project`.
                     project_open = false;
@@ -872,23 +839,23 @@ void SCH_IO_KICAD_SEXPR::saveSymbol( SCH_SYMBOL* aSymbol, const SCHEMATIC& aSche
                 continue;
             }
 
-            if( lastProjectUuid != inst.m_Path[0] )
+            if( lastProjectUuid != aSymbol->GetInstances()[i].m_Path[0] )
             {
                 wxString projectName;
 
-                if( inst.m_Path[0] == rootSheetUuid )
+                if( aSymbol->GetInstances()[i].m_Path[0] == rootSheetUuid )
                     projectName = aSchematic.Prj().GetProjectName();
                 else
-                    projectName = inst.m_ProjectName;
+                    projectName = aSymbol->GetInstances()[i].m_ProjectName;
 
-                lastProjectUuid = inst.m_Path[0];
+                lastProjectUuid = aSymbol->GetInstances()[i].m_Path[0];
                 m_out->Print( aNestLevel + 2, "(project %s\n",
                               m_out->Quotew( projectName ).c_str() );
                 project_open = true;
             }
 
             wxString path;
-            KIID_PATH tmp = inst.m_Path;
+            KIID_PATH tmp = aSymbol->GetInstances()[i].m_Path;
 
             if( aForClipboard && aRelativePath )
                 tmp.MakeRelativeTo( aRelativePath->Path() );
@@ -898,12 +865,12 @@ void SCH_IO_KICAD_SEXPR::saveSymbol( SCH_SYMBOL* aSymbol, const SCHEMATIC& aSche
             m_out->Print( aNestLevel + 3, "(path %s\n",
                           m_out->Quotew( path ).c_str() );
             m_out->Print( aNestLevel + 4, "(reference %s) (unit %d)\n",
-                          m_out->Quotew( inst.m_Reference ).c_str(),
+                          m_out->Quotew( aSymbol->GetInstances()[i].m_Reference ).c_str(),
-                          inst.m_Unit );
+                          aSymbol->GetInstances()[i].m_Unit );
             m_out->Print( aNestLevel + 3, ")\n" );
 
-            if( project_open && ( ( &inst == &orderedInstances.back() )
+            if( project_open && ( ( i + 1 == aSymbol->GetInstances().size() )
-              || lastProjectUuid != orderedInstances[ii + 1].m_Path[0] ) )
+              || lastProjectUuid != aSymbol->GetInstances()[i+1].m_Path[0] ) )
             {
                 m_out->Print( aNestLevel + 2, ")\n" );  // Closes `project`.
                 project_open = false;

eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr_lib_cache.cpp

@@ -198,17 +198,6 @@ void SCH_IO_KICAD_SEXPR_LIB_CACHE::SaveSymbol( LIB_SYMBOL* aSymbol, OUTPUTFORMAT
 
         aSymbol->GetFields( fields );
 
-        std::sort( fields.begin(), fields.end(),
-                   []( const LIB_FIELD* a, const LIB_FIELD* b )
-                   {
-                        // Mandatory fields are always first.  They are only ones with fixed
-                        // field IDs.  The rest are sorted by their comparison operator.
-                        if( a->IsMandatory() || b->IsMandatory() )
-                            return a->GetId() < b->GetId();
-
-                        return a->Compare( b ) < 0;
-                   } );
-
         for( LIB_FIELD* field : fields )
             saveField( field, aFormatter, aNestLevel + 1 );
 
@@ -255,16 +244,19 @@ void SCH_IO_KICAD_SEXPR_LIB_CACHE::SaveSymbol( LIB_SYMBOL* aSymbol, OUTPUTFORMAT
                 aFormatter.Print( aNestLevel + 2, "(unit_name %s)\n",
                                   aFormatter.Quotes( name ).c_str() );
             }
+            // Enforce item ordering
+            auto cmp =
+                    []( const SCH_ITEM* a, const SCH_ITEM* b )
+                    {
+                        return *a < *b;
+                    };
 
-            std::vector<SCH_ITEM*> save_vec;
+            std::multiset<SCH_ITEM*, decltype( cmp )> save_map( cmp );
-            save_vec.reserve( unit.m_items.size() );
 
             for( SCH_ITEM* item : unit.m_items )
-                save_vec.push_back( item );
+                save_map.insert( item );
 
-            std::sort( save_vec.begin(), save_vec.end(), SCH_ITEM::cmp_items());
+            for( SCH_ITEM* item : save_map )
-
-            for( SCH_ITEM* item : save_vec )
                 saveSymbolDrawItem( item, aFormatter, aNestLevel + 2 );
 
             aFormatter.Print( aNestLevel + 1, ")\n" );
@@ -282,17 +274,6 @@ void SCH_IO_KICAD_SEXPR_LIB_CACHE::SaveSymbol( LIB_SYMBOL* aSymbol, OUTPUTFORMAT
 
         aSymbol->GetFields( fields );
 
-        std::sort( fields.begin(), fields.end(),
-                   []( const LIB_FIELD* a, const LIB_FIELD* b )
-                   {
-                        // Mandatory fields are always first.  They are only ones with fixed
-                        // field IDs.  The rest are sorted by their comparison operator.
-                        if( a->IsMandatory() || b->IsMandatory() )
-                            return a->GetId() < b->GetId();
-
-                        return a->Compare( b ) < 0;
-                   } );
-
         for( LIB_FIELD* field : fields )
             saveField( field, aFormatter, aNestLevel + 1 );
 

eeschema/sch_item.h

@@ -651,17 +651,17 @@ public:
 
     virtual bool operator<( const SCH_ITEM& aItem ) const;
 
-    struct cmp_items
-    {
-        bool operator()( const SCH_ITEM* aFirst, const SCH_ITEM* aSecond ) const;
-    };
-
 protected:
     SCH_RENDER_SETTINGS* getRenderSettings( PLOTTER* aPlotter ) const
     {
         return static_cast<SCH_RENDER_SETTINGS*>( aPlotter->RenderSettings() );
     }
 
+    struct cmp_items
+    {
+        bool operator()( const SCH_ITEM* aFirst, const SCH_ITEM* aSecond ) const;
+    };
+
     void getSymbolEditorMsgPanelInfo( EDA_DRAW_FRAME* aFrame, std::vector<MSG_PANEL_ITEM>& aList );
 
     /**