Closed Bug 1199887 Opened 6 years ago Closed 6 years ago

Various renaming, code cleanups to do before bug 1187234/bug 1179003


(Core :: JavaScript Engine, defect)

Not set



Tracking Status
firefox43 --- fixed


(Reporter: Waldo, Assigned: Waldo)




(3 files)

This is mostly some dumb stuff noticed around is-class assertions that I removed in bug 1179003.  A little grab-baggy, but pretty minimal.

(Given the size of changes in bug 1179003 and bug 1187234, and my understanding of your school schedule, I held off on requesting review from you, evilpie.  If you want to look/review anyway, go ahead.)
One of the callers checks class directly just prior to calling this.  The other does a proxy-permitting check a little earlier.  Too bad we can't use Handle<RegExpObject*> or something to connote this class-checked status -- at least not with the existing APIs.  Maybe we could invent a template class to convey (X or wrapped X) somehow.  But definitely another bug, if it ever happens.
Attachment #8654465 - Flags: review?(evilpies)
The interpreter-based path where returning as Value makes sense, isn't the primary sense any more.  Return the string that everyone *but* the interpreter wants, directly.

The last patch here does likewise for str_replace_string_raw -- just keeping them separate for smallness.
Attachment #8654466 - Flags: review?(evilpies)
Comment on attachment 8654465 [details] [diff] [review]
Rename StringRegExpGuard::init(JSContext*, JSObject*) to initRegExp

Review of attachment 8654465 [details] [diff] [review]:

::: js/src/jsstr.cpp
@@ +2122,5 @@
>          obj_ = regexp;
>          MOZ_ASSERT(ObjectClassIs(obj_, ESClass_RegExp, cx));
>          if (!RegExpToShared(cx, obj_, &re_))

Can you fix this?
Attachment #8654465 - Flags: review?(evilpies) → review+
Comment on attachment 8654466 [details] [diff] [review]
Make str_replace_regexp_raw return a JSString*, rather than return its always-string result via outparam

Review of attachment 8654466 [details] [diff] [review]:

Very nice.
Attachment #8654466 - Flags: review?(evilpies) → review+
Attachment #8654467 - Flags: review?(evilpies) → review+
You need to log in before you can comment on or make changes to this bug.