The thing I was missing is that encode_utf8 chains to encode which munges the SVf_UTF8 flag to be off. It was also throwing me off the scent to see that we added a is_utf8 check -- confusing me, because that changes the semantics of the loop. Before it was 'unconditionally turn this off' now it's 'turn this off if the data is already valid utf8'.
If the user submits non-utf8 data, it seems like the behavior of this loop is going to change. This may not actually affect us, though, because we might be doing checks for valid input earlier on or later... but still, it just looks like we're changing how it operates.
I didn't miss that part -- and I totally agree. But this seems like a fairly random swap from one method of accomplishing the utf8 flag removal to another method, with an added bonus of being odd code ('each' instead of iterating over 'keys'?), not following the style guide, and apparently being semantically different (with the is_utf8 check thrown in).
I fully admit that I only spent about 20 minutes looking at it. But that's all the time I had, at which point I threw an exception and asked for help to try to understand what this is doing.
no subject
The thing I was missing is that encode_utf8 chains to encode which munges the SVf_UTF8 flag to be off. It was also throwing me off the scent to see that we added a is_utf8 check -- confusing me, because that changes the semantics of the loop. Before it was 'unconditionally turn this off' now it's 'turn this off if the data is already valid utf8'.
If the user submits non-utf8 data, it seems like the behavior of this loop is going to change. This may not actually affect us, though, because we might be doing checks for valid input earlier on or later... but still, it just looks like we're changing how it operates.
I didn't miss that part -- and I totally agree. But this seems like a fairly random swap from one method of accomplishing the utf8 flag removal to another method, with an added bonus of being odd code ('each' instead of iterating over 'keys'?), not following the style guide, and apparently being semantically different (with the is_utf8 check thrown in).
I fully admit that I only spent about 20 minutes looking at it. But that's all the time I had, at which point I threw an exception and asked for help to try to understand what this is doing.