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

bpo-38835: Don't use PyFPE_START_PROTECT and PyFPE_END_PROTECT #17231

Merged
merged 1 commit into from Nov 20, 2019

Conversation

@vstinner
Copy link
Member

@vstinner vstinner commented Nov 18, 2019

PyFPE_START_PROTECT() and PyFPE_END_PROTECT() macros are empty: they
do nothing for one year (since commit
735ae8d), stop using them.

https://bugs.python.org/issue38835

Copy link
Member

@pablogsal pablogsal left a comment

LGTM I also double-checked that after this PR there are no usages of the macro left.

One question and a minor code suggestion for the NEWS file:

Do you want to left the empty macro in Include/pyfpe.h?

@pablogsal
Copy link
Member

@pablogsal pablogsal commented Nov 19, 2019

We don't provide any contract regarding these macros and I don't see a way a user can override them easily so I don't think we should be concerned with that.

@vstinner
Copy link
Member Author

@vstinner vstinner commented Nov 19, 2019

Do you want to left the empty macro in Include/pyfpe.h?

If someone wants to remove them, I would suggest to first deprecate them, and wait at least one Python release before removing them. And do that in a separated issue: https://bugs.python.org/issue38835 title is "pyfpe.h: Exclude PyFPE_START_PROTECT and PyFPE_END_PROTECT from the Py_LIMITED_API", it's not directly related :-)

The PyFPE_START_PROTECT() and PyFPE_END_PROTECT() macros are empty:
they have been doing nothing for the last year  (since commit
735ae8d), so stop using them.
@vstinner vstinner force-pushed the vstinner:remove_pyfpe branch from 5773ec1 to a467fd7 Nov 19, 2019
@vstinner
Copy link
Member Author

@vstinner vstinner commented Nov 19, 2019

I updated the NEWS entry.

I rebased the PR to be able to modify the commit message (copy of the NEWS entry).

@vstinner
Copy link
Member Author

@vstinner vstinner commented Nov 19, 2019

We don't provide any contract regarding these macros and I don't see a way a user can override them easily so I don't think we should be concerned with that.

I think the only way to get non-empty macros is to modify pyfpe.h. I doubt anyone does that.

@vstinner vstinner merged commit be143ec into python:master Nov 20, 2019
4 checks passed
4 checks passed
Azure Pipelines PR #20191119.36 succeeded
Details
@bedevere-bot
bedevere/issue-number Issue number 38835 found
Details
@bedevere-bot
bedevere/news News entry found in Misc/NEWS.d
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@vstinner vstinner deleted the vstinner:remove_pyfpe branch Nov 20, 2019
jacobneiltaylor added a commit to jacobneiltaylor/cpython that referenced this pull request Dec 5, 2019
…nGH-17231)

The PyFPE_START_PROTECT() and PyFPE_END_PROTECT() macros are empty:
they have been doing nothing for the last year  (since commit
735ae8d), so stop using them.
shihai1991 added a commit to shihai1991/cpython that referenced this pull request Jan 31, 2020
…nGH-17231)

The PyFPE_START_PROTECT() and PyFPE_END_PROTECT() macros are empty:
they have been doing nothing for the last year  (since commit
735ae8d), so stop using them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants