JSString's fields are currently public. This is because we need to statically initialize various strings. It also means that JSString doesn't inherit from js::gc::Cell. Luke suggested that we could introduce a StringStaticInitializer class that has public fields that mirror JSString's (for flat atomized strings, which are the only kind that are statically initialized), use that for the statically initialized strings, and then cast them to JSString. We'd need static assertions to ensure the two types have identical layouts. Then JSString's fields could become private, and we could strengthen the safety of field accesses, which is good because JSString's fields are very complicated (eg. there are multiple unions) due to all the different kinds of string. JSString could also inherit from js::gc::Cell and some special-casing could be removed (including Cell::asCell(), IIUC).
Created attachment 493185 [details] [diff] [review] patch (agains TM 58010:9123f97f059c) This patch: - Introduces JSStaticString, which has identical layout to (flat, short) JSString, and converts all the static strings to use the new type. - Casts JSStaticString to JSString where necessary. - Marks the fields in JSString as private. Some new member functions (eg. externalType(), charsOffset()) were required for this. - Makes JSString a sub-class of js::gc::Cell. This allowed Cell::asCell() to be removed. A few |thing->asCell()| occurrences -- ones compared against FreeCell pointers -- had to become |static_cast<Cell *>(thing)|. - In js_IntToString(), does the conversion into a temporary buffer and then copies the result into the short string. This allowed some of the weirder functions -- getINlineStorageBeforeInit(), initAtOffsetInBuffer() -- to be removed. Instruction counts are changed only infinitesimally. - Changes JSShortString to be a sub-class of JSString (with extra storage) rather than containing a JSString. I did this because when it contained a JSString *and* both JSString and JSShortString were subclasses of js::gc::Cell, its size increased by a word -- I'm not sure why -- and the static assertions about layout failed. Making JSShortString a sub-class makes it more like JSExternalString which isn't a bad thing. - Adds JSStringLayoutChecks, a class for holding all the static asserts about the layouts of JSString, JSShortString, JSStaticString. It's a friend of JSString and JSShortString so it can access their private fields. The patch will need a bit of updating after bug 613457 lands.