[ast] Encapsulate AstValue inside Literal AstNode

This removes all but one caller of Literal::raw_value(), thus
hiding AstValue from the rest of the codebase. This is in
preparation to move much of AstValue's implementation up
into Literal itself, thus avoiding the overhead of the
underling ZoneObjects and allowing us to remove complexity
such as the cache of Smi-valued AstValues.

Bug: v8:6984
Change-Id: I1b90aa64b9d26db36ef486afe73cda4473ef866e
Reviewed-on: https://chromium-review.googlesource.com/731109Reviewed-by: Marja Hölttä <marja@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Commit-Queue: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48884}

src/ast/ast-value-factory.cc

@@ -179,19 +179,6 @@ AstValue::AstValue(double n) : next_(nullptr) {
   }
 }
 
-bool AstValue::ToUint32(uint32_t* value) const {
-  if (IsSmi()) {
-    int num = smi_;
-    if (num < 0) return false;
-    *value = static_cast<uint32_t>(num);
-    return true;
-  }
-  if (IsHeapNumber()) {
-    return DoubleToUint32IfEqualToSelf(number_, value);
-  }
-  return false;
-}
-
 bool AstValue::IsPropertyName() const {
   if (type_ == STRING) {
     uint32_t index;

src/ast/ast-value-factory.h

@@ -209,12 +209,6 @@ class AstValue : public ZoneObject {
     return Smi::FromInt(smi_);
   }
 
-  bool ToUint32(uint32_t* value) const;
-
-  bool EqualsString(const AstRawString* string) const {
-    return type_ == STRING && string_ == string;
-  }
-
   bool IsPropertyName() const;
 
   V8_EXPORT_PRIVATE bool BooleanValue() const;

src/ast/ast.cc

@@ -90,15 +90,15 @@ MaterializedLiteral* AstNode::AsMaterializedLiteral() {
 #undef RETURN_NODE
 
 bool Expression::IsSmiLiteral() const {
-  return IsLiteral() && AsLiteral()->raw_value()->IsSmi();
+  return IsLiteral() && AsLiteral()->IsSmi();
 }
 
 bool Expression::IsNumberLiteral() const {
-  return IsLiteral() && AsLiteral()->raw_value()->IsNumber();
+  return IsLiteral() && AsLiteral()->IsNumber();
 }
 
 bool Expression::IsStringLiteral() const {
-  return IsLiteral() && AsLiteral()->raw_value()->IsString();
+  return IsLiteral() && AsLiteral()->IsString();
 }
 
 bool Expression::IsPropertyName() const {
@@ -106,12 +106,15 @@ bool Expression::IsPropertyName() const {
 }
 
 bool Expression::IsNullLiteral() const {
-  if (!IsLiteral()) return false;
-  return AsLiteral()->raw_value()->IsNull();
+  return IsLiteral() && AsLiteral()->IsNull();
+}
+
+bool Expression::IsTheHoleLiteral() const {
+  return IsLiteral() && AsLiteral()->IsTheHole();
 }
 
 bool Expression::IsUndefinedLiteral() const {
-  if (IsLiteral() && AsLiteral()->raw_value()->IsUndefined()) return true;
+  if (IsLiteral() && AsLiteral()->IsUndefined()) return true;
 
   const VariableProxy* var_proxy = AsVariableProxy();
   if (var_proxy == nullptr) return false;
@@ -255,9 +258,8 @@ ObjectLiteralProperty::ObjectLiteralProperty(AstValueFactory* ast_value_factory,
                                              Expression* key, Expression* value,
                                              bool is_computed_name)
     : LiteralProperty(key, value, is_computed_name), emit_store_(true) {
-  if (!is_computed_name &&
-      key->AsLiteral()->raw_value()->EqualsString(
-          ast_value_factory->proto_string())) {
+  if (!is_computed_name && key->AsLiteral()->IsString() &&
+      key->AsLiteral()->AsRawString() == ast_value_factory->proto_string()) {
     kind_ = PROTOTYPE;
   } else if (value_->AsMaterializedLiteral() != nullptr) {
     kind_ = MATERIALIZED_LITERAL;
@@ -373,7 +375,7 @@ int ObjectLiteral::InitDepthAndFlags() {
       needs_initial_allocation_site |= literal->NeedsInitialAllocationSite();
     }
 
-    const AstValue* key = property->key()->AsLiteral()->raw_value();
+    Literal* key = property->key()->AsLiteral();
     Expression* value = property->value();
 
     bool is_compile_time_value = CompileTimeValue::IsCompileTimeValue(value);
@@ -384,10 +386,10 @@ int ObjectLiteral::InitDepthAndFlags() {
     // much larger than the number of elements, creating an object
     // literal with fast elements will be a waste of space.
     uint32_t element_index = 0;
-    if (key->IsString() && key->AsString()->AsArrayIndex(&element_index)) {
+    if (key->IsString() && key->AsRawString()->AsArrayIndex(&element_index)) {
       max_element_index = Max(element_index, max_element_index);
       elements++;
-    } else if (key->ToUint32(&element_index) && element_index != kMaxUInt32) {
+    } else if (key->ToArrayIndex(&element_index)) {
       max_element_index = Max(element_index, max_element_index);
       elements++;
     }
@@ -785,17 +787,33 @@ Call::CallType Call::GetCallType() const {
 CaseClause::CaseClause(Expression* label, ZoneList<Statement*>* statements)
     : label_(label), statements_(statements) {}
 
+bool Literal::ToUint32(uint32_t* value) const {
+  if (IsSmi()) {
+    int num = AsSmiLiteral()->value();
+    if (num < 0) return false;
+    *value = static_cast<uint32_t>(num);
+    return true;
+  }
+  if (IsNumber()) {
+    return DoubleToUint32IfEqualToSelf(AsNumber(), value);
+  }
+  return false;
+}
+
+bool Literal::ToArrayIndex(uint32_t* value) const {
+  return ToUint32(value) && *value != kMaxUInt32;
+}
+
 uint32_t Literal::Hash() {
-  return raw_value()->IsString()
-             ? raw_value()->AsString()->Hash()
-             : ComputeLongHash(double_to_uint64(raw_value()->AsNumber()));
+  return IsString() ? AsRawString()->Hash()
+                    : ComputeLongHash(double_to_uint64(AsNumber()));
 }
 
 
 // static
 bool Literal::Match(void* literal1, void* literal2) {
-  const AstValue* x = static_cast<Literal*>(literal1)->raw_value();
-  const AstValue* y = static_cast<Literal*>(literal2)->raw_value();
+  const AstValue* x = static_cast<Literal*>(literal1)->value_;
+  const AstValue* y = static_cast<Literal*>(literal2)->value_;
   return (x->IsString() && y->IsString() && x->AsString() == y->AsString()) ||
          (x->IsNumber() && y->IsNumber() && x->AsNumber() == y->AsNumber());
 }

src/ast/ast.h

@@ -228,6 +228,9 @@ class Expression : public AstNode {
   // True iff the expression is the null literal.
   bool IsNullLiteral() const;
 
+  // True iff the expression is the hole literal.
+  bool IsTheHoleLiteral() const;
+
   // True if we can prove that the expression is the undefined literal. Note
   // that this also checks for loads of the global "undefined" variable.
   bool IsUndefinedLiteral() const;
@@ -992,13 +995,36 @@ class Literal final : public Expression {
     return value_->AsString();
   }
 
-  Smi* AsSmiLiteral() {
-    DCHECK(IsSmiLiteral());
-    return raw_value()->AsSmi();
+  bool IsSmi() const { return value_->IsSmi(); }
+  Smi* AsSmiLiteral() const {
+    DCHECK(IsSmi());
+    return value_->AsSmi();
+  }
+
+  bool IsNumber() const { return value_->IsNumber(); }
+  double AsNumber() const {
+    DCHECK(IsNumber());
+    return value_->AsNumber();
   }
 
-  bool ToBooleanIsTrue() const { return raw_value()->BooleanValue(); }
-  bool ToBooleanIsFalse() const { return !raw_value()->BooleanValue(); }
+  bool IsString() const { return value_->IsString(); }
+  const AstRawString* AsRawString() {
+    DCHECK(IsString());
+    return value_->AsString();
+  }
+
+  bool IsNull() const { return value_->IsNull(); }
+  bool IsUndefined() const { return value_->IsUndefined(); }
+  bool IsTheHole() const { return value_->IsTheHole(); }
+
+  bool IsTrue() const { return value_->IsTrue(); }
+  bool IsFalse() const { return value_->IsFalse(); }
+
+  bool ToBooleanIsTrue() const { return value_->BooleanValue(); }
+  bool ToBooleanIsFalse() const { return !value_->BooleanValue(); }
+
+  bool ToUint32(uint32_t* value) const;
+  bool ToArrayIndex(uint32_t* value) const;
 
   Handle<Object> value() const { return value_->value(); }
   const AstValue* raw_value() const { return value_; }

src/interpreter/bytecode-array-builder.cc

@@ -586,21 +586,9 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::LoadLiteral(const Scope* scope) {
 
 BytecodeArrayBuilder& BytecodeArrayBuilder::LoadLiteral(
     const AstValue* ast_value) {
-  if (ast_value->IsSmi()) {
-    return LoadLiteral(ast_value->AsSmi());
-  } else if (ast_value->IsUndefined()) {
-    return LoadUndefined();
-  } else if (ast_value->IsTrue()) {
-    return LoadTrue();
-  } else if (ast_value->IsFalse()) {
-    return LoadFalse();
-  } else if (ast_value->IsNull()) {
-    return LoadNull();
-  } else if (ast_value->IsTheHole()) {
-    return LoadTheHole();
-  } else if (ast_value->IsString()) {
-    return LoadLiteral(ast_value->AsString());
-  } else if (ast_value->IsHeapNumber() || ast_value->IsBigInt()) {
+  DCHECK(ast_value->IsHeapNumber() || ast_value->IsBigInt() ||
+         ast_value->IsSymbol());
+  if (ast_value->IsHeapNumber() || ast_value->IsBigInt()) {
     size_t entry = GetConstantPoolEntry(ast_value);
     OutputLdaConstant(entry);
     return *this;

src/interpreter/bytecode-flags.cc

@@ -43,7 +43,7 @@ uint8_t CreateClosureFlags::Encode(bool pretenure, bool is_function_scope) {
 // static
 TestTypeOfFlags::LiteralFlag TestTypeOfFlags::GetFlagForLiteral(
     const AstStringConstants* ast_constants, Literal* literal) {
-  const AstRawString* raw_literal = literal->raw_value()->AsString();
+  const AstRawString* raw_literal = literal->AsRawString();
   if (raw_literal == ast_constants->number_string()) {
     return LiteralFlag::kNumber;
   } else if (raw_literal == ast_constants->string_string()) {

src/interpreter/bytecode-generator.cc

@@ -1967,9 +1967,25 @@ void BytecodeGenerator::VisitConditional(Conditional* expr) {
 
 void BytecodeGenerator::VisitLiteral(Literal* expr) {
   if (!execution_result()->IsEffect()) {
-    const AstValue* raw_value = expr->raw_value();
-    builder()->LoadLiteral(raw_value);
-    if (raw_value->IsTrue() || raw_value->IsFalse()) {
+    if (expr->IsSmi()) {
+      builder()->LoadLiteral(expr->AsSmiLiteral());
+    } else if (expr->IsUndefined()) {
+      builder()->LoadUndefined();
+    } else if (expr->IsTrue()) {
+      builder()->LoadTrue();
+    } else if (expr->IsFalse()) {
+      builder()->LoadFalse();
+    } else if (expr->IsNull()) {
+      builder()->LoadNull();
+    } else if (expr->IsTheHole()) {
+      builder()->LoadTheHole();
+    } else if (expr->IsString()) {
+      builder()->LoadLiteral(expr->AsRawString());
+    } else {
+      // TODO(adamk): Get rid of this case.
+      builder()->LoadLiteral(expr->raw_value());
+    }
+    if (expr->IsTrue() || expr->IsFalse()) {
       execution_result()->SetResultIsBoolean();
     }
   }

src/parsing/parser.cc

@@ -245,10 +245,9 @@ FunctionLiteral* Parser::DefaultConstructor(const AstRawString* name,
 bool Parser::ShortcutNumericLiteralBinaryExpression(Expression** x,
                                                     Expression* y,
                                                     Token::Value op, int pos) {
-  if ((*x)->AsLiteral() && (*x)->AsLiteral()->raw_value()->IsNumber() &&
-      y->AsLiteral() && y->AsLiteral()->raw_value()->IsNumber()) {
-    double x_val = (*x)->AsLiteral()->raw_value()->AsNumber();
-    double y_val = y->AsLiteral()->raw_value()->AsNumber();
+  if ((*x)->IsNumberLiteral() && y->IsNumberLiteral()) {
+    double x_val = (*x)->AsLiteral()->AsNumber();
+    double y_val = y->AsLiteral()->AsNumber();
     switch (op) {
       case Token::ADD:
         *x = factory()->NewNumberLiteral(x_val + y_val, pos);
@@ -311,13 +310,12 @@ bool Parser::ShortcutNumericLiteralBinaryExpression(Expression** x,
 Expression* Parser::BuildUnaryExpression(Expression* expression,
                                          Token::Value op, int pos) {
   DCHECK_NOT_NULL(expression);
-  if (expression->IsLiteral()) {
-    const AstValue* literal = expression->AsLiteral()->raw_value();
+  const Literal* literal = expression->AsLiteral();
+  if (literal != nullptr) {
     if (op == Token::NOT) {
       // Convert the literal to a boolean condition and negate it.
-      bool condition = literal->BooleanValue();
-      return factory()->NewBooleanLiteral(!condition, pos);
-    } else if (literal->IsNumber()) {
+      return factory()->NewBooleanLiteral(literal->ToBooleanIsFalse(), pos);
+    } else if (literal->IsNumberLiteral()) {
       // Compute some expressions involving only number literals.
       double value = literal->AsNumber();
       switch (op) {
@@ -3548,7 +3546,7 @@ int32_t Parser::ComputeTemplateLiteralHash(const TemplateLiteral* lit) {
     }
 
     const AstRawString* raw_string =
-        raw_strings->at(index)->AsLiteral()->raw_value()->AsString();
+        raw_strings->at(index)->AsLiteral()->AsRawString();
     if (raw_string->is_one_byte()) {
       const char* data = reinterpret_cast<const char*>(raw_string->raw_data());
       running_hash = StringHasher::ComputeRunningHashOneByte(
@@ -3912,8 +3910,7 @@ Expression* Parser::RewriteSpreads(ArrayLiteral* lit) {
       // %AppendElement($R, value)
       // or, in case of a hole,
       // ++($R.length)
-      if (!value->IsLiteral() ||
-          !value->AsLiteral()->raw_value()->IsTheHole()) {
+      if (!value->IsTheHoleLiteral()) {
         ZoneList<Expression*>* append_element_args = NewExpressionList(2);
         append_element_args->Add(factory()->NewVariableProxy(result), zone());
         append_element_args->Add(value, zone());

src/parsing/parser.h

@@ -685,8 +685,8 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
     ExpressionStatement* e_stat = statement->AsExpressionStatement();
     if (e_stat == nullptr) return false;
     Literal* literal = e_stat->expression()->AsLiteral();
-    if (literal == nullptr || !literal->raw_value()->IsString()) return false;
-    return arg == nullptr || literal->raw_value()->AsString() == arg;
+    if (literal == nullptr || !literal->IsString()) return false;
+    return arg == nullptr || literal->AsRawString() == arg;
   }
 
   V8_INLINE void GetDefaultStrings(

src/parsing/pattern-rewriter.cc

@@ -539,7 +539,7 @@ void PatternRewriter::VisitArrayLiteral(ArrayLiteral* node,
     }
     block_->statements()->Add(if_not_done, zone());
 
-    if (!(value->IsLiteral() && value->AsLiteral()->raw_value()->IsTheHole())) {
+    if (!value->IsTheHoleLiteral()) {
       {
         // completion = kAbruptCompletion;
         Expression* proxy = factory()->NewVariableProxy(completion);

test/cctest/interpreter/test-interpreter.cc

@@ -390,6 +390,27 @@ TEST(InterpreterBinaryOpsHeapNumber) {
   }
 }
 
+namespace {
+// Follows the same logic as BytecodeGraphBuilder::VisitLiteral().
+void LoadLiteralForTest(BytecodeArrayBuilder* builder, const AstValue* value) {
+  if (value->IsString()) {
+    builder->LoadLiteral(value->AsString());
+  } else if (value->IsSmi()) {
+    builder->LoadLiteral(value->AsSmi());
+  } else if (value->IsUndefined()) {
+    builder->LoadUndefined();
+  } else if (value->IsNull()) {
+    builder->LoadNull();
+  } else if (value->IsTrue()) {
+    builder->LoadTrue();
+  } else if (value->IsFalse()) {
+    builder->LoadFalse();
+  } else {
+    builder->LoadLiteral(value);
+  }
+}
+}  // namespace
+
 TEST(InterpreterStringAdd) {
   HandleAndZoneScope handles;
   Isolate* isolate = handles.main_isolate();
@@ -442,11 +463,9 @@ TEST(InterpreterStringAdd) {
         NewFeedbackMetadata(isolate, &feedback_spec);
 
     Register reg(0);
-    builder.LoadLiteral(test_cases[i].lhs)
-        .StoreAccumulatorInRegister(reg)
-        .LoadLiteral(test_cases[i].rhs)
-        .BinaryOperation(Token::Value::ADD, reg, GetIndex(slot))
-        .Return();
+    builder.LoadLiteral(test_cases[i].lhs).StoreAccumulatorInRegister(reg);
+    LoadLiteralForTest(&builder, test_cases[i].rhs);
+    builder.BinaryOperation(Token::Value::ADD, reg, GetIndex(slot)).Return();
     ast_factory.Internalize(isolate);
     Handle<BytecodeArray> bytecode_array = builder.ToBytecodeArray(isolate);
 
@@ -659,11 +678,10 @@ TEST(InterpreterBinaryOpTypeFeedback) {
         i::NewFeedbackMetadata(isolate, &feedback_spec);
 
     Register reg(0);
-    builder.LoadLiteral(test_case.arg1)
-        .StoreAccumulatorInRegister(reg)
-        .LoadLiteral(test_case.arg2)
-        .BinaryOperation(test_case.op, reg, GetIndex(slot0))
-        .Return();
+    LoadLiteralForTest(&builder, test_case.arg1);
+    builder.StoreAccumulatorInRegister(reg);
+    LoadLiteralForTest(&builder, test_case.arg2);
+    builder.BinaryOperation(test_case.op, reg, GetIndex(slot0)).Return();
 
     ast_factory.Internalize(isolate);
     Handle<BytecodeArray> bytecode_array = builder.ToBytecodeArray(isolate);
@@ -772,8 +790,8 @@ TEST(InterpreterBinaryOpSmiTypeFeedback) {
         i::NewFeedbackMetadata(isolate, &feedback_spec);
 
     Register reg(0);
-    builder.LoadLiteral(test_case.arg1)
-        .StoreAccumulatorInRegister(reg)
+    LoadLiteralForTest(&builder, test_case.arg1);
+    builder.StoreAccumulatorInRegister(reg)
         .LoadLiteral(Smi::FromInt(test_case.arg2))
         .BinaryOperation(test_case.op, reg, GetIndex(slot0))
         .Return();
@@ -1359,25 +1377,25 @@ TEST(InterpreterCall) {
         .StoreAccumulatorInRegister(reg)
         .LoadAccumulatorWithRegister(builder.Receiver())
         .StoreAccumulatorInRegister(args[0])
-        .LoadLiteral(ast_factory.NewString(ast_factory.GetOneByteString("a")))
+        .LoadLiteral(ast_factory.GetOneByteString("a"))
         .StoreAccumulatorInRegister(args[1])
-        .LoadLiteral(ast_factory.NewString(ast_factory.GetOneByteString("b")))
+        .LoadLiteral(ast_factory.GetOneByteString("b"))
         .StoreAccumulatorInRegister(args[2])
-        .LoadLiteral(ast_factory.NewString(ast_factory.GetOneByteString("c")))
+        .LoadLiteral(ast_factory.GetOneByteString("c"))
         .StoreAccumulatorInRegister(args[3])
-        .LoadLiteral(ast_factory.NewString(ast_factory.GetOneByteString("d")))
+        .LoadLiteral(ast_factory.GetOneByteString("d"))
         .StoreAccumulatorInRegister(args[4])
-        .LoadLiteral(ast_factory.NewString(ast_factory.GetOneByteString("e")))
+        .LoadLiteral(ast_factory.GetOneByteString("e"))
         .StoreAccumulatorInRegister(args[5])
-        .LoadLiteral(ast_factory.NewString(ast_factory.GetOneByteString("f")))
+        .LoadLiteral(ast_factory.GetOneByteString("f"))
         .StoreAccumulatorInRegister(args[6])
-        .LoadLiteral(ast_factory.NewString(ast_factory.GetOneByteString("g")))
+        .LoadLiteral(ast_factory.GetOneByteString("g"))
         .StoreAccumulatorInRegister(args[7])
-        .LoadLiteral(ast_factory.NewString(ast_factory.GetOneByteString("h")))
+        .LoadLiteral(ast_factory.GetOneByteString("h"))
         .StoreAccumulatorInRegister(args[8])
-        .LoadLiteral(ast_factory.NewString(ast_factory.GetOneByteString("i")))
+        .LoadLiteral(ast_factory.GetOneByteString("i"))
         .StoreAccumulatorInRegister(args[9])
-        .LoadLiteral(ast_factory.NewString(ast_factory.GetOneByteString("j")))
+        .LoadLiteral(ast_factory.GetOneByteString("j"))
         .StoreAccumulatorInRegister(args[10]);
 
     builder.CallProperty(reg, args, call_slot_index);
@@ -1854,9 +1872,9 @@ static void LoadStringAndAddSpace(BytecodeArrayBuilder* builder,
   Register string_reg = builder->register_allocator()->NewRegister();
 
   (*builder)
-      .LoadLiteral(ast_factory->NewString(ast_factory->GetOneByteString(cstr)))
+      .LoadLiteral(ast_factory->GetOneByteString(cstr))
       .StoreAccumulatorInRegister(string_reg)
-      .LoadLiteral(ast_factory->NewString(ast_factory->GetOneByteString(" ")))
+      .LoadLiteral(ast_factory->GetOneByteString(" "))
       .BinaryOperation(Token::Value::ADD, string_reg,
                        GetIndex(string_add_slot));
 }
@@ -1913,8 +1931,7 @@ TEST(InterpreterMixedComparisons) {
 
               if (string_type == kInternalizedStringConstant) {
                 // rhs string is internalized.
-                builder.LoadLiteral(ast_factory.NewString(
-                    ast_factory.GetOneByteString(rhs_cstr)));
+                builder.LoadLiteral(ast_factory.GetOneByteString(rhs_cstr));
               } else {
                 CHECK_EQ(string_type, kComputedString);
                 // rhs string is not internalized (append a space to the end).
@@ -1928,8 +1945,7 @@ TEST(InterpreterMixedComparisons) {
 
               if (string_type == kInternalizedStringConstant) {
                 // lhs string is internalized
-                builder.LoadLiteral(ast_factory.NewString(
-                    ast_factory.GetOneByteString(lhs_cstr)));
+                builder.LoadLiteral(ast_factory.GetOneByteString(lhs_cstr));
               } else {
                 CHECK_EQ(string_type, kComputedString);
                 // lhs string is not internalized (append a space to the end).
@@ -2211,9 +2227,8 @@ TEST(InterpreterUnaryNotNonBoolean) {
     BytecodeArrayBuilder builder(isolate, zone, 1, 0);
 
     Register r0(0);
-    builder.LoadLiteral(object_type_tuples[i].first);
-    builder.LogicalNot(ToBooleanMode::kConvertToBoolean);
-    builder.Return();
+    LoadLiteralForTest(&builder, object_type_tuples[i].first);
+    builder.LogicalNot(ToBooleanMode::kConvertToBoolean).Return();
     ast_factory.Internalize(isolate);
     Handle<BytecodeArray> bytecode_array = builder.ToBytecodeArray(isolate);
     InterpreterTester tester(isolate, bytecode_array);

test/unittests/interpreter/bytecode-array-builder-unittest.cc

@@ -57,8 +57,7 @@ TEST_F(BytecodeArrayBuilderTest, AllBytecodesGenerated) {
       .StoreAccumulatorInRegister(reg)
       .LoadLiteral(Smi::FromInt(10000000))
       .StoreAccumulatorInRegister(reg)
-      .LoadLiteral(
-          ast_factory.NewString(ast_factory.GetOneByteString("A constant")))
+      .LoadLiteral(ast_factory.GetOneByteString("A constant"))
       .StoreAccumulatorInRegister(reg)
       .LoadUndefined()
       .StoreAccumulatorInRegister(reg)
@@ -517,10 +516,9 @@ TEST_F(BytecodeArrayBuilderTest, Constants) {
 
   const AstValue* heap_num_1 = ast_factory.NewNumber(3.14);
   const AstValue* heap_num_2 = ast_factory.NewNumber(5.2);
-  const AstValue* string =
-      ast_factory.NewString(ast_factory.GetOneByteString("foo"));
-  const AstValue* string_copy =
-      ast_factory.NewString(ast_factory.GetOneByteString("foo"));
+  const AstValue* heap_num_2_copy = ast_factory.NewNumber(5.2);
+  const AstRawString* string = ast_factory.GetOneByteString("foo");
+  const AstRawString* string_copy = ast_factory.GetOneByteString("foo");
 
   builder.LoadLiteral(heap_num_1)
       .LoadLiteral(heap_num_2)
@@ -528,12 +526,13 @@ TEST_F(BytecodeArrayBuilderTest, Constants) {
       .LoadLiteral(heap_num_1)
       .LoadLiteral(heap_num_1)
       .LoadLiteral(string_copy)
+      .LoadLiteral(heap_num_2_copy)
       .Return();
 
   ast_factory.Internalize(isolate());
   Handle<BytecodeArray> array = builder.ToBytecodeArray(isolate());
-  // Should only have one entry for each identical constant.
-  CHECK_EQ(array->constant_pool()->length(), 3);
+  // Should only have one entry for each identical string constant.
+  EXPECT_EQ(4, array->constant_pool()->length());
 }
 
 TEST_F(BytecodeArrayBuilderTest, ForwardJumps) {