Skip to content

bpo-38376: Fix build error when enabling assertions but not Py_DEBUG.#16594

Closed
Yhg1s wants to merge 1 commit intopython:masterfrom
Yhg1s:pyunicode_checkconsistency
Closed

bpo-38376: Fix build error when enabling assertions but not Py_DEBUG.#16594
Yhg1s wants to merge 1 commit intopython:masterfrom
Yhg1s:pyunicode_checkconsistency

Conversation

@Yhg1s
Copy link
Copy Markdown
Member

@Yhg1s Yhg1s commented Oct 5, 2019 •
edited by bedevere-bot
Loading

_PyUnicode_CheckConsistency is now defined even without Py_DEBUG set, so get rid of the #define hack that was there to deal with calls in assert statements not explicitly guarded by Py_DEBUG.

https://bugs.python.org/issue38376

@Yhg1s
Fix build error when enabling assertions but not Py_DEBUG:
84875bb 
_PyUnicode_CheckConsistency is now defined even without Py_DEBUG set, so get
rid of the #define hack that was there to deal with calls in assert
statements not explicitly guarded by Py_DEBUG.
@Yhg1s Yhg1s requested a review from vsajip October 5, 2019 15:15
vstinner
vstinner reviewed Oct 7, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

My intent was to also provide _PyObject_CheckConsistency() in release mode to help debugging GC crash, especially in visit_decref(). I wrote PR #16612 to fix my implementation.

@vstinner
Copy link
Copy Markdown
Member

vstinner commented Oct 7, 2019

In short, _PyUnicode_CheckConsistency() is now always available in Python 3.8. I wrote PR #16623 to fix its definition in header files.

This change doesn't look correct. I'm not sure that _PyUnicode_CheckConsistency() definition depends on NDEBUG macro.

@vstinner
Copy link
Copy Markdown
Member

vstinner commented Oct 7, 2019

I merged my PR #16623 instead.

@vstinner vstinner closed this Oct 7, 2019
@Yhg1s
Copy link
Copy Markdown
Member Author

Yhg1s commented Oct 7, 2019

FYI, this change was intended to keep visibility parity, because the existing declaration (not definition) of _PyUnicode_CheckConsistency in Include/unicodeobject.h was already conditional on NDEBUG or Py_DEBUG. The only other declaration was in Include/internal/pycore_object.h, which wouldn't be visible to user code in the same way as before. Moving the declaration to Include/cpython/unicodeobject.h as you did also solved that same problem, because that header is included by Include/unicodeobject.h when not using the limited API. We should be all good now.

@Yhg1s Yhg1s deleted the pyunicode_checkconsistency branch October 7, 2019 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner left review comments

@vsajip vsajip vsajip approved these changes

Assignees

No one assigned

Labels

awaiting merge skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Yhg1s @vstinner @vsajip @the-knights-who-say-ni @bedevere-bot