Strings now exposed externally as array of UCS-2 codepoints

[natevw]

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" characters in all the places needed? See the original PR thread 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. 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.

[natevw]

Alrighty @tcr this is all yours.

[tcr]

@natevw Reviewing now. I've added a branch of runtime which allows you to attach a byte of metadata to strings: #543 Tested that all like strings ("apple", "app" .. "le") have the same data. The same API can be used on the JIT branch after some ifdefs.

[tcr]

If the expectation is that we'll back out of utf8_proc, can this function be copied and moved to tm_utf8? I'm interested in keeping the deps/ unmodified, or preferably gone.

[natevw]

If go back to ASCII-only .toUpperCase/.toLowerCase (which is not spec, but TBH I'm not sure what real-world use it serves outside of case-insensitive ASCII protocol stuff — since they don't do normalization they aren't properly useful for much "natural language" stuff anyway but perhaps that's debatable…)

Anyway, if all we're using utf8_proc for is for basic UTF-8CESU-8 (and occasionally UTF-8) iteration/generation we could easily make our own versions in tm_str and drop it.

[tcr]

I'll do a more thorough code review tomorrow; I'm very excited by what's here.

As an aside, if you think utf8_proc doesn't add much and will require more modifications, we might as well rip out what we need. In particular, if the upper and lower tables can be recreated in a much smaller data structure, we'll save nearly 300kb in the runtime (no small feat!)

[natevw]

More on utf8_proc: I didn't realize until yesterday that this very branch is where utf8_proc and the Unicode-savvy upper/lower casing was introduced. The custom modification definitely felt dirty, but conveniently the dep wasn't a proper submodule and YOLO right?

So given that ASCII-only upper/lower would not be a regression, why don't we:

1. implement the two CESU-8/UTF-8 functions we actually need for the core string encoding work (bytes→uchar, uchar→bytes)
2. remove utf8_proc dep, leave ourselves with ASCII-only case changing for now
3. look into incorporating the code tables later (and in the context of full ES6-level Unicode support or even Intl…locale stuff is simply going to be a lot of data but happy to look into how tightly we can pack a good subset)

Should I go ahead and do the first two steps on this PR?

[tcr]

Tangent: taking a short break, decided to see how good GCC (well, clang) is at compressing uppercase/lowercase tables: https://github.com/tcr/unicode-tables

We can handle this later though, plain ASCII is good for now.

[natevw]

Alright, utf8proc is gone. We now have tm_utf8_decode/tm_utf8_encode functions which are used for both UTF-8 and CESU-8 needs. String casing now just crosses its fingers and uses lua.upper/lua.lower.

I noticed that our implementation of tm_str_codeat is outdated (includes unnecessary manual handling of UTF-8 pairs that are no longer possible) and tm_str_fromcode is a bit sketch (implicit buf_len and input codepoint range check) — I may try clean up those yet too but they are "correct" within current usage.

[tcr]

Rebased over master and removed utf8proc separately so this PR remains slim.

[tcr]

Okay, I've re-read through the patch. I'm happy with its current state and think that we can tackle individual issues in separate issues:

• Add string UTF-8 and UCS-2 length as internal properties of Lua string
• Optimize tm_str_codeat and tm_str_fromcode
• Expose colony_ functions for external (really just internal, but clean) use.
• Flag strings as ASCII-only as an optimization.
• Speed up iteration/decode with optimized C functions.

The blockers for getting this merged are creating those issues on the issue tracker, and resolving the Travis commit that's broken (it's complaining -Weverything style about signedness). Once that's fixed, we can attempt merge on Rampart.

[natevw]

Just pushed a quick fix to the signedness trouble, but unfortunately seems the rebase broke the build somehow:

$ make colony
gyp  colony.gyp --depth=. -f ninja -D enable_ssl=1 -D enable_net=1 -D compiler_path="/Users/natevw/Desktop/Clients/Technical_Machine/runtime/node_modules/colony-compiler/bin/colony-compiler.js" && ninja -C out/Release
ninja: Entering directory `out/Release'
ninja: error: '../../src/colony/modules/events.js', needed by 'gen/dir_builtin.c', missing and no known rule to make it
make: *** [colony] Error 1

UPDATE: removed the offending line from libcolony.gyp and was able to "successfully" build but it's somehow horribly uninitialized when actually running:

$ colony test/suite/string.js 
(error traceback is not a string)
lua_pcall error 2

./test/tap.js:8: attempt to index global 'console' (a nil value)
[T]: src/colony/lua/colony-node.lua:71: attempt to index field 'process' (a nil value)
[T]: src/colony/lua/colony-node.lua:71: attempt to index field 'process' (a nil value)
[T]: src/colony/lua/colony-node.lua:71: attempt to index field 'process' (a nil value)
[T]: src/colony/lua/colony-node.lua:71: attempt to index field 'process' (a nil value)
[T]: src/colony/lua/colony-node.lua:71: attempt to index field 'process' (a nil value)
[T]: src/colony/lua/colony-node.lua:71: attempt to index field 'process' (a nil value)
[T]: src/colony/lua/colony-node.lua:71: attempt to index field 'process' (a nil value)
…

Wish I hadn't deleted my properly merged local branch to "force pull" over your rebase :-(

[natevw]

@tcr — I'm not going to keep playing whack-a-mole on this. Unless it's actually working fine and I just somehow have a bad working copy, can you please get npm test passing again on this branch?

[tcr]

The last commit should fix one rebase issue, .n vs .length.

Your specific issue: run make update before you make colony. That's a temporary but needed fix, it adds events.js and domain.js from the deps/node-libs folder.

I can not rebase branches in the future if that's going to be an issue. Sorry, tests passed fine on my machine after the rebase.

[tcr]

Final rebase. Tests are not passing on Tessel, bisecting now.

[tm-rampart]

Approved by @tcr. Running tests.

[tcr]

Landed! Beautiful! On to smaller patches.