Fix minor ES6 compliance issue in RegExp.prototype.toString and optimize performance a little
My first cut at a patch doesn’t have a regression test because I don’t know where to put it.
Created attachment 254813 [details] Patch
Comment on attachment 254813 [details] Patch Don’t really want to get this reviewed until I add a test for the behavior change.
LGTM, thanks for cleaning up my patch too!
Created attachment 254814 [details] Patch
Created attachment 254844 [details] Patch
Now this has a test. Added a few people who might review the patch.
Comment on attachment 254844 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=254844&action=review > Source/JavaScriptCore/runtime/RegExpPrototype.cpp:140 > +typedef char FlagsString[3 + 1]; // 3 different flags and a null character terminator I'd use std::array for this. > Source/JavaScriptCore/runtime/RegExpPrototype.cpp:142 > +static inline void getFlags(ExecState* exec, JSObject* regexp, FlagsString& flags) I don't think this needs to use the get pattern - it can just return a FlagsString (if FlagsString were an std::array that is). > Source/JavaScriptCore/runtime/RegExpPrototype.cpp:159 > + flags[index] = 0; I'd assert that index < sizeof(FlagsString) here.
(In reply to comment #8) > > Source/JavaScriptCore/runtime/RegExpPrototype.cpp:142 > > +static inline void getFlags(ExecState* exec, JSObject* regexp, FlagsString& flags) > > I don't think this needs to use the get pattern - it can just return a > FlagsString (if FlagsString were an std::array that is). > I just tested, and if a function returns an std::array<char, 4>, the compiler will just put the entire value in a register - no memory access needed!
Committed r185528: <http://trac.webkit.org/changeset/185528>
(In reply to comment #10) > Committed r185528: <http://trac.webkit.org/changeset/185528> I think you need to initialize std::array (You can use std::fill for example).
(In reply to comment #11) > (In reply to comment #10) > > Committed r185528: <http://trac.webkit.org/changeset/185528> > > I think you need to initialize std::array (You can use std::fill for > example). I don’t think I do. If there is an exception, the return value is ignored. If the array is used, we initialize the characters in the string and the null character terminator. We don't need to initialize the rest of the array. Might need to initialize it if we use some strict memory debugger tool or validator perhaps.