Output string literals with CESU-8 sequences #32
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ork if colony used as library in unsuitable environment, though…)
natevw
changed the title
Contributor
Author
|
Although I say "coordinated" above, AFAICT this could be integrated right now with no particular change to the correctness/incorrectness of 'master' branch. Basically this needs to go out either before or alongside the runtime changes. |
Member
|
Approved. r+ |
tcr
added a commit
that referenced
this pull request
Oct 7, 2014
Output string literals with CESU-8 sequences
tm-rampart
added a commit
to tessel/t1-runtime
that referenced
this pull request
Oct 10, 2014
…odepoints This fixes most of colony's String compatibility issues stemming from the mismatch between JavaScript's pre–Unicode 2.0, and Lua's pre-historic, handling of Unicode string representations. ## Background Javascript's string object — like many of its era — was intended represent "Unicode" strings. However, when Unicode 2.0 was introduced it changed the definition of a codepoint (± think "character") so that it no longer fit within the16-bit unsigned integer type which `String` was designed around. (I'd wager that most JS code running out there still does not bother to process characters in the supplementary planes correctly; perhaps justifiably so, in a pre-ES6 world.\*\*) In light of this history, it is *now* fair to say that Javascript's strings are merely immutable buffers storing a block of 16-bit values; they are essentially "raw" UTF-16/UCS-2 encoded data. Lua's strings are basically immutable buffers storing a block of 8-bit values, and (before this patch) colony was using these to store UTF-8 string data. This string representation discrepancy (between JavaScript's UCS-2 and Colony's UTF-8) meant that only ASCII strings were fully compatible between implementations — even characters in the BMP would cause a mismatch in string lengths. For example, `"€5".length` equalled 2 in V8 (`[0x20ac, 0x0035]`), but 4 in colony (`[0xe2, 0x82, 0xac, 0x35]`). There were a few options for remedying this discrepancy. Storing UTF-16 to Lua's string blocks? Keeping UTF-8 internally and splitting ["astral"](https://mathiasbynens.be/notes/javascript-unicode) characters in all the places needed? See the [original PR thread](#137) for some discussion. In the end something of a compromise/hybrid approach was chosen. ## How strings are handled in Colony now This pull request changes Colony's internal string representation to [CESU-8](http://en.wikipedia.org/wiki/CESU-8). This has the advantage of being as compact as UTF-8 for BMP characters, but also keeping surrogate pairs split as UTF-16 does. So we can trivially match JS's ability to extract `"👀"[1]` while also (in theory\*) handling I/O in the default UTF-8 encoding of node.js as a straight memcpy in common cases. This made it reasonably easy to implement the basic string methods/properties — those taking in a UCS-2 index convert it to a CESU-8 index ("JsToLua") before calling the Lua methods. Those needing to return a UCS-2 index can convert the opposite direction ("LuaToJs") from a Lua method result. Outside of string itself, code needed to be audited and in many cases corrected to make sure it distinguished between an "internal colony string" (which should be ± treated as opaque, unless accessed via JS methods) and externally needed representations (usually UTF-8). ## Miscellany This patch **depends on tessel/colony-compiler#32 for pre-converting string literals into the correct internal representation. This patch also adds toLowerCase/toUpperCase methods, which may not work quite right in case of strings that get longer on case change. (Personally I @natevw wonder if we could just implement these ASCII-only; this would basically let us get rid of the utf8proc dependency and its concomitant code tables.) This patch **does not** finish adding/auditing all the encoding handling required of the `Buffer` object. `Buffer.prototype.toString` should be mostly correct, but does not yet handle 'utf16le'. And `new Buffer(str, enc)` is still in pretty bad shape. IMO that work is important but belongs in an additional patch once this lands. This patch **does not** fix RegExp index values, which were completely missing throughout most of this work and so were deemed out of scope. LA LA LA CAN'T SEE THE PULL REQUEST THAT TRIES TO ADD THOSE LA LA IGNORING LA LA LA LA \* Right now no optimizations are done. One simple one was added (for ASCII↔︎CESU-8 conversions) and led to a slight performance *regression* running `npm test` and was backed out. Right now all string "lookups" are O(m) based on target index. This will especially kill code that loops character-by-character through large strings; also note that each access off `.length` is unmitigatedly O(n). \*\* ES6 adds a number of facilities to help with *full* (post-2.0) Unicode support e.g. `String.fromCodePoint` and an extended literal character escaping syntax. This patch focuses on basic correctness and does not add support for these new methods/syntaxes.
tm-rampart
added a commit
to tessel/t1-runtime
that referenced
this pull request
Oct 11, 2014
…odepoints This fixes most of colony's String compatibility issues stemming from the mismatch between JavaScript's pre–Unicode 2.0, and Lua's pre-historic, handling of Unicode string representations. ## Background Javascript's string object — like many of its era — was intended represent "Unicode" strings. However, when Unicode 2.0 was introduced it changed the definition of a codepoint (± think "character") so that it no longer fit within the16-bit unsigned integer type which `String` was designed around. (I'd wager that most JS code running out there still does not bother to process characters in the supplementary planes correctly; perhaps justifiably so, in a pre-ES6 world.\*\*) In light of this history, it is *now* fair to say that Javascript's strings are merely immutable buffers storing a block of 16-bit values; they are essentially "raw" UTF-16/UCS-2 encoded data. Lua's strings are basically immutable buffers storing a block of 8-bit values, and (before this patch) colony was using these to store UTF-8 string data. This string representation discrepancy (between JavaScript's UCS-2 and Colony's UTF-8) meant that only ASCII strings were fully compatible between implementations — even characters in the BMP would cause a mismatch in string lengths. For example, `"€5".length` equalled 2 in V8 (`[0x20ac, 0x0035]`), but 4 in colony (`[0xe2, 0x82, 0xac, 0x35]`). There were a few options for remedying this discrepancy. Storing UTF-16 to Lua's string blocks? Keeping UTF-8 internally and splitting ["astral"](https://mathiasbynens.be/notes/javascript-unicode) characters in all the places needed? See the [original PR thread](#137) for some discussion. In the end something of a compromise/hybrid approach was chosen. ## How strings are handled in Colony now This pull request changes Colony's internal string representation to [CESU-8](http://en.wikipedia.org/wiki/CESU-8). This has the advantage of being as compact as UTF-8 for BMP characters, but also keeping surrogate pairs split as UTF-16 does. So we can trivially match JS's ability to extract `"👀"[1]` while also (in theory\*) handling I/O in the default UTF-8 encoding of node.js as a straight memcpy in common cases. This made it reasonably easy to implement the basic string methods/properties — those taking in a UCS-2 index convert it to a CESU-8 index ("JsToLua") before calling the Lua methods. Those needing to return a UCS-2 index can convert the opposite direction ("LuaToJs") from a Lua method result. Outside of string itself, code needed to be audited and in many cases corrected to make sure it distinguished between an "internal colony string" (which should be ± treated as opaque, unless accessed via JS methods) and externally needed representations (usually UTF-8). ## Miscellany This patch **depends on tessel/colony-compiler#32 for pre-converting string literals into the correct internal representation. This patch also adds toLowerCase/toUpperCase methods, which may not work quite right in case of strings that get longer on case change. (Personally I @natevw wonder if we could just implement these ASCII-only; this would basically let us get rid of the utf8proc dependency and its concomitant code tables.) This patch **does not** finish adding/auditing all the encoding handling required of the `Buffer` object. `Buffer.prototype.toString` should be mostly correct, but does not yet handle 'utf16le'. And `new Buffer(str, enc)` is still in pretty bad shape. IMO that work is important but belongs in an additional patch once this lands. This patch **does not** fix RegExp index values, which were completely missing throughout most of this work and so were deemed out of scope. LA LA LA CAN'T SEE THE PULL REQUEST THAT TRIES TO ADD THOSE LA LA IGNORING LA LA LA LA \* Right now no optimizations are done. One simple one was added (for ASCII↔︎CESU-8 conversions) and led to a slight performance *regression* running `npm test` and was backed out. Right now all string "lookups" are O(m) based on target index. This will especially kill code that loops character-by-character through large strings; also note that each access off `.length` is unmitigatedly O(n). \*\* ES6 adds a number of facilities to help with *full* (post-2.0) Unicode support e.g. `String.fromCodePoint` and an extended literal character escaping syntax. This patch focuses on basic correctness and does not add support for these new methods/syntaxes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This changes colony's internal string encoding from UTF-8 to CESU-8.
Will need a coordinated release with tessel/t1-runtime#542.