Useful for testing.

Forgot to qref.

This patch uses AutoAssertNoGC, but I really want AutoCheckCannotGC (bug 1013531). Terrence, could one of the GC people take bug 1013531 this week? I want to convert a lot of code the coming days/weeks, so it'd be great to have this work as expected.

Comment on attachment 8428632 [details] [diff] [review] Patch Review of attachment 8428632 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit-test/tests/basic/latin1.js @@ +10,5 @@ > + assertEq(s == "foo02", false); > + > + // Non-atomized to force char comparison. > + var nonAtomized = "afoo01".substr(1); > + assertEq(isLatin1(nonAtomized), false); Later, substr() should return a latin1 string right? To prepare for that, perhaps you should make the first letter a unicode character? ::: js/src/jsstr.cpp @@ +4201,5 @@ > +static bool > +EqualCharsLatin1TwoByte(const char *s1, const jschar *s2, size_t len) > +{ > + for (const char *s1end = s1 + len; s1 < s1end; s1++, s2++) { > + if (jschar(*s1) != *s2) I think there is an issue here in that it isn't defined whether 'char' is signed or unsigned which could make this cast to jschar to weird things. Can you explicitly cast to 'unsigned char' first and a testcase to latin1.js? ::: js/src/vm/String.h @@ +749,5 @@ > + > + MOZ_ALWAYS_INLINE > + const char *inlineLatin1Chars(const JS::AutoAssertNoGC &nogc) const { > + JS_ASSERT(hasLatin1Chars()); > + return d.inlineStorageLatin1; Since these are members of JSInlineString, could the 'inline' prefix be dropped? In effect, these are just optimized versions of JSLinearString::chars, so it seems "safe".

(In reply to Jan de Mooij [:jandem] from comment #2) > This patch uses AutoAssertNoGC, but I really want AutoCheckCannotGC (bug > 1013531). > > Terrence, could one of the GC people take bug 1013531 this week? I want to > convert a lot of code the coming days/weeks, so it'd be great to have this > work as expected. On m-i. Should be good to go, unless it bounces.

And it bounced. :-(

https://hg.mozilla.org/integration/mozilla-inbound/rev/062236a88303 (In reply to Luke Wagner [:luke] from comment #3) > I think there is an issue here in that it isn't defined whether 'char' is > signed or unsigned which could make this cast to jschar to weird things. > Can you explicitly cast to 'unsigned char' first and a testcase to latin1.js? Ah, good catch. I modified the test to fail without this change. Bug 1018243 makes latin1 chars unsigned and removes the cast again. > Since these are members of JSInlineString, could the 'inline' prefix be > dropped? In effect, these are just optimized versions of > JSLinearString::chars, so it seems "safe". Done.