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

report type of passed argument when conversion fails #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

N-Coder
Copy link
Contributor

@N-Coder N-Coder commented Oct 24, 2021

No description provided.

Copy link
Owner

@wlav wlav left a comment

Choose a reason for hiding this comment

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

If you use PyObject_Type instead of Py_TYPE, then you get a new reference and its result needs a Py_DECREF.

__name__ is interned as CPyCppyy::PyStrings::gName, for use with PyObject_GetAttr.

Instead of PyUnicode_AsUTF8, use CPyCppyy_PyText_AsString for portability.

There is also std::string ClassName(PyObject* pyobj); in Utility.h. It differs in that it looks for __cpp_name__ before looking for __name__ but otherwise has all the ref-counting, conversions, etc. as they should be.

@N-Coder
Copy link
Contributor Author

N-Coder commented Oct 25, 2021

Could the PyErr_Clear in Utility::ClassName, which gets called when the passed object has no __cpp_name__, interfere with the following SetPyError_ and the PyErr_Fetch it calls?

std::string CPyCppyy::Utility::ClassName(PyObject* pyobj) {
    std::string clname = "<unknown>";
    PyObject* pyclass = (PyObject*)Py_TYPE(pyobj);
    PyObject* pyname = PyObject_GetAttr(pyclass, PyStrings::gCppName);
    if (!pyname) {
        PyErr_Clear();
        pyname = PyObject_GetAttr(pyclass, PyStrings::gName);
    }

@N-Coder
Copy link
Contributor Author

N-Coder commented Nov 7, 2021

I changed it to ClassName and removed the ref counting, but I'm still unsure what'll happen in case of an exception.

@wlav
Copy link
Owner

wlav commented Nov 8, 2021

I'm thinking (not tested) that in case of an exception, PyErr_Clear would need to be called for ClassName() to succeed. I'm also thinking that if there is an exception, fetching it and using it as part of the message may be useful.

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.

2 participants