pcbnew: Cut only copied objects

Fixes a bug where objects where accessed after being freed by the cut

Fixes: lp:1811456
* https://bugs.launchpad.net/kicad/+bug/1811456

common/view/view.cpp

@@ -279,6 +279,7 @@ void VIEW::OnDestroy( VIEW_ITEM* aItem )
         data->m_view->VIEW::Remove( aItem );
 
     delete data;
+    aItem->ClearViewPrivData();
 }
 
 

include/tool/actions.h

@@ -104,7 +104,7 @@ public:
                              CURSOR_CLICK, CURSOR_DBL_CLICK, CURSOR_FAST_MOVE = 0x8000 };
 
     ///> Remove event modifier flags
-    enum class REMOVE_FLAGS { NORMAL = 0x00, ALT = 0x01 };
+    enum class REMOVE_FLAGS { NORMAL = 0x00, ALT = 0x01, CUT = 0x02 };
 };
 
 #endif

include/view/view_item.h

@@ -150,6 +150,11 @@ public:
         return m_viewPrivData;
     }
 
+    void ClearViewPrivData()
+    {
+        m_viewPrivData = NULL;
+    }
+
 private:
     friend class VIEW;
 

pcbnew/tools/edit_tool.cpp

@@ -882,12 +882,19 @@ int EDIT_TOOL::Remove( const TOOL_EVENT& aEvent )
     std::vector<BOARD_ITEM*> lockedItems;
 
     // get a copy instead of reference (as we're going to clear the selection before removing items)
-    auto selectionCopy = m_selectionTool->RequestSelection(
+    SELECTION selectionCopy;
+    bool isCut = aEvent.Parameter<intptr_t>() == static_cast<intptr_t>( PCB_ACTIONS::REMOVE_FLAGS::CUT );
+    bool isAlt = aEvent.Parameter<intptr_t>() == static_cast<intptr_t>( PCB_ACTIONS::REMOVE_FLAGS::ALT );
+
+    // If we are in a "Cut" operation, then the copied selection exists already
+    if( isCut )
+        selectionCopy = m_selectionTool->GetSelection();
+    else
+        selectionCopy = m_selectionTool->RequestSelection(
             []( const VECTOR2I& aPt, GENERAL_COLLECTOR& aCollector )
             { EditToolSelectionFilter( aCollector, EXCLUDE_LOCKED_PADS | EXCLUDE_TRANSIENTS ); } );
 
     bool isHover = selectionCopy.IsHover();
-    const bool isAlt = aEvent.Parameter<intptr_t>() == (int) PCB_ACTIONS::REMOVE_FLAGS::ALT;
 
     // in "alternative" mode, deletion is not just a simple list of selected items,
     // it removes whole tracks, not just segments
@@ -900,7 +907,10 @@ int EDIT_TOOL::Remove( const TOOL_EVENT& aEvent )
     if( selectionCopy.Empty() )
         return 0;
 
-    if( !m_lockedSelected )
+    // N.B. Setting the CUT flag prevents lock filtering as we only want to delete the items that
+    // were copied to the clipboard, no more, no fewer.  Filtering for locked item, if any will be done
+    // in the copyToClipboard() routine
+    if( !m_lockedSelected && !isCut )
     {
         // Second RequestSelection removes locked items but keeps a copy of their pointers
         selectionCopy = m_selectionTool->RequestSelection(
@@ -942,7 +952,10 @@ int EDIT_TOOL::Remove( const TOOL_EVENT& aEvent )
         }
     }
 
-    m_commit->Push( _( "Delete" ) );
+    if( isCut )
+        m_commit->Push( _( "Cut" ) );
+    else
+        m_commit->Push( _( "Delete" ) );
 
     if( !m_lockedSelected && lockedItems.size() > 0 )
     {
@@ -1533,7 +1546,14 @@ int EDIT_TOOL::copyToClipboardWithAnchor( const TOOL_EVENT& aEvent )
 int EDIT_TOOL::cutToClipboard( const TOOL_EVENT& aEvent )
 {
     if( !copyToClipboard( aEvent ) )
-        Remove( aEvent );
+    {
+        // N.B. Setting the CUT flag prevents lock filtering as we only want to delete the items that
+        // were copied to the clipboard, no more, no fewer.  Filtering for locked item, if any will be done
+        // in the copyToClipboard() routine
+        TOOL_EVENT evt = aEvent;
+        evt.SetParameter( PCB_ACTIONS::REMOVE_FLAGS::CUT );
+        Remove( evt );
+    }
 
     return 0;
 }