gh-139353: Add Objects/unicode_format.c file

[vstinner]

• Move PyUnicode_Format() implementation from unicodeobject.c to unicode_format.c.
• Replace unicode_modifiable() with _PyUnicode_IsModifiable()
• Add empty lines to have two empty lines between functions.

• Issue: Split large Objects/unicodeobject.c file into smaller files #139353

[vstinner]

cc @serhiy-storchaka @ZeroIntensity @gpshead @picnixz

[picnixz]

Should I understand that it's to avoid a refactoring in this file? do you plan to remove those defines later? (I think we could do it but in another PR)

[vstinner]

I don't plan to remove those defines. I don't think that it's worth it.

[picnixz]

Can I assume that, starting from here, it's just a huge copy-paste, possibly with some names changed due to some private functions that are now _Py* something? I can otherwise run a diff myself but I wanted to be ask (if you say "no", I'll just run the diff locally)

[vstinner]

I made few changes:

• Replace unicode_modifiable() with _PyUnicode_IsModifiable()
• Replace unicode_fill() with _PyUnicode_Fill()
• Add empty lines to have two empty lines between functions.

[StanFromIreland]

This is a good time to consider PEP 7'ing the code, it would be nice (I would volunteer to do it :-). IIRC the main argument was that it'll ruin the blame, but that's not an issue any more.

[AA-Turner]

The split to a new file should have as few changes as possible so Git can calculate cross-file moves, formatting changes can be left to their own PR.

[vstinner]

I would prefer to reduce changes to the bare minimum. If someone wants to apply coding style changes, I would suggest to make a separated PR. I prefer to stick to just "move code around".

[serhiy-storchaka]

Please take your time. Some people will only be able to participate in the discussion on weekends.

There are three formatters: str.__mod__() (PyUnicode_Format()), str.format() and PyUnicode_FromFormat(). The code for str.format() is scattered between Objects/stringlib/unicode_format.h and Python/formatter_unicode.c (3000 lines total). The code that you moved here is for str.__mod__() (1000 lines). How many lines are for PyUnicode_FromFormat()?

Do we want all three formatters in one file or in three separate files? In any case I would start with merging Objects/stringlib/unicode_format.h and Python/formatter_unicode.c into a single file.

[vstinner]

How many lines are for PyUnicode_FromFormat()?

In PR gh-139354, PyUnicode_FromFormat() takes 657 lines.

Do we want all three formatters in one file or in three separate files?

I propose to have one file per formatter. IMO it's already tedious to navigate in a 1000 lines file (unicode_format.c).

In any case I would start with merging Objects/stringlib/unicode_format.h and Python/formatter_unicode.c into a single file.

I started with PyUnicode_Format() since it's easy to split it. It has few "dependencies" to other unicodeobject.c functions.

I can create another PR for Python/formatter_unicode.c.

[serhiy-storchaka]

Then what will be the names for three files?

Some code can be shared between formatters, so we may add the forth file for shared code and the fifth file for the header.

[vstinner]

Then what will be the names for three files?

In PR #139354, there are:

• Objects/unicode_fromformat.c: PyUnicode_FromFormat()
• Objects/unicode_format.c: PyUnicode_Format()

For the 3rd one (Python/formatter_unicode.c), it can be Objects/unicode_formatter.c or another name. Or it can stay in Python/formatter_unicode.c.

Some code can be shared between formatters, so we may add the forth file for shared code and the fifth file for the header.

So far, I didn't find any code which is shared between these 3 functions. If later we discover shared code, _PyUnicode functions can be added and code can stay where it is (no need to add an helper C file), and declared in pycore_unicodeobject.h.

[vstinner]

I rebased the PR on the main branch: on top of the "gh-139353: Rename formatter_unicode.c to unicode_formatter.c (#139723)" change.

[vstinner]

@serhiy-storchaka: So what do you think about adding Objects/unicode_format.c?

[serhiy-storchaka]

LGTM. 👍

[vstinner]

Merged, thanks for the review.