Symbol editor: fix various crash and behavior bugs.

Abandon the previous behavior of always creating a new copy of the
buffered symbol which required a re-parenting of every derived symbol
to prevent accidentally orphaning a derived symbol parent symbol
pointers.

This change required something that should have been done a long time
ago by adding an assignment operator to LIB_PART.  The assignment
operator makes it possible to create a single copy of the symbol to
buffer which gets updated rather than deleting the last copy and
replacing it with a new copy.

Add a clear method to the MULTIVECTOR class so that existing LIB_PART
object draw items can be removed before assigning the draw items from
the part being assigned.

Fixes #3672

https://gitlab.com/kicad/code/kicad/issues/3672

eeschema/class_libentry.cpp

@@ -156,6 +156,49 @@ LIB_PART::~LIB_PART()
 }
 
 
+const LIB_PART& LIB_PART::operator=( const LIB_PART& aPart )
+{
+    if( &aPart == this )
+        return aPart;
+
+    LIB_ITEM* newItem;
+
+    m_library             = aPart.m_library;
+    m_name                = aPart.m_name;
+    m_FootprintList       = wxArrayString( aPart.m_FootprintList );
+    m_unitCount           = aPart.m_unitCount;
+    m_unitsLocked         = aPart.m_unitsLocked;
+    m_pinNameOffset       = aPart.m_pinNameOffset;
+    m_showPinNumbers      = aPart.m_showPinNumbers;
+    m_showPinNames        = aPart.m_showPinNames;
+    m_dateLastEdition     = aPart.m_dateLastEdition;
+    m_options             = aPart.m_options;
+    m_libId               = aPart.m_libId;
+    m_description         = aPart.m_description;
+    m_keyWords            = aPart.m_keyWords;
+    m_docFileName         = aPart.m_docFileName;
+
+    m_drawings.clear();
+
+    for( const LIB_ITEM& oldItem : aPart.m_drawings )
+    {
+        if( ( oldItem.GetFlags() & ( IS_NEW | STRUCT_DELETED ) ) != 0 )
+            continue;
+
+        newItem = (LIB_ITEM*) oldItem.Clone();
+        newItem->SetParent( this );
+        m_drawings.push_back( newItem );
+    }
+
+    PART_SPTR parent = aPart.m_parent.lock();
+
+    if( parent )
+        SetParent( parent.get() );
+
+    return *this;
+}
+
+
 int LIB_PART::Compare( const LIB_PART& aRhs ) const
 {
     if( m_me == aRhs.m_me )

eeschema/class_libentry.h

@@ -572,6 +572,8 @@ public:
     bool operator==( const LIB_PART* aPart ) const { return this == aPart; }
     bool operator==( const LIB_PART& aPart ) const { return Compare( aPart ) == 0; }
 
+    const LIB_PART& operator=( const LIB_PART& aPart );
+
     /**
      * Return a flattened symbol inheritance to the caller.
      *

eeschema/libedit/lib_manager.cpp

@@ -384,36 +384,22 @@ bool LIB_MANAGER::UpdatePart( LIB_PART* aPart, const wxString& aLibrary )
     wxCHECK( aPart, false );
     LIB_BUFFER& libBuf = getLibraryBuffer( aLibrary );
     auto partBuf = libBuf.GetBuffer( aPart->GetName() );
-    LIB_PART* partCopy = new LIB_PART( *aPart, nullptr );
 
-    partCopy->SetLibId( LIB_ID( aLibrary, aPart->GetLibId().GetLibItemName() ) );
+    if( partBuf )     // Existing symbol.
-
-    if( partBuf )
     {
-        if( partBuf->GetPart()->IsRoot() && libBuf.HasDerivedSymbols( aPart->GetName() ) )
+        LIB_PART* bufferedPart = const_cast< LIB_PART* >( partBuf->GetPart() );
-        {
-            // Reparent derived symbols.
-            for( auto entry : libBuf.GetBuffers() )
-            {
-                if( entry->GetPart()->IsRoot() )
-                    continue;
 
-                if( entry->GetPart()->GetParent().lock() == partBuf->GetPart()->SharedPtr() )
+        wxCHECK( bufferedPart, false );
-                {
-                    if( !libBuf.DeleteBuffer( entry ) )
-                        return false;
 
-                    LIB_PART* reparentedPart = new LIB_PART( *entry->GetPart() );
+        *bufferedPart = *aPart;
-                    reparentedPart->SetParent( partCopy );
+        partBuf->GetScreen()->SetModify();
-                    libBuf.CreateBuffer( reparentedPart, new SCH_SCREEN( &m_frame.Kiway() ) );
-                }
-            }
-        }
-
-        libBuf.UpdateBuffer( partBuf, partCopy );
     }
-    else    // New entry
+    else              // New symbol
     {
+        LIB_PART* partCopy = new LIB_PART( *aPart, nullptr );
+
+        partCopy->SetLibId( LIB_ID( aLibrary, aPart->GetLibId().GetLibItemName() ) );
+
         SCH_SCREEN* screen = new SCH_SCREEN( &m_frame.Kiway() );
         libBuf.CreateBuffer( partCopy, screen );
         screen->SetModify();
@@ -431,11 +417,7 @@ bool LIB_MANAGER::UpdatePartAfterRename( LIB_PART* aPart, const wxString& aOldNa
 
     wxCHECK( partBuf, false );
 
-    LIB_PART* bufferedPart = const_cast< LIB_PART* >( partBuf->GetPart() );
+    libBuf.UpdateBuffer( partBuf, aPart );
-
-    wxCHECK( bufferedPart, false );
-
-    bufferedPart->SetName( aPart->GetName() );
     SetCurrentPart( aPart->GetName() );
     m_frame.SyncLibraries( false );
 
@@ -846,12 +828,16 @@ bool LIB_MANAGER::LIB_BUFFER::CreateBuffer( LIB_PART* aCopy, SCH_SCREEN* aScreen
 bool LIB_MANAGER::LIB_BUFFER::UpdateBuffer( LIB_MANAGER::PART_BUFFER::PTR aPartBuf,
                                             LIB_PART* aCopy )
 {
-    bool ret = true;
+    wxCHECK( aCopy && aPartBuf, false );
 
-    aPartBuf->SetPart( aCopy );
+    LIB_PART* bufferedPart = aPartBuf->GetPart();
+
+    wxCHECK( bufferedPart, false );
+
+    *bufferedPart = *aCopy;
     ++m_hash;
 
-    return ret;
+    return true;
 }
 
 

eeschema/libedit/lib_manager.h

@@ -379,7 +379,7 @@ private:
         ///> Creates a new buffer to store a part. LIB_BUFFER takes ownership of aCopy.
         bool CreateBuffer( LIB_PART* aCopy, SCH_SCREEN* aScreen );
 
-        ///> Updates the stored part. LIB_BUFFER takes ownership of aCopy.
+        ///> Updates the buffered part with the contents of \a aCopy.
         bool UpdateBuffer( PART_BUFFER::PTR aPartBuf, LIB_PART* aCopy );
 
         bool DeleteBuffer( PART_BUFFER::PTR aPartBuf );

include/multivector.h

@@ -204,6 +204,19 @@ public:
         return CONST_ITERATOR( this, operator[]( bucket ).end(), bucket, aType );
     }
 
+    void clear( int aType = UNDEFINED_TYPE )
+    {
+        if( aType != UNDEFINED_TYPE )
+        {
+            operator[]( aType ).clear();
+        }
+        else
+        {
+            for( int i = 0; i < TYPES_COUNT; ++i)
+                m_data[ i ].clear();
+        }
+    }
+
     size_t size( int aType = UNDEFINED_TYPE ) const
     {
         if( aType != UNDEFINED_TYPE )