Skip to content

gh-125196: Use PyUnicodeWriter in parser #125271

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Oct 12, 2024
Merged

gh-125196: Use PyUnicodeWriter in parser #125271

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions Parser/action_helpers.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1615,7 +1615,6 @@ _PyPegen_concatenate_strings(Parser *p, asdl_expr_seq *strings,
}

/* build folded list */
_PyUnicodeWriter writer;
current_pos = 0;
for (i = 0; i < n_flattened_elements; i++) {
expr_ty elem = asdl_seq_GET(flattened, i);
Expand All @@ -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
Member

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).

if (writer == NULL) {
return NULL;
}
expr_ty last_elem = elem;
for (j = i; j < n_flattened_elements; j++) {
expr_ty current_elem = asdl_seq_GET(flattened, j);
if (current_elem->kind == Constant_kind) {
if (_PyUnicodeWriter_WriteStr(
&writer, current_elem->v.Constant.value)) {
_PyUnicodeWriter_Dealloc(&writer);
if (PyUnicodeWriter_WriteStr(writer,
current_elem->v.Constant.value)) {
PyUnicodeWriter_Discard(writer);
return NULL;
}
last_elem = current_elem;
Expand All @@ -1652,9 +1654,8 @@ _PyPegen_concatenate_strings(Parser *p, asdl_expr_seq *strings,
}
i = j - 1;

PyObject *concat_str = _PyUnicodeWriter_Finish(&writer);
PyObject *concat_str = PyUnicodeWriter_Finish(writer);
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().

return NULL;
}
if (_PyArena_AddPyObject(p->arena, concat_str) < 0) {
Expand Down
Loading