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-125196: Use PyUnicodeWriter in parser #125271

Merged
merged 1 commit into from
Oct 12, 2024
Merged

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Oct 10, 2024

Replace the private _PyUnicodeWriter API with the public PyUnicodeWriter API in _PyPegen_concatenate_strings().

Replace the private _PyUnicodeWriter API with the public
PyUnicodeWriter API in _PyPegen_concatenate_strings().
if (concat_str == NULL) {
_PyUnicodeWriter_Dealloc(&writer);
Copy link
Member Author

Choose a reason for hiding this comment

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

The writer must not be used after _PyUnicodeWriter_Finish().

Copy link
Member

@pablogsal pablogsal left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -1635,14 +1634,17 @@ _PyPegen_concatenate_strings(Parser *p, asdl_expr_seq *strings,
"abc" u"abc" -> "abcabc" */
PyObject *kind = elem->v.Constant.kind;

_PyUnicodeWriter_Init(&writer);
PyUnicodeWriter *writer = PyUnicodeWriter_Create(0);
Copy link
Member

Choose a reason for hiding this comment

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

What's the 0 here?

Copy link
Contributor

Choose a reason for hiding this comment

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

IIRC that's the number of pre-allocated characters. If you know that you will need N characters, I think it's better to put N in advance so that you don't have to resize the buffer. Otherwise, 0 is a good choice if you don't know in advance how many characters you'll eventually need (at the cost of resizes).

@vstinner vstinner merged commit 4a943c3 into python:main Oct 12, 2024
39 checks passed
@vstinner vstinner deleted the writer_pegen branch October 12, 2024 07:28
@vstinner
Copy link
Member Author

Merged, thanks for the review @pablogsal.

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.

3 participants