Skip to content

Conversation

@vstinner
Copy link
Member

@vstinnervstinner 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().
PyObject*concat_str=_PyUnicodeWriter_Finish(&writer);
PyObject*concat_str=PyUnicodeWriter_Finish(writer);
if (concat_str==NULL){
_PyUnicodeWriter_Dealloc(&writer);
Copy link
MemberAuthor

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

@pablogsalpablogsal left a comment

Choose a reason for hiding this comment

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

LGTM

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

@vstinnervstinner merged commit 4a943c3 into python:mainOct 12, 2024
@vstinnervstinner deleted the writer_pegen branch October 12, 2024 07:28
@vstinner
Copy link
MemberAuthor

Merged, thanks for the review @pablogsal.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@vstinner@picnixz@pablogsal