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

Py_DECREF: only pass filename if Py_REF_DEBUG is defined #17870

Merged
merged 2 commits into from Jan 8, 2020

Conversation

@vstinner
Copy link
Member

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

This comment has been minimized.

Copy link
Member Author

vstinner commented Jan 6, 2020

See PR #16781 for my first attempt.

@vstinner

This comment has been minimized.

Copy link
Member Author

vstinner commented Jan 6, 2020

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

@vstinner

This comment has been minimized.

Copy link
Member Author

vstinner commented Jan 8, 2020

@Yhg1s
Yhg1s approved these changes Jan 8, 2020
Copy link
Member

Yhg1s left a comment

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

#ifdef Py_REF_DEBUG
const char *filename, int lineno,
#endif
PyObject *op)
{
(void)filename; /* may be unused, shut up -Wunused-parameter */

This comment has been minimized.

Copy link
@Yhg1s

Yhg1s Jan 8, 2020

Member

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

This comment has been minimized.

Copy link
@vstinner

vstinner Jan 8, 2020

Author Member

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
8 checks passed
8 checks passed
Windows (x86)
Details
Windows (x64)
Details
macOS
Details
Ubuntu
Details
Azure Pipelines PR #20200106.53 succeeded
Details
bedevere/issue-number Issue report skipped
bedevere/news "skip news" label found
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@vstinner vstinner deleted the vstinner:decref2 branch Jan 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.