← Back to team overview

kicad-developers team mailing list archive

[PATCH 1/1] Make EDA_ITEM::Clone() abstract

 

This avoids missing Clone functions in derived classes, at the expense of
six bytes per call site, or so.
---
 common/base_struct.cpp                       |  7 -------
 eeschema/class_libentry.h                    | 12 ++++++++++++
 eeschema/sch_collectors.cpp                  |  6 ++++++
 eeschema/sch_screen.h                        |  6 ++++++
 gerbview/gbr_layout.h                        |  6 ++++++
 gerbview/gbr_screen.h                        |  6 ++++++
 gerbview/gerber_draw_item.h                  |  6 ++++++
 gerbview/gerber_file_image.h                 |  6 ++++++
 gerbview/gerber_file_image_list.h            |  6 ++++++
 include/base_struct.h                        |  9 +--------
 include/pcb_screen.h                         |  6 ++++++
 include/preview_items/arc_assistant.h        |  5 +++++
 include/preview_items/bright_box.h           |  5 +++++
 include/preview_items/centreline_rect_item.h |  5 +++++
 include/preview_items/polygon_item.h         |  5 +++++
 include/preview_items/ruler_item.h           |  5 +++++
 include/preview_items/selection_area.h       |  5 +++++
 include/worksheet_viewitem.h                 |  6 ++++++
 pagelayout_editor/pl_editor_screen.h         |  6 ++++++
 pagelayout_editor/pl_editor_undo_redo.cpp    |  6 ++++++
 pcbnew/class_board.h                         |  6 ++++++
 pcbnew/class_marker_pcb.h                    |  6 ++++++
 pcbnew/netinfo.h                             | 12 ++++++++++++
 pcbnew/ratsnest_viewitem.h                   |  6 ++++++
 pcbnew/router/router_preview_item.h          |  6 ++++++
 pcbnew/tools/edit_points.h                   |  5 +++++
 26 files changed, 150 insertions(+), 15 deletions(-)

diff --git a/common/base_struct.cpp b/common/base_struct.cpp
index ca8876a96..c9e398fd0 100644
--- a/common/base_struct.cpp
+++ b/common/base_struct.cpp
@@ -121,13 +121,6 @@ const EDA_RECT EDA_ITEM::GetBoundingBox() const
 }
 
 
-EDA_ITEM* EDA_ITEM::Clone() const
-{
-    wxCHECK_MSG( false, NULL, wxT( "Clone not implemented in derived class " ) + GetClass() +
-                 wxT( ".  Bad programmer!" ) );
-}
-
-
 SEARCH_RESULT EDA_ITEM::IterateForward( EDA_ITEM*       listStart,
                                         INSPECTOR       inspector,
                                         void*           testData,
diff --git a/eeschema/class_libentry.h b/eeschema/class_libentry.h
index bff3e545e..a3ae72d9c 100644
--- a/eeschema/class_libentry.h
+++ b/eeschema/class_libentry.h
@@ -157,6 +157,12 @@ public:
 #if defined(DEBUG)
     void Show( int nestLevel, std::ostream& os ) const override { ShowDummy( os ); }
 #endif
+
+private:
+    virtual EDA_ITEM* Clone() const override
+    {
+        wxCHECK_MSG( false, nullptr, wxT( "Cannot clone LIB_ALIAS" ) );
+    }
 };
 
 extern bool operator<( const LIB_ALIAS& aItem1, const LIB_ALIAS& aItem2 );
@@ -766,6 +772,12 @@ public:
 #if defined(DEBUG)
     void Show( int nestLevel, std::ostream& os ) const override { ShowDummy( os ); }
 #endif
+
+private:
+    virtual EDA_ITEM* Clone() const override
+    {
+        wxCHECK_MSG( false, nullptr, wxT( "Cannot clone LIB_PART" ) );
+    }
 };
 
 #endif  //  CLASS_LIBENTRY_H
diff --git a/eeschema/sch_collectors.cpp b/eeschema/sch_collectors.cpp
index a1db6b8f0..07b7fc880 100644
--- a/eeschema/sch_collectors.cpp
+++ b/eeschema/sch_collectors.cpp
@@ -361,6 +361,12 @@ public:
     void MirrorY( int  ) override {}
     void MirrorX( int  ) override {}
     void Rotate( wxPoint  ) override {}
+
+private:
+    virtual EDA_ITEM* Clone() const override
+    {
+        wxCHECK_MSG( false, nullptr, wxT( "Cannot clone DELETED_SCH_ITEM" ) );
+    }
 };
 
 
diff --git a/eeschema/sch_screen.h b/eeschema/sch_screen.h
index 4a4e23f8a..4efe6fc92 100644
--- a/eeschema/sch_screen.h
+++ b/eeschema/sch_screen.h
@@ -492,6 +492,12 @@ public:
 #if defined(DEBUG)
     void Show( int nestLevel, std::ostream& os ) const override;
 #endif
+
+private:
+    virtual EDA_ITEM* Clone() const override
+    {
+        wxCHECK_MSG( false, nullptr, wxT( "Cannot clone SCH_SCREEN" ) );
+    }
 };
 
 
diff --git a/gerbview/gbr_layout.h b/gerbview/gbr_layout.h
index 1c3312178..507d43ca0 100644
--- a/gerbview/gbr_layout.h
+++ b/gerbview/gbr_layout.h
@@ -191,6 +191,12 @@ public:
     void    Show( int nestLevel, std::ostream& os ) const override { ShowDummy( os ); }
 
 #endif
+
+private:
+    virtual EDA_ITEM* Clone() const override
+    {
+        wxCHECK_MSG( false, nullptr, wxT( "Cannot clone GBR_LAYOUT" ) );
+    }
 };
 
 #endif      // #ifndef GBR_LAYOUT_H
diff --git a/gerbview/gbr_screen.h b/gerbview/gbr_screen.h
index 691b4d4f5..88792c9af 100644
--- a/gerbview/gbr_screen.h
+++ b/gerbview/gbr_screen.h
@@ -57,6 +57,12 @@ public:
      * virtual pure in BASE_SCREEN, so it must be defined here
      */
     void ClearUndoORRedoList( UNDO_REDO_CONTAINER& aList, int aItemCount = -1 ) override;
+
+private:
+    virtual EDA_ITEM* Clone() const override
+    {
+        wxCHECK_MSG( false, nullptr, wxT( "Cannot clone GBR_SCREEN" ) );
+    }
 };
 
 
diff --git a/gerbview/gerber_draw_item.h b/gerbview/gerber_draw_item.h
index 9c3531564..6e8000ef9 100644
--- a/gerbview/gerber_draw_item.h
+++ b/gerbview/gerber_draw_item.h
@@ -327,6 +327,12 @@ public:
 
     ///> @copydoc EDA_ITEM::GetMenuImage()
     BITMAP_DEF GetMenuImage() const override;
+
+private:
+    virtual EDA_ITEM* Clone() const override
+    {
+        wxCHECK_MSG( false, nullptr, wxT( "Cannot clone GERBER_DRAW_ITEM" ) );
+    }
 };
 
 
diff --git a/gerbview/gerber_file_image.h b/gerbview/gerber_file_image.h
index f071231d0..6d3d69c7d 100644
--- a/gerbview/gerber_file_image.h
+++ b/gerbview/gerber_file_image.h
@@ -392,6 +392,12 @@ public:
     void    Show( int nestLevel, std::ostream& os ) const override { ShowDummy( os ); }
 
 #endif
+
+private:
+    virtual EDA_ITEM* Clone() const override
+    {
+        wxCHECK_MSG( false, nullptr, wxT( "Cannot clone GERBER_FILE_IMAGE" ) );
+    }
 };
 
 #endif  // ifndef GERBER_FILE_IMAGE_H
diff --git a/gerbview/gerber_file_image_list.h b/gerbview/gerber_file_image_list.h
index 6880a9187..508428a76 100644
--- a/gerbview/gerber_file_image_list.h
+++ b/gerbview/gerber_file_image_list.h
@@ -129,6 +129,12 @@ public:
         void    Show( int nestLevel, std::ostream& os ) const override { ShowDummy( os ); }
 
     #endif
