From 14cb9090ec3ef15efd487accfc3be3346bcc4aee Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Tue, 8 Jan 2019 21:53:10 +0100 Subject: [PATCH 1/7] Provide operator-- on SortedCellIndexIterator to fix #220 --- src/db/db/dbCellMapping.cc | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/db/db/dbCellMapping.cc b/src/db/db/dbCellMapping.cc index 8888a9ebf..66490629d 100644 --- a/src/db/db/dbCellMapping.cc +++ b/src/db/db/dbCellMapping.cc @@ -72,13 +72,25 @@ struct SortedCellIndexIterator SortedCellIndexIterator &operator++() { ++m_n; - return *this; + return *this; } SortedCellIndexIterator &operator+=(size_t n) { m_n += n; - return *this; + return *this; + } + + SortedCellIndexIterator &operator--() + { + --m_n; + return *this; + } + + SortedCellIndexIterator &operator-=(size_t n) + { + m_n -= n; + return *this; } bool operator==(const SortedCellIndexIterator &d) const From aeeb6d7c875c7b78a54daa6a1efa2f94967ccc1c Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Wed, 9 Jan 2019 01:06:11 +0100 Subject: [PATCH 2/7] Fixed #200 by introducing layout locking during iteration The cause for the problem was that the layout got updated while iterating causing the mess within the iterator. This solution is to lock the layout while an iterator is present. This happens for various Cell and Shapes iterator, so it's a major enhancement. --- src/db/db/db.pro | 3 +- src/db/db/dbLayout.h | 27 ++++++++- src/db/db/gsiDeclDbCell.cc | 108 +++++++++++++++++++++++------------ src/db/db/gsiDeclDbHelpers.h | 83 +++++++++++++++++++++++++++ src/db/db/gsiDeclDbShapes.cc | 42 +++++++------- src/tl/tl/tlReuseVector.h | 27 +++++---- testdata/gds/t200.gds | Bin 0 -> 1354 bytes testdata/ruby/dbLayout.rb | 39 +++++++++++++ 8 files changed, 256 insertions(+), 73 deletions(-) create mode 100644 src/db/db/gsiDeclDbHelpers.h create mode 100644 testdata/gds/t200.gds diff --git a/src/db/db/db.pro b/src/db/db/db.pro index 8c0a4e558..90b03bf16 100644 --- a/src/db/db/db.pro +++ b/src/db/db/db.pro @@ -208,7 +208,8 @@ HEADERS = \ dbForceLink.h \ dbPlugin.h \ dbInit.h \ - dbConverters.h + dbConverters.h \ + gsiDeclDbHelpers.h !equals(HAVE_QT, "0") { diff --git a/src/db/db/dbLayout.h b/src/db/db/dbLayout.h index 8a1d0a591..94e5cd184 100644 --- a/src/db/db/dbLayout.h +++ b/src/db/db/dbLayout.h @@ -1725,9 +1725,32 @@ public: } } + LayoutLocker (const LayoutLocker &other) + : mp_layout (other.mp_layout) + { + if (mp_layout) { + mp_layout->start_changes (); + } + } + + LayoutLocker &operator= (const LayoutLocker &other) + { + if (this == &other) { + return *this; + } + + if (mp_layout) { + mp_layout->end_changes (); + } + mp_layout = other.mp_layout; + if (mp_layout) { + mp_layout->start_changes (); + } + + return *this; + } + private: - LayoutLocker (const LayoutLocker &); - LayoutLocker &operator= (const LayoutLocker &); db::Layout *mp_layout; }; diff --git a/src/db/db/gsiDeclDbCell.cc b/src/db/db/gsiDeclDbCell.cc index f0e3a3637..0865ab1e7 100644 --- a/src/db/db/gsiDeclDbCell.cc +++ b/src/db/db/gsiDeclDbCell.cc @@ -23,6 +23,8 @@ #include "gsiDecl.h" + +#include "gsiDeclDbHelpers.h" #include "dbLayout.h" #include "dbBoxConvert.h" #include "dbRegion.h" @@ -711,70 +713,70 @@ static void dump_mem_statistics (const db::Cell *cell, bool detailed) ms.print (); } -static db::Shapes::shape_iterator begin_shapes (const db::Cell *s, unsigned int layer_index, unsigned int flags) +static gsi::layout_locking_iterator1 begin_shapes (const db::Cell *s, unsigned int layer_index, unsigned int flags) { - return s->begin (layer_index, flags); + return gsi::layout_locking_iterator1 (s->layout (), s->begin (layer_index, flags)); } -static db::Shapes::shape_iterator begin_shapes_all (const db::Cell *s, unsigned int layer_index) +static gsi::layout_locking_iterator1 begin_shapes_all (const db::Cell *s, unsigned int layer_index) { - return s->begin (layer_index, db::ShapeIterator::All); + return gsi::layout_locking_iterator1 (s->layout (), s->begin (layer_index, db::ShapeIterator::All)); } -static db::Shapes::shape_iterator begin_touching_shapes (const db::Cell *s, unsigned int layer_index, const db::Box &box, unsigned int flags) +static gsi::layout_locking_iterator1 begin_touching_shapes (const db::Cell *s, unsigned int layer_index, const db::Box &box, unsigned int flags) { - return s->begin_touching (layer_index, box, flags); + return gsi::layout_locking_iterator1 (s->layout (), s->begin_touching (layer_index, box, flags)); } -static db::Shapes::shape_iterator begin_touching_shapes_all (const db::Cell *s, unsigned int layer_index, const db::Box &box) +static gsi::layout_locking_iterator1 begin_touching_shapes_all (const db::Cell *s, unsigned int layer_index, const db::Box &box) { - return s->begin_touching (layer_index, box, db::ShapeIterator::All); + return gsi::layout_locking_iterator1 (s->layout (), s->begin_touching (layer_index, box, db::ShapeIterator::All)); } -static db::Shapes::shape_iterator begin_overlapping_shapes (const db::Cell *s, unsigned int layer_index, const db::Box &box, unsigned int flags) +static gsi::layout_locking_iterator1 begin_overlapping_shapes (const db::Cell *s, unsigned int layer_index, const db::Box &box, unsigned int flags) { - return s->begin_overlapping (layer_index, box, flags); + return gsi::layout_locking_iterator1 (s->layout (), s->begin_overlapping (layer_index, box, flags)); } -static db::Shapes::shape_iterator begin_overlapping_shapes_all (const db::Cell *s, unsigned int layer_index, const db::Box &box) +static gsi::layout_locking_iterator1 begin_overlapping_shapes_all (const db::Cell *s, unsigned int layer_index, const db::Box &box) { - return s->begin_overlapping (layer_index, box, db::ShapeIterator::All); + return gsi::layout_locking_iterator1 (s->layout (), s->begin_overlapping (layer_index, box, db::ShapeIterator::All)); } -static db::Shapes::shape_iterator begin_touching_shapes_um (const db::Cell *s, unsigned int layer_index, const db::DBox &box, unsigned int flags) +static gsi::layout_locking_iterator1 begin_touching_shapes_um (const db::Cell *s, unsigned int layer_index, const db::DBox &box, unsigned int flags) { const db::Layout *layout = s->layout (); if (! layout) { throw tl::Exception (tl::to_string (tr ("Cell does not reside inside a layout - cannot use a micrometer search box"))); } - return s->begin_touching (layer_index, db::CplxTrans (layout->dbu ()).inverted () * box, flags); + return gsi::layout_locking_iterator1 (s->layout (), s->begin_touching (layer_index, db::CplxTrans (layout->dbu ()).inverted () * box, flags)); } -static db::Shapes::shape_iterator begin_touching_shapes_all_um (const db::Cell *s, unsigned int layer_index, const db::DBox &box) +static gsi::layout_locking_iterator1 begin_touching_shapes_all_um (const db::Cell *s, unsigned int layer_index, const db::DBox &box) { const db::Layout *layout = s->layout (); if (! layout) { throw tl::Exception (tl::to_string (tr ("Cell does not reside inside a layout - cannot use a micrometer search box"))); } - return s->begin_touching (layer_index, db::CplxTrans (layout->dbu ()).inverted () * box, db::ShapeIterator::All); + return gsi::layout_locking_iterator1 (s->layout (), s->begin_touching (layer_index, db::CplxTrans (layout->dbu ()).inverted () * box, db::ShapeIterator::All)); } -static db::Shapes::shape_iterator begin_overlapping_shapes_um (const db::Cell *s, unsigned int layer_index, const db::DBox &box, unsigned int flags) +static gsi::layout_locking_iterator1 begin_overlapping_shapes_um (const db::Cell *s, unsigned int layer_index, const db::DBox &box, unsigned int flags) { const db::Layout *layout = s->layout (); if (! layout) { throw tl::Exception (tl::to_string (tr ("Cell does not reside inside a layout - cannot use a micrometer search box"))); } - return s->begin_overlapping (layer_index, db::CplxTrans (layout->dbu ()).inverted () * box, flags); + return gsi::layout_locking_iterator1 (s->layout (), s->begin_overlapping (layer_index, db::CplxTrans (layout->dbu ()).inverted () * box, flags)); } -static db::Shapes::shape_iterator begin_overlapping_shapes_all_um (const db::Cell *s, unsigned int layer_index, const db::DBox &box) +static gsi::layout_locking_iterator1 begin_overlapping_shapes_all_um (const db::Cell *s, unsigned int layer_index, const db::DBox &box) { const db::Layout *layout = s->layout (); if (! layout) { throw tl::Exception (tl::to_string (tr ("Cell does not reside inside a layout - cannot use a micrometer search box"))); } - return s->begin_overlapping (layer_index, db::CplxTrans (layout->dbu ()).inverted () * box, db::ShapeIterator::All); + return gsi::layout_locking_iterator1 (s->layout (), s->begin_overlapping (layer_index, db::CplxTrans (layout->dbu ()).inverted () * box, db::ShapeIterator::All)); } static db::Instance insert_inst_with_props (db::Cell *c, const db::Cell::cell_inst_array_type &inst, db::properties_id_type id) @@ -1679,18 +1681,12 @@ static db::DBox cell_dbbox_per_layer (const db::Cell *cell, unsigned int layer_i return cell->bbox (layer_index) * layout->dbu (); } -static db::Cell::overlapping_iterator cell_begin_overlapping_inst_um (const db::Cell *cell, const db::DBox &db) +gsi::layout_locking_iterator1 begin_overlapping_inst (const db::Cell *cell, const db::Cell::box_type &b) { - const db::Layout *layout = cell->layout (); - if (! layout) { - throw tl::Exception (tl::to_string (tr ("Cell does not reside inside a layout - cannot use a micrometer-unit search boxes"))); - } - - db::CplxTrans dbu_trans (layout->dbu ()); - return cell->begin_overlapping (dbu_trans.inverted () * db); + return gsi::layout_locking_iterator1 (cell->layout (), cell->begin_overlapping (b)); } -static db::Cell::touching_iterator cell_begin_touching_inst_um (const db::Cell *cell, const db::DBox &db) +gsi::layout_locking_iterator1 begin_overlapping_inst_um (const db::Cell *cell, const db::DBox &dbox) { const db::Layout *layout = cell->layout (); if (! layout) { @@ -1698,7 +1694,43 @@ static db::Cell::touching_iterator cell_begin_touching_inst_um (const db::Cell * } db::CplxTrans dbu_trans (layout->dbu ()); - return cell->begin_touching (dbu_trans.inverted () * db); + return gsi::layout_locking_iterator1 (cell->layout (), cell->begin_overlapping (dbu_trans.inverted () * dbox)); +} + +gsi::layout_locking_iterator1 begin_touching_inst (const db::Cell *cell, const db::Cell::box_type &b) +{ + return gsi::layout_locking_iterator1 (cell->layout (), cell->begin_touching (b)); +} + +gsi::layout_locking_iterator1 begin_touching_inst_um (const db::Cell *cell, const db::DBox &dbox) +{ + const db::Layout *layout = cell->layout (); + if (! layout) { + throw tl::Exception (tl::to_string (tr ("Cell does not reside inside a layout - cannot use a micrometer-unit search boxes"))); + } + + db::CplxTrans dbu_trans (layout->dbu ()); + return gsi::layout_locking_iterator1 (cell->layout (), cell->begin_touching (dbu_trans.inverted () * dbox)); +} + +gsi::layout_locking_iterator1 begin_child_cells (const db::Cell *cell) +{ + return gsi::layout_locking_iterator1 (cell->layout (), cell->begin_child_cells ()); +} + +gsi::layout_locking_iterator1 begin_parent_insts (const db::Cell *cell) +{ + return gsi::layout_locking_iterator1 (cell->layout (), cell->begin_parent_insts ()); +} + +gsi::layout_locking_iterator2 begin_parent_cells (const db::Cell *cell) +{ + return gsi::layout_locking_iterator2 (cell->layout (), cell->begin_parent_cells (), cell->end_parent_cells ()); +} + +static layout_locking_iterator1 begin_inst (db::Cell *cell) +{ + return layout_locking_iterator1 (cell->layout (), cell->begin ()); } Class decl_Cell ("db", "Cell", @@ -2569,7 +2601,7 @@ Class decl_Cell ("db", "Cell", "\n" "This method has been introduced in version 0.25." ) + - gsi::iterator ("each_overlapping_inst", (db::Cell::overlapping_iterator (db::Cell::*) (const db::Cell::box_type &b) const) &db::Cell::begin_overlapping, gsi::arg ("b"), + gsi::iterator_ext ("each_overlapping_inst", &begin_overlapping_inst, gsi::arg ("b"), "@brief Gets the instances overlapping the given rectangle\n" "\n" "This will iterate over all child cell\n" @@ -2579,7 +2611,7 @@ Class decl_Cell ("db", "Cell", "\n" "Starting with version 0.15, this iterator delivers \\Instance objects rather than \\CellInstArray objects." ) + - gsi::iterator_ext ("each_overlapping_inst", &cell_begin_overlapping_inst_um, gsi::arg ("b"), + gsi::iterator_ext ("each_overlapping_inst", &begin_overlapping_inst_um, gsi::arg ("b"), "@brief Gets the instances overlapping the given rectangle, with the rectangle in micrometer units\n" "\n" "This will iterate over all child cell\n" @@ -2592,7 +2624,7 @@ Class decl_Cell ("db", "Cell", "\n" "This variant has been introduced in version 0.25." ) + - gsi::iterator ("each_touching_inst", (db::Cell::touching_iterator (db::Cell::*) (const db::Cell::box_type &b) const) &db::Cell::begin_touching, gsi::arg ("b"), + gsi::iterator_ext ("each_touching_inst", &begin_touching_inst, gsi::arg ("b"), "@brief Gets the instances touching the given rectangle\n" "\n" "This will iterate over all child cell\n" @@ -2602,7 +2634,7 @@ Class decl_Cell ("db", "Cell", "\n" "Starting with version 0.15, this iterator delivers \\Instance objects rather than \\CellInstArray objects." ) + - gsi::iterator_ext ("each_touching_inst", &cell_begin_touching_inst_um, gsi::arg ("b"), + gsi::iterator_ext ("each_touching_inst", &begin_touching_inst_um, gsi::arg ("b"), "@brief Gets the instances touching the given rectangle, with the rectangle in micrometer units\n" "\n" "This will iterate over all child cell\n" @@ -2615,7 +2647,7 @@ Class decl_Cell ("db", "Cell", "\n" "This variant has been introduced in version 0.25." ) + - gsi::iterator ("each_child_cell", &db::Cell::begin_child_cells, + gsi::iterator_ext ("each_child_cell", &begin_child_cells, "@brief Iterates over all child cells\n" "\n" "This iterator will report the child cell indices, not every instance.\n" @@ -2626,12 +2658,12 @@ Class decl_Cell ("db", "Cell", "The number of child cells (not child instances!) is returned.\n" "CAUTION: this method is SLOW, in particular if many instances are present.\n" ) + - gsi::iterator ("each_inst", (db::Cell::const_iterator (db::Cell::*) () const) &db::Cell::begin, + gsi::iterator_ext ("each_inst", &begin_inst, "@brief Iterates over all child instances (which may actually be instance arrays)\n" "\n" "Starting with version 0.15, this iterator delivers \\Instance objects rather than \\CellInstArray objects." ) + - gsi::iterator ("each_parent_inst", &db::Cell::begin_parent_insts, + gsi::iterator_ext ("each_parent_inst", &begin_parent_insts, "@brief Iterates over the parent instance list (which may actually be instance arrays)\n" "\n" "The parent instances are basically inversions of the instances. Using parent instances " @@ -2642,7 +2674,7 @@ Class decl_Cell ("db", "Cell", "\n" "The number of parent cells (cells which reference our cell) is reported." ) + - gsi::iterator ("each_parent_cell", &db::Cell::begin_parent_cells, &db::Cell::end_parent_cells, + gsi::iterator_ext ("each_parent_cell", &begin_parent_cells, "@brief Iterates over all parent cells\n" "\n" "This iterator will iterate over the parent cells, just returning their\n" diff --git a/src/db/db/gsiDeclDbHelpers.h b/src/db/db/gsiDeclDbHelpers.h new file mode 100644 index 000000000..22b5f5e50 --- /dev/null +++ b/src/db/db/gsiDeclDbHelpers.h @@ -0,0 +1,83 @@ + +/* + + KLayout Layout Viewer + Copyright (C) 2006-2019 Matthias Koefferlein + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software + Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + +*/ + + +#ifndef HDR_gsiDeclDbHelpers +#define HDR_gsiDeclDbHelpers + +#include "dbLayoutUtils.h" + +namespace gsi +{ + /** + * @brief A safe iterator locking the layout while iterating a container within it + */ + template + class layout_locking_iterator2 + : private db::LayoutLocker + { + public: + typedef typename I::value_type value_type; + typedef typename I::reference reference; + typedef typename I::pointer pointer; + typedef typename I::difference_type difference_type; + typedef typename I::iterator_category iterator_category; + + layout_locking_iterator2 (const db::Layout *layout, const I &b, const I &e) : db::LayoutLocker (const_cast (layout)), m_b (b), m_e (e) {} + bool at_end () const { return m_b == m_e; } + void operator++ () { ++m_b; } + + reference operator* () const { return *m_b; } + pointer operator-> () const { return m_b.operator-> (); } + + private: + I m_b, m_e; + }; + + /** + * @brief A safe iterator locking the layout while iterating a container within it + */ + template + class layout_locking_iterator1 + : private db::LayoutLocker + { + public: + typedef typename I::value_type value_type; + typedef typename I::reference reference; + typedef typename I::pointer pointer; + typedef typename I::difference_type difference_type; + typedef typename I::iterator_category iterator_category; + + layout_locking_iterator1 (const db::Layout *layout, const I &i) : db::LayoutLocker (const_cast (layout)), m_i (i) { } + bool at_end () const { return m_i.at_end (); } + void operator++ () { ++m_i; } + + reference operator* () const { return *m_i; } + pointer operator-> () const { return m_i.operator-> (); } + + private: + I m_i; + }; + +} + +#endif diff --git a/src/db/db/gsiDeclDbShapes.cc b/src/db/db/gsiDeclDbShapes.cc index 05d8a5217..19b7661be 100644 --- a/src/db/db/gsiDeclDbShapes.cc +++ b/src/db/db/gsiDeclDbShapes.cc @@ -22,6 +22,8 @@ #include "gsiDecl.h" + +#include "gsiDeclDbHelpers.h" #include "dbShapes.h" #include "dbShape.h" #include "dbLayout.h" @@ -101,54 +103,54 @@ static db::Shape dinsert_with_properties (db::Shapes *s, const Sh &p, db::proper return s->insert (db::object_with_properties (db::CplxTrans (shapes_dbu (s)).inverted () * p, id)); } -static db::Shapes::shape_iterator begin (const db::Shapes *s, unsigned int flags) +static gsi::layout_locking_iterator1 begin (const db::Shapes *s, unsigned int flags) { - return s->begin (flags); + return gsi::layout_locking_iterator1 (s->layout (), s->begin (flags)); } -static db::Shapes::shape_iterator begin_all (const db::Shapes *s) +static gsi::layout_locking_iterator1begin_all (const db::Shapes *s) { - return s->begin (db::ShapeIterator::All); + return gsi::layout_locking_iterator1 (s->layout (), s->begin (db::ShapeIterator::All)); } -static db::Shapes::shape_iterator begin_overlapping (const db::Shapes *s, unsigned int flags, const db::Box ®ion) +static gsi::layout_locking_iterator1begin_overlapping (const db::Shapes *s, unsigned int flags, const db::Box ®ion) { - return s->begin_overlapping (region, flags); + return gsi::layout_locking_iterator1 (s->layout (), s->begin_overlapping (region, flags)); } -static db::Shapes::shape_iterator begin_doverlapping (const db::Shapes *s, unsigned int flags, const db::DBox ®ion) +static gsi::layout_locking_iterator1begin_doverlapping (const db::Shapes *s, unsigned int flags, const db::DBox ®ion) { - return s->begin_overlapping (db::CplxTrans (shapes_dbu (s)).inverted () * region, flags); + return gsi::layout_locking_iterator1 (s->layout (), s->begin_overlapping (db::CplxTrans (shapes_dbu (s)).inverted () * region, flags)); } -static db::Shapes::shape_iterator begin_overlapping_all (const db::Shapes *s, const db::Box ®ion) +static gsi::layout_locking_iterator1begin_overlapping_all (const db::Shapes *s, const db::Box ®ion) { - return s->begin_overlapping (region, db::ShapeIterator::All); + return gsi::layout_locking_iterator1 (s->layout (), s->begin_overlapping (region, db::ShapeIterator::All)); } -static db::Shapes::shape_iterator begin_doverlapping_all (const db::Shapes *s, const db::DBox ®ion) +static gsi::layout_locking_iterator1begin_doverlapping_all (const db::Shapes *s, const db::DBox ®ion) { - return s->begin_overlapping (db::CplxTrans (shapes_dbu (s)).inverted () * region, db::ShapeIterator::All); + return gsi::layout_locking_iterator1 (s->layout (), s->begin_overlapping (db::CplxTrans (shapes_dbu (s)).inverted () * region, db::ShapeIterator::All)); } -static db::Shapes::shape_iterator begin_touching (const db::Shapes *s, unsigned int flags, const db::Box ®ion) +static gsi::layout_locking_iterator1begin_touching (const db::Shapes *s, unsigned int flags, const db::Box ®ion) { - return s->begin_touching (region, flags); + return gsi::layout_locking_iterator1 (s->layout (), s->begin_touching (region, flags)); } -static db::Shapes::shape_iterator begin_dtouching (const db::Shapes *s, unsigned int flags, const db::DBox ®ion) +static gsi::layout_locking_iterator1begin_dtouching (const db::Shapes *s, unsigned int flags, const db::DBox ®ion) { - return s->begin_touching (db::CplxTrans (shapes_dbu (s)).inverted () * region, flags); + return gsi::layout_locking_iterator1 (s->layout (), s->begin_touching (db::CplxTrans (shapes_dbu (s)).inverted () * region, flags)); } -static db::Shapes::shape_iterator begin_touching_all (const db::Shapes *s, const db::Box ®ion) +static gsi::layout_locking_iterator1begin_touching_all (const db::Shapes *s, const db::Box ®ion) { - return s->begin_touching (region, db::ShapeIterator::All); + return gsi::layout_locking_iterator1 (s->layout (), s->begin_touching (region, db::ShapeIterator::All)); } -static db::Shapes::shape_iterator begin_dtouching_all (const db::Shapes *s, const db::DBox ®ion) +static gsi::layout_locking_iterator1begin_dtouching_all (const db::Shapes *s, const db::DBox ®ion) { - return s->begin_touching (db::CplxTrans (shapes_dbu (s)).inverted () * region, db::ShapeIterator::All); + return gsi::layout_locking_iterator1 (s->layout (), s->begin_touching (db::CplxTrans (shapes_dbu (s)).inverted () * region, db::ShapeIterator::All)); } static void transform_shapes (db::Shapes *s, const db::Trans &trans) diff --git a/src/tl/tl/tlReuseVector.h b/src/tl/tl/tlReuseVector.h index f31236a3a..c7cad2b69 100644 --- a/src/tl/tl/tlReuseVector.h +++ b/src/tl/tl/tlReuseVector.h @@ -887,6 +887,21 @@ public: init (); } + /** + * @brief Returns a value indicating whether the given index is valid + */ + bool is_used (size_type n) const + { + if (n >= first () && n < last ()) { + if (mp_rdata) { + return mp_rdata->is_used (n); + } else { + return true; + } + } + return false; + } + /** * @brief For diagnostics purposes only */ @@ -908,18 +923,6 @@ private: mp_rdata = 0; } - bool is_used (size_type n) const - { - if (n >= first () && n < last ()) { - if (mp_rdata) { - return mp_rdata->is_used (n); - } else { - return true; - } - } - return false; - } - size_type first () const { if (mp_rdata) { diff --git a/testdata/gds/t200.gds b/testdata/gds/t200.gds new file mode 100644 index 0000000000000000000000000000000000000000..08552d3a49985d8f9354b16bffd037f25b26f6f6 GIT binary patch literal 1354 zcmb7EJxc>Y5FIaZEqcQCR;> z2``wP&J$%~@v(`$k(u&ob#&@{=;U~@KwWV+E*S)qYg^0fE957Lf+mj|C}{SGsIN7q zze!auO6nzdi$lrlUXCVDewx1F23lj%4=gySB|nZJc&R(_1}B=p@i9D>_a}Wlx5Ysd z7=yQY#)b5&GbwSVZ`khWeeve`U|zl6ui{XmhYa6`H|IeH3H*fTgpjNe|JC1Khj}%`?D@5;ML)D)4 zZT+Vi>+tG5bBKOm84q6YAcEh4mwv`-4hmD-;F39{?!-frwWA#*zDXHokC-v^#ooLEW^{lNAmP@X>s@xQ)|LQWT J>nVMoJ^(V&Jjeh5 literal 0 HcmV?d00001 diff --git a/testdata/ruby/dbLayout.rb b/testdata/ruby/dbLayout.rb index 04b418eb9..cb89a9e0d 100644 --- a/testdata/ruby/dbLayout.rb +++ b/testdata/ruby/dbLayout.rb @@ -1994,6 +1994,45 @@ END end + # Iterating while flatten + def test_issue200 + + ly = RBA::Layout.new + ly.read(ENV["TESTSRC"] + "/testdata/gds/t200.gds") + l1 = ly.layer(1, 0) + l2 = ly.layer(2, 0) + l3 = ly.layer(3, 0) + + tc_name = ly.top_cell.name + r1 = RBA::Region::new(ly.top_cell.begin_shapes_rec(l1)) + r2 = RBA::Region::new(ly.top_cell.begin_shapes_rec(l2)) + r3 = RBA::Region::new(ly.top_cell.begin_shapes_rec(l3)) + assert_equal(r1.size > 0, true) + assert_equal(r2.size > 0, true) + assert_equal(r3.size == 0, true) + + ly.top_cell.each_inst do |ci| + ci.flatten + end + + tc = ly.cell(tc_name) + assert_equal(ly.top_cells.size, 4) + assert_equal(tc.child_cells, 0) + assert_equal(tc.parent_cells, 0) + + rr1 = RBA::Region::new(tc.begin_shapes_rec(l1)) + rr2 = RBA::Region::new(tc.begin_shapes_rec(l2)) + rr3 = RBA::Region::new(tc.begin_shapes_rec(l3)) + assert_equal(r1.size, rr1.size) + assert_equal(r2.size, rr2.size) + assert_equal(r3.size, rr3.size) + + assert_equal((rr1 ^ r1).is_empty?, true) + assert_equal((rr2 ^ r2).is_empty?, true) + assert_equal((rr3 ^ r3).is_empty?, true) + + end + end load("test_epilogue.rb") From e5925dfa1d8ca25171654e94f629e15cbacca01f Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Thu, 10 Jan 2019 00:50:13 +0100 Subject: [PATCH 3/7] Formal URL for issues in Changelog. --- Changelog | 136 +++++++++++++++++++++++++++--------------------------- 1 file changed, 68 insertions(+), 68 deletions(-) diff --git a/Changelog b/Changelog index 138367007..9a4524b08 100644 --- a/Changelog +++ b/Changelog @@ -3,21 +3,21 @@ * YET TO BE RELEASED * Enhancement: Standalone Python modules provided For details see: - https://github.com/klayoutmatthias/klayout/wiki/klayout---Standalone-KLayout-Python-Module + %GITHUB%/wiki/klayout---Standalone-KLayout-Python-Module 0.25.5 (2018-09-30): -* Bugfix: https://github.com/klayoutmatthias/klayout/issues/162 +* Bugfix: %GITHUB%/issues/162 GDS2 LIBNAME was not maintained on "File/Save". -* Bugfix: https://github.com/klayoutmatthias/klayout/issues/166 +* Bugfix: %GITHUB%/issues/166 Internal error when writing GDS files (breaking of polygons) -* Bugfix: https://github.com/klayoutmatthias/klayout/issues/172 +* Bugfix: %GITHUB%/issues/172 DEF reader did not pull vias from LEF -* Bugfix: https://github.com/klayoutmatthias/klayout/issues/174 +* Bugfix: %GITHUB%/issues/174 Performance issue with many layers with width >1 -* Bugfix: https://github.com/klayoutmatthias/klayout/issues/176 +* Bugfix: %GITHUB%/issues/176 Painting issue with texts -* Bugfix: https://github.com/klayoutmatthias/klayout/issues/185 +* Bugfix: %GITHUB%/issues/185 Hash values available as __hash__ standard method now for Python * Bugfix: some potential memory corruption issues fixed @@ -27,36 +27,36 @@ These fixes are included in this release. 0.25.4 (2018-08-25): -* Bugfix: https://github.com/klayoutmatthias/klayout/issues/121 +* Bugfix: %GITHUB%/issues/121 Issue with multiple reads of GDS2 layouts including PCells -* Bugfix: https://github.com/klayoutmatthias/klayout/issues/134 +* Bugfix: %GITHUB%/issues/134 Error in cell.fill_region caused by big polygon with spikes -* Bugfix: https://github.com/klayoutmatthias/klayout/issues/139 +* Bugfix: %GITHUB%/issues/139 Libraries have not been reassigned when loading a GDS file from command line (does not happen on File/Open) -* Bugfix: https://github.com/klayoutmatthias/klayout/issues/141 +* Bugfix: %GITHUB%/issues/141 Issue with RBA::QHostAddress (ambiguous overload) on Qt5 -* Bugfix: https://github.com/klayoutmatthias/klayout/issues/142 +* Bugfix: %GITHUB%/issues/142 Issue with RBA::RecursiveShapeIterator#region= -* Bugfix: https://github.com/klayoutmatthias/klayout/issues/144 +* Bugfix: %GITHUB%/issues/144 The Salt package descriptions are not shown with Motif style -* Bugfix: https://github.com/klayoutmatthias/klayout/issues/148 +* Bugfix: %GITHUB%/issues/148 Wrong font is used -* Bugfix: https://github.com/klayoutmatthias/klayout/issues/152 +* Bugfix: %GITHUB%/issues/152 Shapes#size reported a wrong shape count in viewer mode -* Bugfix: https://github.com/klayoutmatthias/klayout/issues/153 +* Bugfix: %GITHUB%/issues/153 Application crash when editing guiding shape properties -* Bugfix: https://github.com/klayoutmatthias/klayout/issues/155 +* Bugfix: %GITHUB%/issues/155 Program freezes after replacing nothing by something in Macro editor -* Bugfix: https://github.com/klayoutmatthias/klayout/issues/157 +* Bugfix: %GITHUB%/issues/157 "Replace cell with ..." rejected cell names with a library prefix -* Bugfix: https://github.com/klayoutmatthias/klayout/issues/158 +* Bugfix: %GITHUB%/issues/158 Repaint issue on cell context -* Bugfix: https://github.com/klayoutmatthias/klayout/issues/159 +* Bugfix: %GITHUB%/issues/159 Tech specific macros and DRC scripts were not shown in tech manager * Bugfix: 8 bit indexed GIF images can be used for package icons now @@ -118,41 +118,41 @@ * Bugfix: XOR progress is more realistic The progress is updated after the layer has been computed, not before. -* Bugfix: https://github.com/klayoutmatthias/klayout/issues/117 +* Bugfix: %GITHUB%/issues/117 DTrans#itype was broken. -* Bugfix: https://github.com/klayoutmatthias/klayout/issues/116 +* Bugfix: %GITHUB%/issues/116 Fixed a polygon decomposition bug when writing GDS files with big polygons with many holes. As a side effect, the polygons with many holes computed by a NOT operation for example are less complex and spikes in the cutlines are avoided. -* Bugfix: https://github.com/klayoutmatthias/klayout/issues/115 +* Bugfix: %GITHUB%/issues/115 Reader options were not persisted. -* Bugfix: https://github.com/klayoutmatthias/klayout/issues/114 +* Bugfix: %GITHUB%/issues/114 Custom line styles not loaded from tech's layer properties file. -* Enhancement: https://github.com/klayoutmatthias/klayout/issues/113 +* Enhancement: %GITHUB%/issues/113 The XOR tool has an option now to heal result shapes which cross tile boundaries. The result shape count of tiled and non-tiled mode should basically be the same then. -* Bugfix: https://github.com/klayoutmatthias/klayout/issues/112 +* Bugfix: %GITHUB%/issues/112 Salt package repository relative paths have not been working. -* Bugfix: https://github.com/klayoutmatthias/klayout/issues/109 +* Bugfix: %GITHUB%/issues/109 Issues with Python 3 and shape properties - property -* Bugfix: https://github.com/klayoutmatthias/klayout/issues/108 +* Bugfix: %GITHUB%/issues/108 Bugfix on Box#enlarge and Box#moved for empty boxes. keys generated with Python 3 could not be written to GDS2. -* Bugfix: https://github.com/klayoutmatthias/klayout/issues/107 +* Bugfix: %GITHUB%/issues/107 Undo not working with shapes. -* Enhancement: https://github.com/klayoutmatthias/klayout/issues/106 +* Enhancement: %GITHUB%/issues/106 Search & replace help page enhancements. 0.25.2 (2018-03-20): -* Bugfix: https://github.com/klayoutmatthias/klayout/issues/90 +* Bugfix: %GITHUB%/issues/90 DRC: "extended" was not working as expected with "joined = true" -* Bugfix: https://github.com/klayoutmatthias/klayout/issues/89 +* Bugfix: %GITHUB%/issues/89 Display issue on MacOS fixed -* Enhancement: https://github.com/klayoutmatthias/klayout/issues/85 +* Enhancement: %GITHUB%/issues/85 IDE debugger: files can be excluded from showing exceptions when they are thrown. To exclude a file press the new "Ignore" button when the debugger tells you an exception has been generated. @@ -160,7 +160,7 @@ in the IDE settings ("Debugging" tab) The macro IDE settings can now be edited in the File/Setup dialog. -* Bugfix: https://github.com/klayoutmatthias/klayout/issues/94 +* Bugfix: %GITHUB%/issues/94 Retina displays are support to some extend on MacOS. An open topic is the quality of the icons. * Enhancement: build system for MacOS @@ -174,40 +174,40 @@ - A potential crash ob removing packages was fixed * Enhancement: 64 bit coordinate support enabled on Windows builds * Further bugfixes: See links - - https://github.com/klayoutmatthias/klayout/issues/21 (Autorun(-early) doesn't seem to run when lym files are inside a package) - - https://github.com/klayoutmatthias/klayout/issues/24 (Text insert dialog bug - Ok button isn't working) - - https://github.com/klayoutmatthias/klayout/issues/26 (Exceptions are reported every time they propagate up in the call chain in the ruby debugger) - - https://github.com/klayoutmatthias/klayout/issues/28 (CIF format detection failed) - - https://github.com/klayoutmatthias/klayout/issues/30 (Writer options dialog non-functional on a fresh configuration) - - https://github.com/klayoutmatthias/klayout/issues/32 (Rounding issue with instance properties) - - https://github.com/klayoutmatthias/klayout/issues/33 (Plugin factory not working when using with Python) - - https://github.com/klayoutmatthias/klayout/issues/36 (Hardening against destruction of object inside event handler) - - https://github.com/klayoutmatthias/klayout/issues/39 (Action cannot be reassigned) - - https://github.com/klayoutmatthias/klayout/issues/40 (Crash in Python binding) - - https://github.com/klayoutmatthias/klayout/issues/41 (Polygon#touches? issue) - - https://github.com/klayoutmatthias/klayout/issues/42 (Headless mode support with Qt5/-zz) - - https://github.com/klayoutmatthias/klayout/issues/43 (Crash when using Qt specific command line options) - - https://github.com/klayoutmatthias/klayout/issues/44 (Transformation constructor with x,y not working) - - https://github.com/klayoutmatthias/klayout/issues/45 (Partial selection does not capture instance) - - https://github.com/klayoutmatthias/klayout/issues/48 (Cancel does not reset current tool) - - https://github.com/klayoutmatthias/klayout/issues/51 (Segmentation fault on return to main window and other opportunities) - - https://github.com/klayoutmatthias/klayout/issues/53 (Unreadable 'about' text) - - https://github.com/klayoutmatthias/klayout/issues/62 (QXmlSimpleReader#parse cannot be called) - - https://github.com/klayoutmatthias/klayout/issues/63 (Wrong output on DRC non_interacting with empty second input) - - https://github.com/klayoutmatthias/klayout/issues/64 (Crash on exit) - - https://github.com/klayoutmatthias/klayout/issues/68 (OASIS reader issue with degenerated shapes) - - https://github.com/klayoutmatthias/klayout/issues/69 (DRC: 'inside' does not merge shapes of second input) - - https://github.com/klayoutmatthias/klayout/issues/71 (Target cell argument is required) - - https://github.com/klayoutmatthias/klayout/issues/72 (Edges/Region NOT issue) - - https://github.com/klayoutmatthias/klayout/issues/73 (Allow 'change layers' on PCells which support a single layer parameter) - - https://github.com/klayoutmatthias/klayout/issues/74 (Small-corner boolean issue) - - https://github.com/klayoutmatthias/klayout/issues/75 (Python PCell issue when parameters are called 'layer') - - https://github.com/klayoutmatthias/klayout/issues/79 (Replace function enabled also for read-only macros) + - %GITHUB%/issues/21 (Autorun(-early) doesn't seem to run when lym files are inside a package) + - %GITHUB%/issues/24 (Text insert dialog bug - Ok button isn't working) + - %GITHUB%/issues/26 (Exceptions are reported every time they propagate up in the call chain in the ruby debugger) + - %GITHUB%/issues/28 (CIF format detection failed) + - %GITHUB%/issues/30 (Writer options dialog non-functional on a fresh configuration) + - %GITHUB%/issues/32 (Rounding issue with instance properties) + - %GITHUB%/issues/33 (Plugin factory not working when using with Python) + - %GITHUB%/issues/36 (Hardening against destruction of object inside event handler) + - %GITHUB%/issues/39 (Action cannot be reassigned) + - %GITHUB%/issues/40 (Crash in Python binding) + - %GITHUB%/issues/41 (Polygon#touches? issue) + - %GITHUB%/issues/42 (Headless mode support with Qt5/-zz) + - %GITHUB%/issues/43 (Crash when using Qt specific command line options) + - %GITHUB%/issues/44 (Transformation constructor with x,y not working) + - %GITHUB%/issues/45 (Partial selection does not capture instance) + - %GITHUB%/issues/48 (Cancel does not reset current tool) + - %GITHUB%/issues/51 (Segmentation fault on return to main window and other opportunities) + - %GITHUB%/issues/53 (Unreadable 'about' text) + - %GITHUB%/issues/62 (QXmlSimpleReader#parse cannot be called) + - %GITHUB%/issues/63 (Wrong output on DRC non_interacting with empty second input) + - %GITHUB%/issues/64 (Crash on exit) + - %GITHUB%/issues/68 (OASIS reader issue with degenerated shapes) + - %GITHUB%/issues/69 (DRC: 'inside' does not merge shapes of second input) + - %GITHUB%/issues/71 (Target cell argument is required) + - %GITHUB%/issues/72 (Edges/Region NOT issue) + - %GITHUB%/issues/73 (Allow 'change layers' on PCells which support a single layer parameter) + - %GITHUB%/issues/74 (Small-corner boolean issue) + - %GITHUB%/issues/75 (Python PCell issue when parameters are called 'layer') + - %GITHUB%/issues/79 (Replace function enabled also for read-only macros) * Further enhancements: see links - - https://github.com/klayoutmatthias/klayout/issues/29 (Permissive mode for OASIS writer on odd-width paths) - - https://github.com/klayoutmatthias/klayout/issues/59 (Async download of package index and details) - - https://github.com/klayoutmatthias/klayout/issues/66 (Authentication dialog indicates retry) - - https://github.com/klayoutmatthias/klayout/issues/77 (Layout#copy_tree now works in non-editable mode too) + - %GITHUB%/issues/29 (Permissive mode for OASIS writer on odd-width paths) + - %GITHUB%/issues/59 (Async download of package index and details) + - %GITHUB%/issues/66 (Authentication dialog indicates retry) + - %GITHUB%/issues/77 (Layout#copy_tree now works in non-editable mode too) 0.25 (2017-11-04): * Enhancement: Menu customization @@ -269,7 +269,7 @@ Packages can be published on GitHub or any server supporting WebDAV. After registering a package, users can install or update packages with a few clicks. - For more information see https://github.com/klayoutmatthias/klayout/wiki + For more information see %GITHUB%/wiki or "About Packages" in the "Various Topics" area of the main documentation. The package manager is found in the "Tools" menu under "Manage Packages". From b6eeb4bef34ae7e734a71cb6617dacb8f7cd8e21 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Wed, 16 Jan 2019 23:12:47 +0100 Subject: [PATCH 4/7] Fixed #225 by fixing the READER. --- src/laybasic/laybasic/layDitherPattern.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/laybasic/laybasic/layDitherPattern.cc b/src/laybasic/laybasic/layDitherPattern.cc index f4afad80b..c3fbd3085 100644 --- a/src/laybasic/laybasic/layDitherPattern.cc +++ b/src/laybasic/laybasic/layDitherPattern.cc @@ -726,8 +726,6 @@ DitherPatternInfo::from_strings (const std::vector &strv) uint_from_string (strv [strv.size () - 1 - i].c_str (), data [i], w); } - std::reverse (&data[0], &data[h]); - set_pattern (data, w, h); } From 93f4cbf294b5c9d64d6f84aa32f1ae06930005f3 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Wed, 23 Jan 2019 21:16:26 +0100 Subject: [PATCH 5/7] Fixed Windows installer: stream plugins have not been included. --- scripts/klayout-inst.nsis | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/klayout-inst.nsis b/scripts/klayout-inst.nsis index 7d6244294..91d36a508 100644 --- a/scripts/klayout-inst.nsis +++ b/scripts/klayout-inst.nsis @@ -68,6 +68,8 @@ section file *.dll file strm*.exe file .*-paths.txt + file /r db_plugins + file /r lay_plugins file /r audio file /r generic file /r iconengines From fe0d3e28bab5862b0f73a7929346a01fbf565ad5 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sat, 26 Jan 2019 23:59:22 +0100 Subject: [PATCH 6/7] Fixed issue #228 Reason for the problem: the interaction test has to keep separate "inside" records for both below and above the scanline, not just once. With the single record, the step in the left polygon erased the "inside" condition. --- src/db/db/dbEdgeProcessor.cc | 28 ++++++++++++++++++++-------- src/db/db/dbEdgeProcessor.h | 4 ++-- src/db/unit_tests/dbRegion.cc | 24 ++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 10 deletions(-) diff --git a/src/db/db/dbEdgeProcessor.cc b/src/db/db/dbEdgeProcessor.cc index 0a1481cd1..756f6af72 100644 --- a/src/db/db/dbEdgeProcessor.cc +++ b/src/db/db/dbEdgeProcessor.cc @@ -639,7 +639,8 @@ InteractionDetector::reset () { m_wcv_n.clear (); m_wcv_s.clear (); - m_inside.clear (); + m_inside_n.clear (); + m_inside_s.clear (); } void @@ -649,7 +650,8 @@ InteractionDetector::reserve (size_t n) m_wcv_s.clear (); m_wcv_n.resize (n, 0); m_wcv_s.resize (n, 0); - m_inside.clear (); + m_inside_n.clear (); + m_inside_s.clear (); } int @@ -667,9 +669,11 @@ InteractionDetector::edge (bool north, bool enter, property_type p) // we have to catch interactions between objects north and south to the scanline if (north || (m_mode == 0 && m_include_touching)) { + std::set *inside = north ? &m_inside_n : &m_inside_s; + if (inside_after < inside_before) { - m_inside.erase (p); + inside->erase (p); if (m_mode != 0) { @@ -677,7 +681,7 @@ InteractionDetector::edge (bool north, bool enter, property_type p) // (due to prefer_touch == true and the sorting of coincident edges by property id) // hence every remaining parts count as non-interacting (outside) if (p == m_container_id) { - for (std::set ::const_iterator i = m_inside.begin (); i != m_inside.end (); ++i) { + for (std::set ::const_iterator i = inside->begin (); i != inside->end (); ++i) { if (*i != m_container_id) { m_non_interactions.insert (*i); } @@ -695,13 +699,13 @@ InteractionDetector::edge (bool north, bool enter, property_type p) // note that the container parts will be delivered first of all coincident // edges hence we can check whether the container is present even for coincident // edges - if (m_inside.find (m_container_id) != m_inside.end ()) { + if (inside->find (m_container_id) != inside->end ()) { m_interactions.insert (std::make_pair (m_container_id, p)); } else { m_non_interactions.insert (p); } } else { - for (std::set ::const_iterator i = m_inside.begin (); i != m_inside.end (); ++i) { + for (std::set ::const_iterator i = inside->begin (); i != inside->end (); ++i) { if (*i != m_container_id) { m_interactions.insert (std::make_pair (m_container_id, *i)); } @@ -710,7 +714,15 @@ InteractionDetector::edge (bool north, bool enter, property_type p) } else { - for (std::set ::const_iterator i = m_inside.begin (); i != m_inside.end (); ++i) { + for (std::set ::const_iterator i = m_inside_n.begin (); i != m_inside_n.end (); ++i) { + if (*i < p) { + m_interactions.insert (std::make_pair (*i, p)); + } else if (*i > p) { + m_interactions.insert (std::make_pair (p, *i)); + } + } + + for (std::set ::const_iterator i = m_inside_s.begin (); i != m_inside_s.end (); ++i) { if (*i < p) { m_interactions.insert (std::make_pair (*i, p)); } else if (*i > p) { @@ -720,7 +732,7 @@ InteractionDetector::edge (bool north, bool enter, property_type p) } - m_inside.insert (p); + inside->insert (p); } diff --git a/src/db/db/dbEdgeProcessor.h b/src/db/db/dbEdgeProcessor.h index bca103bc2..9cca1308a 100644 --- a/src/db/db/dbEdgeProcessor.h +++ b/src/db/db/dbEdgeProcessor.h @@ -299,7 +299,7 @@ public: virtual void reserve (size_t n); virtual int edge (bool north, bool enter, property_type p); virtual int compare_ns () const; - virtual bool is_reset () const { return m_inside.empty (); } + virtual bool is_reset () const { return m_inside_s.empty () && m_inside_n.empty (); } virtual bool prefer_touch () const { return m_include_touching; } private: @@ -307,7 +307,7 @@ private: bool m_include_touching; property_type m_container_id; std::vector m_wcv_n, m_wcv_s; - std::set m_inside; + std::set m_inside_n, m_inside_s; std::set > m_interactions; std::set m_non_interactions; }; diff --git a/src/db/unit_tests/dbRegion.cc b/src/db/unit_tests/dbRegion.cc index 686ec4062..55f61df92 100644 --- a/src/db/unit_tests/dbRegion.cc +++ b/src/db/unit_tests/dbRegion.cc @@ -1329,3 +1329,27 @@ TEST(30c) EXPECT_EQ (r.to_string (), "(-100,-100;-100,0;0,0;0,200;100,200;100,0;0,0;0,-100)"); } +TEST(issue_228) +{ + db::Region r; + db::Point pts[] = { + db::Point (0, 10), + db::Point (0, 290), + db::Point (280, 290), + db::Point (280, 230), + db::Point (360, 230), + db::Point (360, 70), + db::Point (280,70), + db::Point (280,10) + }; + + db::Polygon poly; + poly.assign_hull (pts, pts + sizeof (pts) / sizeof (pts [0])); + r.insert (poly); + + db::Region rr; + rr.insert (db::Box (360, 70, 480, 230)); + + EXPECT_EQ (r.selected_interacting (rr).to_string (), r.to_string ()); + EXPECT_EQ (rr.selected_interacting (r).to_string (), rr.to_string ()); +} From 7b07782cdf922a8c5d98a675dbcf5f182f7905fe Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Thu, 31 Jan 2019 00:36:44 +0100 Subject: [PATCH 7/7] Avoid an issue with virtual functions Reimplementing virtual functions with "const &" arguments wasn't behaving as expected because these arguments were copied. Now, "const &" for arguments (in virtual function reimplementation) is not implemented as a copy. In addition, now it's possible to declare results as references always (also if const &). See gsiTest.cc:1078 for example: // gsi::arg_make_reference makes the function's return value // always being taken as a reference gsi::method ("pass_cd_cref_as_ref", &C_P::pass_cd_cref) --- src/gsi/gsi/gsiMethods.h | 73 ++++++++++---- src/gsi/gsi/gsiMethodsVar.h | 132 ++++++++++++++++--------- src/gsi/gsi/gsiTypes.cc | 7 +- src/gsi/gsi/gsiTypes.h | 165 ++++++++++++++++++++++++++++++-- src/gsi/gsi_test/gsiTest.cc | 25 +++++ src/gsi/gsi_test/gsiTest.h | 51 ++++++++++ src/pya/pya/pyaConvert.cc | 2 +- src/rba/rba/rbaConvert.cc | 2 +- testdata/python/basic.py | 74 ++++++++++++++ testdata/ruby/basic_testcore.rb | 84 ++++++++++++++++ 10 files changed, 539 insertions(+), 76 deletions(-) diff --git a/src/gsi/gsi/gsiMethods.h b/src/gsi/gsi/gsiMethods.h index 9dedc3694..d29d70f25 100644 --- a/src/gsi/gsi/gsiMethods.h +++ b/src/gsi/gsi/gsiMethods.h @@ -334,11 +334,23 @@ public: /** * @brief Adds an argument to the argument list (of type X) */ - template + template void add_arg () { ArgType a; - a.template init (); + a.template init (); + m_arg_types.push_back (a); + m_argsize += a.size (); + } + + /** + * @brief Adds an argument to the argument list (of type X) + */ + template + void add_arg () + { + ArgType a; + a.template init (); m_arg_types.push_back (a); m_argsize += a.size (); } @@ -346,11 +358,23 @@ public: /** * @brief Adds an argument to the argument list (of type X plus additional specs) */ - template + template void add_arg (const ArgSpecBase &spec) { ArgType a; - a.template init (spec); + a.template init (spec); + m_arg_types.push_back (a); + m_argsize += a.size (); + } + + /** + * @brief Adds an argument to the argument list (of type X plus additional specs) + */ + template + void add_arg (const ArgSpecBase &spec) + { + ArgType a; + a.template init (spec); m_arg_types.push_back (a); m_argsize += a.size (); } @@ -358,11 +382,23 @@ public: /** * @brief This version will take the ownership of the ArgSpecBase object */ - template + template void add_arg (ArgSpecBase *spec) { ArgType a; - a.template init (spec); + a.template init (spec); + m_arg_types.push_back (a); + m_argsize += a.size (); + } + + /** + * @brief This version will take the ownership of the ArgSpecBase object + */ + template + void add_arg (ArgSpecBase *spec) + { + ArgType a; + a.template init (spec); m_arg_types.push_back (a); m_argsize += a.size (); } @@ -379,10 +415,19 @@ public: /** * @brief Sets the return type to "X" */ - template + template void set_return () { - m_ret_type.template init (); + m_ret_type.template init (); + } + + /** + * @brief Sets the return type to "X" + */ + template + void set_return () + { + m_ret_type.template init (); } /** @@ -391,7 +436,7 @@ public: template void set_return_new () { - m_ret_type.template init (true); + m_ret_type.template init (); } /** @@ -810,16 +855,6 @@ constant (const std::string &name, R (*m) (), const std::string &doc = std::stri return Methods (new ConstantGetter (name, m, doc)); } -struct return_by_value -{ - typedef tl::False is_factory; -}; - -struct return_new_object -{ - typedef tl::True is_factory; -}; - // 0 argument #define _COUNT 0 diff --git a/src/gsi/gsi/gsiMethodsVar.h b/src/gsi/gsi/gsiMethodsVar.h index db383beaf..c0511038f 100644 --- a/src/gsi/gsi/gsiMethodsVar.h +++ b/src/gsi/gsi/gsiMethodsVar.h @@ -220,7 +220,7 @@ private: _ARGSPECMEM }; -template +template class _NAME(Method) : public MethodSpecificBase { @@ -240,11 +240,7 @@ public: { this->clear (); _ADDARGS - if (tl::value_from_type (typename F::is_factory ())) { - this->template set_return_new (); - } else { - this->template set_return (); - } + this->template set_return (); } virtual MethodBase *clone () const @@ -268,7 +264,7 @@ private: _ARGSPECMEM }; -template +template class _NAME(ConstMethod) : public MethodSpecificBase { @@ -288,11 +284,7 @@ public: { this->clear (); _ADDARGS - if (tl::value_from_type (typename F::is_factory ())) { - this->template set_return_new (); - } else { - this->template set_return (); - } + this->template set_return (); } virtual MethodBase *clone () const @@ -316,7 +308,7 @@ private: _ARGSPECMEM }; -template +template class _NAME(ExtMethod) : public MethodBase { @@ -336,11 +328,7 @@ public: { this->clear (); _ADDARGS - if (tl::value_from_type (typename F::is_factory ())) { - this->template set_return_new (); - } else { - this->template set_return (); - } + this->template set_return (); } virtual MethodBase *clone () const @@ -364,7 +352,7 @@ private: _ARGSPECMEM }; -template +template class _NAME(StaticMethod) : public StaticMethodBase { @@ -384,11 +372,7 @@ public: { this->clear (); _ADDARGS - if (tl::value_from_type (typename F::is_factory ())) { - this->template set_return_new (); - } else { - this->template set_return (); - } + this->template set_return (); } virtual MethodBase *clone () const @@ -1300,7 +1284,14 @@ template Methods method (const std::string &name, R (X::*m) (_FUNCARGLIST), const std::string &doc = std::string ()) { - return Methods (new _NAME(Method) (name, m, doc)); + return Methods (new _NAME(Method) (name, m, doc)); +} + +template +Methods +method (const std::string &name, R (X::*m) (_FUNCARGLIST), const std::string &doc = std::string ()) +{ + return Methods (new _NAME(Method) (name, m, doc)); } #if _COUNT != 0 @@ -1308,7 +1299,14 @@ template Methods method (const std::string &name, R (X::*m) (_FUNCARGLIST) _COMMA _ARGSPECS, const std::string &doc = std::string ()) { - return Methods ((new _NAME(Method) (name, m, doc))->add_args (_ARGSPECARGS)); + return Methods ((new _NAME(Method) (name, m, doc))->add_args (_ARGSPECARGS)); +} + +template +Methods +method (const std::string &name, R (X::*m) (_FUNCARGLIST) _COMMA _ARGSPECS, const std::string &doc = std::string ()) +{ + return Methods ((new _NAME(Method) (name, m, doc))->add_args (_ARGSPECARGS)); } #endif @@ -1316,7 +1314,7 @@ template Methods factory (const std::string &name, R *(X::*m) (_FUNCARGLIST), const std::string &doc = std::string ()) { - return Methods (new _NAME(Method) (name, m, doc)); + return Methods (new _NAME(Method) (name, m, doc)); } #if _COUNT != 0 @@ -1324,7 +1322,7 @@ template Methods factory (const std::string &name, R *(X::*m) (_FUNCARGLIST) _COMMA _ARGSPECS, const std::string &doc = std::string ()) { - return Methods ((new _NAME(Method) (name, m, doc))->add_args (_ARGSPECARGS)); + return Methods ((new _NAME(Method) (name, m, doc))->add_args (_ARGSPECARGS)); } #endif @@ -1332,7 +1330,14 @@ template Methods method_ext (const std::string &name, R (*xm) (X * _COMMA _FUNCARGLIST), const std::string &doc = std::string ()) { - return Methods (new _NAME(ExtMethod) (name, xm, doc)); + return Methods (new _NAME(ExtMethod) (name, xm, doc)); +} + +template +Methods +method_ext (const std::string &name, R (*xm) (X * _COMMA _FUNCARGLIST), const std::string &doc = std::string ()) +{ + return Methods (new _NAME(ExtMethod) (name, xm, doc)); } #if _COUNT != 0 @@ -1340,7 +1345,14 @@ template Methods method_ext (const std::string &name, R (*xm) (X * _COMMA _FUNCARGLIST) _COMMA _ARGSPECS, const std::string &doc = std::string ()) { - return Methods ((new _NAME(ExtMethod) (name, xm, doc))->add_args (_ARGSPECARGS)); + return Methods ((new _NAME(ExtMethod) (name, xm, doc))->add_args (_ARGSPECARGS)); +} + +template +Methods +method_ext (const std::string &name, R (*xm) (X * _COMMA _FUNCARGLIST) _COMMA _ARGSPECS, const std::string &doc = std::string ()) +{ + return Methods ((new _NAME(ExtMethod) (name, xm, doc))->add_args (_ARGSPECARGS)); } #endif @@ -1348,7 +1360,7 @@ template Methods factory_ext (const std::string &name, R *(*xm) (X * _COMMA _FUNCARGLIST), const std::string &doc = std::string ()) { - return Methods (new _NAME(ExtMethod) (name, xm, doc)); + return Methods (new _NAME(ExtMethod) (name, xm, doc)); } #if _COUNT != 0 @@ -1356,7 +1368,7 @@ template Methods factory_ext (const std::string &name, R *(*xm) (X * _COMMA _FUNCARGLIST) _COMMA _ARGSPECS, const std::string &doc = std::string ()) { - return Methods ((new _NAME(ExtMethod) (name, xm, doc))->add_args (_ARGSPECARGS)); + return Methods ((new _NAME(ExtMethod) (name, xm, doc))->add_args (_ARGSPECARGS)); } #endif @@ -1364,7 +1376,7 @@ template Methods constructor (const std::string &name, X *(*m) (_FUNCARGLIST), const std::string &doc = std::string ()) { - return Methods (new _NAME(StaticMethod) (name, m, doc)); + return Methods (new _NAME(StaticMethod) (name, m, doc)); } #if _COUNT != 0 @@ -1372,7 +1384,7 @@ template Methods constructor (const std::string &name, X *(*m) (_FUNCARGLIST) _COMMA _ARGSPECS, const std::string &doc = std::string ()) { - return Methods ((new _NAME(StaticMethod) (name, m, doc))->add_args (_ARGSPECARGS)); + return Methods ((new _NAME(StaticMethod) (name, m, doc))->add_args (_ARGSPECARGS)); } #endif @@ -1380,7 +1392,14 @@ template Methods method (const std::string &name, R (*m) (_FUNCARGLIST), const std::string &doc = std::string ()) { - return Methods (new _NAME(StaticMethod) (name, m, doc)); + return Methods (new _NAME(StaticMethod) (name, m, doc)); +} + +template +Methods +method (const std::string &name, R (*m) (_FUNCARGLIST), const std::string &doc = std::string ()) +{ + return Methods (new _NAME(StaticMethod) (name, m, doc)); } #if _COUNT != 0 @@ -1388,7 +1407,14 @@ template Methods method (const std::string &name, R (*m) (_FUNCARGLIST) _COMMA _ARGSPECS, const std::string &doc = std::string ()) { - return Methods ((new _NAME(StaticMethod) (name, m, doc))->add_args (_ARGSPECARGS)); + return Methods ((new _NAME(StaticMethod) (name, m, doc))->add_args (_ARGSPECARGS)); +} + +template +Methods +method (const std::string &name, R (*m) (_FUNCARGLIST) _COMMA _ARGSPECS, const std::string &doc = std::string ()) +{ + return Methods ((new _NAME(StaticMethod) (name, m, doc))->add_args (_ARGSPECARGS)); } #endif @@ -1396,7 +1422,7 @@ template Methods factory (const std::string &name, R *(*m) (_FUNCARGLIST), const std::string &doc = std::string ()) { - return Methods (new _NAME(StaticMethod) (name, m, doc)); + return Methods (new _NAME(StaticMethod) (name, m, doc)); } #if _COUNT != 0 @@ -1404,7 +1430,7 @@ template Methods factory (const std::string &name, R *(*m) (_FUNCARGLIST) _COMMA _ARGSPECS, const std::string &doc = std::string ()) { - return Methods ((new _NAME(StaticMethod) (name, m, doc))->add_args (_ARGSPECARGS)); + return Methods ((new _NAME(StaticMethod) (name, m, doc))->add_args (_ARGSPECARGS)); } #endif @@ -1428,7 +1454,7 @@ template Methods factory_callback (const std::string &name, R (X::*m) (_FUNCARGLIST), Callback X::*cb, const std::string &doc = std::string ()) { - return Methods (new _NAME(Method) (name, m, doc, cb)); + return Methods (new _NAME(Method) (name, m, doc, cb)); } #if _COUNT != 0 @@ -1436,7 +1462,7 @@ template Methods factory_callback (const std::string &name, R (X::*m) (_FUNCARGLIST), Callback X::*cb _COMMA _ARGSPECS, const std::string &doc = std::string ()) { - return Methods ((new _NAME(Method) (name, m, doc, cb))->add_args (_ARGSPECARGS)); + return Methods ((new _NAME(Method) (name, m, doc, cb))->add_args (_ARGSPECARGS)); } #endif @@ -1444,7 +1470,14 @@ template Methods method (const std::string &name, R (X::*m) (_FUNCARGLIST) const, const std::string &doc = std::string ()) { - return Methods (new _NAME(ConstMethod) (name, m, doc)); + return Methods (new _NAME(ConstMethod) (name, m, doc)); +} + +template +Methods +method (const std::string &name, R (X::*m) (_FUNCARGLIST) const, const std::string &doc = std::string ()) +{ + return Methods (new _NAME(ConstMethod) (name, m, doc)); } #if _COUNT != 0 @@ -1452,7 +1485,14 @@ template Methods method (const std::string &name, R (X::*m) (_FUNCARGLIST) const _COMMA _ARGSPECS, const std::string &doc = std::string ()) { - return Methods ((new _NAME(ConstMethod) (name, m, doc))->add_args (_ARGSPECARGS)); + return Methods ((new _NAME(ConstMethod) (name, m, doc))->add_args (_ARGSPECARGS)); +} + +template +Methods +method (const std::string &name, R (X::*m) (_FUNCARGLIST) const _COMMA _ARGSPECS, const std::string &doc = std::string ()) +{ + return Methods ((new _NAME(ConstMethod) (name, m, doc))->add_args (_ARGSPECARGS)); } #endif @@ -1460,7 +1500,7 @@ template Methods factory (const std::string &name, R *(X::*m) (_FUNCARGLIST) const, const std::string &doc = std::string ()) { - return Methods (new _NAME(ConstMethod) (name, m, doc)); + return Methods (new _NAME(ConstMethod) (name, m, doc)); } #if _COUNT != 0 @@ -1468,7 +1508,7 @@ template Methods factory (const std::string &name, R *(X::*m) (_FUNCARGLIST) const _COMMA _ARGSPECS, const std::string &doc = std::string ()) { - return Methods ((new _NAME(ConstMethod) (name, m, doc))->add_args (_ARGSPECARGS)); + return Methods ((new _NAME(ConstMethod) (name, m, doc))->add_args (_ARGSPECARGS)); } #endif @@ -1492,7 +1532,7 @@ template Methods factory_callback (const std::string &name, R (X::*m) (_FUNCARGLIST) const, Callback X::*cb, const std::string &doc = std::string ()) { - return Methods (new _NAME(ConstMethod) (name, m, doc, cb)); + return Methods (new _NAME(ConstMethod) (name, m, doc, cb)); } #if _COUNT != 0 @@ -1500,7 +1540,7 @@ template Methods factory_callback (const std::string &name, R (X::*m) (_FUNCARGLIST) const, Callback X::*cb _COMMA _ARGSPECS, const std::string &doc = std::string ()) { - return Methods ((new _NAME(ConstMethod) (name, m, doc, cb))->add_args (_ARGSPECARGS)); + return Methods ((new _NAME(ConstMethod) (name, m, doc, cb))->add_args (_ARGSPECARGS)); } #endif diff --git a/src/gsi/gsi/gsiTypes.cc b/src/gsi/gsi/gsiTypes.cc index c79711b86..237e154ac 100644 --- a/src/gsi/gsi/gsiTypes.cc +++ b/src/gsi/gsi/gsiTypes.cc @@ -112,7 +112,7 @@ ArgType::to_string () const ArgType::ArgType () : m_type (T_void), mp_spec (0), mp_inner (0), mp_inner_k (0), m_is_ref (false), m_is_ptr (false), m_is_cref (false), m_is_cptr (false), m_is_iter (false), - m_owns_spec (false), m_pass_obj (false), + m_owns_spec (false), m_pass_obj (false), m_prefer_copy (false), mp_cls (0), m_size (0) { } @@ -132,7 +132,7 @@ ArgType::~ArgType () ArgType::ArgType (const ArgType &other) : m_type (T_void), mp_spec (0), mp_inner (0), mp_inner_k (0), m_is_ref (false), m_is_ptr (false), m_is_cref (false), m_is_cptr (false), m_is_iter (false), - m_owns_spec (false), m_pass_obj (false), + m_owns_spec (false), m_pass_obj (false), m_prefer_copy (false), mp_cls (0), m_size (0) { operator= (other); @@ -156,6 +156,7 @@ ArgType::operator= (const ArgType &other) m_type = other.m_type; m_pass_obj = other.m_pass_obj; + m_prefer_copy = other.m_prefer_copy; m_is_ref = other.m_is_ref; m_is_cref = other.m_is_cref; m_is_ptr = other.m_is_ptr; @@ -204,7 +205,7 @@ ArgType::operator== (const ArgType &b) const } return m_type == b.m_type && m_is_iter == b.m_is_iter && m_is_ref == b.m_is_ref && m_is_cref == b.m_is_cref && m_is_ptr == b.m_is_ptr && m_is_cptr == b.m_is_cptr && - mp_cls == b.mp_cls && m_pass_obj == b.m_pass_obj; + mp_cls == b.mp_cls && m_pass_obj == b.m_pass_obj && m_prefer_copy == b.m_prefer_copy; } void diff --git a/src/gsi/gsi/gsiTypes.h b/src/gsi/gsi/gsiTypes.h index 3347772be..4f4065285 100644 --- a/src/gsi/gsi/gsiTypes.h +++ b/src/gsi/gsi/gsiTypes.h @@ -1385,6 +1385,67 @@ public: } }; +/** + * @brief A tag indicating ownership transfer + * This tag will transfer ownership of an object (either back- or forward). + */ +struct arg_pass_ownership { }; + +/** + * @brief A tag indicating copy preference + * By specifying this tag, making a copy of the object is the preferred transfer method + */ +struct arg_make_copy { }; + +/** + * @brief A tag indicating to take the reference of an object + * By specifying this tag, taking a reference is the preferred transfer method + */ +struct arg_make_reference { }; + +/** + * @brief A tag indicating the default return value preference + * The default return value preference is "copy" for const references and direct values + * and reference otherwise. + */ +struct arg_default_return_value_preference { }; + +/** + * @brief A function computing the "prefer_copy" value + */ +template +struct compute_prefer_copy +{ + static bool value () { return false; } +}; + +template +struct compute_prefer_copy +{ + static bool value () { return type_traits::is_cref (); } +}; + +template +struct compute_prefer_copy +{ + static bool value () { return true; } +}; + +/** + * @brief A function computing the "pass_obj" value + */ +template +struct compute_pass_obj +{ + static bool value () { return false; } +}; + +template +struct compute_pass_obj +{ + static bool value () { return true; } +}; + /** * @brief Generic argument type declaration * @@ -1427,7 +1488,22 @@ public: template void init (const ArgSpecBase &spec) { - init (); + init (); + mp_spec = &spec; + m_owns_spec = false; + } + + /** + * @brief Initializes with an external specification + * + * This method will initialize the type object referring to an external + * spec object. The spec object will specify name and default + * value for arguments + */ + template + void init (const ArgSpecBase &spec) + { + init (); mp_spec = &spec; m_owns_spec = false; } @@ -1442,7 +1518,22 @@ public: template void init (ArgSpecBase *spec) { - init (); + init (); + mp_spec = spec; + m_owns_spec = true; + } + + /** + * @brief Initializes with a specification + * + * This method will initialize the type object with a specification. + * The ownership over the specification object will be transferred to + * the type object. + */ + template + void init (ArgSpecBase *spec) + { + init (); mp_spec = spec; m_owns_spec = true; } @@ -1454,7 +1545,7 @@ public: * to the case of object pointers mainly. */ template - void init (bool pass_obj = false) + void init () { release_spec (); @@ -1462,7 +1553,8 @@ public: m_is_iter = type_traits::is_iter (); mp_cls = type_traits::cls_decl (); - m_pass_obj = pass_obj; + m_pass_obj = compute_pass_obj::value (); + m_prefer_copy = compute_prefer_copy::value (); m_is_ref = type_traits::is_ref (); m_is_ptr = type_traits::is_ptr (); m_is_cref = type_traits::is_cref (); @@ -1481,12 +1573,56 @@ public: if (type_traits::inner_type>::code () != T_void) { mp_inner = new ArgType; - mp_inner->init::inner_type> (); + mp_inner->init::inner_type, arg_make_reference> (); } if (type_traits::inner_k_type>::code () != T_void) { mp_inner_k = new ArgType; - mp_inner_k->init::inner_k_type> (); + mp_inner_k->init::inner_k_type, arg_make_reference> (); + } + } + + /** + * @brief Initialize the type from a given type X + * If "pass_obj" is true, the receiver of an object will always + * take over the ownership over the passed object. This applies + * to the case of object pointers mainly. + */ + template + void init () + { + release_spec (); + + m_type = type_traits::code (); + m_is_iter = type_traits::is_iter (); + mp_cls = type_traits::cls_decl (); + + m_pass_obj = compute_pass_obj::value (); + m_prefer_copy = compute_prefer_copy::value (); + m_is_ref = type_traits::is_ref (); + m_is_ptr = type_traits::is_ptr (); + m_is_cref = type_traits::is_cref (); + m_is_cptr = type_traits::is_cptr (); + m_size = (unsigned int) type_traits::serial_size (); + + if (mp_inner) { + delete mp_inner; + mp_inner = 0; + } + + if (mp_inner_k) { + delete mp_inner_k; + mp_inner_k = 0; + } + + if (type_traits::inner_type>::code () != T_void) { + mp_inner = new ArgType; + mp_inner->init::inner_type, arg_make_reference> (); + } + + if (type_traits::inner_k_type>::code () != T_void) { + mp_inner_k = new ArgType; + mp_inner_k->init::inner_k_type, arg_make_reference> (); } } @@ -1571,6 +1707,22 @@ public: m_pass_obj = b; } + /** + * @brief Returns a value indicating that the value prefers to be copied + */ + bool prefer_copy () const + { + return m_prefer_copy; + } + + /** + * @brief Sets a value indicating that the value prefers to be copied + */ + void set_prefer_copy (bool b) + { + m_prefer_copy = b; + } + /** * @brief Returns a value indicating whether the type is a reference */ @@ -1702,6 +1854,7 @@ private: bool m_is_iter : 1; bool m_owns_spec : 1; bool m_pass_obj : 1; + bool m_prefer_copy : 1; mutable const ClassBase *mp_cls; unsigned int m_size; diff --git a/src/gsi/gsi_test/gsiTest.cc b/src/gsi/gsi_test/gsiTest.cc index 4750e7ad3..d115a54a7 100644 --- a/src/gsi/gsi_test/gsiTest.cc +++ b/src/gsi/gsi_test/gsiTest.cc @@ -1057,9 +1057,34 @@ static gsi::ClassExt b_ext ( gsi::iterator_ext ("b10p", &b10bp_ext, &b10ep_ext) ); +CopyDetector *new_cd (int x) +{ + return new CopyDetector (x); +} + +static gsi::Class decl_copy_detector ("", "CopyDetector", + gsi::constructor ("new", &new_cd) + + gsi::method ("x", &CopyDetector::x) + + gsi::method ("xx", &CopyDetector::xx) +); static gsi::Class decl_c ("", "C", gsi::callback ("f", &C_P::f, &C_P::f_cb) + + gsi::callback ("vfunc", &C_P::vfunc, &C_P::vfunc_cb) + + gsi::method ("call_vfunc", &C_P::call_vfunc) + + gsi::method ("pass_cd_direct", &C_P::pass_cd_direct) + + gsi::method ("pass_cd_cref", &C_P::pass_cd_cref) + + gsi::method ("pass_cd_cref_as_copy", &C_P::pass_cd_cref) + + gsi::method ("pass_cd_cref_as_ref", &C_P::pass_cd_cref) + + gsi::method ("pass_cd_cptr", &C_P::pass_cd_cptr) + + gsi::method ("pass_cd_cptr_as_copy", &C_P::pass_cd_cptr) + + gsi::method ("pass_cd_cptr_as_ref", &C_P::pass_cd_cptr) + + gsi::method ("pass_cd_ref", &C_P::pass_cd_ref) + + gsi::method ("pass_cd_ref_as_copy", &C_P::pass_cd_ref) + + gsi::method ("pass_cd_ref_as_ref", &C_P::pass_cd_ref) + + gsi::method ("pass_cd_ptr", &C_P::pass_cd_ptr) + + gsi::method ("pass_cd_ptr_as_copy", &C_P::pass_cd_ptr) + + gsi::method ("pass_cd_ptr_as_ref", &C_P::pass_cd_ptr) + gsi::method ("g", &C_P::g) + gsi::method ("s1", &C::s1) + gsi::method ("s2", &C::s2) + diff --git a/src/gsi/gsi_test/gsiTest.h b/src/gsi/gsi_test/gsiTest.h index 7992a42b3..a7e6938f6 100644 --- a/src/gsi/gsi_test/gsiTest.h +++ b/src/gsi/gsi_test/gsiTest.h @@ -846,6 +846,35 @@ struct B static B *b_inst; }; +class CopyDetector +{ +public: + CopyDetector (int x) + : m_x (x), m_xx (x) + { } + + CopyDetector () + : m_x (0), m_xx (0) + { } + + CopyDetector (const CopyDetector &d) + : m_x (d.m_x), m_xx (d.m_xx + 1) // this detects the copy + { } + + CopyDetector &operator= (const CopyDetector &d) + { + m_x = d.m_x; + m_xx = d.m_xx + 1; + return *this; + } + + int x () const { return m_x; } + int xx () const { return m_xx; } + +private: + int m_x, m_xx; +}; + class C { public: @@ -861,6 +890,22 @@ public: return f(s); } + virtual void vfunc (const CopyDetector &) + { + // .. nothing yet .. + } + + void call_vfunc (const CopyDetector &cd) + { + vfunc (cd); + } + + CopyDetector pass_cd_direct (const CopyDetector &cd) { return cd; } + const CopyDetector &pass_cd_cref (const CopyDetector &cd) { return cd; } + const CopyDetector *pass_cd_cptr (const CopyDetector &cd) { return &cd; } + CopyDetector *pass_cd_ptr (const CopyDetector &cd) { return const_cast (&cd); } + CopyDetector &pass_cd_ref (const CopyDetector &cd) { return const_cast (cd); } + static int s1 (); static std::vector::const_iterator s1a (); static std::vector::const_iterator s1b (); @@ -880,7 +925,13 @@ public: return f_cb.can_issue () ? f_cb.issue (&C::f, s) : C::f (s); } + virtual void vfunc (const CopyDetector &cd) + { + return vfunc_cb.can_issue () ? vfunc_cb.issue (&C::vfunc, cd) : C::vfunc (cd); + } + gsi::Callback f_cb; + gsi::Callback vfunc_cb; }; struct E diff --git a/src/pya/pya/pyaConvert.cc b/src/pya/pya/pyaConvert.cc index 4ceff8ecd..57918f95b 100644 --- a/src/pya/pya/pyaConvert.cc +++ b/src/pya/pya/pyaConvert.cc @@ -345,7 +345,7 @@ object_to_python (void *obj, PYAObjectBase *self, const gsi::ArgType &atype) bool is_direct = !(atype.is_ptr () || atype.is_ref () || atype.is_cptr () || atype.is_cref ()); bool pass_obj = atype.pass_obj () || is_direct; bool is_const = atype.is_cptr () || atype.is_cref (); - bool prefer_copy = atype.is_cref (); + bool prefer_copy = atype.prefer_copy (); bool can_destroy = prefer_copy || atype.is_ptr (); return object_to_python (obj, self, cls, pass_obj, is_const, prefer_copy, can_destroy); diff --git a/src/rba/rba/rbaConvert.cc b/src/rba/rba/rbaConvert.cc index 1210624c5..e54bfb73b 100644 --- a/src/rba/rba/rbaConvert.cc +++ b/src/rba/rba/rbaConvert.cc @@ -133,7 +133,7 @@ VALUE object_to_ruby (void *obj, Proxy *self, const gsi::ArgType &atype) bool is_direct = !(atype.is_ptr () || atype.is_ref () || atype.is_cptr () || atype.is_cref ()); bool pass_obj = atype.pass_obj () || is_direct; bool is_const = atype.is_cptr () || atype.is_cref (); - bool prefer_copy = atype.is_cref (); + bool prefer_copy = atype.prefer_copy (); bool can_destroy = prefer_copy || atype.is_ptr (); return object_to_ruby (obj, self, cls, pass_obj, is_const, prefer_copy, can_destroy); diff --git a/testdata/python/basic.py b/testdata/python/basic.py index 568a9ee16..ec7f58ef0 100644 --- a/testdata/python/basic.py +++ b/testdata/python/basic.py @@ -73,6 +73,14 @@ class C_IMP2(pya.C): class C_IMP3(pya.C): anything = None +class C_IMP4(pya.C): + def __init__(self): + self.x = None + self.xx = None + def vfunc(self, cd): + self.x = cd.x() + self.xx = cd.xx() + class Z_IMP1(pya.Z): def f(self, x): return x.cls_name() @@ -1072,6 +1080,11 @@ class BasicTest(unittest.TestCase): arr.append(i) self.assertEqual( arr, [ 0, 1, 2, 3, 4, 5, 6, 0, 0, 1 ] ) + c4 = C_IMP4() + c4.call_vfunc(pya.CopyDetector(17)) + self.assertEqual(c4.x, 17) + self.assertEqual(c4.xx, 17) + def test_20(self): b = pya.B() @@ -1538,6 +1551,67 @@ class BasicTest(unittest.TestCase): b = None self.assertEqual(pya.B.has_inst(), False) + def test_29(self): + + # copy/ref semantics on return + + c = pya.C() + + cd = pya.CopyDetector(42) + + cd2 = c.pass_cd_direct(cd) + self.assertEqual(cd2.x(), 42) + # two copies: one for return statement and then one for the new object + self.assertEqual(cd2.xx(), 44) + + cd2 = c.pass_cd_cref(cd) + self.assertEqual(cd2.x(), 42) + self.assertEqual(cd2.xx(), 43) + + cd2 = c.pass_cd_cref_as_copy(cd) + self.assertEqual(cd2.x(), 42) + self.assertEqual(cd2.xx(), 43) + + cd2 = c.pass_cd_cref_as_ref(cd) + self.assertEqual(cd2.x(), 42) + self.assertEqual(cd2.xx(), 42) + + cd2 = c.pass_cd_cptr(cd) + self.assertEqual(cd2.x(), 42) + self.assertEqual(cd2.xx(), 42) + + cd2 = c.pass_cd_cptr_as_copy(cd) + self.assertEqual(cd2.x(), 42) + self.assertEqual(cd2.xx(), 43) + + cd2 = c.pass_cd_cptr_as_ref(cd) + self.assertEqual(cd2.x(), 42) + self.assertEqual(cd2.xx(), 42) + + cd2 = c.pass_cd_ptr(cd) + self.assertEqual(cd2.x(), 42) + self.assertEqual(cd2.xx(), 42) + + cd2 = c.pass_cd_ptr_as_copy(cd) + self.assertEqual(cd2.x(), 42) + self.assertEqual(cd2.xx(), 43) + + cd2 = c.pass_cd_ptr_as_ref(cd) + self.assertEqual(cd2.x(), 42) + self.assertEqual(cd2.xx(), 42) + + cd2 = c.pass_cd_ref(cd) + self.assertEqual(cd2.x(), 42) + self.assertEqual(cd2.xx(), 42) + + cd2 = c.pass_cd_ref_as_copy(cd) + self.assertEqual(cd2.x(), 42) + self.assertEqual(cd2.xx(), 43) + + cd2 = c.pass_cd_ref_as_ref(cd) + self.assertEqual(cd2.x(), 42) + self.assertEqual(cd2.xx(), 42) + def test_30(self): # some basic tests for the *Value boxing classes diff --git a/testdata/ruby/basic_testcore.rb b/testdata/ruby/basic_testcore.rb index 467ceaccf..8d9cd8493 100644 --- a/testdata/ruby/basic_testcore.rb +++ b/testdata/ruby/basic_testcore.rb @@ -936,6 +936,22 @@ class Basic_TestClass < TestBase class C_IMP3 < RBA::C end + class C_IMP4 < RBA::C + def initialize + @x = @xx = nil + end + def x + @x + end + def xx + @xx + end + def vfunc(cd) + @x = cd.x + @xx = cd.xx + end + end + def test_19 c0 = RBA::C.new @@ -978,6 +994,11 @@ class Basic_TestClass < TestBase C_IMP2.each { |i| arr.push i } assert_equal( arr, [ 0, 1, 2, 3, 4, 5, 6, 0, 0, 1 ] ) + c4 = C_IMP4.new + c4.call_vfunc(RBA::CopyDetector::new(17)) + assert_equal(c4.x, 17) + assert_equal(c4.xx, 17) + end def test_20 @@ -1414,6 +1435,69 @@ class Basic_TestClass < TestBase end + def test_29 + + # copy/ref semantics on return + + c = RBA::C::new + + cd = RBA::CopyDetector::new(42) + + cd2 = c.pass_cd_direct(cd) + assert_equal(cd2.x, 42) + # two copies: one for return statement and then one for the new object + assert_equal(cd2.xx, 44) + + cd2 = c.pass_cd_cref(cd) + assert_equal(cd2.x, 42) + assert_equal(cd2.xx, 43) + + cd2 = c.pass_cd_cref_as_copy(cd) + assert_equal(cd2.x, 42) + assert_equal(cd2.xx, 43) + + cd2 = c.pass_cd_cref_as_ref(cd) + assert_equal(cd2.x, 42) + assert_equal(cd2.xx, 42) + + cd2 = c.pass_cd_cptr(cd) + assert_equal(cd2.x, 42) + assert_equal(cd2.xx, 42) + + cd2 = c.pass_cd_cptr_as_copy(cd) + assert_equal(cd2.x, 42) + assert_equal(cd2.xx, 43) + + cd2 = c.pass_cd_cptr_as_ref(cd) + assert_equal(cd2.x, 42) + assert_equal(cd2.xx, 42) + + cd2 = c.pass_cd_ptr(cd) + assert_equal(cd2.x, 42) + assert_equal(cd2.xx, 42) + + cd2 = c.pass_cd_ptr_as_copy(cd) + assert_equal(cd2.x, 42) + assert_equal(cd2.xx, 43) + + cd2 = c.pass_cd_ptr_as_ref(cd) + assert_equal(cd2.x, 42) + assert_equal(cd2.xx, 42) + + cd2 = c.pass_cd_ref(cd) + assert_equal(cd2.x, 42) + assert_equal(cd2.xx, 42) + + cd2 = c.pass_cd_ref_as_copy(cd) + assert_equal(cd2.x, 42) + assert_equal(cd2.xx, 43) + + cd2 = c.pass_cd_ref_as_ref(cd) + assert_equal(cd2.x, 42) + assert_equal(cd2.xx, 42) + + end + def test_30 # some basic tests for the *Value boxing classes