luke
/
klayout
mirror of https://github.com/KLayout/klayout.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Bugfix for passing default arguments to GSI calls 

This happens when default arguments (specifically
user class or enum types) and passed between keyword
and positional arguments. We must not use a temporary
tl::Variant object as it gets out of scope and a
reference is stored.

In addition: better error messages for Python when
a method can't be matched to arguments.
This commit is contained in:
Matthias Koefferlein 2024-05-25 17:48:17 +02:00
parent 370abf7ab3
commit 1861abc68c
6 changed files with 139 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/gsi/gsi/gsiExpression.cc
View File

@ -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<tl::Variant &> (def_value), &heap);
} else {
throw tl::Exception (tl::to_string ("No argument provided (positional or keyword) and no default value available"));
}

10
src/gsi/gsi_test/gsiTest.cc
View File

@ -1715,9 +1715,15 @@ gsi::EnumIn<B3, B3::E> 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<BB> 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<BB> b2_in_bb (decl_b2);
gsi::ClassExt<BB> b3_in_bb (decl_b3);

43
src/pya/pya/pyaCallables.cc
View File

@ -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<tl::Variant &> (def_value), &heap);
} else {
throw tl::Exception (tl::to_string (tr ("No argument provided (positional or keyword) and no default value available")));
}

6
src/rba/rba/rba.cc
View File

@ -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<tl::Variant &> (def_value), &heap);
} else {
throw tl::Exception (tl::to_string (tr ("No argument provided (positional or keyword) and no default value available")));
}

42
testdata/python/basic.py vendored
View File

@ -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()

47
testdata/ruby/basic_testcore.rb vendored
View File

@ -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