Simplify template literal raw string creation

src/parser.cc

Index: src/parser.cc
diff --git a/src/parser.cc b/src/parser.cc
index 22c0f5c142c918cc45043fc1b11efb6f4a46488d..485a23a882b58dd5999f32c85bcabcdaf2067af6 100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -5291,8 +5291,6 @@ ZoneList<Expression*>* Parser::TemplateRawStrings(const TemplateLiteral* lit,
 
   DCHECK(total);
 
-  Handle<String> source(String::cast(script()->source()));
-
   raw_strings = new (zone()) ZoneList<Expression*>(total, zone());
 
   uint32_t running_hash = 0;
@@ -5300,7 +5298,7 @@ ZoneList<Expression*>* Parser::TemplateRawStrings(const TemplateLiteral* lit,
   for (int index = 0; index < total; ++index) {
     int span_start = cooked_strings->at(index)->position() + 1;
     int span_end = lengths->at(index) - 1;
-    int length;
+    int length = span_end;
     int to_index = 0;
 
     if (index) {
@@ -5308,42 +5306,31 @@ ZoneList<Expression*>* Parser::TemplateRawStrings(const TemplateLiteral* lit,
           running_hash, "${}", 3);
     }
 
-    SmartArrayPointer<char> raw_chars =
-        source->ToCString(ALLOW_NULLS, FAST_STRING_TRAVERSAL, span_start,
-                          span_end, &length);
-
     // Normalize raw line-feeds. [U+000D U+000A] (CRLF) and [U+000D] (CR) must
     // be translated into U+000A (LF).
-    for (int from_index = 0; from_index < length; ++from_index) {
-      char ch = raw_chars[from_index];
-      if (ch == '\r') {
-        ch = '\n';
-        if (from_index + 1 < length && raw_chars[from_index + 1] == '\n') {
-          ++from_index;
-        }
+    StringCharacterStream stream(String::cast(script()->source()), span_start);

[marja, 2014/12/01 20:01:34]

Oops; this problem was already present before this CL: We might not have the
source string yet. In the streaming parsing case, we don't have it. :/

Can we get the string from somewhere else? Can we delay constructing the raw
strings until they're really needed? (I probably thought that they're
constructed on demand, not parse time, when I originally thought this would
work...)

[caitp (gmail), 2014/12/01 20:05:15]

I see.

What happens is, we are setting up some array literals to pass to
GetTemplateCallSite(), which will use them to produce the objects it needs ---
perhaps we could pass the offsets to that function instead, and use a runtime
function to fetch the strings?

But the trouble with this is that it would always be a runtime operation, and
we'd have to do it every single time a tagged template is invoked....

An alternative solution would be to ensure that source is available, somehow...

[arv (Not doing code reviews), 2014/12/01 20:18:44]

We'll have to think a bit more about this. The source code up to the end of the
template literal has been parsed before we get here. Maybe we should just get
the source from the Scanner here.

+    uc16* normalized_raw_chars = zone()->NewArray<uc16>(length);
+    bool last_was_cr = false;
+    for (int from_index = 0; from_index < length && stream.HasMore();

[caitp (gmail), 2014/12/01 19:57:36]

are there cases where the character stream would not have enough characters (eg
stream.HasMore() returns false)? It seems like it should always be available,
but maybe not.

[Dmitry Lomov (no reviews), 2014/12/01 19:58:05]

Can you assert that at the end of this loop, always !stream.HasMore() (I believe
that should be true if this algo is correct?)

[arv (Not doing code reviews), 2014/12/01 20:18:43]

The stream is on the entire script so it will most likely have more characters.

+         ++from_index) {
+      uc16 ch = stream.GetNext();
+      switch (ch) {
+        case '\r':
+          normalized_raw_chars[to_index++] = '\n';
+          last_was_cr = true;
+          continue;
+        case '\n':
+          if (last_was_cr) break;
+        // Fall through.
+        default:
+          normalized_raw_chars[to_index++] = ch;

[caitp (gmail), 2014/12/01 19:57:36]

move `last_was_cr = false;` here I think? (then can remove the `continue` also)

[arv (Not doing code reviews), 2014/12/01 20:18:44]

We need to reset it even if last_was_cr was true.

I tried a different forms for this logic and none of them turned out nice.

       }
-      raw_chars[to_index++] = ch;
+      last_was_cr = false;
     }
 
-    Access<UnicodeCache::Utf8Decoder>
-        decoder(isolate()->unicode_cache()->utf8_decoder());
-    decoder->Reset(raw_chars.get(), to_index);
-    int utf16_length = decoder->Utf16Length();
-    Literal* raw_lit = NULL;
-    if (utf16_length > 0) {
-      uc16* utf16_buffer = zone()->NewArray<uc16>(utf16_length);
-      to_index = decoder->WriteUtf16(utf16_buffer, utf16_length);
-      running_hash = StringHasher::ComputeRunningHash(
-          running_hash, utf16_buffer, to_index);
-      const uint16_t* data = reinterpret_cast<const uint16_t*>(utf16_buffer);
-      const AstRawString* raw_str = ast_value_factory()->GetTwoByteString(
-          Vector<const uint16_t>(data, to_index));
-      raw_lit = factory()->NewStringLiteral(raw_str, span_start - 1);
-    } else {
-      raw_lit = factory()->NewStringLiteral(
-          ast_value_factory()->empty_string(), span_start - 1);
-    }
-    DCHECK_NOT_NULL(raw_lit);
+    const AstRawString* raw_str = ast_value_factory()->GetTwoByteString(
+        Vector<const uc16>(normalized_raw_chars, to_index));
+    Literal* raw_lit = factory()->NewStringLiteral(raw_str, span_start - 1);
     raw_strings->Add(raw_lit, zone());
   }