Proof-of-concept matchPrefix on Pattern.

Proof-of-concept matchPrefix on Pattern.

Can be used to implement startsWith (and endsWith with a loop).

R=floitsch@google.com

Committed: https://code.google.com/p/dart/source/detail?r=23897

[Lasse Reichstein Nielsen, 2013-06-11 11:11:21]

I think this would be acceptable.

[floitsch, 2013-06-11 12:45:24]

LGTM with tests.

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

https://codereview.chromium.org/15709018/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/js_string.dart:33: for (int i = 0;
i < this.length; i++) {
TODO that this could be made much faster.

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

https://codereview.chromium.org/15709018/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/regexp_helper.dart:106: if
(match._groups[match._groups.length - 1] is String) return null;
isn't the groups entry 'null' if it hasn't matched?

https://codereview.chromium.org/15709018/diff/1/sdk/lib/core/pattern.dart
File sdk/lib/core/pattern.dart (right):

https://codereview.chromium.org/15709018/diff/1/sdk/lib/core/pattern.dart#new...
sdk/lib/core/pattern.dart:9: Match matchPrefix(String string, [int index = 0]);
matchAsPrefix?

string.matchAsPrefix(otherString) somehow reads more like "match the left as
prefix of the other", than just string.matchPrefix(otherString).

[Lasse Reichstein Nielsen, 2013-06-12 08:37:49]

Committed patchset #3 manually as r23897 (presubmit successful).

[Lasse Reichstein Nielsen, 2013-06-12 09:24:05]

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

https://codereview.chromium.org/15709018/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/js_string.dart:33: for (int i = 0;
i < this.length; i++) {
I'm not sure it can be made that much faster. If it's translated to similar JS
code, I think an optimizing VM like V8 should do fine on it.

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

https://codereview.chromium.org/15709018/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/regexp_helper.dart:105: if (match
== null) return null;
This line is really a sanity check - it can't happen since the regexp can match
the empty string.

https://codereview.chromium.org/15709018/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/regexp_helper.dart:106: if
(match._groups[match._groups.length - 1] is String) return null;
That's my assumption too - a non participating capture will have undefined as JS
value.
Let's change it to "!= null" instead of "is String".

https://codereview.chromium.org/15709018/diff/1/sdk/lib/core/pattern.dart
File sdk/lib/core/pattern.dart (right):

https://codereview.chromium.org/15709018/diff/1/sdk/lib/core/pattern.dart#new...
sdk/lib/core/pattern.dart:9: Match matchPrefix(String string, [int index = 0]);
On 2013/06/11 12:45:24, floitsch wrote:
> matchAsPrefix?
> 
> string.matchAsPrefix(otherString) somehow reads more like "match the left as
> prefix of the other", than just string.matchPrefix(otherString).

Done.

[sra1, 2013-06-13 17:30:53]

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

https://codereview.chromium.org/15709018/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/js_string.dart:33: for (int i = 0;
i < this.length; i++) {
On 2013/06/12 09:24:05, Lasse Reichstein Nielsen wrote:
> I'm not sure it can be made that much faster. If it's translated to similar JS
> code, I think an optimizing VM like V8 should do fine on it.

The code is pretty horrible.
dart2js inlines codeUnitAt which contains bounds checks.



  matchAsPrefix$2: function(receiver, string, start) {
    var t1, t2, i, t3;
    if (start < 0 || start > string.length)
      throw $.wrapException(new $.RangeError("value " + $.S(start) + " not in
range 0.." + string.length));
    t1 = receiver.length;
    t2 = string.length;
    if (start + t1 > t2)
      return;
    for (i = 0; i < t1; ++i) {
      t3 = start + i;
      if (t3 < 0)
        $.throwExpression(new $.RangeError("value " + $.S(t3)));
      if (t3 >= t2)
        $.throwExpression(new $.RangeError("value " + $.S(t3)));
      t3 = string.charCodeAt(t3);
      if (i >= t1)
        $.throwExpression(new $.RangeError("value " + i));
      if (t3 !== receiver.charCodeAt(i))
        return;
    }
    return new $.StringMatch(start, string, receiver);
  },

Options:
1. Improve the (hand written) bounds checking elimination
2. Tweak the code by replacing  x.codeUnitAt(i) by JS('int', '#.charCodeAt(#)',
x, i)
3. Change the code to use JavaScript string level operations

I think #2 is fair game.