[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..021fc77746a4849c0983e5855706adf5ba979dfb 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.h"
 
 #include "include/libplatform/libplatform.h"
 #include "include/v8.h"
@@ -12,23 +14,34 @@
 #include "src/base/smart-pointers.h"
 #include "src/compiler.h"
 

[rmcilroy, 2016/02/12 15:29:05]

rm extra newline

[Stefano Sanfilippo, 2016/02/12 18:29:40]

Done.

-#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 = "       ";
+struct CommandLineOptions {

[rmcilroy, 2016/02/12 15:29:05]

Make this a class if it has non-neglagible functionality. Also move fields to
the bottom, make them private, and add "_" to their name and add getters.

[Stefano Sanfilippo, 2016/02/12 18:29:40]

Done.

+  bool print_help = false;
+  bool read_raw_snippet = false;

[rmcilroy, 2016/02/12 15:29:05]

read_raw_js_snippet

[Stefano Sanfilippo, 2016/02/12 18:29:41]

Done.

+  ConstantPoolType const_pool_type = kConstantPoolTypeMixed;
+  bool read_from_stdin = false;

[rmcilroy, 2016/02/12 15:29:05]

nit - move up with other bools

[Stefano Sanfilippo, 2016/02/12 18:29:40]

Done.

+  std::vector<std::string> filename_list;
+
+  bool Validate() const {
+    if (print_help) return true;
+
+    if (!read_from_stdin && filename_list.empty()) {
+      std::cerr << "ERROR: No input file specified.\n";
+      return false;
+    }
+
+    if (read_from_stdin && !filename_list.empty()) {
+      std::cerr << "ERROR: Reading from stdin, but input files supplied.\n";
+      return false;
+    }
 
-enum ConstantPoolType {
-  kConstantPoolTypeUnknown,
-  kConstantPoolTypeString,
-  kConstantPoolTypeInteger,
-  kConstantPoolTypeDouble,
-  kConstantPoolTypeMixed,
+    return true;
+  }
 };
 
 class ArrayBufferAllocator final : public v8::ArrayBuffer::Allocator {
@@ -86,284 +99,88 @@ 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;
+bool ReadAll(std::string* body, std::istream& stream) {

[rmcilroy, 2016/02/12 15:29:05]

ReadRawJsSnippet

[Stefano Sanfilippo, 2016/02/12 18:29:40]

Done.

+  std::stringstream body_buffer;
+  bool success = body_buffer << stream.rdbuf();
+  *body = body_buffer.str();
+  return success;
 }
 
-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/12 15:29:05]

Just return the string instead of having it as a out pointer, and check for end
of file in the while loop in ExtractSnippets (with a CHECK or UNREACHABLE assert
here if the format is not as expected)

[Stefano Sanfilippo, 2016/02/12 18:29:40]

The fact is that EOF might test true after that function even if we read a valid
snippet, because that snippet was the very last thing in the file. The boolean
flag makes this difference through the two possible exit paths from the
function.

So, either we make the assumption that this is never going to happen, or we
should keep this IMO. The other possibility would be to return a string and the
success flag as an output pointer, but this would just complicate the while loop
below, again IMHO.

We might also want to use the empty string as a condition to break the loop, but
there's a test case whose input is the empty string itself, which anyway is a
valid snippet.

I am updating the fix to all the other issues but this one since I feel we might
want to talk about it further, hope you don't mind.

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

Ok, this is fine as is

+  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;
     }
-
-    stream << ')';
+    *string += '\n';
   }
+  return false;
 }
 
-void PrintBytecode(std::ostream& stream,
-                   const BytecodeArrayIterator& bytecode_iter) {
-  Bytecode bytecode = bytecode_iter.current_bytecode();
-
-  stream << "B(" << Bytecodes::ToString(bytecode) << ')';
+bool ExtractSnippets(std::vector<std::string>* snippet_list,

[rmcilroy, 2016/02/12 15:29:05]

const std::vector<std::string>*

[Stefano Sanfilippo, 2016/02/12 18:29:40]

We are modifying the vector in the function, const won't do :)

+                     const CommandLineOptions& options) {
+  std::string snippet;
 
-  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);
-  }
-}
-
-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));
+  if (options.read_from_stdin) {
+    if (!ReadAll(&snippet, std::cin)) {
+      std::cerr << "ERROR: Could not read from stanard input.\n";
+      return false;
+    }
+    snippet_list->push_back(snippet);
+    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();
+  for (const std::string& body_filename : options.filename_list) {
+    std::ifstream body_file{body_filename};
+    if (!body_file.is_open()) {
+      std::cerr << "ERROR: Could not open '" << body_filename << "'.\n";
+      return false;
+    }
+    if (options.read_raw_snippet) {
+      if (!ReadAll(&snippet, body_file)) {
+        std::cerr << "ERROR: Could read from '" << body_filename << "'.\n";
+        return false;
       }
-      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();
+      snippet_list->push_back(snippet);
+    } else {
+      while (ReadNextSnippet(&snippet, body_file)) {
+        snippet_list->push_back(snippet);
       }
-      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;
-  }
-
-  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)));
     }
-    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,
+                              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);
-
-    std::string source_code = WrapCodeInFunction(wrapper_function_name, body);
-    CompileAndRun(platform.isolate(), context, source_code.c_str());
+    v8::Context::Scope context_scope{context};
 
-    i::Handle<i::BytecodeArray> bytecode_array = GetBytecodeArrayForGlobal(
-        platform.isolate(), context, wrapper_function_name);
-
-    PrintBytecodeArray(std::cout, bytecode_array, body, platform.isolate(),
-                       constant_pool_type);
-  }
-}
+    stream << "#\n# Autogenerated by generate-bytecode-expectations\n#\n\n";
 
-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();
+    ExpectationPrinter printer{platform.isolate(), const_pool_type};

[rmcilroy, 2016/02/12 15:29:05]

replace "{" with "("

[Stefano Sanfilippo, 2016/02/12 18:29:40]

Done.

+    for (const std::string& snippet : snippet_list) {
+      printer.PrintExpectation(stream, snippet);
+    }
   }
-  *body = body_buffer.str();
-  return true;
 }
 
 ConstantPoolType ParseConstantPoolType(const char* type_string) {
@@ -379,40 +196,72 @@ ConstantPoolType ParseConstantPoolType(const char* type_string) {
   return kConstantPoolTypeUnknown;
 }
 
+CommandLineOptions ParseOptionsFromCommandLine(int argc, char** argv) {

[rmcilroy, 2016/02/12 15:29:05]

This should probably be a constructor for CommandLineOptions, or a static Create
method 

[Stefano Sanfilippo, 2016/02/12 18:29:41]

Done.

+  CommandLineOptions 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]);
+      if (options.const_pool_type == kConstantPoolTypeUnknown) {

[rmcilroy, 2016/02/12 15:29:05]

Move this logic into CommandLineArguments::Validate

[Stefano Sanfilippo, 2016/02/12 18:29:40]

Done.

+        std::cerr << "ERROR: Unknown constant pool type " << argv[i] << ".\n";
+        return options;
+      }
+      expecting_const_pool_type = false;
+    } else if (strcmp(argv[i], "--help") == 0) {
+      options.print_help = true;
+    } else if (strcmp(argv[i], "--js") == 0) {
+      options.read_raw_snippet = true;
+    } else if (strcmp(argv[i], "--pool-type") == 0) {
+      expecting_const_pool_type = true;
+    } else if (strcmp(argv[i], "--stdin") == 0) {
+      options.read_from_stdin = true;
+    } else if (strncmp(argv[i], "--", 2) != 0) {  // If it doesn't start with --
+      options.filename_list.push_back(argv[i]);
+    } else {
+      std::cerr << "ERROR: Unknonwn option " << argv[i] << "\n";
+      return options;
+    }
+  }
+
+  return options;
+}
+
 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 FILES]...\n\n"

[rmcilroy, 2016/02/12 15:29:05]

Let's make this tool only take a single file - the use case would be fixing a
golden expectation file and replacing it, so I don't see much usecase for
multiple files (which just complicates thing IMO).

[Stefano Sanfilippo, 2016/02/12 18:29:40]

Done.

+         "Options:\n"
+         "  --help   Print this help message.\n"
+         "  --js     Read raw JavaScript, instead of the output format.\n"

[rmcilroy, 2016/02/12 15:29:05]

nit / --raw-js

[Stefano Sanfilippo, 2016/02/12 18:29:40]

Done.

+         "  --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;
-  }
+  CommandLineOptions options = ParseOptionsFromCommandLine(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]);
 }