+
+private:
+    virtual EDA_ITEM* Clone() const override
+    {
+        wxCHECK_MSG( false, nullptr, wxT( "Cannot clone GERBER_FILE_IMAGE_LIST" ) );
+    }
 };
 
 #endif  // ifndef GERBER_FILE_IMAGE_LIST_H
diff --git a/include/base_struct.h b/include/base_struct.h
index 0de994f2c..ff4da2771 100644
--- a/include/base_struct.h
+++ b/include/base_struct.h
@@ -317,16 +317,9 @@ public:
      * Function Clone
      * creates a duplicate of this item with linked list members set to NULL.
      *
-     * The default version will return NULL in release builds and likely crash the
-     * program.  In debug builds, a warning message indicating the derived class
-     * has not implemented cloning.  This really should be a pure virtual function.
-     * Due to the fact that there are so many objects derived from EDA_ITEM, the
-     * decision was made to return NULL until all the objects derived from EDA_ITEM
-     * implement cloning.  Once that happens, this function should be made pure.
-     *
      * @return A clone of the item.
      */
-    virtual EDA_ITEM* Clone() const; // should not be inline, to save the ~ 6 bytes per call site.
+    virtual EDA_ITEM* Clone() const = 0;
 
     /**
      * Function IterateForward
diff --git a/include/pcb_screen.h b/include/pcb_screen.h
index 130c1989d..b14ef67ba 100644
--- a/include/pcb_screen.h
+++ b/include/pcb_screen.h
@@ -100,6 +100,12 @@ public:
      * So this function can be called to remove old commands
      */
     void ClearUndoORRedoList( UNDO_REDO_CONTAINER& aList, int aItemCount = -1 ) override;
+
+private:
+    virtual EDA_ITEM* Clone() const override
+    {
+        wxCHECK_MSG( false, nullptr, wxT( "Cannot clone PCB_SCREEN" ) );
+    }
 };
 
 #endif  // PCB_SCREEN_H
diff --git a/include/preview_items/arc_assistant.h b/include/preview_items/arc_assistant.h
index 9c3c411ae..1b86971e9 100644
--- a/include/preview_items/arc_assistant.h
+++ b/include/preview_items/arc_assistant.h
@@ -67,6 +67,11 @@ public:
 private:
 
     const ARC_GEOM_MANAGER& m_constructMan;
+
+    virtual EDA_ITEM* Clone() const override
+    {
+        wxCHECK_MSG( false, nullptr, wxT( "Cannot clone ARC_ASSISTANT" ) );
+    }
 };
 }       // PREVIEW
 }       // KIGFX
diff --git a/include/preview_items/bright_box.h b/include/preview_items/bright_box.h
index 2af9beeaa..ff81a7a2e 100644
--- a/include/preview_items/bright_box.h
+++ b/include/preview_items/bright_box.h
@@ -95,6 +95,11 @@ protected:
     EDA_ITEM* m_item;
     double m_lineWidth;
     KIGFX::COLOR4D m_color;
+
+    virtual EDA_ITEM* Clone() const override
+    {
+        wxCHECK_MSG( false, nullptr, wxT( "Cannot clone BRIGHT_BOX" ) );
+    }
 };
 
 #endif
diff --git a/include/preview_items/centreline_rect_item.h b/include/preview_items/centreline_rect_item.h
index b75a4b5ae..cf380aed8 100644
--- a/include/preview_items/centreline_rect_item.h
+++ b/include/preview_items/centreline_rect_item.h
@@ -71,6 +71,11 @@ private:
 
     ///> The aspect ratio of the rectangle to draw
     double m_aspect;
+
+    virtual EDA_ITEM* Clone() const override
+    {
+        wxCHECK_MSG( false, nullptr, wxT( "Cannot clone CENTRELINE_RECT_ITEM" ) );
+    }
 };
 
 } // PREVIEW
diff --git a/include/preview_items/polygon_item.h b/include/preview_items/polygon_item.h
index 61d59a9b3..ea1e949c1 100644
--- a/include/preview_items/polygon_item.h
+++ b/include/preview_items/polygon_item.h
@@ -75,6 +75,11 @@ private:
 
     ///> polygon fill
     SHAPE_POLY_SET m_polyfill;
+
+    virtual EDA_ITEM* Clone() const override
+    {
+        wxCHECK_MSG( false, nullptr, wxT( "Cannot clone POLYGON_ITEM" ) );
+    }
 };
 
 } // PREVIEW
diff --git a/include/preview_items/ruler_item.h b/include/preview_items/ruler_item.h
index c651ca4c0..128bb84fe 100644
--- a/include/preview_items/ruler_item.h
+++ b/include/preview_items/ruler_item.h
@@ -74,6 +74,11 @@ public:
 private:
 
     const TWO_POINT_GEOMETRY_MANAGER& m_geomMgr;
+
+    virtual EDA_ITEM* Clone() const override
+    {
+        wxCHECK_MSG( false, nullptr, wxT( "Cannot clone RULER_ITEM" ) );
+    }
 };
 
 } // PREVIEW
diff --git a/include/preview_items/selection_area.h b/include/preview_items/selection_area.h
index d2dd1aef2..107b1f4e5 100644
--- a/include/preview_items/selection_area.h
+++ b/include/preview_items/selection_area.h
@@ -94,6 +94,11 @@ private:
     bool m_subtractive;
 
     VECTOR2I m_origin, m_end;
+
+    virtual EDA_ITEM* Clone() const override
+    {
+        wxCHECK_MSG( false, nullptr, wxT( "Cannot clone SELECTION_AREA" ) );
+    }
 };
 
 } // PREVIEW
diff --git a/include/worksheet_viewitem.h b/include/worksheet_viewitem.h
index 2a6115e70..b31db3aaa 100644
--- a/include/worksheet_viewitem.h
+++ b/include/worksheet_viewitem.h
@@ -166,6 +166,12 @@ protected:
 
     /// Draws a border that determines the page size.
     void drawBorder( GAL* aGal ) const;
+
+private:
+    virtual EDA_ITEM* Clone() const override
+    {
+        wxCHECK_MSG( false, nullptr, wxT( "Cannot clone ORIGIN_VIEWITEM" ) );
+    }
 };
 }
 
diff --git a/pagelayout_editor/pl_editor_screen.h b/pagelayout_editor/pl_editor_screen.h
index c9e86af57..01f784dc0 100644
--- a/pagelayout_editor/pl_editor_screen.h
+++ b/pagelayout_editor/pl_editor_screen.h
@@ -74,6 +74,12 @@ public:
      * @param aItem Any object derived from WORKSHEET_DATAITEM
      */
     void SetCurItem( WORKSHEET_DATAITEM* aItem ) { BASE_SCREEN::SetCurItem( (EDA_ITEM*)aItem ); }
+
+private:
+    virtual EDA_ITEM* Clone() const override
+    {
+        wxCHECK_MSG( false, nullptr, wxT( "Cannot clone PL_EDITOR_SCREEN" ) );
+    }
 };
 
 
diff --git a/pagelayout_editor/pl_editor_undo_redo.cpp b/pagelayout_editor/pl_editor_undo_redo.cpp
index 387ffb2e3..6f03dbd22 100644
--- a/pagelayout_editor/pl_editor_undo_redo.cpp
+++ b/pagelayout_editor/pl_editor_undo_redo.cpp
@@ -65,6 +65,12 @@ public:
     {
         return wxT( "PL_ITEM_LAYOUT" );
     }
+
+private:
+    virtual EDA_ITEM* Clone() const override
+    {
+        wxCHECK_MSG( false, nullptr, wxT( "Cannot clone PL_ITEM_LAYOUT" ) );
+    }
 };
 
 void PL_EDITOR_FRAME::SaveCopyInUndoList()
diff --git a/pcbnew/class_board.h b/pcbnew/class_board.h
index d03d7defd..4a5b9b696 100644
--- a/pcbnew/class_board.h
+++ b/pcbnew/class_board.h
@@ -1353,6 +1353,12 @@ public:
      * Resets all items' netcodes to 0 (no net).
      */
     void ClearAllNetCodes();
