700752 - Various RegExpPrivate cleanups

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jeff Walden [:Waldo]]

Attachment: Patch

Right now the OS X opt warn-as-errors shell build is red because the compiler can't tell that a RegExpPrivateCache::AddPtr will never be used uninitialized.  I fixed that with some slight refactoring which also moves not-cached cases out-of-line.  I also noticed some type changes mean that we have pointless ensureLinear(cx) calls a few places, so I removed those.  (I have an idea for avoiding such mistakes in the future, but I'll leave that to a separate bug.)

Comment 1

[Chris Leary [:cdleary] (not checking bugmail)]

Comment on attachment 572927 [details] [diff] [review]
Patch

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

Thanks, looks good.

::: js/src/vm/RegExpObject.cpp
@@ +469,4 @@
>      return true;
>  }
>  
> +/* static */ RegExpPrivate *

Nit: I don't think we usually make these comments, do we? Without precedent or a style guide note, I think I'd rather we leave it out.

@@ +469,5 @@
>      return true;
>  }
>  
> +/* static */ RegExpPrivate *
> +RegExpPrivate::createUncached(JSContext *cx, JSLinearString *source, RegExpFlag flags, TokenStream *ts)

Nit: I think we're s/ts/tokenStream these days while we're in here, right?

Comment 2

[Jeff Walden [:Waldo]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d9cc509bfb07

There seems to be a reasonable amount of precedent for /* static */, so I left that as-is.  I did change s/ts/tokenStream/, although for utterly trivial cases like that an initialism doesn't really bother me much.

Comment 3

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

https://hg.mozilla.org/mozilla-central/rev/d9cc509bfb07