Replace stale pin references with UNDO copy

When we replace a symbol with one that has fewer pins, the old pins
are released, which leaves points to them in the connection graph
dangling.  This updates the pointer to use the cloned copy in the undo
stack until the connection graph is rebuilt with the new data

Fixes https://gitlab.com/kicad/code/kicad/-/issues/17851

eeschema/connection_graph.cpp

@@ -85,6 +85,47 @@ void CONNECTION_SUBGRAPH::RemoveItem( SCH_ITEM* aItem )
 }
 
 
+void CONNECTION_SUBGRAPH::ExchangeItem( SCH_ITEM* aOldItem, SCH_ITEM* aNewItem )
+{
+    m_items.erase( aOldItem );
+    m_items.insert( aNewItem );
+
+    m_drivers.erase( aOldItem );
+    m_drivers.insert( aNewItem );
+
+    if( aOldItem == m_driver )
+    {
+        m_driver = aNewItem;
+        m_driver_connection = aNewItem->GetOrInitConnection( m_sheet, m_graph );
+    }
+
+    SCH_CONNECTION* old_conn = aOldItem->Connection( &m_sheet );
+    SCH_CONNECTION* new_conn = aNewItem->GetOrInitConnection( m_sheet, m_graph );
+
+    if( old_conn && new_conn )
+    {
+        new_conn->Clone( *old_conn );
+
+        if( old_conn->IsDriver() )
+            new_conn->SetDriver( aNewItem );
+
+        new_conn->ClearDirty();
+    }
+
+    if( aOldItem->Type() == SCH_SHEET_PIN_T )
+    {
+        m_hier_pins.erase( static_cast<SCH_SHEET_PIN*>( aOldItem ) );
+        m_hier_pins.insert( static_cast<SCH_SHEET_PIN*>( aNewItem ) );
+    }
+
+    if( aOldItem->Type() == SCH_HIER_LABEL_T )
+    {
+        m_hier_ports.erase( static_cast<SCH_HIERLABEL*>( aOldItem ) );
+        m_hier_ports.insert( static_cast<SCH_HIERLABEL*>( aNewItem ) );
+    }
+}
+
+
 bool CONNECTION_SUBGRAPH::ResolveDrivers( bool aCheckMultipleDrivers )
 {
     std::lock_guard lock( m_driver_mutex );
@@ -580,6 +621,53 @@ void CONNECTION_GRAPH::Merge( CONNECTION_GRAPH& aGraph )
 }
 
 
