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-129173: Use _PyUnicodeError_GetParams in PyCodec_NameReplaceErrors #129135

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

picnixz
Copy link
Member

@picnixz picnixz commented Jan 21, 2025

I extracted the logic of rendering hexadecimal digits but this requires a double pointer (alternatively, I could make the function return the number of characters that were written and advance the pointer after the call).

If inlined calls are preferred, then we can convert the function into a macro to avoid duplications but I think a function call is not that slow (especially if I add the inline qualifier to force a bit the compiler to inline it and then allow it to possibly vectorize it if it's smart). Ideally, I would like to extract that logic from PyCodec_XMLCharRefReplaceErrors as well but it's the only place where we use the decimal base instead. OTOH, if we extract the logic, it's a bit cleaner to read.

After the refactoring of surrogate handlers has been done, I will wrap up this list of PRs by cleaning up the handlers that I fixed previously (I just didn't want to do both cleanup and fixes in the same PR). The idea is to extract the handling of each unicode error type into a separate function (unless the handler only handles a single exception type as it's the case for the namereplace handler).

@picnixz picnixz changed the title Use _PyUnicodeError_GetParams for the 'namereplace' handler gh-129173: Use _PyUnicodeError_GetParams for the 'namereplace' handler Jan 22, 2025
@picnixz picnixz changed the title gh-129173: Use _PyUnicodeError_GetParams for the 'namereplace' handler gh-129173: Use _PyUnicodeError_GetParams in PyCodec_NameReplaceErrors Jan 22, 2025
@picnixz picnixz marked this pull request as ready for review January 24, 2025 15:21
@picnixz picnixz marked this pull request as draft January 24, 2025 15:23
@picnixz
Copy link
Member Author

picnixz commented Jan 24, 2025

Since I will be leaving for two weeks and won't be able to commit or review anything (except on mobile), I kept it as a draft and we'll discuss about it later. What remains do to on the codecs part is mainly refactoring so it's just a feature.

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.

1 participant