Skip to content

Conversation

@vstinner
Copy link
Member

@vstinner vstinner commented Jan 6, 2020

Filename and line numbers are not needed when Py_REF_DEBUG are not
defined.

The static inline _Py_DECREF() function was introduced by
commit 2aaf0c1.

Filename and line numbers are not needed when Py_REF_DEBUG are not
defined.

The static inline _Py_DECREF() function was introduced by
commit 2aaf0c1.
@vstinner
Copy link
Member Author

vstinner commented Jan 6, 2020

See PR #16781 for my first attempt.

@vstinner
Copy link
Member Author

vstinner commented Jan 6, 2020

@Yhg1s @serhiy-storchaka: Does this version look better than PR #16781?

@vstinner
Copy link
Member Author

vstinner commented Jan 8, 2020

cc @pablogsal

Copy link
Member

@Yhg1s Yhg1s left a comment

Choose a reason for hiding this comment

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

Yes, this looks better to me. The ifdef dance isn't too bad, and it's no longer duplicating logic.

Include/object.h Outdated
#endif
PyObject *op)
{
(void)filename; /* may be unused, shut up -Wunused-parameter */
Copy link
Member

Choose a reason for hiding this comment

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

These suppressions shouldn't be necessary anymore, but if they are, they should be conditional.

Copy link
Member Author

Choose a reason for hiding this comment

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

The suppression are needed with the current code, since filename & fileno are not used for a release build. It's the purpose of my change: don't pass them if we don't need them ;-)

@vstinner vstinner merged commit f3a0a6b into python:master Jan 8, 2020
@vstinner vstinner deleted the decref2 branch January 8, 2020 20:03
shihai1991 pushed a commit to shihai1991/cpython that referenced this pull request Jan 31, 2020
)

Filename and line numbers are not needed when Py_REF_DEBUG are not
defined.

The static inline _Py_DECREF() function was introduced by
commit 2aaf0c1.
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.

4 participants