String.prototype.charAt() currently always reifies JSString. This is wasteful if the JSString is a substring. It also uses jsSingleCharacterSubstring() which is a counter-productive optimization, so let's remove that.
Created attachment 253651 [details] Patch
Comment on attachment 253651 [details] Patch Clearing flags on attachment: 253651 Committed r184865: <http://trac.webkit.org/changeset/184865>
All reviewed patches have been landed. Closing bug.
Comment on attachment 253651 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=253651&action=review > Source/JavaScriptCore/runtime/StringPrototype.cpp:788 > + StringView string = thisValue.toString(exec)->view(exec); This code is wrong. It takes a temporary reference to a string's backing store, but nothing prevents that backing store from being deleted or garbage collected. This code is only possible because JSString::SafeView will automatically convert to StringView, due to its operator StringView(). It is very weird that the safe type automatically converts to the unsafe type. That makes the safe type unsafe too.
Comment on attachment 253651 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=253651&action=review >> Source/JavaScriptCore/runtime/StringPrototype.cpp:788 >> + StringView string = thisValue.toString(exec)->view(exec); > > This code is wrong. It takes a temporary reference to a string's backing store, but nothing prevents that backing store from being deleted or garbage collected. > > This code is only possible because JSString::SafeView will automatically convert to StringView, due to its operator StringView(). It is very weird that the safe type automatically converts to the unsafe type. That makes the safe type unsafe too. I spotted the same mistake in another patch recently.
crab That operator StringView() is kind of a footgun eh.
Oh, all we need to do is to make the return type be SafeView instead of StringView.
(In reply to comment #4) > It is very weird > that the safe type automatically converts to the unsafe type. That makes the > safe type unsafe too. The job of SafeView is to make code like this OK: function(value->view(exec)); It doesn’t make code with local variables safe. Perhaps we can find an even better solution.
(In reply to comment #7) > Oh, all we need to do is to make the return type be SafeView instead of > StringView. local variable type, not return type Never occurred to me to actually use SafeView as a local variable.