+
+private:
+    virtual EDA_ITEM* Clone() const override
+    {
+        wxCHECK_MSG( false, nullptr, wxT( "Cannot clone BOARD" ) );
+    }
 };
 
 #endif      // CLASS_BOARD_H_
diff --git a/pcbnew/class_marker_pcb.h b/pcbnew/class_marker_pcb.h
index 9a95e5ac2..78b229997 100644
--- a/pcbnew/class_marker_pcb.h
+++ b/pcbnew/class_marker_pcb.h
@@ -136,6 +136,12 @@ public:
 protected:
     ///> Pointer to BOARD_ITEM that causes DRC error.
     const BOARD_ITEM* m_item;
+
+private:
+    virtual EDA_ITEM* Clone() const override
+    {
+        wxCHECK_MSG( false, nullptr, wxT( "Cannot clone PCB_MARKER" ) );
+    }
 };
 
 #endif      //  CLASS_MARKER_PCB_H
diff --git a/pcbnew/netinfo.h b/pcbnew/netinfo.h
index e379c7ea8..4085346b7 100644
--- a/pcbnew/netinfo.h
+++ b/pcbnew/netinfo.h
@@ -262,6 +262,12 @@ public:
     {
         return m_parent;
     }
+
+private:
+    virtual EDA_ITEM* Clone() const override
+    {
+        wxCHECK_MSG( false, nullptr, wxT( "Cannot clone NETINFO_ITEM" ) );
+    }
 };
 
 
@@ -573,6 +579,12 @@ private:
     NETCODES_MAP m_netCodes;        ///< map of <int, NETINFO_ITEM*> is NOT owner
 
     int m_newNetCode;               ///< possible value for new net code assignment
+
+private:
+    virtual EDA_ITEM* Clone() const
+    {
+        wxCHECK_MSG( false, nullptr, wxT( "Cannot clone NETINFO_LIST" ) );
+    }
 };
 
 
diff --git a/pcbnew/ratsnest_viewitem.h b/pcbnew/ratsnest_viewitem.h
index a6551c801..e39986b92 100644
--- a/pcbnew/ratsnest_viewitem.h
+++ b/pcbnew/ratsnest_viewitem.h
@@ -71,6 +71,12 @@ public:
 protected:
     ///> Object containing ratsnest data.
     std::shared_ptr<CONNECTIVITY_DATA> m_data;
+
+private:
+    virtual EDA_ITEM* Clone() const override
+    {
+        wxCHECK_MSG( false, nullptr, wxT( "Cannot clone RATSNEST_VIEWITEM" ) );
+    }
 };
 
 }   // namespace KIGFX
diff --git a/pcbnew/router/router_preview_item.h b/pcbnew/router/router_preview_item.h
index 05c3aeb4c..7f9b53f5c 100644
--- a/pcbnew/router/router_preview_item.h
+++ b/pcbnew/router/router_preview_item.h
@@ -140,6 +140,12 @@ private:
 
     KIGFX::COLOR4D m_color;
     VECTOR2I m_pos;
+
+private:
+    virtual EDA_ITEM* Clone() const override
+    {
+        wxCHECK_MSG( false, nullptr, wxT( "Cannot clone ROUTER_PREVIEW_ITEM" ) );
+    }
 };
 
 #endif
diff --git a/pcbnew/tools/edit_points.h b/pcbnew/tools/edit_points.h
index f4d925f34..11d42b941 100644
--- a/pcbnew/tools/edit_points.h
+++ b/pcbnew/tools/edit_points.h
@@ -524,6 +524,11 @@ private:
     std::deque<EDIT_POINT> m_points;    ///< EDIT_POINTs for modifying m_parent
     std::deque<EDIT_LINE> m_lines;      ///< EDIT_LINEs for modifying m_parent
     std::list<int> m_contours;          ///< Indices of end contour points
+
+    virtual EDA_ITEM* Clone() const override
+    {
+        wxCHECK_MSG( false, nullptr, wxT( "Cannot clone EDIT_POINTS" ) );
+    }
 };
 
 #endif /* EDIT_POINTS_H_ */

References