-
-
Notifications
You must be signed in to change notification settings - Fork 33.9k
bpo-38205: Convert Py_UNREACHABLE() macro to a function #16280
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
Conversation
|
This change:
I didn't keep the long version of the joke since RANDALL_WAS_HERE compilation flag is not documented anyway. If someone wants to keep it, I suggest to document it in Misc/SpecialBuilds.txt. Moreover, I dislike untested code :-/ I'm also fine with merging the two xkcd texts into a single longer one if you prefer. Recently, @serhiy-storchaka fixed "from future import barry_as_FLUFL": this easteregg doesn't need to rebuild Python, so I'm fine with keep it. |
|
You can test this change by (1) applying this change: (2) run this script: Output with this change (in debug mode): Output without this change (in debug mode): This main enhancement is that you now get the Python traceback where the bug occurred. |
|
This change fix https://bugs.python.org/issue38205 |
zware
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One nit about the release-mode message, but the overall change looks reasonable to me.
|
@zware: @serhiy-storchaka prefers to keep a macro rather converting it to a function: https://bugs.python.org/issue38205#msg352782 What's the opinion? |
|
I'm somewhat in favor of keeping it as a macro just for complete backwards compatibility, even if it's implemented as a private |
|
I wrote PR #16290 which fix https://bugs.python.org/issue38205 but keeps Py_UNREACHABLE() as a macro. |
|
It was decided to merge PR #16290 instead. |
https://bugs.python.org/issue38205