Do not cache utf8 offsets for non-canonical lengths

[Leont]

In particular, if the length is beyond the end, it should not be stored as the end.

This fixes #18588

[hvds]

Looking through this commit, I'm slightly confused - when is canonical_position ever true?

[Leont]

Looking through this commit, I'm slightly confused - when is canonical_position ever true?

sv_pos_u2b_forwards contains this piece of code.

while (s < send && uoffset) {
    --uoffset;
    s += UTF8SKIP(s);
}

So either it stops because we're at the right offset, or because we're at the end of the string (or both, but that's really just a special case of the former).

In the former case, canonical_position would be true, in the latter it wouldn't.

[hvds]

Looking through this commit, I'm slightly confused - when is canonical_position ever true?

sv_pos_u2b_forwards contains this piece of code.

while (s < send && uoffset) {
    --uoffset;
    s += UTF8SKIP(s);
}

So either it stops because we're at the right offset, or because we're at the end of the string (or both, but that's really just a special case of the former).

In the former case, canonical_position would be true, in the latter it wouldn't.

Thanks, I see my mistake now - I had (repeatedly) read it as *canonical_position = uoffset = 0, rolling two assignments into a single statement rather than *canonical_position = uoffset == 0, a single assignment of the result of a comparison.

In part it was natural for me to read it the wrong way because I would always write such a construct as *canonical_position = (uoffset == 0); I think it would be a good idea to add the parens in this case too.

[khwilliamson]

I had the same initial reaction as @hvds, but did spot the == when I looked more slowly. Parens would have helped me see it immediately. I would be more comfortable with a comment. Unless a bug fix is face-palm worthy, it wasn't obviously a problem, and won't be again 6 months from now

[Leont]

Fair enough, I'll add it