Reviewers: Vitaly Repeshko,
Message:
Hi Vitaly,
here is something I noticed when implementing stuff for slices. Please take
a
look.
Cheers,
Yang
http://codereview.chromium.org/7685005/diff/1/src/runtime.cc
File src/runtime.cc (left):
http://codereview.chromium.org/7685005/diff/1/src/runtime.cc#oldcode3154
src/runtime.cc:3154:
This check is obsolete as the caller in string.js already makes sure
that this cannot happen.
http://codereview.chromium.org/7685005/diff/1/src/runtime.cc
File src/runtime.cc (right):
http://codereview.chromium.org/7685005/diff/1/src/runtime.cc#newcode3163
src/runtime.cc:3163:
The same check for cons string exists in Runtime_StringIndexOf. I quote
you:
Possible scenario:
1. a cons is flattened (its first component is a flat ASCII string now)
2. the first component is extracted (we do it in some places to improve.
performance, could be hard to guard against)
3. the first component is externalized.
We have to guard against this case. Though I have to say that I haven't
been able to produce a test case for this scenario using javascript and
the v8-API. Could you give me some pointers here?
Description:
Inserted a missing string encoding check in lastIndexOf.
Please review this at http://codereview.chromium.org/7685005/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files:
M src/runtime.cc
M src/string.js
Index: src/runtime.cc
diff --git a/src/runtime.cc b/src/runtime.cc
index
f629970ff28209a568124a23ef07bed9194cbe3c..dfd9d14585ed7d0a012167fa0b53d4731fc2c804
100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -3145,14 +3145,7 @@ RUNTIME_FUNCTION(MaybeObject*,
Runtime_StringLastIndexOf) {
uint32_t start_index;
if (!index->ToArrayIndex(&start_index)) return Smi::FromInt(-1);
- uint32_t pat_length = pat->length();
- uint32_t sub_length = sub->length();
-
- if (start_index + pat_length > sub_length) {
- start_index = sub_length - pat_length;
- }
-
- if (pat_length == 0) {
+ if (pat->length() == 0) {
return Smi::FromInt(start_index);
}
@@ -3163,25 +3156,30 @@ RUNTIME_FUNCTION(MaybeObject*,
Runtime_StringLastIndexOf) {
int position = -1;
- if (pat->IsAsciiRepresentation()) {
- Vector<const char> pat_vector = pat->ToAsciiVector();
- if (sub->IsAsciiRepresentation()) {
- position = StringMatchBackwards(sub->ToAsciiVector(),
+ String* seq_sub = *sub;
+ if (seq_sub->IsConsString()) seq_sub =
ConsString::cast(seq_sub)->first();
+ String* seq_pat = *pat;
+ if (seq_pat->IsConsString()) seq_pat =
ConsString::cast(seq_pat)->first();
+
+ if (seq_pat->IsAsciiRepresentation()) {
+ Vector<const char> pat_vector = seq_pat->ToAsciiVector();
+ if (seq_sub->IsAsciiRepresentation()) {
+ position = StringMatchBackwards(seq_sub->ToAsciiVector(),
pat_vector,
start_index);
} else {
- position = StringMatchBackwards(sub->ToUC16Vector(),
+ position = StringMatchBackwards(seq_sub->ToUC16Vector(),
pat_vector,
start_index);
}
} else {
- Vector<const uc16> pat_vector = pat->ToUC16Vector();
- if (sub->IsAsciiRepresentation()) {
- position = StringMatchBackwards(sub->ToAsciiVector(),
+ Vector<const uc16> pat_vector = seq_pat->ToUC16Vector();
+ if (seq_sub->IsAsciiRepresentation()) {
+ position = StringMatchBackwards(seq_sub->ToAsciiVector(),
pat_vector,
start_index);
} else {
- position = StringMatchBackwards(sub->ToUC16Vector(),
+ position = StringMatchBackwards(seq_sub->ToUC16Vector(),
pat_vector,
start_index);
}
Index: src/string.js
diff --git a/src/string.js b/src/string.js
index
50531822be15f65ec9718c208de357252274c4cb..5c20dde5eb59114852b7514e9ccc210a64b3e4e9
100644
--- a/src/string.js
+++ b/src/string.js
@@ -149,7 +149,7 @@ function StringLastIndexOf(pat /* position */) { //
length == 1
position = 0;
}
if (position + patLength < subLength) {
- index = position
+ index = position;
}
}
}
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