+void CONNECTION_GRAPH::ExchangeItem( SCH_ITEM* aOldItem, SCH_ITEM* aNewItem )
+{
+    wxCHECK2( aOldItem->Type() == aNewItem->Type(), return );
+
+    auto exchange = [&]( SCH_ITEM* aOld, SCH_ITEM* aNew )
+    {
+        auto it = m_item_to_subgraph_map.find( aOld );
+
+        if( it == m_item_to_subgraph_map.end() )
+            return;
+
+        CONNECTION_SUBGRAPH* sg = it->second;
+
+        sg->ExchangeItem( aOld, aNew );
+
+        m_item_to_subgraph_map.erase( it );
+        m_item_to_subgraph_map.emplace( aNew, sg );
+
+        for( auto it2 = m_items.begin(); it2 != m_items.end(); ++it2 )
+        {
+            if( *it2 == aOld )
+            {
+                *it2 = aNew;
+                break;
+            }
+        }
+    };
+
+    exchange( aOldItem, aNewItem );
+
+    if( aOldItem->Type() == SCH_SYMBOL_T )
+    {
+        SCH_SYMBOL* oldSymbol = static_cast<SCH_SYMBOL*>( aOldItem );
+        SCH_SYMBOL* newSymbol = static_cast<SCH_SYMBOL*>( aNewItem );
+        std::vector<SCH_PIN*> oldPins = oldSymbol->GetPins( &m_schematic->CurrentSheet() );
+        std::vector<SCH_PIN*> newPins = newSymbol->GetPins( &m_schematic->CurrentSheet() );
+
+        wxCHECK2( oldPins.size() == newPins.size(), return );
+
+        for( size_t ii = 0; ii < oldPins.size(); ii++ )
+        {
+            exchange( oldPins[ii], newPins[ii] );
+        }
+    }
+}
+
+
 void CONNECTION_GRAPH::Reset()
 {
     for( auto& subgraph : m_subgraphs )

eeschema/connection_graph.h

@@ -199,6 +199,11 @@ public:
 
     void RemoveItem( SCH_ITEM* aItem );
 
+    /**
+     * Replaces all references to #aOldItem with #aNewItem in the subgraph.
+    */
+    void ExchangeItem( SCH_ITEM* aOldItem, SCH_ITEM* aNewItem );
+
     // Use this to keep a connection pointer that is not owned by any item
     // This will be destroyed with the subgraph
     void StoreImplicitConnection( SCH_CONNECTION* aConnection )
@@ -456,6 +461,11 @@ public:
 
     void RemoveItem( SCH_ITEM* aItem );
 
+    /**
+     * Replace all references to #aOldItem with #aNewItem in the graph.
+    */
+    void ExchangeItem( SCH_ITEM* aOldItem, SCH_ITEM* aNewItem );
+
 private:
     /**
      * Update the graphical connectivity between items (i.e. where they touch)

eeschema/dialogs/dialog_change_symbols.cpp

@@ -23,6 +23,7 @@
 #include <algorithm>
 
 #include <bitmaps.h>
+#include <connection_graph.h>
 #include <string_utils.h>   // WildCompareString
 #include <kiway.h>
 #include <refdes_utils.h>
@@ -602,7 +603,17 @@ int DIALOG_CHANGE_SYMBOLS::processSymbols( SCH_COMMIT* aCommit,
         wxCHECK( screen, 0 );
 
         screen->Remove( symbol );
-        aCommit->Modify( symbol, screen );
+        SCH_SYMBOL* symbol_copy = static_cast<SCH_SYMBOL*>( symbol->Clone() );
+        aCommit->Modified( symbol, symbol_copy, screen );
+
+        CONNECTION_GRAPH*           connectionGraph = screen->Schematic()->ConnectionGraph();
+
+        // When we replace the lib symbol below, we free the associated pins if the new symbol has
+        // fewer than the original.  This will cause the connection graph to be out of date unless
+        // we replace references in the graph to the old symbol/pins with references to the ones stored
+        // in the undo stack.
+        if( connectionGraph )
+            connectionGraph->ExchangeItem( symbol, symbol_copy );
     }
 
     for( const auto& [ symbol, symbol_change_info ] : symbols )

eeschema/sch_edit_frame.cpp

@@ -1769,7 +1769,7 @@ void SCH_EDIT_FRAME::RecalculateConnections( SCH_COMMIT* aCommit, SCH_CLEANUP_FL
     {
         PICKED_ITEMS_LIST* changed_list = m_undoList.m_CommandsList.back();
         std::set<SCH_ITEM*> changed_items;
-        std::vector<VECTOR2I> pts;
+        std::set<VECTOR2I> pts;
         std::set<std::pair<SCH_SHEET_PATH, SCH_ITEM*>> item_paths;
 
         for( unsigned ii = 0; ii < changed_list->GetCount(); ++ii )
@@ -1784,7 +1784,7 @@ void SCH_EDIT_FRAME::RecalculateConnections( SCH_COMMIT* aCommit, SCH_CLEANUP_FL
             SCH_SHEET_PATHS& paths = screen->GetClientSheetPaths();
 
             std::vector<VECTOR2I> tmp_pts = item->GetConnectionPoints();
-            pts.insert( pts.end(), tmp_pts.begin(), tmp_pts.end() );
+            pts.insert( tmp_pts.begin(), tmp_pts.end() );
             changed_items.insert( item );
 
             for( SCH_SHEET_PATH& path : paths )
@@ -1796,7 +1796,7 @@ void SCH_EDIT_FRAME::RecalculateConnections( SCH_COMMIT* aCommit, SCH_CLEANUP_FL
                 continue;
 
             tmp_pts = item->GetConnectionPoints();
-            pts.insert( pts.end(), tmp_pts.begin(), tmp_pts.end() );
+            pts.insert( tmp_pts.begin(), tmp_pts.end() );
             changed_items.insert( item );
 
             // We have to directly add the pins here because the link may not exist on the schematic
@@ -1811,7 +1811,7 @@ void SCH_EDIT_FRAME::RecalculateConnections( SCH_COMMIT* aCommit, SCH_CLEANUP_FL
                 item_paths.insert( std::make_pair( path, item ) );
         }
 
-        for( VECTOR2I& pt: pts )
+        for( const VECTOR2I& pt: pts )
         {
             for( SCH_ITEM* item : GetScreen()->Items().Overlapping( pt ) )
             {

include/commit.h

@@ -111,7 +111,7 @@ public:
     ///< before the modification.
     COMMIT& Modified( EDA_ITEM* aItem, EDA_ITEM* aCopy, BASE_SCREEN *aScreen = nullptr )
     {
-        return createModified( aItem, aCopy );
+        return createModified( aItem, aCopy, 0, aScreen );
     }
 
     template<class Range>