Bug 1199887 - Make str_replace_regexp_raw return a JSString*, rather than return its always-string result via outparam. r=evilpie
authorJeff Walden <jwalden@mit.edu>
Wed, 26 Aug 2015 11:58:41 -0700
changeset 261075 9de14ef7802e2bb614ac09a3a0e1c81ea7c0e307
parent 261074 dde65be15e240024e41a25c4a7ac009dbb82675c
child 261076 e0d198f50e59bcaebea8fa4eaa27d647f5eb8356
push id29333
push userphilringnalda@gmail.com
push dateSun, 06 Sep 2015 03:20:00 +0000
treeherdermozilla-central@56f5ffd44b7f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersevilpie
bugs1199887
milestone43.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1199887 - Make str_replace_regexp_raw return a JSString*, rather than return its always-string result via outparam. r=evilpie
js/src/jit/Recover.cpp
js/src/jit/VMFunctions.cpp
js/src/jsstr.cpp
js/src/vm/RegExpObject.h
--- a/js/src/jit/Recover.cpp
+++ b/js/src/jit/Recover.cpp
@@ -1049,22 +1049,22 @@ RRegExpReplace::RRegExpReplace(CompactBu
 { }
 
 bool
 RRegExpReplace::recover(JSContext* cx, SnapshotIterator& iter) const
 {
     RootedString string(cx, iter.read().toString());
     Rooted<RegExpObject*> regexp(cx, &iter.read().toObject().as<RegExpObject>());
     RootedString repl(cx, iter.read().toString());
-    RootedValue result(cx);
 
-    if (!js::str_replace_regexp_raw(cx, string, regexp, repl, &result))
+    JSString* result = js::str_replace_regexp_raw(cx, string, regexp, repl);
+    if (!result)
         return false;
 
-    iter.storeInstructionResult(result);
+    iter.storeInstructionResult(StringValue(result));
     return true;
 }
 
 bool
 MTypeOf::writeRecoverData(CompactBufferWriter& writer) const
 {
     MOZ_ASSERT(canRecoverOnBailout());
     writer.writeUnsigned(uint32_t(RInstruction::Recover_TypeOf));
--- a/js/src/jit/VMFunctions.cpp
+++ b/js/src/jit/VMFunctions.cpp
@@ -1036,21 +1036,17 @@ CreateDerivedTypedObj(JSContext* cx, Han
 }
 
 JSString*
 RegExpReplace(JSContext* cx, HandleString string, HandleObject regexp, HandleString repl)
 {
     MOZ_ASSERT(string);
     MOZ_ASSERT(repl);
 
-    RootedValue rval(cx);
-    if (!str_replace_regexp_raw(cx, string, regexp.as<RegExpObject>(), repl, &rval))
-        return nullptr;
-
-    return rval.toString();
+    return str_replace_regexp_raw(cx, string, regexp.as<RegExpObject>(), repl);
 }
 
 JSString*
 StringReplace(JSContext* cx, HandleString string, HandleString pattern, HandleString repl)
 {
     MOZ_ASSERT(string);
     MOZ_ASSERT(pattern);
     MOZ_ASSERT(repl);
--- a/js/src/jsstr.cpp
+++ b/js/src/jsstr.cpp
@@ -3186,48 +3186,48 @@ AppendSubstrings(JSContext* cx, HandleLi
         /* Appending to the rope permanently roots the substring. */
         if (!rope.append(part))
             return nullptr;
     }
 
     return rope.result();
 }
 
-static bool
-StrReplaceRegexpRemove(JSContext* cx, HandleString str, RegExpShared& re, MutableHandleValue rval)
+static JSString*
+StrReplaceRegexpRemove(JSContext* cx, HandleString str, RegExpShared& re)
 {
     RootedLinearString linearStr(cx, str->ensureLinear(cx));
     if (!linearStr)
-        return false;
+        return nullptr;
 
     Vector<StringRange, 16, SystemAllocPolicy> ranges;
 
     size_t charsLen = linearStr->length();
 
     ScopedMatchPairs matches(&cx->tempLifoAlloc());
     size_t startIndex = 0; /* Index used for iterating through the string. */
     size_t lastIndex = 0;  /* Index after last successful match. */
     size_t lazyIndex = 0;  /* Index before last successful match. */
 
     /* Accumulate StringRanges for unmatched substrings. */
     while (startIndex <= charsLen) {
         if (!CheckForInterrupt(cx))
-            return false;
+            return nullptr;
 
         RegExpRunStatus status = re.execute(cx, linearStr, startIndex, &matches);
         if (status == RegExpRunStatus_Error)
-            return false;
+            return nullptr;
         if (status == RegExpRunStatus_Success_NotFound)
             break;
         MatchPair& match = matches[0];
 
         /* Include the latest unmatched substring. */
         if (size_t(match.start) > lastIndex) {
             if (!ranges.append(StringRange(lastIndex, match.start - lastIndex)))
-                return false;
+                return nullptr;
         }
 
         lazyIndex = lastIndex;
         lastIndex = match.limit;
 
         startIndex = match.isEmpty() ? match.limit + 1 : match.limit;
 
         /* Non-global removal executes at most once. */
@@ -3237,144 +3237,136 @@ StrReplaceRegexpRemove(JSContext* cx, Ha
 
     RegExpStatics* res;
 
     /* If unmatched, return the input string. */
     if (!lastIndex) {
         if (startIndex > 0) {
             res = cx->global()->getRegExpStatics(cx);
             if (!res)
-                return false;
+                return nullptr;
             res->updateLazily(cx, linearStr, &re, lazyIndex);
         }
-        rval.setString(str);
-        return true;
+
+        return str;
     }
 
     /* The last successful match updates the RegExpStatics. */
     res = cx->global()->getRegExpStatics(cx);
     if (!res)
-        return false;
+        return nullptr;
 
     res->updateLazily(cx, linearStr, &re, lazyIndex);
 
     /* Include any remaining part of the string. */
     if (lastIndex < charsLen) {
         if (!ranges.append(StringRange(lastIndex, charsLen - lastIndex)))
-            return false;
+            return nullptr;
     }
 
     /* Handle the empty string before calling .begin(). */
-    if (ranges.empty()) {
-        rval.setString(cx->runtime()->emptyString);
-        return true;
-    }
-
-    JSString* result = AppendSubstrings(cx, linearStr, ranges.begin(), ranges.length());
-    if (!result)
-        return false;
-
-    rval.setString(result);
-    return true;
+    if (ranges.empty())
+        return cx->runtime()->emptyString;
+
+    return AppendSubstrings(cx, linearStr, ranges.begin(), ranges.length());
 }
 
-static inline bool
-StrReplaceRegExp(JSContext* cx, ReplaceData& rdata, MutableHandleValue rval)
+static inline JSString*
+StrReplaceRegExp(JSContext* cx, ReplaceData& rdata)
 {
     rdata.leftIndex = 0;
     rdata.calledBack = false;
 
     RegExpStatics* res = cx->global()->getRegExpStatics(cx);
     if (!res)
-        return false;
+        return nullptr;
 
     RegExpShared& re = rdata.g.regExp();
 
     // The spec doesn't describe this function very clearly, so we go ahead and
     // assume that when the input to String.prototype.replace is a global
     // RegExp, calling the replacer function (assuming one was provided) takes
     // place only after the matching is done. See the comment at the beginning
     // of DoMatchGlobal explaining why we can zero the the RegExp object's
     // lastIndex property here.
     if (re.global() && !rdata.g.zeroLastIndex(cx))
-        return false;
+        return nullptr;
 
     /* Optimize removal. */
     if (rdata.repstr && rdata.repstr->length() == 0) {
         MOZ_ASSERT(!rdata.lambda && !rdata.elembase && rdata.dollarIndex == UINT32_MAX);
-        return StrReplaceRegexpRemove(cx, rdata.str, re, rval);
+        return StrReplaceRegexpRemove(cx, rdata.str, re);
     }
 
     RootedLinearString linearStr(cx, rdata.str->ensureLinear(cx));
     if (!linearStr)
-        return false;
+        return nullptr;
 
     if (re.global()) {
         if (!DoMatchForReplaceGlobal(cx, res, linearStr, re, rdata))
-            return false;
+            return nullptr;
     } else {
         if (!DoMatchForReplaceLocal(cx, res, linearStr, re, rdata))
-            return false;
+            return nullptr;
     }
 
     if (!rdata.calledBack) {
         /* Didn't match, so the string is unmodified. */
-        rval.setString(rdata.str);
-        return true;
+        return rdata.str;
     }
 
     JSSubString sub;
     res->getRightContext(&sub);
     if (!rdata.sb.appendSubstring(sub.base, sub.offset, sub.length))
-        return false;
-
-    JSString* retstr = rdata.sb.finishString();
-    if (!retstr)
-        return false;
-
-    rval.setString(retstr);
-    return true;
+        return nullptr;
+
+    return rdata.sb.finishString();
 }
 
 static inline bool
 str_replace_regexp(JSContext* cx, const CallArgs& args, ReplaceData& rdata)
 {
     if (!rdata.g.normalizeRegExp(cx, true, 2, args))
         return false;
 
-    return StrReplaceRegExp(cx, rdata, args.rval());
+    JSString* res = StrReplaceRegExp(cx, rdata);
+    if (!res)
+        return false;
+
+    args.rval().setString(res);
+    return true;
 }
 
-bool
+JSString*
 js::str_replace_regexp_raw(JSContext* cx, HandleString string, Handle<RegExpObject*> regexp,
-                           HandleString replacement, MutableHandleValue rval)
+                           HandleString replacement)
 {
     /* Optimize removal, so we don't have to create ReplaceData */
     if (replacement->length() == 0) {
         StringRegExpGuard guard(cx);
         if (!guard.initRegExp(cx, regexp))
-            return false;
+            return nullptr;
 
         RegExpShared& re = guard.regExp();
-        return StrReplaceRegexpRemove(cx, string, re, rval);
+        return StrReplaceRegexpRemove(cx, string, re);
     }
 
     ReplaceData rdata(cx);
     rdata.str = string;
 
     JSLinearString* repl = replacement->ensureLinear(cx);
     if (!repl)
-        return false;
+        return nullptr;
 
     rdata.setReplacementString(repl);
 
     if (!rdata.g.initRegExp(cx, regexp))
-        return false;
-
-    return StrReplaceRegExp(cx, rdata, rval);
+        return nullptr;
+
+    return StrReplaceRegExp(cx, rdata);
 }
 
 static inline bool
 StrReplaceString(JSContext* cx, ReplaceData& rdata, const FlatMatch& fm, MutableHandleValue rval)
 {
     /*
      * Note: we could optimize the text.length == pattern.length case if we wanted,
      * even in the presence of dollar metachars.
--- a/js/src/vm/RegExpObject.h
+++ b/js/src/vm/RegExpObject.h
@@ -469,19 +469,19 @@ class RegExpObject : public NativeObject
     RegExpShared* maybeShared() const {
         return static_cast<RegExpShared*>(NativeObject::getPrivate(PRIVATE_SLOT));
     }
 
     /* Call setShared in preference to setPrivate. */
     void setPrivate(void* priv) = delete;
 };
 
-bool
+JSString*
 str_replace_regexp_raw(JSContext* cx, HandleString string, Handle<RegExpObject*> regexp,
-                       HandleString replacement, MutableHandleValue rval);
+                       HandleString replacement);
 
 /*
  * Parse regexp flags. Report an error and return false if an invalid
  * sequence of flags is encountered (repeat/invalid flag).
  *
  * N.B. flagStr must be rooted.
  */
 bool