[parser/ast] Remove AstString, in favour of stand-alons Ast(Raw|Cons)String.

Removes the AstString super-class and makes the two sub-classes stand-alone.
That also removes the slightly awkward implementation of a super-class with
dynamically dispatched behaviour, except it was manually implemented because
virtual function calls were considered expensive.

BUG=v8:6902

Change-Id: I079a2372557aa17ee839efcaddb9b7fe86e38b4d
Reviewed-on: https://chromium-review.googlesource.com/458428
Commit-Queue: Daniel Vogelheim <vogelheim@chromium.org>
Reviewed-by: Marja Hölttä <marja@chromium.org>
Cr-Commit-Position: refs/heads/master@{#44104}

src/ast/ast-value-factory.cc

@@ -84,21 +84,8 @@ class AstRawStringInternalizationKey : public HashTableKey {
   const AstRawString* string_;
 };
 
-int AstString::length() const {
-  if (IsRawStringBits::decode(bit_field_)) {
-    return reinterpret_cast<const AstRawString*>(this)->length();
-  }
-  return reinterpret_cast<const AstConsString*>(this)->length();
-}
-
-void AstString::Internalize(Isolate* isolate) {
-  if (IsRawStringBits::decode(bit_field_)) {
-    return reinterpret_cast<AstRawString*>(this)->Internalize(isolate);
-  }
-  return reinterpret_cast<AstConsString*>(this)->Internalize(isolate);
-}
-
 void AstRawString::Internalize(Isolate* isolate) {
+  DCHECK(!has_string_);
   if (literal_bytes_.length() == 0) {
     set_string(isolate->factory()->empty_string());
   } else {
@@ -121,18 +108,26 @@ bool AstRawString::AsArrayIndex(uint32_t* index) const {
 }
 
 bool AstRawString::IsOneByteEqualTo(const char* data) const {
-  int length = static_cast<int>(strlen(data));
-  if (is_one_byte() && literal_bytes_.length() == length) {
-    const char* token = reinterpret_cast<const char*>(literal_bytes_.start());
-    return !strncmp(token, data, length);
-  }
-  return false;
+  if (!is_one_byte()) return false;
+
+  size_t length = static_cast<size_t>(literal_bytes_.length());
+  if (length != strlen(data)) return false;
+
+  return 0 == strncmp(reinterpret_cast<const char*>(literal_bytes_.start()),
+                      data, length);
+}
+
+uint16_t AstRawString::FirstCharacter() const {
+  if (is_one_byte()) return literal_bytes_[0];
+  const uint16_t* c = reinterpret_cast<const uint16_t*>(literal_bytes_.start());
+  return *c;
 }
 
 bool AstRawString::Compare(void* a, void* b) {
   const AstRawString* lhs = static_cast<AstRawString*>(a);
   const AstRawString* rhs = static_cast<AstRawString*>(b);
   DCHECK_EQ(lhs->hash(), rhs->hash());
+
   if (lhs->length() != rhs->length()) return false;
   const unsigned char* l = lhs->raw_data();
   const unsigned char* r = rhs->raw_data();
@@ -161,11 +156,15 @@ bool AstRawString::Compare(void* a, void* b) {
 }
 
 void AstConsString::Internalize(Isolate* isolate) {
-  // AstRawStrings are internalized before AstConsStrings so left and right are
-  // already internalized.
-  set_string(isolate->factory()
-                 ->NewConsString(left_->string(), right_->string())
-                 .ToHandleChecked());
+  Handle<String> tmp(isolate->factory()->empty_string());
+  for (const AstRawString* current : strings_) {
+    // AstRawStrings are internalized before AstConsStrings, so *current is
+    // already internalized.
+    tmp = isolate->factory()
+              ->NewConsString(tmp, current->string())
+              .ToHandleChecked();
+  }
+  set_string(tmp);
 }
 
 bool AstValue::IsPropertyName() const {
@@ -285,22 +284,34 @@ const AstRawString* AstValueFactory::GetString(Handle<String> literal) {
   return result;
 }
 
-
-const AstConsString* AstValueFactory::NewConsString(
-    const AstString* left, const AstString* right) {
-  // This Vector will be valid as long as the Collector is alive (meaning that
-  // the AstRawString will not be moved).
-  AstConsString* new_string = new (zone_) AstConsString(left, right);
-  CHECK(new_string != nullptr);
-  AddString(new_string);
+AstConsString* AstValueFactory::NewConsString() {
+  AstConsString* new_string = new (zone_) AstConsString(zone_);
+  DCHECK_NOT_NULL(new_string);
+  AddConsString(new_string);
   return new_string;
 }
 
+AstConsString* AstValueFactory::NewConsString(const AstRawString* str) {
+  return NewConsString()->AddString(str);
+}
+
+AstConsString* AstValueFactory::NewConsString(const AstRawString* str1,
+                                              const AstRawString* str2) {
+  return NewConsString()->AddString(str1)->AddString(str2);
+}
+
 void AstValueFactory::Internalize(Isolate* isolate) {
   // Strings need to be internalized before values, because values refer to
   // strings.
-  for (AstString* current = strings_; current != nullptr;) {
-    AstString* next = current->next();
+  for (AstRawString* current = strings_; current != nullptr;) {
+    AstRawString* next = current->next();
+    current->Internalize(isolate);
+    current = next;
+  }
+
+  // AstConsStrings refer to AstRawStrings.
+  for (AstConsString* current = cons_strings_; current != nullptr;) {
+    AstConsString* next = current->next();
     current->Internalize(isolate);
     current = next;
   }

src/ast/ast-value-factory.h

@@ -35,123 +35,128 @@
 #include "src/isolate.h"
 #include "src/utils.h"
 
-// AstString, AstValue and AstValueFactory are for storing strings and values
-// independent of the V8 heap and internalizing them later. During parsing,
-// AstStrings and AstValues are created and stored outside the heap, in
-// AstValueFactory. After parsing, the strings and values are internalized
-// (moved into the V8 heap).
+// Ast(Raw|Cons)String, AstValue and AstValueFactory are for storing strings and
+// values independent of the V8 heap and internalizing them later. During
+// parsing, they are created and stored outside the heap, in AstValueFactory.
+// After parsing, the strings and values are internalized (moved into the V8
+// heap).
 namespace v8 {
 namespace internal {
 
-class AstString : public ZoneObject {
- public:
-  explicit AstString(bool is_raw)
-      : next_(nullptr), bit_field_(IsRawStringBits::encode(is_raw)) {}
-
-  int length() const;
-  bool IsEmpty() const { return length() == 0; }
-
-  // Puts the string into the V8 heap.
-  void Internalize(Isolate* isolate);
-
-  // This function can be called after internalizing.
-  V8_INLINE Handle<String> string() const {
-    DCHECK_NOT_NULL(string_);
-    return Handle<String>(string_);
-  }
-
-  AstString* next() { return next_; }
-  AstString** next_location() { return &next_; }
-
- protected:
-  void set_string(Handle<String> string) { string_ = string.location(); }
-  // {string_} is stored as String** instead of a Handle<String> so it can be
-  // stored in a union with {next_}.
-  union {
-    AstString* next_;
-    String** string_;
-  };
-  // Poor-man's virtual dispatch to AstRawString / AstConsString. Takes less
-  // memory.
-  class IsRawStringBits : public BitField<bool, 0, 1> {};
-  int bit_field_;
-};
-
-
-class AstRawString final : public AstString {
+class AstRawString final : public ZoneObject {
  public:
+  bool IsEmpty() const { return literal_bytes_.length() == 0; }
   int length() const {
-    if (is_one_byte()) return literal_bytes_.length();
-    return literal_bytes_.length() / 2;
+    return is_one_byte() ? literal_bytes_.length()
+                         : literal_bytes_.length() / 2;
   }
-
-  int byte_length() const { return literal_bytes_.length(); }
+  bool AsArrayIndex(uint32_t* index) const;
+  bool IsOneByteEqualTo(const char* data) const;
+  uint16_t FirstCharacter() const;
 
   void Internalize(Isolate* isolate);
 
-  bool AsArrayIndex(uint32_t* index) const;
-
-  // The string is not null-terminated, use length() to find out the length.
+  // Access the physical representation:
+  bool is_one_byte() const { return is_one_byte_; }
+  int byte_length() const { return literal_bytes_.length(); }
   const unsigned char* raw_data() const {
     return literal_bytes_.start();
   }
 
-  bool is_one_byte() const { return IsOneByteBits::decode(bit_field_); }
-
-  bool IsOneByteEqualTo(const char* data) const;
-  uint16_t FirstCharacter() const {
-    if (is_one_byte()) return literal_bytes_[0];
-    const uint16_t* c =
-        reinterpret_cast<const uint16_t*>(literal_bytes_.start());
-    return *c;
-  }
-
-  static bool Compare(void* a, void* b);
-
   // For storing AstRawStrings in a hash map.
   uint32_t hash() const {
     return hash_;
   }
 
+  // This function can be called after internalizing.
+  V8_INLINE Handle<String> string() const {
+    DCHECK_NOT_NULL(string_);
+    DCHECK(has_string_);
+    return Handle<String>(string_);
+  }
+
  private:
   friend class AstRawStringInternalizationKey;
   friend class AstStringConstants;
   friend class AstValueFactory;
 
+  // Members accessed only by the AstValueFactory & related classes:
+  static bool Compare(void* a, void* b);
   AstRawString(bool is_one_byte, const Vector<const byte>& literal_bytes,
                uint32_t hash)
-      : AstString(true), hash_(hash), literal_bytes_(literal_bytes) {
-    bit_field_ |= IsOneByteBits::encode(is_one_byte);
+      : next_(nullptr),
+        literal_bytes_(literal_bytes),
+        hash_(hash),
+        is_one_byte_(is_one_byte) {}
+  AstRawString* next() {
+    DCHECK(!has_string_);
+    return next_;
+  }
+  AstRawString** next_location() {
+    DCHECK(!has_string_);
+    return &next_;
   }
 
-  AstRawString() : AstString(true), hash_(0) {
-    bit_field_ |= IsOneByteBits::encode(true);
+  void set_string(Handle<String> string) {
+    DCHECK(!string.is_null());
+    DCHECK(!has_string_);
+    string_ = string.location();
+#ifdef DEBUG
+    has_string_ = true;
+#endif
   }
 
-  class IsOneByteBits : public BitField<bool, IsRawStringBits::kNext, 1> {};
+  // {string_} is stored as String** instead of a Handle<String> so it can be
+  // stored in a union with {next_}.
+  union {
+    AstRawString* next_;
+    String** string_;
+  };
 
+  Vector<const byte> literal_bytes_;  // Memory owned by Zone.
   uint32_t hash_;
-  // Points to memory owned by Zone.
-  Vector<const byte> literal_bytes_;
+  bool is_one_byte_;
+#ifdef DEBUG
+  // (Debug-only:) Verify the object life-cylce: Some functions may only be
+  // called after internalization (that is, after a v8::internal::String has
+  // been set); some only before.
+  bool has_string_ = false;
+#endif
 };
 
-
-class AstConsString final : public AstString {
+class AstConsString final : public ZoneObject {
  public:
-  AstConsString(const AstString* left, const AstString* right)
-      : AstString(false),
-        length_(left->length() + right->length()),
-        left_(left),
-        right_(right) {}
+  AstConsString* AddString(const AstRawString* s) {
+    if (s && !s->IsEmpty()) strings_.push_back(s);
+    return this;
+  }
 
-  int length() const { return length_; }
+  bool IsEmpty() const { return strings_.empty(); }
 
   void Internalize(Isolate* isolate);
 
+  V8_INLINE Handle<String> string() const {
+    DCHECK_NOT_NULL(string_);
+    return Handle<String>(string_);
+  }
+
  private:
-  const int length_;
-  const AstString* left_;
-  const AstString* right_;
+  friend class AstValueFactory;
+
+  explicit AstConsString(Zone* zone) : next_(nullptr), strings_(zone) {}
+
+  AstConsString* next() const { return next_; }
+  AstConsString** next_location() { return &next_; }
+
+  // {string_} is stored as String** instead of a Handle<String> so it can be
+  // stored in a union with {next_}.
+  void set_string(Handle<String> string) { string_ = string.location(); }
+  union {
+    AstConsString* next_;
+    String** string_;
+  };
+
+  ZoneLinkedList<const AstRawString*> strings_;
 };
 
 enum class AstSymbol : uint8_t { kHomeObjectSymbol };
@@ -412,7 +417,10 @@ class AstValueFactory {
         values_(nullptr),
         strings_(nullptr),
         strings_end_(&strings_),
+        cons_strings_(nullptr),
+        cons_strings_end_(&cons_strings_),
         string_constants_(string_constants),
+        empty_cons_string_(nullptr),
         zone_(zone),
         hash_seed_(hash_seed) {
 #define F(name) name##_ = nullptr;
@@ -423,6 +431,7 @@ class AstValueFactory {
     std::fill(one_character_strings_,
               one_character_strings_ + arraysize(one_character_strings_),
               nullptr);
+    empty_cons_string_ = NewConsString();
   }
 
   Zone* zone() const { return zone_; }
@@ -438,17 +447,20 @@ class AstValueFactory {
     return GetTwoByteStringInternal(literal);
   }
   const AstRawString* GetString(Handle<String> literal);
-  const AstConsString* NewConsString(const AstString* left,
-                                     const AstString* right);
+  V8_EXPORT_PRIVATE AstConsString* NewConsString();
+  AstConsString* NewConsString(const AstRawString* str);
+  AstConsString* NewConsString(const AstRawString* str1,
+                               const AstRawString* str2);
 
   V8_EXPORT_PRIVATE void Internalize(Isolate* isolate);
 
 #define F(name, str)                           \
-  const AstRawString* name##_string() {        \
+  const AstRawString* name##_string() const {  \
     return string_constants_->name##_string(); \
   }
   STRING_CONSTANTS(F)
 #undef F
+  const AstConsString* empty_cons_string() const { return empty_cons_string_; }
 
   V8_EXPORT_PRIVATE const AstValue* NewString(const AstRawString* string);
   // A JavaScript symbol (ECMA-262 edition 6).
@@ -472,14 +484,21 @@ class AstValueFactory {
     values_ = value;
     return value;
   }
-  AstString* AddString(AstString* string) {
+  AstRawString* AddString(AstRawString* string) {
     *strings_end_ = string;
     strings_end_ = string->next_location();
     return string;
   }
+  AstConsString* AddConsString(AstConsString* string) {
+    *cons_strings_end_ = string;
+    cons_strings_end_ = string->next_location();
+    return string;
+  }
   void ResetStrings() {
     strings_ = nullptr;
     strings_end_ = &strings_;
+    cons_strings_ = nullptr;
+    cons_strings_end_ = &cons_strings_;
   }
   V8_EXPORT_PRIVATE AstRawString* GetOneByteStringInternal(
       Vector<const uint8_t> literal);
@@ -495,11 +514,14 @@ class AstValueFactory {
 
   // We need to keep track of strings_ in order since cons strings require their
   // members to be internalized first.
-  AstString* strings_;
-  AstString** strings_end_;
+  AstRawString* strings_;
+  AstRawString** strings_end_;
+  AstConsString* cons_strings_;
+  AstConsString** cons_strings_end_;
 
   // Holds constant string values which are shared across the isolate.
   const AstStringConstants* string_constants_;
+  const AstConsString* empty_cons_string_;
 
   // Caches for faster access: small numbers, one character lowercase strings
   // (for minified code).

src/ast/ast.cc

@@ -164,7 +164,7 @@ void Expression::MarkTail() {
 bool DoExpression::IsAnonymousFunctionDefinition() const {
   // This is specifically to allow DoExpressions to represent ClassLiterals.
   return represented_function_ != nullptr &&
-         represented_function_->raw_name()->length() == 0;
+         represented_function_->raw_name()->IsEmpty();
 }
 
 bool Statement::IsJump() const {

src/ast/ast.h

@@ -2595,8 +2595,8 @@ class FunctionLiteral final : public Expression {
   enum EagerCompileHint { kShouldEagerCompile, kShouldLazyCompile };
 
   Handle<String> name() const { return raw_name_->string(); }
-  const AstString* raw_name() const { return raw_name_; }
-  void set_raw_name(const AstString* name) { raw_name_ = name; }
+  const AstConsString* raw_name() const { return raw_name_; }
+  void set_raw_name(const AstConsString* name) { raw_name_ = name; }
   DeclarationScope* scope() const { return scope_; }
   ZoneList<Statement*>* body() const { return body_; }
   void set_function_token_position(int pos) { function_token_position_ = pos; }
@@ -2658,7 +2658,7 @@ class FunctionLiteral final : public Expression {
     raw_inferred_name_ = NULL;
   }
 
-  void set_raw_inferred_name(const AstString* raw_inferred_name) {
+  void set_raw_inferred_name(const AstConsString* raw_inferred_name) {
     DCHECK(raw_inferred_name != NULL);
     raw_inferred_name_ = raw_inferred_name;
     DCHECK(inferred_name_.is_null());
@@ -2729,7 +2729,7 @@ class FunctionLiteral final : public Expression {
  private:
   friend class AstNodeFactory;
 
-  FunctionLiteral(Zone* zone, const AstString* name,
+  FunctionLiteral(Zone* zone, const AstRawString* name,
                   AstValueFactory* ast_value_factory, DeclarationScope* scope,
                   ZoneList<Statement*>* body, int expected_property_count,
                   int parameter_count, int function_length,
@@ -2744,10 +2744,10 @@ class FunctionLiteral final : public Expression {
         function_token_position_(kNoSourcePosition),
         suspend_count_(0),
         has_braces_(has_braces),
-        raw_name_(name),
+        raw_name_(ast_value_factory->NewConsString(name)),
         scope_(scope),
         body_(body),
-        raw_inferred_name_(ast_value_factory->empty_string()),
+        raw_inferred_name_(ast_value_factory->empty_cons_string()),
         ast_properties_(zone),
         function_literal_id_(function_literal_id) {
     bit_field_ |= FunctionTypeBits::encode(function_type) |
@@ -2776,10 +2776,10 @@ class FunctionLiteral final : public Expression {
   int suspend_count_;
   bool has_braces_;
 
-  const AstString* raw_name_;
+  const AstConsString* raw_name_;
   DeclarationScope* scope_;
   ZoneList<Statement*>* body_;
-  const AstString* raw_inferred_name_;
+  const AstConsString* raw_inferred_name_;
   Handle<String> inferred_name_;
   AstProperties ast_properties_;
   int function_literal_id_;

src/parsing/func-name-inferrer.cc

@@ -53,35 +53,26 @@ void FuncNameInferrer::RemoveAsyncKeywordFromEnd() {
   }
 }
 
-const AstString* FuncNameInferrer::MakeNameFromStack() {
-  return MakeNameFromStackHelper(0, ast_value_factory_->empty_string());
-}
-
-const AstString* FuncNameInferrer::MakeNameFromStackHelper(
-    int pos, const AstString* prev) {
-  if (pos >= names_stack_.length()) return prev;
-  if (pos < names_stack_.length() - 1 &&
-      names_stack_.at(pos).type == kVariableName &&
-      names_stack_.at(pos + 1).type == kVariableName) {
+const AstConsString* FuncNameInferrer::MakeNameFromStack() {
+  AstConsString* result = ast_value_factory_->NewConsString();
+  for (int pos = 0; pos < names_stack_.length(); pos++) {
     // Skip consecutive variable declarations.
-    return MakeNameFromStackHelper(pos + 1, prev);
-  } else {
-    if (prev->length() > 0) {
-      const AstRawString* name = names_stack_.at(pos).name;
-      if (prev->length() + name->length() + 1 > String::kMaxLength) return prev;
-      const AstConsString* curr = ast_value_factory_->NewConsString(
-          ast_value_factory_->dot_string(), name);
-      curr = ast_value_factory_->NewConsString(prev, curr);
-      return MakeNameFromStackHelper(pos + 1, curr);
-    } else {
-      return MakeNameFromStackHelper(pos + 1, names_stack_.at(pos).name);
+    if (pos + 1 < names_stack_.length() &&
+        names_stack_.at(pos).type == kVariableName &&
+        names_stack_.at(pos + 1).type == kVariableName) {
+      continue;
     }
+    // Add name. Separate names with ".".
+    if (!result->IsEmpty()) {
+      result->AddString(ast_value_factory_->dot_string());
+    }
+    result->AddString(names_stack_.at(pos).name);
   }
+  return result;
 }
 
-
 void FuncNameInferrer::InferFunctionsNames() {
-  const AstString* func_name = MakeNameFromStack();
+  const AstConsString* func_name = MakeNameFromStack();
   for (int i = 0; i < funcs_to_infer_.length(); ++i) {
     funcs_to_infer_[i]->set_raw_inferred_name(func_name);
   }

src/parsing/func-name-inferrer.h

@@ -10,8 +10,8 @@
 namespace v8 {
 namespace internal {
 
+class AstConsString;
 class AstRawString;
-class AstString;
 class AstValueFactory;
 class FunctionLiteral;
 
@@ -105,11 +105,7 @@ class FuncNameInferrer : public ZoneObject {
   Zone* zone() const { return zone_; }
 
   // Constructs a full name in dotted notation from gathered names.
-  const AstString* MakeNameFromStack();
-
-  // A helper function for MakeNameFromStack.
-  const AstString* MakeNameFromStackHelper(int pos,
-                                               const AstString* prev);
+  const AstConsString* MakeNameFromStack();
 
   // Performs name inferring for added functions.
   void InferFunctionsNames();

src/parsing/parser.cc

@@ -3311,8 +3311,8 @@ void Parser::DeclareClassProperty(const AstRawString* class_name,
     class_info->constructor = GetPropertyValue(property)->AsFunctionLiteral();
     DCHECK_NOT_NULL(class_info->constructor);
     class_info->constructor->set_raw_name(
-        class_name != nullptr ? class_name
-                              : ast_value_factory()->empty_string());
+        class_name != nullptr ? ast_value_factory()->NewConsString(class_name)
+                              : ast_value_factory()->empty_cons_string());
     return;
   }
 
@@ -4271,10 +4271,11 @@ void Parser::SetFunctionName(Expression* value, const AstRawString* name) {
   if (!value->IsAnonymousFunctionDefinition()) return;
   auto function = value->AsFunctionLiteral();
   if (function != nullptr) {
-    function->set_raw_name(name);
+    function->set_raw_name(ast_value_factory()->NewConsString(name));
   } else {
     DCHECK(value->IsDoExpression());
-    value->AsDoExpression()->represented_function()->set_raw_name(name);
+    value->AsDoExpression()->represented_function()->set_raw_name(
+        ast_value_factory()->NewConsString(name));
   }
 }