Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix method python method call without self #439

Merged
merged 6 commits into from
Sep 11, 2024

Conversation

philippedistributive
Copy link
Collaborator

Do check if method has 0 args meaning no self and ultimately fail in the correct manner

closes #436

Copy link
Collaborator

@zollqir zollqir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic works, but a simpler solution is available

Comment on lines 382 to 397
bool isMethod;
if (PyMethod_Check(pyFunc)) {
f = PyMethod_Function(pyFunc); // borrowed reference
nNormalArgs -= 1; // don't include the implicit `self` of the method as an argument
isMethod = true;
} else {
isMethod = false;
}
PyCodeObject *bytecode = (PyCodeObject *)PyFunction_GetCode(f); // borrowed reference
PyObject *defaults = PyFunction_GetDefaults(f); // borrowed reference
nDefaultArgs = defaults ? PyTuple_Size(defaults) : 0;
nNormalArgs += bytecode->co_argcount - nDefaultArgs - 1;
if (bytecode->co_argcount == 0 && isMethod) {
nNormalArgs += nDefaultArgs;
} else {
nNormalArgs += bytecode->co_argcount - nDefaultArgs - 1;
}
Copy link
Collaborator

@zollqir zollqir Sep 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes are not necessary, the comparison on line 404 just needs to change

diff --git a/src/jsTypeFactory.cc b/src/jsTypeFactory.cc
index 24eb5c4..7238da3 100644
--- a/src/jsTypeFactory.cc
+++ b/src/jsTypeFactory.cc
@@ -379,29 +379,21 @@ bool callPyFunc(JSContext *cx, unsigned int argc, JS::Value *vp) {
   else {
     nNormalArgs = 1;
     PyObject *f = pyFunc;
-    bool isMethod;
     if (PyMethod_Check(pyFunc)) {
       f = PyMethod_Function(pyFunc); // borrowed reference
       nNormalArgs -= 1; // don't include the implicit `self` of the method as an argument
-      isMethod = true;
-    } else {
-      isMethod = false;
     }
     PyCodeObject *bytecode = (PyCodeObject *)PyFunction_GetCode(f); // borrowed reference
     PyObject *defaults = PyFunction_GetDefaults(f); // borrowed reference
     nDefaultArgs = defaults ? PyTuple_Size(defaults) : 0;
-    if (bytecode->co_argcount == 0 && isMethod) {
-      nNormalArgs += nDefaultArgs;
-    } else {
-      nNormalArgs += bytecode->co_argcount - nDefaultArgs - 1;
-    }
+    nNormalArgs += bytecode->co_argcount - nDefaultArgs - 1;
     if (bytecode->co_flags & CO_VARARGS) {
       varargs = true;
     }
   }
 
   // use faster calling if no arguments are needed
-  if (((nNormalArgs + nDefaultArgs) == 0 && !varargs)) {
+  if (((nNormalArgs + nDefaultArgs) <= 0 && !varargs)) {
     #if PY_VERSION_HEX >= 0x03090000
     pyRval = PyObject_CallNoArgs(pyFunc);
     #else```

tests/python/test_functions_this.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@zollqir zollqir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@philippedistributive philippedistributive merged commit 2ed42d9 into main Sep 11, 2024
32 checks passed
@philippedistributive philippedistributive deleted the philippe/fix-436 branch September 11, 2024 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Seg fault on a method call
2 participants