Move utf8_encode and utf8_decode to ext/standard

[hikari-no-yume]

These are generic string functions which have a use outside of processing XML, and don't have any dependency on libxml. Therefore, this patch moves them to ext/standard.

This patch doesn't touch NEWS and UPGRADING currently. I can do those and merge this myself, if it's approved.

[smalyshev]

looks good! 👍

[nikic]

Why 4?

[smalyshev]

I think 2 should be enough here actually, since from iso-8859-1 to UTF-8 you can't get more than 2 bytes.

[hikari-no-yume]

Good catch. The original function was (needlessly!) more generic and supported encoding 3- and 4-byte characters, which I stripped out, but I missed this.

[hikari-no-yume]

When I copied across the internal encode/decode functions to ext/standard, I simplified the code a little (it's less generic than the ext/xml code). I might want to assess whether ext/xml could possibly share these functions, or at least backport the improvements. I'm not sure how much of their feature set is actually used.

[hikari-no-yume]

On that line of thought: though it's trivial, there's now at least four places in the PHP source where UTF-8 generation is done: the lexer (\u{} literals, this duplication is my fault), ext/xml, ext/standard/string.c (with this pull request) and ext/standard/html.c (php_utf32_utf8). This would be a good opportunity to maybe use the latter function in all these cases, though it might be best to move it to a header for inlining.

(It's interesting that PHP core has a bunch of multibyte stuff in html.c, actually. Should it really live there?…)

[krakjoe]

Can I suggest that this PR stays focused on just moving these string functions to a sensible place, and try to make it "nice" in another PR that addresses the duplication issue ?

[hikari-no-yume]

Yes, that's fair.

[hikari-no-yume]

@yohgaki Do you have any thoughts on this?

[yohgaki]

It seems these functions aimed to support various encodings when it is implemented. It will not happen, I guess. It may be better soft deprecate them. However, I don't have strong opinion on this.

[krakjoe]

I don't think an RFC is required here, would like to see this merged.

Am I wrong about that, do we need an RFC ?

[hikari-no-yume]

Well, it doesn't break backwards-compatibility, and it doesn't even introduce a new feature, it just moves something from one extension to another. It's not even a “bug fix” really.

So… I don't think it requires an RFC? I could just merge this right away if there's no objections.

[hikari-no-yume]

Another change that maybe should be done is renaming the functions and making these names be aliases, because utf8_encode and utf8_decode are not very descriptive names. But of course, aliasing is only meaningful insofar as it means the manual says something different, so we can just edit the manual instead. Aliases would only make much sense if we introduced counterpart functions for other character sets, but I don't think that will happen (these two functions are relics and have better alternatives).

[krakjoe]

I think it's okay to merge the patch as it is, just to move the functions out of xml.

I think if you wanted to tidy up some stuff and create aliases and remove duplication, then that needs an RFC.

I'm happy for this to be merged into master whatever, I'd wait for someone else to +1 that before doing it.

Please remember a news and upgrading entry when you merge.

[nikic]

+1

[hikari-no-yume]

Merged into master, so this will be in PHP 7.2.