Commit fc44d57b authored by jgruber's avatar jgruber Committed by Commit bot

[string] Refactor String::GetSubstitution

Remove deep nesting and repeated code by using a switch-based structure
(instead of if-based), use clearer variable names, and separate cleanly between
immutable and mutable variables.

BUG=v8:5437

Review-Url: https://codereview.chromium.org/2776123002
Cr-Commit-Position: refs/heads/master@{#44147}
parent 960c2a1f
...@@ -11412,40 +11412,59 @@ MaybeHandle<String> String::GetSubstitution(Isolate* isolate, Match* match, ...@@ -11412,40 +11412,59 @@ MaybeHandle<String> String::GetSubstitution(Isolate* isolate, Match* match,
Handle<String> dollar_string = Handle<String> dollar_string =
factory->LookupSingleCharacterStringFromCode('$'); factory->LookupSingleCharacterStringFromCode('$');
int next = String::IndexOf(isolate, replacement, dollar_string, 0); int next_dollar_ix = String::IndexOf(isolate, replacement, dollar_string, 0);
if (next < 0) { if (next_dollar_ix < 0) {
return replacement; return replacement;
} }
IncrementalStringBuilder builder(isolate); IncrementalStringBuilder builder(isolate);
if (next > 0) { if (next_dollar_ix > 0) {
builder.AppendString(factory->NewSubString(replacement, 0, next)); builder.AppendString(factory->NewSubString(replacement, 0, next_dollar_ix));
} }
while (true) { while (true) {
int pos = next + 1; const int peek_ix = next_dollar_ix + 1;
if (pos < replacement_length) { if (peek_ix >= replacement_length) {
const uint16_t peek = replacement->Get(pos); builder.AppendCharacter('$');
if (peek == '$') { // $$ return builder.Finish();
pos++; }
int continue_from_ix = -1;
const uint16_t peek = replacement->Get(peek_ix);
switch (peek) {
case '$': // $$
builder.AppendCharacter('$'); builder.AppendCharacter('$');
} else if (peek == '&') { // $& - match continue_from_ix = peek_ix + 1;
pos++; break;
case '&': // $& - match
builder.AppendString(match->GetMatch()); builder.AppendString(match->GetMatch());
} else if (peek == '`') { // $` - prefix continue_from_ix = peek_ix + 1;
pos++; break;
case '`': // $` - prefix
builder.AppendString(match->GetPrefix()); builder.AppendString(match->GetPrefix());
} else if (peek == '\'') { // $' - suffix continue_from_ix = peek_ix + 1;
pos++; break;
case '\'': // $' - suffix
builder.AppendString(match->GetSuffix()); builder.AppendString(match->GetSuffix());
} else if (peek >= '0' && peek <= '9') { continue_from_ix = peek_ix + 1;
break;
case '0':
case '1':
case '2':
case '3':
case '4':
case '5':
case '6':
case '7':
case '8':
case '9': {
// Valid indices are $1 .. $9, $01 .. $09 and $10 .. $99 // Valid indices are $1 .. $9, $01 .. $09 and $10 .. $99
int scaled_index = (peek - '0'); int scaled_index = (peek - '0');
int advance = 1; int advance = 1;
if (pos + 1 < replacement_length) { if (peek_ix + 1 < replacement_length) {
const uint16_t next_peek = replacement->Get(pos + 1); const uint16_t next_peek = replacement->Get(peek_ix + 1);
if (next_peek >= '0' && next_peek <= '9') { if (next_peek >= '0' && next_peek <= '9') {
const int new_scaled_index = scaled_index * 10 + (next_peek - '0'); const int new_scaled_index = scaled_index * 10 + (next_peek - '0');
if (new_scaled_index < captures_length) { if (new_scaled_index < captures_length) {
...@@ -11455,40 +11474,47 @@ MaybeHandle<String> String::GetSubstitution(Isolate* isolate, Match* match, ...@@ -11455,40 +11474,47 @@ MaybeHandle<String> String::GetSubstitution(Isolate* isolate, Match* match,
} }
} }
if (scaled_index != 0 && scaled_index < captures_length) { if (scaled_index == 0 || scaled_index >= captures_length) {
bool capture_exists;
Handle<String> capture;
ASSIGN_RETURN_ON_EXCEPTION(
isolate, capture,
match->GetCapture(scaled_index, &capture_exists), String);
if (capture_exists) builder.AppendString(capture);
pos += advance;
} else {
builder.AppendCharacter('$'); builder.AppendCharacter('$');
continue_from_ix = peek_ix;
break;
} }
} else {
builder.AppendCharacter('$'); bool capture_exists;
Handle<String> capture;
ASSIGN_RETURN_ON_EXCEPTION(
isolate, capture, match->GetCapture(scaled_index, &capture_exists),
String);
if (capture_exists) builder.AppendString(capture);
continue_from_ix = peek_ix + advance;
break;
} }
} else { default:
builder.AppendCharacter('$'); builder.AppendCharacter('$');
continue_from_ix = peek_ix;
break;
} }
// Go the the next $ in the replacement. // Go the the next $ in the replacement.
next = String::IndexOf(isolate, replacement, dollar_string, pos); // TODO(jgruber): Single-char lookups could be much more efficient.
DCHECK_NE(continue_from_ix, -1);
next_dollar_ix =
String::IndexOf(isolate, replacement, dollar_string, continue_from_ix);
// Return if there are no more $ characters in the replacement. If we // Return if there are no more $ characters in the replacement. If we
// haven't reached the end, we need to append the suffix. // haven't reached the end, we need to append the suffix.
if (next < 0) { if (next_dollar_ix < 0) {
if (pos < replacement_length) { if (continue_from_ix < replacement_length) {
builder.AppendString( builder.AppendString(factory->NewSubString(
factory->NewSubString(replacement, pos, replacement_length)); replacement, continue_from_ix, replacement_length));
} }
return builder.Finish(); return builder.Finish();
} }
// Append substring between the previous and the next $ character. // Append substring between the previous and the next $ character.
if (next > pos) { if (next_dollar_ix > continue_from_ix) {
builder.AppendString(factory->NewSubString(replacement, pos, next)); builder.AppendString(
factory->NewSubString(replacement, continue_from_ix, next_dollar_ix));
} }
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment