Skip to content

Py_IS_TYPE() macro uses Py_TYPE() #22341

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

Merged
merged 1 commit into from
Sep 22, 2020
Merged

Py_IS_TYPE() macro uses Py_TYPE() #22341

merged 1 commit into from
Sep 22, 2020

Conversation

vstinner
Copy link
Member

No description provided.

@vstinner
Copy link
Member Author

cc @corona10 @shihai1991

@@ -141,7 +141,7 @@ static inline PyTypeObject* _Py_TYPE(const PyObject *ob) {


static inline int _Py_IS_TYPE(const PyObject *ob, const PyTypeObject *type) {
return ob->ob_type == type;
Copy link
Member

Choose a reason for hiding this comment

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

Hm, There have any benefit from this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

My plan is to hide implementation details as much as possible: https://www.python.org/dev/peps/pep-0620/

Right now, Py_TYPE() is trivial: just read ob->ob_type, but it may be more complex tomorrow.

Example in my tagged pointer experiment:
https://github.com/vstinner/cpython/blob/tagged_ptr/Include/object.h#L203


static inline PyTypeObject* _Py_TYPE(const PyObject *ob)
{
    if (!_Py_TAGPTR_IS_TAGGED(ob)) {
        return ob->ob_type;
    }
    else {
        return _Py_TAGPTR_TYPE(ob);
    }
}

Experiment: vstinner#6

Copy link
Member Author

@vstinner vstinner Sep 21, 2020

Choose a reason for hiding this comment

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

With this PR, only Py_TYPE() has to be updated, Py_IS_TYPE() remains unchanged.

Copy link
Member

@shihai1991 shihai1991 left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Member

@corona10 corona10 left a comment

Choose a reason for hiding this comment

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

lgtm

@vstinner vstinner merged commit c5cb077 into python:master Sep 22, 2020
@vstinner vstinner deleted the Py_IS_TYPE branch September 22, 2020 10:42
@vstinner
Copy link
Member Author

Thanks for your reviews @shihai1991 and @corona10 ;-)

If you are curious about tagged pointers: https://mail.python.org/archives/list/[email protected]/thread/4ETF7H5COIZWTCXDRTITVMU4P5DLRDLU/

@shihai1991
Copy link
Member

Thanks for your reviews @shihai1991 and @corona10 ;-)

If you are curious about tagged pointers: https://mail.python.org/archives/list/[email protected]/thread/4ETF7H5COIZWTCXDRTITVMU4P5DLRDLU/

copy that, this is an interesting PR.

xzy3 pushed a commit to xzy3/cpython that referenced this pull request Oct 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants