Fixed an issue with wrapping new objects into tl::Variants which are returned directly. For these objects, ownership needs to be transferred to the script.

This commit is contained in:
Matthias Koefferlein 2020-12-27 17:09:06 +01:00
parent 992be87812
commit 4ec00fb129
10 changed files with 222 additions and 108 deletions

View File

@ -223,11 +223,11 @@ static db::CompoundRegionOperationNode *new_edge_pair_to_second_edges_node (db::
return new db::CompoundRegionEdgePairToEdgeProcessingOperationNode (new db::EdgePairToSecondEdgesProcessor (), input, true /*processor is owned*/);
}
static db::CompoundRegionOperationNode *new_check_node (db::edge_relation_type rel, bool different_polygons, db::Coord d, bool whole_edges, const tl::Variant &metrics, const tl::Variant &ignore_angle, const tl::Variant &min_projection, const tl::Variant &max_projection, bool shielded, db::OppositeFilter opposite_filter, db::RectFilter rect_filter)
static db::CompoundRegionOperationNode *new_check_node (db::edge_relation_type rel, bool different_polygons, db::Coord d, bool whole_edges, db::metrics_type metrics, const tl::Variant &ignore_angle, const tl::Variant &min_projection, const tl::Variant &max_projection, bool shielded, db::OppositeFilter opposite_filter, db::RectFilter rect_filter)
{
return new db::CompoundRegionCheckOperationNode (rel, different_polygons, d,
db::RegionCheckOptions (whole_edges,
metrics.is_nil () ? db::Euclidian : db::metrics_type (metrics.to_int ()),
metrics,
ignore_angle.is_nil () ? 90 : ignore_angle.to_double (),
min_projection.is_nil () ? db::Region::distance_type (0) : min_projection.to<db::Region::distance_type> (),
max_projection.is_nil () ? std::numeric_limits<db::Region::distance_type>::max () : max_projection.to<db::Region::distance_type> (),

View File

@ -1106,8 +1106,8 @@ public:
// Variant adaptor framework
/**
* @brief A generic adaptor for strings
* This is the base class for implementing generic access to strings
* @brief A generic adaptor for variants
* This is the base class for implementing generic access to variants
*/
class GSI_PUBLIC VariantAdaptor
: public AdaptorBase
@ -1145,7 +1145,7 @@ public:
};
/**
* @brief Generic string adaptor implementation
* @brief Generic variant adaptor implementation
*/
template <class X>
class GSI_PUBLIC_TEMPLATE VariantAdaptorImpl
@ -1274,7 +1274,12 @@ public:
return *mp_v;
}
virtual void set (const tl::Variant &v)
tl::Variant &var_ref_nc ()
{
return *mp_v;
}
virtual void set (const tl::Variant &v)
{
if (! m_is_const) {
*mp_v = v;

View File

@ -107,6 +107,11 @@ const char *A::a_static ()
return "static_a";
}
tl::Variant A::new_a_by_variant ()
{
return tl::Variant (A ());
}
static A *a_ctor (int i)
{
return new A (i);
@ -128,9 +133,11 @@ A *A::a20_get ()
// Implementation of B
B *B::b_inst = 0;
static int b_count = 0;
B::B ()
{
++b_count;
av.push_back (A (100));
av.push_back (A (121));
av.push_back (A (144));
@ -142,7 +149,7 @@ B::B ()
av_nc.push_back (new A_NC (7169));
}
B::~B()
B::~B ()
{
while (! av_nc.empty ()) {
delete av_nc.back ();
@ -155,14 +162,21 @@ B::~B()
if (b_inst == this) {
b_inst = 0;
}
--b_count;
}
B::B (const B &d)
{
operator=(d);
++b_count;
}
B &B::operator=(const B &d)
int B::instance_count ()
{
return b_count;
}
B &B::operator=(const B &d)
{
if (&d == this) {
return *this;
@ -213,6 +227,11 @@ bool B::has_inst ()
return b_inst != 0;
}
tl::Variant B::new_b_by_variant ()
{
return tl::Variant (B ());
}
std::string B::addr () const
{
char c[50];
@ -790,6 +809,7 @@ static gsi::QFlagsClass<Enum> decl_qflags_enum ("", "Enums");
static gsi::Class<A> decl_a ("", "A",
gsi::constructor ("new_a|new", &a_ctor) +
gsi::method ("instance_count", &A::instance_count) +
gsi::method ("new_a_by_variant", &A::new_a_by_variant) +
gsi::method ("br", &A::br) +
gsi::method ("get_e", &A::get_e) +
gsi::method ("get_eptr", &A::get_eptr) +
@ -918,17 +938,16 @@ static gsi::Class<B> decl_b ("", "B",
gsi::method ("has_inst", &B::has_inst) +
gsi::method ("set_inst", &B::set_inst) +
gsi::method ("del_inst", &B::del_inst) +
gsi::method ("instance_count", &B::instance_count) +
gsi::method ("new_b_by_variant", &B::new_b_by_variant) +
gsi::method ("addr", &B::addr) +
gsi::method ("b1|always_5", &B::b1) +
gsi::method ("b2|str", &B::str) +
gsi::method ("always_5", &B::always_5) +
gsi::method ("str", &B::str) +
gsi::method ("set_str", &B::set_str) +
gsi::method ("str_ccptr", &B::str_ccptr) +
gsi::method ("set_str_combine", &B::set_str_combine) +
gsi::method_ext ("b3|aptr_to_n", &b3_ext) +
gsi::method ("b4|aref_to_s", &B::b4) +
gsi::method ("b5", &B::b5) +
gsi::method ("b5a", &B::b5a) +
gsi::method ("b5b", &B::b5b) +
gsi::method ("make_a", &B::make_a) +
gsi::method ("set_an", &B::set_an) +
gsi::method ("an", &B::an) +

View File

@ -93,6 +93,11 @@ struct A
*/
static const char *a_static ();
/**
* @brief Construction through tl::Variant
*/
static tl::Variant new_a_by_variant ();
/*
* @brief A dummy method providing a chance to set a breakpoint in the script
*/
@ -445,9 +450,16 @@ struct B
static B *inst ();
static bool has_inst ();
static int instance_count ();
/**
* @brief Construction through tl::Variant
*/
static tl::Variant new_b_by_variant ();
std::string addr () const;
int b1 () const {
int always_5 () const {
return 5;
}
@ -469,19 +481,6 @@ struct B
return tl::sprintf ("b4_result: %d", aref.n);
}
void b5 (const char *p) {
m = p;
}
void b5b (const char *p1, const char *p2) {
m = p1;
m += p2;
}
const char *b5a () const {
return m.c_str ();
}
A make_a (int n) {
return A(n);
}

View File

@ -534,11 +534,12 @@ struct reader<gsi::StringType>
};
static
PyObject *object_from_variant (const tl::Variant &var, PYAObjectBase *self, const gsi::ArgType &atype)
PyObject *object_from_variant (tl::Variant &var, PYAObjectBase *self, const gsi::ArgType &atype)
{
if (var.is_user()) {
bool pass_obj = atype.pass_obj() || (! atype.is_cptr() && ! atype.is_ptr () && ! atype.is_cref () && ! atype.is_ref ());
bool is_direct = (! atype.is_cptr() && ! atype.is_ptr () && ! atype.is_cref () && ! atype.is_ref ());
bool pass_obj = atype.pass_obj() || is_direct;
bool is_const = atype.is_cptr() || atype.is_cref();
bool prefer_copy = false;
bool can_destroy = false;
@ -546,6 +547,9 @@ PyObject *object_from_variant (const tl::Variant &var, PYAObjectBase *self, cons
// TODO: ugly const_cast, but there is no "const shared reference" ...
gsi::Proxy *holder = dynamic_cast<gsi::Proxy *>(const_cast<tl::Object *>(var.to_object ()));
void *obj = var.to_user ();
const gsi::ClassBase *cls = var.user_cls ()->gsi_cls ();
if (pass_obj) {
if (holder) {
@ -567,19 +571,20 @@ PyObject *object_from_variant (const tl::Variant &var, PYAObjectBase *self, cons
// If the object was not owned before, it is not owned after (bears risk of invalid
// pointers, but it's probably rarely the case. Non-managed objects are usually copied
// between the ownership spaces.
if (var.user_is_ref()) {
// If the variant holds the user object, we can take it from it and claim ownership.
if (var.user_is_ref ()) {
prefer_copy = false; // unsafe
pass_obj = false;
} else {
prefer_copy = true; // safe
obj = var.user_take ();
can_destroy = true;
}
pass_obj = false;
}
}
return object_to_python ((void *) var.to_user (), self, var.user_cls ()->gsi_cls (), pass_obj, is_const, prefer_copy, can_destroy);
return object_to_python (obj, self, cls, pass_obj, is_const, prefer_copy, can_destroy);
} else {
return c2python (var);
@ -601,9 +606,10 @@ struct reader<gsi::VariantType>
gsi::VariantAdaptorImpl<tl::Variant> *aa = dynamic_cast<gsi::VariantAdaptorImpl<tl::Variant> *> (a.get ());
if (aa) {
// A small optimization that saves one variant copy
*ret = object_from_variant (aa->var_ref (), self, atype);
*ret = object_from_variant (aa->var_ref_nc (), self, atype);
} else {
*ret = object_from_variant (a->var (), self, atype);
tl::Variant v = a->var ();
*ret = object_from_variant (v, self, atype);
}
}
}

View File

@ -556,11 +556,12 @@ struct reader<gsi::StringType>
}
};
static VALUE object_from_variant (const tl::Variant &var, Proxy *self, const gsi::ArgType &atype)
static VALUE object_from_variant (tl::Variant &var, Proxy *self, const gsi::ArgType &atype)
{
if (var.is_user()) {
bool pass_obj = atype.pass_obj() || (! atype.is_cptr() && ! atype.is_ptr () && ! atype.is_cref () && ! atype.is_ref ());
bool is_direct = (! atype.is_cptr() && ! atype.is_ptr () && ! atype.is_cref () && ! atype.is_ref ());
bool pass_obj = atype.pass_obj() || is_direct;
bool is_const = atype.is_cptr() || atype.is_cref();
bool prefer_copy = false;
bool can_destroy = false;
@ -568,6 +569,9 @@ static VALUE object_from_variant (const tl::Variant &var, Proxy *self, const gsi
// TODO: ugly const_cast, but there is no "const shared reference" ...
gsi::Proxy *holder = dynamic_cast<gsi::Proxy *>(const_cast<tl::Object *>(var.to_object ()));
void *obj = var.to_user ();
const gsi::ClassBase *cls = var.user_cls ()->gsi_cls ();
if (pass_obj) {
if (holder) {
@ -589,19 +593,20 @@ static VALUE object_from_variant (const tl::Variant &var, Proxy *self, const gsi
// If the object was not owned before, it is not owned after (bears risk of invalid
// pointers, but it's probably rarely the case. Non-managed objects are usually copied
// between the ownership spaces.
if (var.user_is_ref()) {
// If the variant holds the user object, we can take it from it and claim ownership.
if (var.user_is_ref ()) {
prefer_copy = false; // unsafe
pass_obj = false;
} else {
prefer_copy = true; // safe
obj = var.user_take ();
can_destroy = true;
}
pass_obj = false;
}
}
return object_to_ruby ((void *) var.to_user (), self, var.user_cls ()->gsi_cls (), pass_obj, is_const, prefer_copy, can_destroy);
return object_to_ruby (obj, self, cls, pass_obj, is_const, prefer_copy, can_destroy);
} else {
return c2ruby<tl::Variant> (var);
@ -623,9 +628,10 @@ struct reader<gsi::VariantType>
gsi::VariantAdaptorImpl<tl::Variant> *aa = dynamic_cast<gsi::VariantAdaptorImpl<tl::Variant> *> (a.get ());
if (aa) {
// A small optimization that saves one variant copy
*ret = object_from_variant (aa->var_ref (), self, atype);
*ret = object_from_variant (aa->var_ref_nc (), self, atype);
} else {
*ret = object_from_variant (a->var (), self, atype);
tl::Variant v = a->var ();
*ret = object_from_variant (v, self, atype);
}
}
}

View File

@ -2460,8 +2460,18 @@ void Variant::user_destroy ()
void *obj = to_user ();
if (obj) {
user_cls ()->destroy (obj);
m_type = t_nil;
}
reset ();
}
void *Variant::user_take ()
{
tl_assert (is_user ());
void *obj = to_user ();
if (obj) {
m_type = t_nil;
}
return obj;
}
void Variant::user_assign (const tl::Variant &other)

View File

@ -861,6 +861,11 @@ public:
*/
void user_destroy ();
/**
* @brief Takes the user object and releases it from the variant
*/
void *user_take ();
/**
* @brief Assigns the object stored in other to self
*

View File

@ -393,6 +393,34 @@ class BasicTest(unittest.TestCase):
a.mod_efptr( ee, pya.Enum.a )
self.assertEqual( str(ee), "a|b" )
def test_10(self):
# all references of A are released now:
ic0 = pya.A.instance_count()
self.assertEqual(ic0, 0)
a = pya.A.new_a_by_variant()
self.assertEqual(pya.A.instance_count(), ic0 + 1)
self.assertEqual(a.a1(), 17)
a.a5(-15)
self.assertEqual(a.a1(), -15)
a = None
self.assertEqual(pya.A.instance_count(), ic0)
ic0 = pya.B.instance_count()
self.assertEqual(ic0, 0)
b = pya.B.new_b_by_variant()
self.assertEqual(pya.B.instance_count(), ic0 + 1)
b.set_str_combine("x", "y")
self.assertEqual(b.str(), "xy")
b._destroy()
self.assertEqual(pya.B.instance_count(), ic0)
def test_12(self):
a1 = pya.A()
@ -629,14 +657,14 @@ class BasicTest(unittest.TestCase):
b.amember_cptr().a2()
self.assertEqual( b.b1(), 5 )
self.assertEqual( b.b2(), "" )
b.b5( "xyz" )
self.assertEqual( b.b2(), "xyz" )
self.assertEqual( b.b5a(), "xyz" )
b.b5b( "yx", "zz" )
self.assertEqual( b.b2(), "yxzz" )
self.assertEqual( b.b5a(), "yxzz" )
self.assertEqual(b.always_5(), 5)
self.assertEqual(b.str(), "")
b.set_str("xyz")
self.assertEqual(b.str(), "xyz")
self.assertEqual(b.str_ccptr(), "xyz")
b.set_str_combine("yx", "zz")
self.assertEqual(b.str(), "yxzz")
self.assertEqual(b.str_ccptr(), "yxzz")
arr = []
@ -845,53 +873,53 @@ class BasicTest(unittest.TestCase):
b = pya.B()
bb = pya.B()
bb.b5("a")
bb.set_str("a")
b.push_b(bb)
bb = pya.B()
bb.b5("y")
bb.set_str("y")
b.push_b(bb)
bb = pya.B()
bb.b5("uu")
bb.set_str("uu")
b.push_b(bb)
arr = []
for bb in b.each_b_copy():
arr.append(bb.b2())
arr.append(bb.str())
self.assertEqual(arr, ["a", "y", "uu"])
arr = []
for bb in b.each_b_copy():
bb.b5(bb.b2() + "x")
arr.append(bb.b2())
bb.set_str(bb.str() + "x")
arr.append(bb.str())
self.assertEqual(arr, ["ax", "yx", "uux"])
arr = []
for bb in b.each_b_copy():
arr.append(bb.b2())
arr.append(bb.str())
self.assertEqual(arr, ["a", "y", "uu"])
arr = []
for bb in b.each_b_cref():
arr.append(bb.b2())
arr.append(bb.str())
self.assertEqual(arr, ["a", "y", "uu"])
arr = []
# this works, since the "const B &" will be converted to a copy
for bb in b.each_b_cref():
bb.b5(bb.b2() + "x")
arr.append(bb.b2())
bb.set_str(bb.str() + "x")
arr.append(bb.str())
self.assertEqual(arr, ["ax", "yx", "uux"])
arr = []
for bb in b.each_b_cref():
arr.append(bb.b2())
arr.append(bb.str())
self.assertEqual(arr, ["a", "y", "uu"])
arr = []
for bb in b.each_b_cptr():
arr.append(bb.b2())
arr.append(bb.str())
self.assertEqual(arr, ["a", "y", "uu"])
arr = []
@ -899,8 +927,8 @@ class BasicTest(unittest.TestCase):
err_caught = False
try:
for bb in b.each_b_cptr():
bb.b5(bb.b2() + "x")
arr.append(bb.b2())
bb.set_str(bb.str() + "x")
arr.append(bb.str())
except:
err_caught = True
self.assertEqual(err_caught, True)
@ -908,39 +936,39 @@ class BasicTest(unittest.TestCase):
arr = []
for bb in b.each_b_cptr():
arr.append(bb.b2())
arr.append(bb.str())
self.assertEqual(arr, ["a", "y", "uu"])
arr = []
for bb in b.each_b_ref():
arr.append(bb.b2())
arr.append(bb.str())
self.assertEqual(arr, ["a", "y", "uu"])
arr = []
for bb in b.each_b_ref():
bb.b5(bb.b2() + "x")
arr.append(bb.b2())
bb.set_str(bb.str() + "x")
arr.append(bb.str())
self.assertEqual(arr, ["ax", "yx", "uux"])
arr = []
for bb in b.each_b_ref():
arr.append(bb.b2())
arr.append(bb.str())
self.assertEqual(arr, ["ax", "yx", "uux"])
arr = []
for bb in b.each_b_ptr():
arr.append(bb.b2())
arr.append(bb.str())
self.assertEqual(arr, ["ax", "yx", "uux"])
arr = []
for bb in b.each_b_ptr():
bb.b5(bb.b2() + "x")
arr.append(bb.b2())
bb.set_str(bb.str() + "x")
arr.append(bb.str())
self.assertEqual(arr, ["axx", "yxx", "uuxx"])
arr = []
for bb in b.each_b_ptr():
arr.append(bb.b2())
arr.append(bb.str())
self.assertEqual(arr, ["axx", "yxx", "uuxx"])
def test_14(self):

View File

@ -290,6 +290,42 @@ class Basic_TestClass < TestBase
end
def test_10
GC.start
# all references of A are released now:
ic0 = RBA::A::instance_count
assert_equal(ic0, 0)
a = RBA::A.new_a_by_variant
assert_equal(RBA::A::instance_count, ic0 + 1)
assert_equal(a.a1, 17)
a.a5(-15)
assert_equal(a.a1, -15)
a = nil
GC.start
assert_equal(RBA::A::instance_count, ic0)
# destruction of raw instances (non-gsi-enabled) via c++
GC.start
ic0 = RBA::B::instance_count
assert_equal(ic0, 0)
b = RBA::B.new_b_by_variant
assert_equal(RBA::B::instance_count, ic0 + 1)
b.set_str_combine("x", "y")
assert_equal(b.str, "xy")
b._destroy
assert_equal(RBA::B::instance_count, ic0)
end
def test_12
a1 = RBA::A.new
@ -537,14 +573,14 @@ class Basic_TestClass < TestBase
b.amember_cptr.a2
assert_equal( b.b1, 5 )
assert_equal( b.b2, "" )
b.b5( "xyz" )
assert_equal( b.b2, "xyz" )
assert_equal( b.b5a, "xyz" )
b.b5b( "yx", "zz" )
assert_equal( b.b2, "yxzz" )
assert_equal( b.b5a, "yxzz" )
assert_equal( b.always_5, 5 )
assert_equal( b.str, "" )
b.set_str( "xyz" )
assert_equal( b.str, "xyz" )
assert_equal( b.str_ccptr, "xyz" )
b.set_str_combine( "yx", "zz" )
assert_equal( b.str, "yxzz" )
assert_equal( b.str_ccptr, "yxzz" )
arr = []
@ -719,58 +755,58 @@ class Basic_TestClass < TestBase
b = RBA::B.new
bb = RBA::B.new
bb.b5("a")
bb.set_str("a")
b.push_b(bb)
bb = RBA::B.new
bb.b5("y")
bb.set_str("y")
b.push_b(bb)
bb = RBA::B.new
bb.b5("uu")
bb.set_str("uu")
b.push_b(bb)
arr = []
b.each_b_copy { |bb| arr.push(bb.b2) }
b.each_b_copy { |bb| arr.push(bb.str) }
assert_equal(arr, ["a", "y", "uu"])
# through enumerator
assert_equal(b.each_b_copy.collect(&:b2), ["a", "y", "uu"])
assert_equal(b.each_b_copy.collect(&:str), ["a", "y", "uu"])
arr = []
b.each_b_copy { |bb| bb.b5(bb.b2 + "x"); arr.push(bb.b2) }
b.each_b_copy { |bb| bb.set_str(bb.str + "x"); arr.push(bb.str) }
assert_equal(arr, ["ax", "yx", "uux"])
arr = []
b.each_b_copy { |bb| arr.push(bb.b2) }
b.each_b_copy { |bb| arr.push(bb.str) }
assert_equal(arr, ["a", "y", "uu"])
arr = []
b.each_b_cref { |bb| arr.push(bb.b2) }
b.each_b_cref { |bb| arr.push(bb.str) }
assert_equal(arr, ["a", "y", "uu"])
# through enumerator
assert_equal(b.each_b_cref.collect(&:b2), ["a", "y", "uu"])
assert_equal(b.each_b_cref.collect(&:str), ["a", "y", "uu"])
arr = []
# this works, since the "const B &" will be converted to a copy
b.each_b_cref { |bb| bb.b5(bb.b2 + "x"); arr.push(bb.b2) }
b.each_b_cref { |bb| bb.set_str(bb.str + "x"); arr.push(bb.str) }
assert_equal(arr, ["ax", "yx", "uux"])
arr = []
# since that was a copy, the b children were not modified
b.each_b_cref { |bb| arr.push(bb.b2) }
b.each_b_cref { |bb| arr.push(bb.str) }
assert_equal(arr, ["a", "y", "uu"])
arr = []
b.each_b_cptr { |bb| arr.push(bb.b2) }
b.each_b_cptr { |bb| arr.push(bb.str) }
assert_equal(arr, ["a", "y", "uu"])
# through enumerator
assert_equal(b.each_b_cptr.collect(&:b2), ["a", "y", "uu"])
assert_equal(b.each_b_cptr.collect(&:str), ["a", "y", "uu"])
arr = []
# const references cannot be modified
err_caught = false
begin
b.each_b_cptr { |bb| bb.b5(bb.b2 + "x"); arr.push(bb.b2) }
b.each_b_cptr { |bb| bb.set_str(bb.str + "x"); arr.push(bb.str) }
rescue
err_caught = true
end
@ -778,35 +814,35 @@ class Basic_TestClass < TestBase
assert_equal(arr, [])
arr = []
b.each_b_cptr { |bb| arr.push(bb.b2) }
b.each_b_cptr { |bb| arr.push(bb.str) }
assert_equal(arr, ["a", "y", "uu"])
arr = []
b.each_b_ref { |bb| arr.push(bb.b2) }
b.each_b_ref { |bb| arr.push(bb.str) }
assert_equal(arr, ["a", "y", "uu"])
# through enumerator
assert_equal(b.each_b_ref.collect(&:b2), ["a", "y", "uu"])
assert_equal(b.each_b_ref.collect(&:str), ["a", "y", "uu"])
arr = []
b.each_b_ref { |bb| bb.b5(bb.b2 + "x"); arr.push(bb.b2) }
b.each_b_ref { |bb| bb.set_str(bb.str + "x"); arr.push(bb.str) }
assert_equal(arr, ["ax", "yx", "uux"])
arr = []
b.each_b_ref { |bb| arr.push(bb.b2) }
b.each_b_ref { |bb| arr.push(bb.str) }
assert_equal(arr, ["ax", "yx", "uux"])
arr = []
b.each_b_ptr { |bb| arr.push(bb.b2) }
b.each_b_ptr { |bb| arr.push(bb.str) }
assert_equal(arr, ["ax", "yx", "uux"])
# through enumerator
assert_equal(b.each_b_ptr.collect(&:b2), ["ax", "yx", "uux"])
assert_equal(b.each_b_ptr.collect(&:str), ["ax", "yx", "uux"])
arr = []
b.each_b_ptr { |bb| bb.b5(bb.b2 + "x"); arr.push(bb.b2) }
b.each_b_ptr { |bb| bb.set_str(bb.str + "x"); arr.push(bb.str) }
assert_equal(arr, ["axx", "yxx", "uuxx"])
arr = []
b.each_b_ptr { |bb| arr.push(bb.b2) }
b.each_b_ptr { |bb| arr.push(bb.str) }
assert_equal(arr, ["axx", "yxx", "uuxx"])
end