From 227203cdd10eac485b959de13d17f084007e008a Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Wed, 26 Mar 2025 00:43:31 +0100 Subject: [PATCH] Providing a less strict overload resolution Downcast now has precedence over conversion constructors, hence less ambiguities Solution is implemented for - Ruby - Python - Expressions For Expressions: - The overload resolution is less evolved anyway - There was an additional bug preventing to pass arrays (hashes) in expressions --- src/gsi/gsi/gsiExpression.cc | 6 +- src/gsi/gsi/gsiVariantArgs.cc | 44 +++++++------- src/gsi/gsi/gsiVariantArgs.h | 3 +- src/gsi/unit_tests/gsiExpressionTests.cc | 36 ++++++++++++ src/pya/pya/pyaCallables.cc | 2 + src/pya/pya/pyaMarshal.cc | 2 +- src/rba/rba/rba.cc | 2 + src/rba/rba/rbaMarshal.cc | 2 +- src/rdb/rdb/gsiDeclRdb.cc | 5 ++ testdata/python/rdbTest.py | 72 +++++++++++++++++++++++ testdata/ruby/rdbTest.rb | 74 ++++++++++++++++++++++++ 11 files changed, 223 insertions(+), 25 deletions(-) diff --git a/src/gsi/gsi/gsiExpression.cc b/src/gsi/gsi/gsiExpression.cc index 4ac9fbc10..1727c2bc6 100644 --- a/src/gsi/gsi/gsiExpression.cc +++ b/src/gsi/gsi/gsiExpression.cc @@ -952,9 +952,11 @@ VariantUserClassImpl::execute_gsi (const tl::ExpressionParserContext & /*context const tl::Variant *arg = i >= int (args.size ()) ? get_kwarg (*a, kwargs) : &args[i]; if (! arg) { is_valid = a->spec ()->has_default (); - } else if (gsi::test_arg (*a, *arg, false /*strict*/)) { + } else if (gsi::test_arg (*a, *arg, false /*strict*/, false /*no object substitution*/)) { + sc += 100; + } else if (gsi::test_arg (*a, *arg, true /*loose*/, false /*no object substitution*/)) { ++sc; - } else if (test_arg (*a, *arg, true /*loose*/)) { + } else if (gsi::test_arg (*a, *arg, true /*loose*/, true /*with object substitution*/)) { // non-scoring match } else { is_valid = false; diff --git a/src/gsi/gsi/gsiVariantArgs.cc b/src/gsi/gsi/gsiVariantArgs.cc index 39dc0725e..5e1b4ffb7 100644 --- a/src/gsi/gsi/gsiVariantArgs.cc +++ b/src/gsi/gsi/gsiVariantArgs.cc @@ -46,12 +46,12 @@ inline void *get_object (tl::Variant &var) // ------------------------------------------------------------------- // Test if an argument can be converted to the given type -bool test_arg (const gsi::ArgType &atype, const tl::Variant &arg, bool loose); +bool test_arg (const gsi::ArgType &atype, const tl::Variant &arg, bool loose, bool object_substitution); template struct test_arg_func { - void operator () (bool *ret, const tl::Variant &arg, const gsi::ArgType & /*atype*/, bool /*loose*/) + void operator () (bool *ret, const tl::Variant &arg, const gsi::ArgType & /*atype*/, bool /*loose*/, bool /*object_substitution*/) { *ret = arg.can_convert_to (); } @@ -60,7 +60,16 @@ struct test_arg_func template <> struct test_arg_func { - void operator () (bool *ret, const tl::Variant & /*arg*/, const gsi::ArgType & /*atype*/, bool /*loose*/) + void operator () (bool *ret, const tl::Variant & /*arg*/, const gsi::ArgType & /*atype*/, bool /*loose*/, bool /*object_substitution*/) + { + *ret = true; + } +}; + +template <> +struct test_arg_func +{ + void operator () (bool *ret, const tl::Variant & /*arg*/, const gsi::ArgType & /*atype*/, bool /*loose*/, bool /*object_substitution*/) { *ret = true; } @@ -69,7 +78,7 @@ struct test_arg_func template <> struct test_arg_func { - void operator () (bool *ret, const tl::Variant &arg, const gsi::ArgType &atype, bool loose) + void operator () (bool *ret, const tl::Variant &arg, const gsi::ArgType &atype, bool loose, bool object_substitution) { // allow nil of pointers if ((atype.is_ptr () || atype.is_cptr ()) && arg.is_nil ()) { @@ -77,7 +86,7 @@ struct test_arg_func return; } - if (arg.is_list ()) { + if (object_substitution && arg.is_list ()) { // we may implicitly convert an array into a constructor call of a target object - // for now we only check whether the number of arguments is compatible with the array given. @@ -104,9 +113,9 @@ struct test_arg_func const tl::VariantUserClassBase *cls = arg.user_cls (); if (! cls) { *ret = false; - } else if (! cls->gsi_cls ()->is_derived_from (atype.cls ()) && (! loose || ! cls->gsi_cls ()->can_convert_to(atype.cls ()))) { - *ret = false; - } else if ((atype.is_ref () || atype.is_ptr ()) && cls->is_const ()) { + } else if (! (cls->gsi_cls () == atype.cls () || + (loose && (cls->gsi_cls ()->is_derived_from (atype.cls ()) || + (object_substitution && cls->gsi_cls ()->can_convert_to (atype.cls ())))))) { *ret = false; } else { *ret = true; @@ -117,7 +126,7 @@ struct test_arg_func template <> struct test_arg_func { - void operator () (bool *ret, const tl::Variant &arg, const gsi::ArgType &atype, bool loose) + void operator () (bool *ret, const tl::Variant &arg, const gsi::ArgType &atype, bool loose, bool /*object_substitution*/) { if (! arg.is_list ()) { *ret = false; @@ -129,7 +138,7 @@ struct test_arg_func *ret = true; for (tl::Variant::const_iterator v = arg.begin (); v != arg.end () && *ret; ++v) { - if (! test_arg (ainner, *v, loose)) { + if (! test_arg (ainner, *v, loose, true)) { *ret = false; } } @@ -139,7 +148,7 @@ struct test_arg_func template <> struct test_arg_func { - void operator () (bool *ret, const tl::Variant &arg, const gsi::ArgType &atype, bool loose) + void operator () (bool *ret, const tl::Variant &arg, const gsi::ArgType &atype, bool loose, bool /*object_substitution*/) { // Note: delegating that to the function avoids "injected class name used as template template expression" warning if (! arg.is_array ()) { @@ -152,16 +161,11 @@ struct test_arg_func const ArgType &ainner = *atype.inner (); const ArgType &ainner_k = *atype.inner_k (); - if (! arg.is_list ()) { - *ret = false; - return; - } - *ret = true; for (tl::Variant::const_array_iterator a = arg.begin_array (); a != arg.end_array () && *ret; ++a) { - if (! test_arg (ainner_k, a->first, loose)) { + if (! test_arg (ainner_k, a->first, loose, true)) { *ret = false; - } else if (! test_arg (ainner, a->second, loose)) { + } else if (! test_arg (ainner, a->second, loose, true)) { *ret = false; } } @@ -169,7 +173,7 @@ struct test_arg_func }; bool -test_arg (const gsi::ArgType &atype, const tl::Variant &arg, bool loose) +test_arg (const gsi::ArgType &atype, const tl::Variant &arg, bool loose, bool object_substitution) { // for const X * or X *, nil is an allowed value if ((atype.is_cptr () || atype.is_ptr ()) && arg.is_nil ()) { @@ -177,7 +181,7 @@ test_arg (const gsi::ArgType &atype, const tl::Variant &arg, bool loose) } bool ret = false; - gsi::do_on_type () (atype.type (), &ret, arg, atype, loose); + gsi::do_on_type () (atype.type (), &ret, arg, atype, loose, object_substitution); return ret; } diff --git a/src/gsi/gsi/gsiVariantArgs.h b/src/gsi/gsi/gsiVariantArgs.h index 75f312819..187132289 100644 --- a/src/gsi/gsi/gsiVariantArgs.h +++ b/src/gsi/gsi/gsiVariantArgs.h @@ -70,10 +70,11 @@ GSI_PUBLIC void pull_arg (gsi::SerialArgs &retlist, const gsi::ArgType &atype, t * @param atype The argument type * @param arg The value to pass to it * @param loose true for loose checking + * @param object_substitution true to substitute object arguments by lists (using constructor) or employing conversion constructors * * @return True, if the argument can be passed */ -GSI_PUBLIC bool test_arg (const gsi::ArgType &atype, const tl::Variant &arg, bool loose); +GSI_PUBLIC bool test_arg (const gsi::ArgType &atype, const tl::Variant &arg, bool loose, bool object_substitution); } diff --git a/src/gsi/unit_tests/gsiExpressionTests.cc b/src/gsi/unit_tests/gsiExpressionTests.cc index 0449850d5..105383e92 100644 --- a/src/gsi/unit_tests/gsiExpressionTests.cc +++ b/src/gsi/unit_tests/gsiExpressionTests.cc @@ -863,3 +863,39 @@ TEST(16) } } +// implicit conversions +TEST(17) +{ + tl::Eval e; + tl::Variant v; + + // smoke test + v = e.parse ("var rdb=ReportDatabase.new();" + "var cat=rdb.create_category('name');" + "var cell=rdb.create_cell('TOP');" + "var it=rdb.create_item(cell,cat);" + "var bwp=BoxWithProperties.new(Box.new(0,0,1,2), {1=>'value'});" + "it.add_value(bwp)").execute (); + + v = e.parse ("var rdb=ReportDatabase.new();" + "var cat=rdb.create_category('name');" + "var cell=rdb.create_cell('TOP');" + "var it=rdb.create_item(cell,cat);" + "var bwp=DBoxWithProperties.new(DBox.new(0,0,1,2), {1=>'value'});" + "it.add_value(bwp)").execute (); + + v = e.parse ("var rdb=ReportDatabase.new();" + "var cat=rdb.create_category('name');" + "var cell=rdb.create_cell('TOP');" + "var it=rdb.create_item(cell,cat);" + "var b=DBox.new(0,0,1,2);" + "it.add_value(b)").execute (); + + v = e.parse ("var rdb=ReportDatabase.new();" + "var cat=rdb.create_category('name');" + "var cell=rdb.create_cell('TOP');" + "var it=rdb.create_item(cell,cat);" + "it.add_value(17.5)").execute (); +} + + diff --git a/src/pya/pya/pyaCallables.cc b/src/pya/pya/pyaCallables.cc index f9fd2c75f..829f7076f 100644 --- a/src/pya/pya/pyaCallables.cc +++ b/src/pya/pya/pyaCallables.cc @@ -413,6 +413,8 @@ match_method (int mid, PyObject *self, PyObject *args, PyObject *kwargs, bool st if (! arg) { is_valid = a->spec ()->has_default (); } else if (test_arg (*a, arg.get (), false /*strict*/, false /*object substitution*/)) { + sc += 100; + } else if (test_arg (*a, arg.get (), true /*loose*/, false /*object substitution*/)) { ++sc; } else if (test_arg (*a, arg.get (), true /*loose*/, true /*object substitution*/)) { // non-scoring match diff --git a/src/pya/pya/pyaMarshal.cc b/src/pya/pya/pyaMarshal.cc index 7a50c85e7..0326f2e18 100644 --- a/src/pya/pya/pyaMarshal.cc +++ b/src/pya/pya/pyaMarshal.cc @@ -1232,7 +1232,7 @@ struct test_arg_func return; } - if (! (cls_decl == acls || (loose && (cls_decl->is_derived_from (atype.cls ()) || cls_decl->can_convert_to (atype.cls ()))))) { + if (! (cls_decl == acls || (loose && (cls_decl->is_derived_from (atype.cls ()) || (object_substitution && cls_decl->can_convert_to (atype.cls ())))))) { *ret = false; return; } diff --git a/src/rba/rba/rba.cc b/src/rba/rba/rba.cc index 7979bc6cc..77faa3d88 100644 --- a/src/rba/rba/rba.cc +++ b/src/rba/rba/rba.cc @@ -539,6 +539,8 @@ private: if (arg == Qundef) { is_valid = a->spec ()->has_default (); } else if (test_arg (*a, arg, false /*strict*/, false /*with object substitution*/)) { + sc += 100; + } else if (test_arg (*a, arg, true /*loose*/, false /*with object substitution*/)) { ++sc; } else if (test_arg (*a, arg, true /*loose*/, true /*with object substitution*/)) { // non-scoring match diff --git a/src/rba/rba/rbaMarshal.cc b/src/rba/rba/rbaMarshal.cc index b55f82111..0781f5e7d 100644 --- a/src/rba/rba/rbaMarshal.cc +++ b/src/rba/rba/rbaMarshal.cc @@ -1217,7 +1217,7 @@ struct test_arg_func // in loose mode (second pass) try to match the types via implicit constructors, // in strict mode (first pass) require direct type match - if (p->cls_decl () == atype.cls () || (loose && (p->cls_decl ()->is_derived_from (atype.cls ()) || p->cls_decl ()->can_convert_to (atype.cls ())))) { + if (p->cls_decl () == atype.cls () || (loose && (p->cls_decl ()->is_derived_from (atype.cls ()) || (object_substitution && p->cls_decl ()->can_convert_to (atype.cls ()))))) { // type matches: check constness if ((atype.is_ref () || atype.is_ptr ()) && p->const_ref ()) { *ret = false; diff --git a/src/rdb/rdb/gsiDeclRdb.cc b/src/rdb/rdb/gsiDeclRdb.cc index 10307e96f..d2db8c0d1 100644 --- a/src/rdb/rdb/gsiDeclRdb.cc +++ b/src/rdb/rdb/gsiDeclRdb.cc @@ -999,6 +999,11 @@ Class decl_RdbItem ("rdb", "RdbItem", "@param value The box to add.\n" "This method has been introduced in version 0.25 as a convenience method." ) + + gsi::method_ext ("add_value", &add_value_t, gsi::arg ("value"), + "@brief Adds a text object to the values of this item\n" + "@param value The text to add.\n" + "This method has been introduced in version 0.30.1 to support text objects with properties." + ) + gsi::method_ext ("add_value", &add_value_t, gsi::arg ("value"), "@brief Adds an edge object to the values of this item\n" "@param value The edge to add.\n" diff --git a/testdata/python/rdbTest.py b/testdata/python/rdbTest.py index dfa2194d6..a7c34925f 100644 --- a/testdata/python/rdbTest.py +++ b/testdata/python/rdbTest.py @@ -972,6 +972,78 @@ class RDB_TestClass(unittest.TestCase): _cat_same = None self.assertEqual(_subcat.rdb_id(), _subcat_same.rdb_id()) + def test_15(self): + + p = pya.DPolygon(pya.DBox(0.5, 1, 2, 3)) + pwp = pya.DPolygonWithProperties(p, { 1: "value" }) + e = pya.DEdge(pya.DPoint(0, 0), pya.DPoint(1, 2)) + ewp = pya.DEdgeWithProperties(e, { 1: "value" }) + ep = pya.DEdgePair(e, e.moved(10, 10)) + epwp = pya.DEdgePairWithProperties(ep, { 1: "value" }) + t = pya.DText("text", pya.DTrans.R0) + twp = pya.DTextWithProperties(t, { 1: "value" }) + b = pya.DBox(0, 0, 1, 2) + bwp = pya.DBoxWithProperties(b, { 1: "value" }) + + ip = pya.Polygon(pya.Box(0, 1, 2, 3)) + ipwp = pya.PolygonWithProperties(ip, { 1: "value" }) + ie = pya.Edge(pya.Point(0, 0), pya.Point(1, 2)) + iewp = pya.EdgeWithProperties(ie, { 1: "value" }) + iep = pya.EdgePair(ie, ie.moved(10, 10)) + iepwp = pya.EdgePairWithProperties(iep, { 1: "value" }) + it = pya.Text("text", pya.Trans.R0) + itwp = pya.TextWithProperties(it, { 1: "value" }) + ib = pya.Box(0, 0, 1, 2) + ibwp = pya.BoxWithProperties(ib, { 1: "value" }) + + rdb = pya.ReportDatabase() + + cat = rdb.create_category("name") + cell = rdb.create_cell("TOP") + item = rdb.create_item(cell, cat) + + item.add_value(p) + item.add_value(pwp) + item.add_value(b) + item.add_value(bwp) + item.add_value(t) + item.add_value(twp) + item.add_value(e) + item.add_value(ewp) + item.add_value(ep) + item.add_value(epwp) + + item.add_value(ip) + item.add_value(ipwp) + item.add_value(ib) + item.add_value(ibwp) + item.add_value(it) + item.add_value(itwp) + item.add_value(ie) + item.add_value(iewp) + item.add_value(iep) + item.add_value(iepwp) + + item.add_value("string") + item.add_value(17.5) + + values = [ str(v) for v in item.each_value() ] + + self.assertEqual(values, [ + 'polygon: (0.5,1;0.5,3;2,3;2,1)', 'polygon: (0.5,1;0.5,3;2,3;2,1)', + 'box: (0,0;1,2)', 'box: (0,0;1,2)', + "label: ('text',r0 0,0)", "label: ('text',r0 0,0)", + 'edge: (0,0;1,2)', 'edge: (0,0;1,2)', + 'edge-pair: (0,0;1,2)/(10,10;11,12)', 'edge-pair: (0,0;1,2)/(10,10;11,12)', + 'polygon: (0,1;0,3;2,3;2,1)', 'polygon: (0,1;0,3;2,3;2,1)', + 'box: (0,0;1,2)', 'box: (0,0;1,2)', + "label: ('text',r0 0,0)", "label: ('text',r0 0,0)", + 'edge: (0,0;1,2)', 'edge: (0,0;1,2)', + 'edge-pair: (0,0;1,2)/(10,10;11,12)', 'edge-pair: (0,0;1,2)/(10,10;11,12)', + 'text: string', + 'float: 17.5' + ]) + # run unit tests if __name__ == '__main__': suite = unittest.TestLoader().loadTestsFromTestCase(RDB_TestClass) diff --git a/testdata/ruby/rdbTest.rb b/testdata/ruby/rdbTest.rb index 392e0cfa4..91ecb9bf2 100644 --- a/testdata/ruby/rdbTest.rb +++ b/testdata/ruby/rdbTest.rb @@ -1131,6 +1131,80 @@ class RDB_TestClass < TestBase end + def test_15 + + p = RBA::DPolygon::new(RBA::DBox::new(0.5, 1, 2, 3)) + pwp = RBA::DPolygonWithProperties::new(p, { 1 => "value" }) + e = RBA::DEdge::new(RBA::DPoint::new(0, 0), RBA::DPoint::new(1, 2)) + ewp = RBA::DEdgeWithProperties::new(e, { 1 => "value" }) + ep = RBA::DEdgePair::new(e, e.moved(10, 10)) + epwp = RBA::DEdgePairWithProperties::new(ep, { 1 => "value" }) + t = RBA::DText::new("text", RBA::DTrans::R0) + twp = RBA::DTextWithProperties::new(t, { 1 => "value" }) + b = RBA::DBox::new(0, 0, 1, 2) + bwp = RBA::DBoxWithProperties::new(b, { 1 => "value" }) + + ip = RBA::Polygon::new(RBA::Box::new(0, 1, 2, 3)) + ipwp = RBA::PolygonWithProperties::new(ip, { 1 => "value" }) + ie = RBA::Edge::new(RBA::Point::new(0, 0), RBA::Point::new(1, 2)) + iewp = RBA::EdgeWithProperties::new(ie, { 1 => "value" }) + iep = RBA::EdgePair::new(ie, ie.moved(10, 10)) + iepwp = RBA::EdgePairWithProperties::new(iep, { 1 => "value" }) + it = RBA::Text::new("text", RBA::Trans::R0) + itwp = RBA::TextWithProperties::new(it, { 1 => "value" }) + ib = RBA::Box::new(0, 0, 1, 2) + ibwp = RBA::BoxWithProperties::new(ib, { 1 => "value" }) + + rdb = RBA::ReportDatabase::new + + cat = rdb.create_category("name") + cell = rdb.create_cell("TOP") + item = rdb.create_item(cell, cat) + + item.add_value(p) + item.add_value(pwp) + item.add_value(b) + item.add_value(bwp) + item.add_value(t) + item.add_value(twp) + item.add_value(e) + item.add_value(ewp) + item.add_value(ep) + item.add_value(epwp) + + item.add_value(ip) + item.add_value(ipwp) + item.add_value(ib) + item.add_value(ibwp) + item.add_value(it) + item.add_value(itwp) + item.add_value(ie) + item.add_value(iewp) + item.add_value(iep) + item.add_value(iepwp) + + item.add_value("string") + item.add_value(17.5) + + values = item.each_value.collect(&:to_s) + + assert_equal(values, [ + 'polygon: (0.5,1;0.5,3;2,3;2,1)', 'polygon: (0.5,1;0.5,3;2,3;2,1)', + 'box: (0,0;1,2)', 'box: (0,0;1,2)', + "label: ('text',r0 0,0)", "label: ('text',r0 0,0)", + 'edge: (0,0;1,2)', 'edge: (0,0;1,2)', + 'edge-pair: (0,0;1,2)/(10,10;11,12)', 'edge-pair: (0,0;1,2)/(10,10;11,12)', + 'polygon: (0,1;0,3;2,3;2,1)', 'polygon: (0,1;0,3;2,3;2,1)', + 'box: (0,0;1,2)', 'box: (0,0;1,2)', + "label: ('text',r0 0,0)", "label: ('text',r0 0,0)", + 'edge: (0,0;1,2)', 'edge: (0,0;1,2)', + 'edge-pair: (0,0;1,2)/(10,10;11,12)', 'edge-pair: (0,0;1,2)/(10,10;11,12)', + 'text: string', + 'float: 17.5' + ]) + + end + end load("test_epilogue.rb")