1043683 - Make RegExpStatics::makeMatch sane about not taking two arguments, one of which is always double the other (yet has its parity tested)

Status: RESOLVED FIXED

(Core :: JavaScript: Standard Library, defect)

Description

[Jeff Walden [:Waldo]]

In the process of reading bug 369778's patch I realized RegExpStatics::makeMatch is kind of stupid, and we can remove an argument and save substantially on complexity in the process.  We should do that.

Comment 1

[Jeff Walden [:Waldo]]

Attachment: Make RegExpStatics::makeMatch sane about not taking two arguments, one of which is always double the other (yet has its parity tested). NOT REVIEWED YET

Andy, seeing as this is a relatively simple change what with makeMatch being used only in this file, and seeing as you've looked at this code most recently, you're probably the best person to review this, seems to me.  It'll bitrot your patch slightly, but I can review around the changes.  :-)

Comment 2

[Jeff Walden [:Waldo]]

(Protocol on review requests, if you're not yet familiar with it: set the review flag to + if you think the patch is good, or set it to - if you think it's bad.  If you have comments on anything -- better ways to do one thing or another, suggestions for better comment wording, etc. -- feel free to make them in the process.)

Comment 3

[Andy Anderson]

Comment on attachment 8462097 [details] [diff] [review]
Make RegExpStatics::makeMatch sane about not taking two arguments, one of which is always double the other (yet has its parity tested).  NOT REVIEWED YET

Review of attachment 8462097 [details] [diff] [review]:
-----------------------------------------------------------------

Seems fine to me. I'd like it if there were an additional comment demanding parity on the input, but given that this code shouldn't move around too much it's not vital.

Comment 4

[Andy Anderson]

(In reply to Andy Anderson from comment #3)
> Comment on attachment 8462097 [details] [diff] [review]
> Make RegExpStatics::makeMatch sane about not taking two arguments, one of
> which is always double the other (yet has its parity tested).  NOT REVIEWED
> YET
> 
> Review of attachment 8462097 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Seems fine to me. I'd like it if there were an additional comment demanding
> parity on the input, but given that this code shouldn't move around too much
> it's not vital.

It's late. Patch is fine as is, ignore my earlier comments suggesting otherwise.

Comment 5

[Jeff Walden [:Waldo]]

Thanks!  Landed:

https://hg.mozilla.org/integration/mozilla-inbound/rev/5edaa0a67cb8

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5edaa0a67cb8