Simplify template literal raw string creation

src/parser.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/v8.h"
 
 #include "src/api.h"
 #include "src/ast.h"
 #include "src/bailout-reason.h"
 #include "src/base/platform/platform.h"
@@ skipping 5273 matching lines @@
   // Given a TemplateLiteral, produce a list of raw strings, used for generating
   // a CallSite object for a tagged template invocations.
   //
   // A raw string will consist of the unescaped characters of a template span,
   // with end-of-line sequences normalized to U+000A LINE FEEDs, and without
   // leading or trailing template delimiters.
   //
 
   DCHECK(total);
 
-  Handle<String> source(String::cast(script()->source()));
-
   raw_strings = new (zone()) ZoneList<Expression*>(total, zone());
 
   uint32_t running_hash = 0;
 
   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) {
       running_hash = StringHasher::ComputeRunningHashOneByte(
           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());
   }
 
   // Hash key is used exclusively by template call site caching. There are no
   // real security implications for unseeded hashes, and no issues with changing
   // the hashing algorithm to improve performance or entropy.
   *hash = running_hash;
 
   return raw_strings;
 }
 } }  // namespace v8::internal