[Interpreter] Print constant pool in generate-bytecode-expectations

test/cctest/interpreter/generate-bytecode-expectations.cc

Index: test/cctest/interpreter/generate-bytecode-expectations.cc
diff --git a/test/cctest/interpreter/generate-bytecode-expectations.cc b/test/cctest/interpreter/generate-bytecode-expectations.cc
index 4fc54e12c91010fae925cd30e6ed9ffe0c95f4a3..71ec36d52f40084532ed81304a647f94a348c813 100644
--- a/test/cctest/interpreter/generate-bytecode-expectations.cc
+++ b/test/cctest/interpreter/generate-bytecode-expectations.cc
@@ -164,45 +164,118 @@ void PrintBytecode(std::ostream& stream,
   }
 }
 
+void PrintQuotedUC16Char(std::ostream& stream, uint16_t c) {
+  switch (c) {
+    case '"':
+      stream << "\\\"";
+      break;
+    case '\n':
+      stream << "\\n";
+      break;
+    case '\t':
+      stream << "\\t";
+      break;
+    case '\\':
+      stream << "\\\\";
+      break;
+    default:
+      stream << i::AsUC16(c);
+      break;
+  }
+}
+
 std::string QuoteCString(const std::string& source) {
-  std::string quoted_buffer;
+  std::stringstream quoted_buffer;
   for (char c : source) {
-    switch (c) {
-      case '"':
-        quoted_buffer += "\\\"";
-        break;
-      case '\n':
-        quoted_buffer += "\\n";
-        break;
-      case '\t':
-        quoted_buffer += "\\t";
-        break;
-      case '\\':
-        quoted_buffer += "\\\\";
-        break;
-      default:
-        quoted_buffer += c;
-        break;
-    }
+    PrintQuotedUC16Char(quoted_buffer, c);
   }
-  return quoted_buffer;
+  return quoted_buffer.str();
 }

[rmcilroy, 2016/02/10 15:20:32]

nit - move these PrintQuotedUC16Char and QuoteCStrin above PrintBytecodeOperand
(general helpers functions first, then the element printer functions).

[Stefano Sanfilippo, 2016/02/10 15:40:16]

Done.

Also, please notice that I got rid of the former.

 
-void PrintBytecodeArray(std::ostream& stream,
-                        i::Handle<i::BytecodeArray> bytecode_array,
-                        const std::string& body, bool print_banner = true) {
-  const int kPointerSize = sizeof(void*);
+enum ConstantPoolType {
+  kConstantPoolTypeUnknown,
+  kConstantPoolTypeString,
+  kConstantPoolTypeInteger,
+  kConstantPoolTypeDouble,
+  kConstantPoolTypeMixed,
+};

[rmcilroy, 2016/02/10 15:20:32]

nit - move this to the top of the namespace

[Stefano Sanfilippo, 2016/02/10 15:40:15]

Done.

 
-  if (print_banner) {
-    stream << kIndent << "// === ExpectedSnippet generated by "
-                         "generate-bytecode-expectations. ===\n";
+void PrintConstantString(std::ostream& stream, i::String* constant_string) {

[rmcilroy, 2016/02/10 15:20:32]

/s/PrintConstantString/PrintV8String/
/s/constant_string/string/

[Stefano Sanfilippo, 2016/02/10 15:40:15]

Done.

+  stream << '"';
+  for (int i = 0, length = constant_string->length(); i < length; ++i) {
+    PrintQuotedUC16Char(stream, constant_string->Get(i));
   }
+  stream << '"';
+}
 
-  // Print the code snippet as a quoted C string.
-  stream << kIndent << "{" << '"' << QuoteCString(body) << "\",\n" << kIndent;
+void PrintConstant(std::ostream& stream,
+                   ConstantPoolType expected_constant_type,
+                   i::Handle<i::Object> constant) {
+  switch (expected_constant_type) {
+    case kConstantPoolTypeString:
+      CHECK(constant->IsString());
+      PrintConstantString(std::cout, i::String::cast(*constant));

[rmcilroy, 2016/02/10 15:20:32]

/s/std::cout/stream

[Stefano Sanfilippo, 2016/02/10 15:40:15]

Done.

+      break;
+    case kConstantPoolTypeInteger:
+      if (constant->IsSmi()) {
+        i::Smi::cast(*constant)->SmiPrint(stream);
+      } else if (constant->IsHeapNumber()) {
+        i::HeapNumber::cast(*constant)->HeapNumberPrint(stream);
+      } else {
+        UNREACHABLE();
+      }
+      break;
+    case kConstantPoolTypeDouble:
+      i::HeapNumber::cast(*constant)->HeapNumberPrint(stream);
+      break;
+    case kConstantPoolTypeMixed:
+      if (constant->IsSmi()) {
+        stream << "kInstanceTypeDontCare";
+      } else {
+        stream << "InstanceType::"
+               << i::HeapObject::cast(*constant)->map()->instance_type();
+      }
+      break;
+    default:
+      UNREACHABLE();
+      return;
+  }
+}
+
+void PrintConstantPool(std::ostream& stream, i::FixedArray* constant_pool,
+                       ConstantPoolType expected_constant_type,
+                       v8::Isolate* isolate) {
+  int num_constants = constant_pool->length();
+  stream << "\n" << kIndent << " },\n" << kIndent << ' ' << num_constants;
+  if (num_constants > 0) {

[rmcilroy, 2016/02/10 15:20:32]

I would be fine with just always emitting a '{}' even if the num_constants is
empty, which would avoid you having to special case the handler stuff below.

[Stefano Sanfilippo, 2016/02/10 15:40:15]

You are right, although I'd prefer to skip the {} part since the test cases
actually put 0 without the {}. In the end, the extra code for handling that case
boils down to:

  if (constant_pool->lenght() > 0 && /* have handlers */) {
    stream << ",\n" << kIndent << "{},\n";
  }

WDYT?

+    stream << ",\n" << kIndent << " {";
+    for (int i = 0; i < num_constants; ++i) {
+      // Print separator before each constant, except the first one
+      if (i != 0) stream << ", ";
+      PrintConstant(
+          stream, expected_constant_type,
+          i::FixedArray::get(constant_pool, i, GetInternalIsolate(isolate)));
+    }
+    stream << '}';
+  }
+  stream << '\n';
+}
+
+void PrintBytecodeSequence(std::ostream& stream,
+                           i::Handle<i::BytecodeArray> bytecode_array) {
+  BytecodeArrayIterator bytecode_iter{bytecode_array};
+  for (; !bytecode_iter.done(); bytecode_iter.Advance()) {
+    // Print separator before each instruction, except the first one.
+    if (bytecode_iter.current_offset() > 0) {
+      stream << ",\n" << kIndent << "     ";
+    }
+    PrintBytecode(stream, bytecode_iter);
+  }
+}
+
+void PrintFrameSize(std::ostream& stream, int frame_size) {

[rmcilroy, 2016/02/10 15:20:32]

nit - order the functions in the same order as they are called below (i.e.
PrintFrameSize, PrintBytecodeSequence, PrintConstantPool.

[Stefano Sanfilippo, 2016/02/10 15:40:15]

Done.

+  const int kPointerSize = sizeof(void*);
 
-  // Print the frame size, in multiples of kPointerSize.
-  int frame_size = bytecode_array->frame_size();
   DCHECK(frame_size % kPointerSize == 0);
   if (frame_size > kPointerSize) {
     stream << ' ' << frame_size / kPointerSize << " * kPointerSize,\n"
@@ -212,6 +285,22 @@ void PrintBytecodeArray(std::ostream& stream,
   } else if (frame_size == 0) {
     stream << " 0,\n" << kIndent;
   }
+}
+
+void PrintBytecodeArray(std::ostream& stream,
+                        i::Handle<i::BytecodeArray> bytecode_array,
+                        const std::string& body, v8::Isolate* isolate,
+                        ConstantPoolType constant_pool_type,
+                        bool print_banner = true) {
+  if (print_banner) {
+    stream << kIndent << "// === ExpectedSnippet generated by "
+                         "generate-bytecode-expectations. ===\n";
+  }
+
+  // Print the code snippet as a quoted C string.
+  stream << kIndent << "{" << '"' << QuoteCString(body) << "\",\n" << kIndent;
+
+  PrintFrameSize(stream, bytecode_array->frame_size());
 
   // Print parameter count and size of the bytecode array.
   stream << ' ' << bytecode_array->parameter_count() << ",\n"
@@ -219,22 +308,47 @@ void PrintBytecodeArray(std::ostream& stream,
          << kIndent << " {\n"
          << kIndent << "     ";
 
-  // Print bytecodes.
-  BytecodeArrayIterator bytecode_iter{bytecode_array};
-  for (; !bytecode_iter.done(); bytecode_iter.Advance()) {
-    // Print separator before each instruction, except the first one.
-    if (bytecode_iter.current_offset() > 0) {
-      stream << ",\n" << kIndent << "     ";
-    }
-    PrintBytecode(stream, bytecode_iter);
-  }
+  PrintBytecodeSequence(stream, bytecode_array);
+
+  PrintConstantPool(stream, bytecode_array->constant_pool(), constant_pool_type,
+                    isolate);
+
+  // TODO(ssanfilippo) print handlers.
+  //
+  // Remember that if we have handlers but not constants, we still need
+  // to emit " 0, {},\n" before the following section.
+  //
+  // For now we leave here a check.
+  //

[rmcilroy, 2016/02/10 15:20:32]

nit - remove the comment (other than the TODO) given the comment above about
always emitting the '{}'.

[Stefano Sanfilippo, 2016/02/10 15:40:15]

Done.

+  i::HandlerTable* handlers =
+      i::HandlerTable::cast(bytecode_array->handler_table());
+  CHECK_EQ(handlers->NumberOfRangeEntries(), 0);
 
-  stream << "\n" << kIndent << " },\n";
-  // TODO(ssanfilippo) add representation of constant pool and handlers.
-  stream << kIndent << " // constant pool and handlers here\n";
   stream << kIndent << "}\n";
 }
 
+void PrintExpectedSnippet(ConstantPoolType parsed_constant_pool_type,

[rmcilroy, 2016/02/10 15:20:32]

/s/parsed_constant_pool_type/constant_pool_type/

[Stefano Sanfilippo, 2016/02/10 15:40:15]

Done.

+                          char* exec_path, std::string body) {
+  const char* wrapper_function_name = "__genbckexp_wrapper__";
+
+  V8InitializationScope platform(exec_path);
+  {
+    v8::Isolate::Scope isolate_scope(platform.isolate());
+    v8::HandleScope handle_scope(platform.isolate());
+    v8::Local<v8::Context> context = v8::Context::New(platform.isolate());
+    v8::Context::Scope context_scope(context);
+
+    std::string source_code = WrapCodeInFunction(wrapper_function_name, body);
+    CompileAndRun(platform.isolate(), context, source_code.c_str());
+
+    i::Handle<i::BytecodeArray> bytecode_array = GetBytecodeArrayForGlobal(
+        platform.isolate(), context, wrapper_function_name);
+
+    PrintBytecodeArray(std::cout, bytecode_array, body, platform.isolate(),
+                       parsed_constant_pool_type);
+  }
+}
+
 bool ReadFromFileOrStdin(std::string* body, const char* body_filename) {
   std::stringstream body_buffer;
   if (strcmp(body_filename, "-") == 0) {
@@ -248,10 +362,24 @@ bool ReadFromFileOrStdin(std::string* body, const char* body_filename) {
   return true;
 }
 
+ConstantPoolType ParseConstantPoolType(const char* type_string) {
+  if (strcmp(type_string, "int") == 0) {
+    return kConstantPoolTypeInteger;
+  } else if (strcmp(type_string, "double") == 0) {
+    return kConstantPoolTypeDouble;
+  } else if (strcmp(type_string, "string") == 0) {
+    return kConstantPoolTypeString;
+  } else if (strcmp(type_string, "mixed") == 0) {
+    return kConstantPoolTypeMixed;
+  }
+  return kConstantPoolTypeUnknown;
+}
+
 void PrintUsage(const char* exec_path) {
   std::cerr << "Usage: " << exec_path
-            << " [filename.js|-]\n\n"
-               "No arguments or - reads from standard input.\n"
+            << " (int|double|string|mixed) [filename.js|-]\n\n"
+               "First argument is the type of objects in the constant pool.\n\n"
+               "Omitting the second argument or - reads from standard input.\n"
                "Anything else is interpreted as a filename.\n\n"
                "This tool is intended as a help in writing tests.\n"
                "Please, DO NOT blindly copy and paste the output "
@@ -261,13 +389,18 @@ void PrintUsage(const char* exec_path) {
 }  // namespace
 
 int main(int argc, char** argv) {
+  if (argc < 2) {
+    PrintUsage(argv[0]);
+    return 1;
+  }
+
   if (argc > 1 && strcmp(argv[1], "--help") == 0) {
     PrintUsage(argv[0]);
     return 0;
   }
 
-  const char* body_filename = (argc > 1 ? argv[1] : "-");
-  const char* wrapper_function_name = "__genbckexp_wrapper__";
+  const char* body_filename = (argc > 2 ? argv[2] : "-");
+  const char* const_pool_type_string = argv[1];
 
   std::string body;
   if (!ReadFromFileOrStdin(&body, body_filename)) {
@@ -276,19 +409,6 @@ int main(int argc, char** argv) {
     return 1;
   }
 
-  V8InitializationScope platform(argv[0]);
-  {
-    v8::Isolate::Scope isolate_scope(platform.isolate());
-    v8::HandleScope handle_scope(platform.isolate());
-    v8::Local<v8::Context> context = v8::Context::New(platform.isolate());
-    v8::Context::Scope context_scope(context);
-
-    std::string source_code = WrapCodeInFunction(wrapper_function_name, body);
-    CompileAndRun(platform.isolate(), context, source_code.c_str());
-
-    i::Handle<i::BytecodeArray> bytecode_array = GetBytecodeArrayForGlobal(
-        platform.isolate(), context, wrapper_function_name);
-
-    PrintBytecodeArray(std::cout, bytecode_array, body);
-  }
+  PrintExpectedSnippet(ParseConstantPoolType(const_pool_type_string), argv[0],
+                       body);
 }