I would prefer to test the length first, to make the code more readable.

Like:

if (strlen(str) == len) {    return 1;}return memcmp(...);

Same below.

It is the same in_PyUnicode_EqualToASCIIString().

How

if (!a) {return0;}returnb;

is more readable than simplereturn a && b;? It is what the&& operator for.

is more readable than simple return a && b;?

For me, it's easier to reason about a single test per line when I review code.

Keepa && b if you prefer.

The readability problem as I see it, is that your&& use has side effects; it is not a pure logic expression.

For me, it's easier to reason about a single test per line when I review code.

Fortunately, every condition here is already on a separate line.

The readability problem as I see it, is that your&& use has side effects; it is not a pure logic expression.

It is how&& works in C. There is a lot of code likearg != NULL and PyDict_Check(arg) && PyDict_GET_SIZE(arg) > count. I do not think rewriting it in threeifs withgotos can improve readability.

It looks to me, that "return true" is more often used than "return non-zero". In this case it is more accurate, because it always returns 1, not other non-zero value. Perhaps other functions which return non-zero was a macro that returned not 1 (something like(arg->flags & FLAG))?

unsigned char byte1 = (0xc0 | (ch >> 6));unsigned char byte2 = (0x80 | (ch & 0x3f));if (str[0] != byte1 || str[1] != byte2) return 0;

And declare astr variable asunsigned char* once to avoid casting str at each byte comparison.

If the first comparison fails, you do not need to calculate the second byte. The code looks more compact and uniform in the way it is written right now. All expressions I copied from the UTF-8 encoder which I wrote 11 years ago, so no need to recheck them. Casting to unsigned char is not a large burden, but if you prefer, I can introduce a newunsigned char* variable.

Maybe "ASCII encoded"?

is more readable than simple return a && b;?

For me, it's easier to reason about a single test per line when I review code.

Keepa && b if you prefer.

The readability problem as I see it, is that your&& use has side effects; it is not a pure logic expression.

No, it does not work so.

NULL is defined as None, andequaltoutf8('abc', None) is a TypeError.

Ifequaltoutf8() is called with only one argument, it sets the second argument forPyUnicode_EqualToUTF8() to NULL, so we can test it and ensure that it indeed crashes. It is a common approach used in other tests in this file forconst char * argument. Some functions do not crash, but raise exception or return successfully for NULL, but this function simply crashes in debug build.

Oh ok, I thought that they were just pseudo-code as comments. Sure, you can leave# NULL if you prefer.

Hmm, I copied this pattern from the test forPyUnicode_CompareWithASCIIString() which was one of the first written tests. In newer tests I use "z#" which allows to pass None for NULL. Or perhaps I changed this everywhere except the test forPyUnicode_CompareWithASCIIString(). So perhaps I can change this too.

Oh ok, I thought that they were just pseudo-code as comments. Sure, you can leave# NULL if you prefer.

I suppose that if we return true at this point then we know thatstr is the utf8 representation ofunicode, does it make sense to copy the contents intounicode->utf8 so that future operations can fast-path without needing to encode again?

It needs a separate research and discussion. The disadvantage is that it increases the consumed memory size, also it consumes some CPU time, so the benefit will be only if the UTF-8 cache is used in future.

If the idea turned out to be good, it can simply be implemented in the future.

Makes total sense. I guess this also sits in an awkward place where it's likely that the user is best suited to know whether or not they want the utf-8 cache populated, but it's also an implementation detail that we don't really want to expose to users.For now I'll just mark this comment as resolved. Edit I can't, probably lack permissions I guess.

PyUnicode_EqualToUTF8() doesn't raise exception and cannot fail. Trying to allocate memory should not raise memory, but it sounds like a non-trivial side effect.

Worst case: 1 GB string, you call PyUnicode_EqualToUTF8() andsuddenly, Python allocates 1 GB more. I would besurprised by this behavior.

Maybe it's worth it to add a comment explaining why we don't cache the UTF-8 encoded string.

What do you think about renamingstring toutf8_str? Theutf8_ would be another way to document that it's expected to be encoded to UTF-8 and also it's easier (for me) to distinguish that the second argument is a bytes string, sincestring name is quite generic.

It is a part of the bigger issue. See#62897.

Unrelated question, but is there a plan to generate this file fromDoc/data/stable_abi.dat or the reverse?

I think thatDoc/data/stable_abi.dat is generated fromMisc/stable_abi.toml.

Ah, ok, thank you!