GeezerVison

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) {
    return 0;
}
return b;

is more readable than simple return 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.

Keep a && 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 like arg != NULL and PyDict_Check(arg) && PyDict_GET_SIZE(arg) > count. I do not think rewriting it in three ifs with gotos 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 a str variable as unsigned 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 new unsigned 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.

Keep a && 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, and equaltoutf8('abc', None) is a TypeError.

If equaltoutf8() is called with only one argument, it sets the second argument for PyUnicode_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 for const 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 for PyUnicode_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 for PyUnicode_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 that str is the utf8 representation of unicode, does it make sense to copy the contents into unicode->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() and suddenly, Python allocates 1 GB more. I would be surprised 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 renaming string to utf8_str? The utf8_ 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, since string 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 from Doc/data/stable_abi.dat or the reverse?

I think that Doc/data/stable_abi.dat is generated from Misc/stable_abi.toml.

Ah, ok, thank you!