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

gh-111178: fix UBSan for custom.c examples #131606

Merged
merged 5 commits into from
Mar 24, 2025

Conversation

picnixz
Copy link
Member

@picnixz picnixz commented Mar 23, 2025

@picnixz picnixz self-assigned this Mar 24, 2025
Copy link
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.

LGTM

cc @encukou

@encukou
Copy link
Member

encukou commented Mar 24, 2025

These are included in the docs (newtypes_tutorial.rst), which redundantly define most of those in the prose, and have some notes that reference the casts. Could you change those as well?
(Perhaps there's a way to consolidate this with something like Sphinx's :start-at:, but that might be a distraction right now. In any case, some of the actual prose does need to be updated.)

@picnixz
Copy link
Member Author

picnixz commented Mar 24, 2025

I observed that I incorrectly called a function as well, so thanks Petr for pointing out the docs.

@picnixz picnixz requested a review from encukou March 24, 2025 10:52
Co-authored-by: Petr Viktorin <encukou@gmail.com>
@picnixz
Copy link
Member Author

picnixz commented Mar 24, 2025

@encukou feel free to merge this one tomorrow or whenever you want, I'm not on my dev session so I can only edit from the web UI.

@encukou encukou merged commit 1d6a2e6 into python:main Mar 24, 2025
27 checks passed
@encukou
Copy link
Member

encukou commented Mar 24, 2025

Thanks!
FWIW, there's also newtypes.rst with some UB examples.

@picnixz picnixz deleted the fix/ubsan/doc-custom-111178 branch March 24, 2025 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants