501265 - Shell load() interprets file as not-UTF-8 (always ASCII, I think)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jeff Walden [:Waldo]]

Sputnik seems to assume that files loaded from disk are interpreted as UTF-8, which affects the behavior of a test with a UTF-8-encoded non-breaking space in it.  At a glance I think we effectively load files as though they were the equivalent file with every char in it exploded into a two-byte UCS-2 character, so high-bit-set UTF-8 bytes make us fall over and die.  Since the shell's just our own custom repl, how about if we switch it over to parsing input entirely as UTF-8?  Breaking change, sure -- but it's our baby, and I see no reason to waste much effort on making the shell handle multiple encodings, not when we could be fixing actual bugs instead.

This affects at a minimum the following Sputnik test:

07_Lexical_Conventions\7.2_White_Space\S7.2_A1.5_T2

Note that the eval in there runs correctly, it's only the top-level file parsing that falls over.

Comment 1

[Brendan Eich [:brendan]]

No free lunch, switching to treat input as UTF-8 will prolly break something or things, even our js/tests. But, sounds like it could be a net win in spite of any such breakage. Cc'ing Wes, who cares about UTF-8 input.

/be

Comment 2

[Brendan Eich [:brendan]]

Not just load, though -- why not all source file reading by the shell?

/be

Comment 3

[Jason Orendorff [:jorendorff]]

The load builtin calls JS_CompileFile, so I think this depends-on/duplicates bug 495790.

Comment 4

[Jeff Walden [:Waldo]]

(In reply to comment #2)
> Not just load, though -- why not all source file reading by the shell?

Certainly all source, by -f and by load(), pretty sure Sputnik just splotches the code to run in a file and then concats that to the end of the JS shell command.


(In reply to comment #3)
> I think this depends-on/duplicates bug 495790.

I think so, except that that was making load-as-UTF-8 conditional on JS_CStringsAreUTF8(), subtle difference.  Wes, if you don't have time to get to this or don't feel like grunging through the encoding fun, I'm fine picking it up from you.

Comment 5

[Terrence Cole [:terrence]]

Attachment: v0

This is trivial to implement now.  The attached one-liner passes all jit-tests.  I've started a try run (https://tbpl.mozilla.org/?tree=Try&rev=7cab4f0eac06) but I don't expect any major problems to crop up there.

Comment 6

[Jeff Walden [:Waldo]]

Comment on attachment 678830 [details] [diff] [review]
v0

Review of attachment 678830 [details] [diff] [review]:
-----------------------------------------------------------------

rs=me

Comment 7

[Terrence Cole [:terrence]]

Green try run at:
https://tbpl.mozilla.org/?tree=Try&rev=7cab4f0eac06

Pushed at:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e092cd2b3205

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e092cd2b3205