Small cleanups in RegExp to avoid generating lots of code just because there is a try/catch in our …

Small cleanups in RegExp to avoid generating lots of code just because there is a try/catch in our RegExp implementation.
Committed: https://code.google.com/p/dart/source/detail?r=19037

[ngeoffray, 2013-02-25 12:46:59]

https://codereview.chromium.org/12310103/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/lib/string_helper.dart (right):

https://codereview.chromium.org/12310103/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/string_helper.dart:97: var quoter
= JS('', "new RegExp(#, 'g')", r'[-[\]{}()*+?.,\\^$|#\s]');
I believe quoter and replacer will not throw a JS RegExp exception. Peter, can
you confirm?

[floitsch, 2013-02-25 13:42:34]

LGTM.

https://codereview.chromium.org/12310103/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/lib/regexp_helper.dart (right):

https://codereview.chromium.org/12310103/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/regexp_helper.dart:82:
JSSyntaxRegExp._globalVersionOf(JSSyntaxRegExp other)
move up to other constructors.

[ahe, 2013-02-26 10:18:21]

LGTM!

https://codereview.chromium.org/12310103/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/lib/regexp_helper.dart (right):

https://codereview.chromium.org/12310103/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/regexp_helper.dart:28: static
_makeNative(
This doesn't need to be private.

https://codereview.chromium.org/12310103/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/regexp_helper.dart:31:
StringBuffer sb = new StringBuffer();
It does not make sense to use a StringBuffer.

https://codereview.chromium.org/12310103/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/lib/string_helper.dart (right):

https://codereview.chromium.org/12310103/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/string_helper.dart:97: var quoter
= JS('', "new RegExp(#, 'g')", r'[-[\]{}()*+?.,\\^$|#\s]');
On 2013/02/25 12:46:59, ngeoffray wrote:
> I believe quoter and replacer will not throw a JS RegExp exception. Peter, can
> you confirm?

It does not throw an exception, it is a valid regular expression (and if not, a
lot of tests will break).

https://codereview.chromium.org/12310103/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/string_helper.dart:99: var
replacer = JS('', "new RegExp(#, 'g')", quoted);
This shouldn't throw either.  The quoter takes care of that.

[ngeoffray, 2013-02-26 11:40:34]

Thanks Florian and Peter.

Note that this change also revealed a bug in our SSA optimizer (that we did not
hit before because of tree shaking): when replacing an instruction with another
and narrowing the type to the intersection of both instructions' types, we did
not check if the type was conflicting. Since conflicting also means "giving up
doing the intersection", and we should not have a conflicting type in the graph,
I now explicitly check the intersection for conflicting.

https://codereview.chromium.org/12310103/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/lib/regexp_helper.dart (right):

https://codereview.chromium.org/12310103/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/regexp_helper.dart:28: static
_makeNative(
On 2013/02/26 10:18:21, ahe wrote:
> This doesn't need to be private.

Done.

https://codereview.chromium.org/12310103/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/regexp_helper.dart:31:
StringBuffer sb = new StringBuffer();
On 2013/02/26 10:18:21, ahe wrote:
> It does not make sense to use a StringBuffer.

Done.

https://codereview.chromium.org/12310103/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/regexp_helper.dart:82:
JSSyntaxRegExp._globalVersionOf(JSSyntaxRegExp other)
On 2013/02/25 13:42:34, floitsch wrote:
> move up to other constructors.

Done.

[ngeoffray, 2013-02-26 11:55:06]

Committed patchset #2 manually as r19037 (presubmit successful).