diff --git a/src/gsi/gsi/gsiExpression.cc b/src/gsi/gsi/gsiExpression.cc index 3c7da5694..af0e9ac6e 100644 --- a/src/gsi/gsi/gsiExpression.cc +++ b/src/gsi/gsi/gsiExpression.cc @@ -980,8 +980,10 @@ VariantUserClassImpl::execute_gsi (const tl::ExpressionParserContext & /*context // leave it to the consumer to establish the default values (that is faster) break; } - tl::Variant def_value = a->spec ()->default_value (); - gsi::push_arg (arglist, *a, def_value, &heap); + const tl::Variant &def_value = a->spec ()->default_value (); + // NOTE: this const_cast means we need to take care that we do not use default values on "out" parameters. + // Otherwise there is a chance we will modify the default value. + gsi::push_arg (arglist, *a, const_cast (def_value), &heap); } else { throw tl::Exception (tl::to_string ("No argument provided (positional or keyword) and no default value available")); } diff --git a/src/gsi/gsi_test/gsiTest.cc b/src/gsi/gsi_test/gsiTest.cc index ee75058a1..04b5dbaca 100644 --- a/src/gsi/gsi_test/gsiTest.cc +++ b/src/gsi/gsi_test/gsiTest.cc @@ -1715,9 +1715,15 @@ gsi::EnumIn enum_in_b3 ("", "E", gsi::enum_const ("E3C", B3::E3C) ); -// 3 base classes +static std::string d4 (BB *, int a, std::string b, double c, B3::E d, tl::Variant e) +{ + return tl::sprintf ("%d,%s,%.12g,%d,%s", a, b, c, int (d), e.to_string ()); +} + +// 3 base classes and enums static gsi::Class decl_bb (decl_b1, "", "BB", - gsi::method ("d3", &BB::d3) + gsi::method ("d3", &BB::d3) + + gsi::method_ext ("d4", &d4, gsi::arg ("a"), gsi::arg ("b"), gsi::arg ("c"), gsi::arg ("d", B3::E3A, "E3A"), gsi::arg ("e", tl::Variant (), "nil"), "") ); gsi::ClassExt b2_in_bb (decl_b2); gsi::ClassExt b3_in_bb (decl_b3); diff --git a/src/pya/pya/pyaCallables.cc b/src/pya/pya/pyaCallables.cc index 77f3c1728..043bd95a2 100644 --- a/src/pya/pya/pyaCallables.cc +++ b/src/pya/pya/pyaCallables.cc @@ -194,18 +194,29 @@ num_args (const gsi::MethodBase *m) } static bool -compatible_with_args (const gsi::MethodBase *m, int argc, PyObject *kwargs) +compatible_with_args (const gsi::MethodBase *m, int argc, PyObject *kwargs, std::string *why_not = 0) { int nargs = num_args (m); - if (argc >= nargs) { + if (argc > nargs) { + if (why_not) { + *why_not = tl::sprintf (tl::to_string (tr ("%d argument(s) expected, but %d given")), nargs, argc); + } + return false; + } else if (argc == nargs) { // no more arguments to consider - return argc == nargs && (kwargs == NULL || PyDict_Size (kwargs) == 0); + if (kwargs != NULL && PyDict_Size (kwargs) > 0) { + if (why_not) { + *why_not = tl::to_string (tr ("all arguments given, but additional keyword arguments specified")); + } + return false; + } else { + return true; + } } if (kwargs != NULL) { - int nkwargs = int (PyDict_Size (kwargs)); int kwargs_taken = 0; while (argc < nargs) { @@ -213,6 +224,9 @@ compatible_with_args (const gsi::MethodBase *m, int argc, PyObject *kwargs) pya::PythonPtr py_arg = PyDict_GetItemString (kwargs, atype.spec ()->name ().c_str ()); if (! py_arg) { if (! atype.spec ()->has_default ()) { + if (why_not) { + *why_not = tl::sprintf (tl::to_string (tr ("no argument specified for '%s' (neither positional or keyword)")), atype.spec ()->name ()); + } return false; } } else { @@ -221,14 +235,20 @@ compatible_with_args (const gsi::MethodBase *m, int argc, PyObject *kwargs) ++argc; } - // matches if all keyword arguments are taken - return kwargs_taken == nkwargs; + return true; } else { while (argc < nargs) { const gsi::ArgType &atype = m->begin_arguments () [argc]; if (! atype.spec ()->has_default ()) { + if (why_not) { + if (argc < nargs - 1 && ! m->begin_arguments () [argc + 1].spec ()->has_default ()) { + *why_not = tl::sprintf (tl::to_string (tr ("no value given for argument #%d and following")), argc + 1); + } else { + *why_not = tl::sprintf (tl::to_string (tr ("no value given for argument #%d")), argc + 1); + } + } return false; } ++argc; @@ -243,8 +263,11 @@ static std::string describe_overload (const gsi::MethodBase *m, int argc, PyObject *kwargs) { std::string res = m->to_string (); - if (compatible_with_args (m, argc, kwargs)) { + std::string why_not; + if (compatible_with_args (m, argc, kwargs, &why_not)) { res += " " + tl::to_string (tr ("[match candidate]")); + } else if (! why_not.empty ()) { + res += " [" + why_not + "]"; } return res; } @@ -705,8 +728,10 @@ push_args (gsi::SerialArgs &arglist, const gsi::MethodBase *meth, PyObject *args // leave it to the consumer to establish the default values (that is faster) break; } - tl::Variant def_value = a->spec ()->default_value (); - gsi::push_arg (arglist, *a, def_value, &heap); + const tl::Variant &def_value = a->spec ()->default_value (); + // NOTE: this const_cast means we need to take care that we do not use default values on "out" parameters. + // Otherwise there is a chance we will modify the default value. + gsi::push_arg (arglist, *a, const_cast (def_value), &heap); } else { throw tl::Exception (tl::to_string (tr ("No argument provided (positional or keyword) and no default value available"))); } diff --git a/src/rba/rba/rba.cc b/src/rba/rba/rba.cc index 0cef54c62..f05031a2c 100644 --- a/src/rba/rba/rba.cc +++ b/src/rba/rba/rba.cc @@ -1066,8 +1066,10 @@ push_args (gsi::SerialArgs &arglist, const gsi::MethodBase *meth, VALUE *argv, i // leave it to the consumer to establish the default values (that is faster) break; } - tl::Variant def_value = a->spec ()->default_value (); - gsi::push_arg (arglist, *a, def_value, &heap); + const tl::Variant &def_value = a->spec ()->default_value (); + // NOTE: this const_cast means we need to take care that we do not use default values on "out" parameters. + // Otherwise there is a chance we will modify the default value. + gsi::push_arg (arglist, *a, const_cast (def_value), &heap); } else { throw tl::Exception (tl::to_string (tr ("No argument provided (positional or keyword) and no default value available"))); } diff --git a/testdata/python/basic.py b/testdata/python/basic.py index 944989467..c24e2d477 100644 --- a/testdata/python/basic.py +++ b/testdata/python/basic.py @@ -3236,6 +3236,48 @@ class BasicTest(unittest.TestCase): self.assertEqual(pc.x, 3) self.assertEqual(pdc.x, 4) + # keyword arguments, enums and error messages + + def test_92(self): + + bb = pya.BB() + + m = "" + try: + bb.d4() + except Exception as ex: + m = str(ex) + self.assertEqual(m, "Can't match arguments. Variants are:\n string d4(int a, string b, double c, B3::E d = E3A, variant e = nil) [no value given for argument #1 and following]\n in BB.d4") + + m = "" + try: + bb.d4(1, "a") + except Exception as ex: + m = str(ex) + self.assertEqual(m, "Can't match arguments. Variants are:\n string d4(int a, string b, double c, B3::E d = E3A, variant e = nil) [no value given for argument #3]\n in BB.d4") + + m = "" + try: + bb.d4(1, "a", 2.0, xxx=17) + except Exception as ex: + m = str(ex) + self.assertEqual(m, "Unknown keyword parameter: xxx in BB.d4") + + m = "" + try: + bb.d4(a=1, b="a", c=2.0, xxx=17) + except Exception as ex: + m = str(ex) + self.assertEqual(m, "Unknown keyword parameter: xxx in BB.d4") + + self.assertEqual(bb.d4(1, "a", 2.0), "1,a,2,100,nil") + self.assertEqual(bb.d4(1, "a", 2.0, e=42), "1,a,2,100,42") + self.assertEqual(bb.d4(1, "a", c=2.0, e=42), "1,a,2,100,42") + self.assertEqual(bb.d4(c=2.0, a=1, b="a", e=42), "1,a,2,100,42") + self.assertEqual(bb.d4(1, "a", 2.0, d=pya.BB.E.E3B), "1,a,2,101,nil") + self.assertEqual(bb.d4(1, "a", d=pya.BB.E.E3B, c=2.5), "1,a,2.5,101,nil") + self.assertEqual(bb.d4(1, "a", 2.0, pya.BB.E.E3B, 42), "1,a,2,101,42") + # run unit tests if __name__ == '__main__': suite = unittest.TestSuite() diff --git a/testdata/ruby/basic_testcore.rb b/testdata/ruby/basic_testcore.rb index bd042373d..60aeff65d 100644 --- a/testdata/ruby/basic_testcore.rb +++ b/testdata/ruby/basic_testcore.rb @@ -3154,4 +3154,51 @@ class Basic_TestClass < TestBase end + # keyword arguments, enums and error messages + def test_81 + + bb = RBA::BB::new + + m = "" + begin + bb.d4() + rescue => ex + m = ex.to_s + end + assert_equal(m, "Can't match arguments. Variants are:\n string d4(int a, string b, double c, B3::E d = E3A, variant e = nil) [no value given for argument #1 and following]\n in BB.d4") + + m = "" + begin + bb.d4(1, "a") + rescue => ex + m = ex.to_s + end + assert_equal(m, "Can't match arguments. Variants are:\n string d4(int a, string b, double c, B3::E d = E3A, variant e = nil) [no value given for argument #3]\n in BB.d4") + + m = "" + begin + bb.d4(1, "a", 2.0, xxx=17) + rescue => ex + m = ex.to_s + end + assert_equal(m, "Unknown keyword parameter: xxx in BB.d4") + + m = "" + begin + bb.d4(a=1, b="a", c=2.0, xxx=17) + rescue => ex + m = ex.to_s + end + assert_equal(m, "Unknown keyword parameter: xxx in BB.d4") + + assert_equal(bb.d4(1, "a", 2.0), "1,a,2,100,nil") + assert_equal(bb.d4(1, "a", 2.0, e=42), "1,a,2,100,42") + assert_equal(bb.d4(1, "a", c=2.0, e=42), "1,a,2,100,42") + assert_equal(bb.d4(c=2.0, a=1, b="a", e=42), "1,a,2,100,42") + assert_equal(bb.d4(1, "a", 2.0, d=RBA::BB::E::E3B), "1,a,2,101,nil") + assert_equal(bb.d4(1, "a", d=RBA::BB::E::E3B, c=2.5), "1,a,2.5,101,nil") + assert_equal(bb.d4(1, "a", 2.0, RBA::BB::E::E3B, 42), "1,a,2,101,42") + + end + end