KEMBAR78
gh-123465: Allow Py_RELATIVE_OFFSET for __*offset__ members by encukou · Pull Request #123474 · python/cpython · GitHub
Skip to content

Conversation

@encukou
Copy link
Member

@encukou encukou commented Aug 29, 2024

Allow flags = Py_READONLY | Py_RELATIVE_OFFSET for the special offset members,
which previously required Py_READONLY.


📚 Documentation preview 📚: https://cpython-previews--123474.org.readthedocs.build/

@encukou encukou marked this pull request as ready for review August 30, 2024 05:54
@wjakob
Copy link
Contributor

wjakob commented Sep 3, 2024

Thank you, I confirm that this PR fixes the issue I encountered.

@encukou encukou merged commit 16be8db into python:main Sep 5, 2024
@encukou encukou deleted the special_relative_offset branch September 5, 2024 12:14
Comment on lines +4740 to +4748
if (strcmp(memb->name, "__weaklistoffset__") == 0) {
weaklistoffset_member = memb;
}
if (strcmp(memb->name, "__dictoffset__") == 0) {
dictoffset_member = memb;
}
if (strcmp(memb->name, "__vectorcalloffset__") == 0) {
vectorcalloffset_member = memb;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer to use if-else or continue. Non-debug builds on MSVC currently avoid a crash here by relying on a default SSA-optimization option (partial redundancy elimination) for some reason, which is usually disabled (/d2ssa-pre-) to diagnose or work around a crash unlike this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Adding else sounds like a great change! continue would work too.
You could reuse gh-123465 to keep the fix-up commit grouped with this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants