[Interpreter] Change the output format of 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 74c3407d286d9915b69bdd33bd1789c02f0732f1..afc8e5d6504392a96bd05fb4e4e1069c8fb898e4 100644
--- a/test/cctest/interpreter/generate-bytecode-expectations.cc
+++ b/test/cctest/interpreter/generate-bytecode-expectations.cc
@@ -2,8 +2,10 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <cstring>
 #include <fstream>
-#include <iostream>
+
+#include "test/cctest/interpreter/bytecode-expectations-printer.h"
 
 #include "include/libplatform/libplatform.h"
 #include "include/v8.h"
@@ -11,24 +13,47 @@
 #include "src/base/logging.h"
 #include "src/base/smart-pointers.h"
 #include "src/compiler.h"
-
-#include "src/interpreter/bytecode-array-iterator.h"
-#include "src/interpreter/bytecode-generator.h"
-#include "src/interpreter/bytecodes.h"
 #include "src/interpreter/interpreter.h"
 
-using namespace i::interpreter;
+using namespace v8::internal::interpreter;
 
 namespace {
 
-const char* kIndent = "       ";
+BytecodeExpectationsPrinter::ConstantPoolType ParseConstantPoolType(
+    const char* type_string) {
+  if (strcmp(type_string, "int") == 0) {
+    return BytecodeExpectationsPrinter::ConstantPoolType::kInteger;
+  } else if (strcmp(type_string, "double") == 0) {
+    return BytecodeExpectationsPrinter::ConstantPoolType::kDouble;
+  } else if (strcmp(type_string, "string") == 0) {
+    return BytecodeExpectationsPrinter::ConstantPoolType::kString;
+  } else if (strcmp(type_string, "mixed") == 0) {
+    return BytecodeExpectationsPrinter::ConstantPoolType::kMixed;
+  }
+  return BytecodeExpectationsPrinter::ConstantPoolType::kUnknown;
+}

[rmcilroy, 2016/02/15 11:15:19]

nit - move below V8InitializationScope

[Stefano Sanfilippo, 2016/02/15 13:26:33]

Done.

+
+struct ProgramOptions {
+  static ProgramOptions FromCommandLine(int argc, char** argv);
+  bool Validate() const;
+
+  bool has_unknown_options() const { return has_unknown_options_; }
+  bool print_help() const { return print_help_; }
+  bool read_raw_js_snippet() const { return read_raw_js_snippet_; }
+  bool read_from_stdin() const { return read_from_stdin_; }
+  std::string filename() const { return filename_; }
+  BytecodeExpectationsPrinter::ConstantPoolType const_pool_type() const {
+    return const_pool_type_;
+  }
 
-enum ConstantPoolType {
-  kConstantPoolTypeUnknown,
-  kConstantPoolTypeString,
-  kConstantPoolTypeInteger,
-  kConstantPoolTypeDouble,
-  kConstantPoolTypeMixed,
+ private:
+  bool has_unknown_options_ = false;
+  bool print_help_ = false;
+  bool read_raw_js_snippet_ = false;
+  bool read_from_stdin_ = false;
+  std::string filename_;
+  BytecodeExpectationsPrinter::ConstantPoolType const_pool_type_ =
+      BytecodeExpectationsPrinter::ConstantPoolType::kMixed;

[rmcilroy, 2016/02/15 11:15:19]

Don't use default values in fields, initialize them to their default values in
the constructor instead.

[Stefano Sanfilippo, 2016/02/15 13:26:34]

Done.

 };
 
 class ArrayBufferAllocator final : public v8::ArrayBuffer::Allocator {
@@ -57,6 +82,59 @@ class V8InitializationScope final {
   DISALLOW_COPY_AND_ASSIGN(V8InitializationScope);
 };
 
+/* static */

[rmcilroy, 2016/02/15 11:15:19]

nit - line comments instead of block comments (i.e. "// static")

[Stefano Sanfilippo, 2016/02/15 13:26:33]

Done.

+ProgramOptions ProgramOptions::FromCommandLine(int argc, char** argv) {
+  ProgramOptions options;
+
+  if (argc <= 1) return options;
+
+  bool expecting_const_pool_type = false;
+  for (int i = 1; i < argc; ++i) {
+    if (expecting_const_pool_type) {
+      options.const_pool_type_ = ParseConstantPoolType(argv[i]);
+      expecting_const_pool_type = false;
+    } else if (strcmp(argv[i], "--help") == 0) {
+      options.print_help_ = true;
+    } else if (strcmp(argv[i], "--raw-js") == 0) {
+      options.read_raw_js_snippet_ = true;
+    } else if (strcmp(argv[i], "--pool-type") == 0) {

[rmcilroy, 2016/02/15 11:15:20]

Could you just make this a --pool-type=..." argument to avoid having to have the
expecting_const_pool_type bool?

[Stefano Sanfilippo, 2016/02/15 13:26:33]

Done.

+      expecting_const_pool_type = true;
+    } else if (strcmp(argv[i], "--stdin") == 0) {
+      options.read_from_stdin_ = true;
+    } else if (strncmp(argv[i], "--", 2) != 0) {  // It doesn't start with --
+      options.filename_ = argv[i];

[rmcilroy, 2016/02/15 11:15:19]

check options.filename_ is empty beforehand (i.e., only one filename passed)

[Stefano Sanfilippo, 2016/02/15 13:26:33]

Done.

+    } else {
+      std::cerr << "ERROR: Unknonwn option " << argv[i] << "\n";
+      options.has_unknown_options_ = true;
+    }
+  }
+
+  return options;
+}
+
+bool ProgramOptions::Validate() const {
+  if (has_unknown_options_) return false;
+  if (print_help_) return true;
+
+  if (const_pool_type_ ==
+      BytecodeExpectationsPrinter::ConstantPoolType::kUnknown) {
+    std::cerr << "ERROR: Unknown constant pool type.\n";
+    return false;
+  }
+
+  if (!read_from_stdin_ && filename_.empty()) {
+    std::cerr << "ERROR: No input file specified.\n";
+    return false;
+  }
+
+  if (read_from_stdin_ && !filename_.empty()) {
+    std::cerr << "ERROR: Reading from stdin, but input files supplied.\n";
+    return false;
+  }
+
+  return true;
+}
+
 i::Isolate* GetInternalIsolate(v8::Isolate* isolate) {
   return reinterpret_cast<i::Isolate*>(isolate);
 }
@@ -86,333 +164,111 @@ V8InitializationScope::~V8InitializationScope() {
   v8::V8::ShutdownPlatform();
 }
 
-v8::Local<v8::String> V8StringFromUTF8(v8::Isolate* isolate, const char* data) {
-  return v8::String::NewFromUtf8(isolate, data, v8::NewStringType::kNormal)
-      .ToLocalChecked();
-}
-
-std::string WrapCodeInFunction(const char* function_name,
-                               const std::string& function_body) {
-  std::ostringstream program_stream;
-  program_stream << "function " << function_name << "() {" << function_body
-                 << "}\n"
-                 << function_name << "();";
-
-  return program_stream.str();
-}
-
-v8::Local<v8::Value> CompileAndRun(v8::Isolate* isolate,
-                                   const v8::Local<v8::Context>& context,
-                                   const char* program) {
-  v8::Local<v8::String> source = V8StringFromUTF8(isolate, program);
-  v8::Local<v8::Script> script =
-      v8::Script::Compile(context, source).ToLocalChecked();
-
-  v8::Local<v8::Value> result;
-  CHECK(script->Run(context).ToLocal(&result));
-
-  return result;
-}
-
-i::Handle<v8::internal::BytecodeArray> GetBytecodeArrayForGlobal(
-    v8::Isolate* isolate, const v8::Local<v8::Context>& context,
-    const char* global_name) {
-  v8::Local<v8::String> v8_global_name = V8StringFromUTF8(isolate, global_name);
-  v8::Local<v8::Function> function = v8::Local<v8::Function>::Cast(
-      context->Global()->Get(context, v8_global_name).ToLocalChecked());
-  i::Handle<i::JSFunction> js_function =
-      i::Handle<i::JSFunction>::cast(v8::Utils::OpenHandle(*function));
-
-  i::Handle<i::BytecodeArray> bytecodes = i::handle(
-      js_function->shared()->bytecode_array(), GetInternalIsolate(isolate));
-
-  return bytecodes;
+std::string ReadRawJSSnippet(std::istream& stream) {

[rmcilroy, 2016/02/15 11:15:19]

All references must be const
(https://google.github.io/styleguide/cppguide.html#Reference_Arguments).

[Stefano Sanfilippo, 2016/02/15 13:26:34]

As discussed, I think it's best if we keep it consistent with how streams are
passed around in the C++ standard library.

+  std::stringstream body_buffer;
+  CHECK(body_buffer << stream.rdbuf());
+  return body_buffer.str();
 }
 
-std::string QuoteCString(const std::string& source) {
-  std::string 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;
+bool ReadNextSnippet(std::string* string, std::istream& stream) {

[rmcilroy, 2016/02/15 11:15:20]

ditto. Also rename string->string_out and put it at the end of the parameter
list.

[Stefano Sanfilippo, 2016/02/15 13:26:33]

Done.

+  std::string line;
+  bool found_begin_snippet = false;
+  string->clear();
+  while (std::getline(stream, line)) {
+    if (line == "snippet: \"") {
+      found_begin_snippet = true;
+      continue;
     }
-  }
-  return quoted_buffer;
-}
-
-void PrintBytecodeOperand(std::ostream& stream,
-                          const BytecodeArrayIterator& bytecode_iter,
-                          const Bytecode& bytecode, int op_index) {
-  OperandType op_type = Bytecodes::GetOperandType(bytecode, op_index);
-  OperandSize op_size = Bytecodes::GetOperandSize(bytecode, op_index);
-
-  const char* size_tag;
-  switch (op_size) {
-    case OperandSize::kByte:
-      size_tag = "8";
-      break;
-    case OperandSize::kShort:
-      size_tag = "16";
-      break;
-    default:
-      UNREACHABLE();
-      return;
-  }
-
-  if (Bytecodes::IsRegisterOperandType(op_type)) {
-    Register register_value = bytecode_iter.GetRegisterOperand(op_index);
-    stream << 'R';
-    if (op_size != OperandSize::kByte) stream << size_tag;
-    stream << '(' << register_value.index() << ')';
-  } else {
-    stream << 'U' << size_tag << '(';
-
-    if (Bytecodes::IsImmediateOperandType(op_type)) {
-      // We need a cast, otherwise the result is printed as char.
-      stream << static_cast<int>(bytecode_iter.GetImmediateOperand(op_index));
-    } else if (Bytecodes::IsRegisterCountOperandType(op_type)) {
-      stream << bytecode_iter.GetRegisterCountOperand(op_index);
-    } else if (Bytecodes::IsIndexOperandType(op_type)) {
-      stream << bytecode_iter.GetIndexOperand(op_index);
+    if (!found_begin_snippet) continue;
+    if (line == "\"") return true;
+    // Skip two leading spaces we added, if the line is not shorter
+    if (line.size() >= 2) {
+      string->append(line.begin() + 2, line.end());
     } else {
-      UNREACHABLE();
+      *string += line;

[rmcilroy, 2016/02/15 11:15:20]

What string gets added by this? Maybe just only append lines which are greater
than/equal to 2 characters?

[Stefano Sanfilippo, 2016/02/15 13:26:34]

This is done to preserve the original format, i.e. if we have blank lines in the
input.

     }
-
-    stream << ')';
-  }
-}
-
-void PrintBytecode(std::ostream& stream,
-                   const BytecodeArrayIterator& bytecode_iter) {
-  Bytecode bytecode = bytecode_iter.current_bytecode();
-
-  stream << "B(" << Bytecodes::ToString(bytecode) << ')';
-
-  int operands_count = Bytecodes::NumberOfOperands(bytecode);
-  for (int op_index = 0; op_index < operands_count; ++op_index) {
-    stream << ", ";
-    PrintBytecodeOperand(stream, bytecode_iter, bytecode, op_index);
+    *string += '\n';
   }
+  return false;
 }
 
-void PrintV8String(std::ostream& stream, i::String* string) {
-  stream << '"';
-  for (int i = 0, length = string->length(); i < length; ++i) {
-    stream << i::AsEscapedUC16ForJSON(string->Get(i));
+bool ExtractSnippets(std::vector<std::string>* snippet_list,
+                     const ProgramOptions& options) {
+  if (options.read_from_stdin()) {
+    snippet_list->push_back(ReadRawJSSnippet(std::cin));

[rmcilroy, 2016/02/15 11:15:19]

You can pull this into the code below (e.g., if options.read_from_stdin() then
std::ostream = std::cin, else std::ostream = std::ifstream(options.filename())

[Stefano Sanfilippo, 2016/02/15 13:26:33]

Done, although in a slightly different way because fstreams cannot be assigned.

+    return true;
   }
-  stream << '"';
-}
-
-void PrintConstant(std::ostream& stream,
-                   ConstantPoolType expected_constant_type,
-                   i::Handle<i::Object> constant) {
-  switch (expected_constant_type) {
-    case kConstantPoolTypeString:
-      CHECK(constant->IsString());
-      PrintV8String(stream, i::String::cast(*constant));
-      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 PrintFrameSize(std::ostream& stream,
-                    i::Handle<i::BytecodeArray> bytecode_array) {
-  const int kPointerSize = sizeof(void*);
-  int frame_size = bytecode_array->frame_size();
-
-  stream << kIndent;
-
-  DCHECK(frame_size % kPointerSize == 0);
-  if (frame_size > kPointerSize) {
-    stream << ' ' << frame_size / kPointerSize << " * kPointerSize,\n"
-           << kIndent;
-  } else if (frame_size == kPointerSize) {
-    stream << " kPointerSize,\n" << kIndent;
-  } else if (frame_size == 0) {
-    stream << " 0,\n" << kIndent;
+  std::string snippet;
+  std::ifstream body_file{options.filename()};

[rmcilroy, 2016/02/15 11:15:19]

Please don't use curly bracket initialization for these callse - use normal
brackets (thoughout the CL).

[Stefano Sanfilippo, 2016/02/15 13:26:34]

Done.

+  if (!body_file.is_open()) {
+    std::cerr << "ERROR: Could not open '" << options.filename() << "'.\n";
+    return false;
   }
-
-  stream << ' ' << bytecode_array->parameter_count() << ",\n";
-}
-
-void PrintBytecodeSequence(std::ostream& stream,
-                           i::Handle<i::BytecodeArray> bytecode_array) {
-  stream << kIndent << ' ' << bytecode_array->length() << ",\n"
-         << kIndent << " {\n"
-         << kIndent << "     ";
-
-  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 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) {
-    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)));
+  if (options.read_raw_js_snippet()) {
+    snippet_list->push_back(ReadRawJSSnippet(body_file));
+  } else {
+    while (ReadNextSnippet(&snippet, body_file)) {
+      snippet_list->push_back(snippet);
     }
-    stream << '}';
   }
-  stream << '\n';
-}
-
-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";
-
-  PrintFrameSize(stream, bytecode_array);
-  PrintBytecodeSequence(stream, bytecode_array);
-  PrintConstantPool(stream, bytecode_array->constant_pool(), constant_pool_type,
-                    isolate);
 
-  // TODO(ssanfilippo) print handlers.
-  i::HandlerTable* handlers =
-      i::HandlerTable::cast(bytecode_array->handler_table());
-  CHECK_EQ(handlers->NumberOfRangeEntries(), 0);
-
-  stream << kIndent << "}\n";
+  return true;
 }
 
-void PrintExpectedSnippet(ConstantPoolType constant_pool_type, char* exec_path,
-                          std::string body) {
-  const char* wrapper_function_name = "__genbckexp_wrapper__";
-
+void GenerateExpectationsFile(
+    std::ostream& stream, const std::vector<std::string>& snippet_list,
+    BytecodeExpectationsPrinter::ConstantPoolType const_pool_type,
+    const char* exec_path) {
   V8InitializationScope platform(exec_path);
   {
-    v8::Isolate::Scope isolate_scope(platform.isolate());
-    v8::HandleScope handle_scope(platform.isolate());
+    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);
+    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(),
-                       constant_pool_type);
-  }
-}
-
-bool ReadFromFileOrStdin(std::string* body, const char* body_filename) {
-  std::stringstream body_buffer;
-  if (strcmp(body_filename, "-") == 0) {
-    body_buffer << std::cin.rdbuf();
-  } else {
-    std::ifstream body_file{body_filename};
-    if (!body_file) return false;
-    body_buffer << body_file.rdbuf();
-  }
-  *body = body_buffer.str();
-  return true;
-}
+    stream << "#\n# Autogenerated by generate-bytecode-expectations\n#\n\n";
 
-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;
+    BytecodeExpectationsPrinter printer(platform.isolate(), const_pool_type);
+    for (const std::string& snippet : snippet_list) {
+      printer.PrintExpectation(stream, snippet);
+    }
   }
-  return kConstantPoolTypeUnknown;
 }
 
 void PrintUsage(const char* exec_path) {
-  std::cerr << "Usage: " << exec_path
-            << " (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 "
-               "into the test suite.\n";
+  std::cerr
+      << "\nUsage: " << exec_path
+      << " [OPTIONS]... [INPUT FILE]\n\n"
+         "Options:\n"
+         "  --help    Print this help message.\n"
+         "  --raw-js  Read raw JavaScript, instead of the output format.\n"
+         "  --stdin   Read from standard input instead of file.\n"
+         "  --pool-type (int|double|string|mixed)\n"
+         "      specify the type of the entries in the constant pool "
+         "(default: mixed).\n"
+         "\n"
+         "Each raw JavaScript file is interpreted as a single snippet.\n\n"
+         "This tool is intended as a help in writing tests.\n"
+         "Please, DO NOT blindly copy and paste the output "
+         "into the test suite.\n";
 }
 
 }  // namespace
 
 int main(int argc, char** argv) {
-  if (argc < 2) {
-    PrintUsage(argv[0]);
-    return 1;
-  }
+  ProgramOptions options = ProgramOptions::FromCommandLine(argc, argv);
 
-  if (argc > 1 && strcmp(argv[1], "--help") == 0) {
+  if (!options.Validate() || options.print_help()) {
     PrintUsage(argv[0]);
-    return 0;
+    return options.print_help() ? 0 : 1;
   }
 
-  const char* body_filename = (argc > 2 ? argv[2] : "-");
-  const char* const_pool_type_string = argv[1];
-
-  std::string body;
-  if (!ReadFromFileOrStdin(&body, body_filename)) {
-    std::cerr << "Could not open '" << body_filename << "'.\n\n";
-    PrintUsage(argv[0]);
-    return 1;
+  std::vector<std::string> snippet_list;
+  if (!ExtractSnippets(&snippet_list, options)) {
+    return 2;
   }
 
-  PrintExpectedSnippet(ParseConstantPoolType(const_pool_type_string), argv[0],
-                       body);
+  GenerateExpectationsFile(std::cout, snippet_list, options.const_pool_type(),
+                           argv[0]);
 }