Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(937)

Unified Diff: test/cctest/interpreter/generate-bytecode-expectations.cc

Issue 1698403002: [Interpreter] generate-bytecode-expectations improvements. (Closed) Base URL: https://chromium.googlesource.com/v8/v8.git@master
Patch Set: Created 4 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 8bf9ba6dfb9ac28e459e71bf1f7480c1962bd074..8b13dcf812a3e16ae872fa17243c689d48cb82e5 100644
--- a/test/cctest/interpreter/generate-bytecode-expectations.cc
+++ b/test/cctest/interpreter/generate-bytecode-expectations.cc
@@ -28,6 +28,10 @@ class ProgramOptions {
print_help_(false),
read_raw_js_snippet_(false),
read_from_stdin_(false),
+ fix_(false),
+ print_banner_(true),
+ wrap_(true),
+ execute_(true),
const_pool_type_(
BytecodeExpectationsPrinter::ConstantPoolType::kMixed) {}
@@ -37,18 +41,32 @@ class ProgramOptions {
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_; }
+ bool fix() const { return fix_; }
+ bool print_banner() const { return print_banner_; }
+ bool wrap() const { return wrap_; }
+ bool execute() const { return execute_; }
BytecodeExpectationsPrinter::ConstantPoolType const_pool_type() const {
return const_pool_type_;
}
+ std::string input_filename() const { return input_filename_; }
+ std::string output_filename() const { return output_file_name_; }
+ std::string top_function_name() const { return top_function_name_; }
private:
bool parsing_failed_;
bool print_help_;
bool read_raw_js_snippet_;
bool read_from_stdin_;
+ bool fix_;
+ bool print_banner_;
+ bool wrap_;
+ bool execute_;
BytecodeExpectationsPrinter::ConstantPoolType const_pool_type_;
- std::string filename_;
+ std::string input_filename_;
+ std::string output_file_name_;
+ std::string top_function_name_;
+
+ friend void ParseOptionsHeader(ProgramOptions*, std::istream&);
};
class ArrayBufferAllocator final : public v8::ArrayBuffer::Allocator {
@@ -95,8 +113,6 @@ BytecodeExpectationsPrinter::ConstantPoolType ParseConstantPoolType(
ProgramOptions ProgramOptions::FromCommandLine(int argc, char** argv) {
ProgramOptions options;
- if (argc <= 1) return options;
-
for (int i = 1; i < argc; ++i) {
if (strcmp(argv[i], "--help") == 0) {
options.print_help_ = true;
@@ -106,13 +122,25 @@ ProgramOptions ProgramOptions::FromCommandLine(int argc, char** argv) {
options.const_pool_type_ = ParseConstantPoolType(argv[i] + 12);
} else if (strcmp(argv[i], "--stdin") == 0) {
options.read_from_stdin_ = true;
+ } else if (strcmp(argv[i], "--fix") == 0) {
+ options.fix_ = true;
+ } else if (strcmp(argv[i], "--no-banner") == 0) {
+ options.print_banner_ = false;
+ } else if (strcmp(argv[i], "--no-wrap") == 0) {
+ options.wrap_ = false;
+ } else if (strcmp(argv[i], "--no-execute") == 0) {
+ options.execute_ = false;
+ } else if (strncmp(argv[i], "--output=", 9) == 0) {
+ options.output_file_name_ = argv[i] + 9;
+ } else if (strncmp(argv[i], "--wrapper-name=", 15) == 0) {
+ options.top_function_name_ = argv[i] + 15;
} else if (strncmp(argv[i], "--", 2) != 0) { // It doesn't start with --
- if (!options.filename_.empty()) {
+ if (!options.input_filename_.empty()) {
std::cerr << "ERROR: More than one input file specified\n";
options.parsing_failed_ = true;
break;
}
- options.filename_ = argv[i];
+ options.input_filename_ = argv[i];
} else {
std::cerr << "ERROR: Unknonwn option " << argv[i] << "\n";
options.parsing_failed_ = true;
@@ -123,6 +151,75 @@ ProgramOptions ProgramOptions::FromCommandLine(int argc, char** argv) {
return options;
}
+bool ParseBoolean(const char* string) {
+ if (strcmp(string, "yes") == 0) {
+ return true;
+ } else if (strcmp(string, "no") == 0) {
+ return false;
+ } else {
+ UNREACHABLE();
+ return false;
+ }
+}
+
+void ParseOptionsHeader(ProgramOptions* options,
+ std::istream& stream) { // NOLINT
+ std::string line;
+
+ // Skip to the beginning of the options header
+ while (std::getline(stream, line)) {
+ if (line == "---") break;
+ }
+
+ while (std::getline(stream, line)) {
+ if (line.compare(0, 11, "pool type: ") == 0) {
+ options->const_pool_type_ = ParseConstantPoolType(line.c_str() + 11);
+ } else if (line.compare(0, 9, "execute: ") == 0) {
+ options->execute_ = ParseBoolean(line.c_str() + 9);
+ } else if (line.compare(0, 6, "wrap: ") == 0) {
+ options->wrap_ = ParseBoolean(line.c_str() + 6);
+ } else if (line.compare(0, 13, "wrapper name: ") == 0) {
+ options->top_function_name_ = line.c_str() + 14;
+ } else {
+ break;
+ }
+ }
+}
+
+const char* BooleanString(bool value) { return value ? "yes" : "no"; }
rmcilroy 2016/02/16 16:43:48 nit - move up near ParseBoolean
Stefano Sanfilippo 2016/02/16 20:28:26 Done.
+
+const char* ConstantPoolTypeString(
+ BytecodeExpectationsPrinter::ConstantPoolType type) {
+ switch (type) {
+ case BytecodeExpectationsPrinter::ConstantPoolType::kDouble:
+ return "double";
+ case BytecodeExpectationsPrinter::ConstantPoolType::kInteger:
+ return "integer";
+ case BytecodeExpectationsPrinter::ConstantPoolType::kMixed:
+ return "mixed";
+ case BytecodeExpectationsPrinter::ConstantPoolType::kString:
+ return "string";
+ default:
+ UNREACHABLE();
+ return nullptr;
+ }
+}
+
+void EmitOptionsHeader(std::ostream& stream, // NOLINT
+ const ProgramOptions& options) {
+ stream << "---"
+ "\npool type: "
+ << ConstantPoolTypeString(options.const_pool_type())
+ << "\nexecute: " << BooleanString(options.execute())
+ << "\nwrap: " << BooleanString(options.wrap());
+
+ if (!options.top_function_name().empty()) {
+ stream << "\nwrapper name: " << options.top_function_name();
+ }
rmcilroy 2016/02/16 16:43:48 Could you emit a seperator between the options and
Stefano Sanfilippo 2016/02/16 20:28:26 I have updated the code to use the separator of th
+
+ stream << "\n\n";
+}
+
bool ProgramOptions::Validate() const {
if (parsing_failed_) return false;
if (print_help_) return true;
@@ -133,16 +230,21 @@ bool ProgramOptions::Validate() const {
return false;
}
- if (!read_from_stdin_ && filename_.empty()) {
+ if (!read_from_stdin_ && input_filename_.empty()) {
std::cerr << "ERROR: No input file specified.\n";
return false;
}
- if (read_from_stdin_ && !filename_.empty()) {
+ if (read_from_stdin_ && !input_filename_.empty()) {
std::cerr << "ERROR: Reading from stdin, but input files supplied.\n";
return false;
}
+ if (!wrap_ && !top_function_name_.empty()) {
+ std::cerr << "ERROR: not wrapping, but wrapper name specified.\n";
+ return false;
+ }
+
rmcilroy 2016/02/16 16:43:48 You should also validate that --rebaseline and --r
Stefano Sanfilippo 2016/02/16 20:28:26 Done.
return true;
}
@@ -194,41 +296,64 @@ bool ReadNextSnippet(std::istream& stream, std::string* string_out) { // NOLINT
return false;
}
+std::string UnescapeString(const std::string& escaped_string) {
+ std::string unescaped_string;
+ bool previous_was_backslash = false;
+ for (char c : escaped_string) {
+ if (previous_was_backslash) {
+ // If it was not an escape sequence, emit the backslash
rmcilroy 2016/02/16 16:43:48 /s/the backslash/the previous backslash./
Stefano Sanfilippo 2016/02/16 20:28:26 Done.
+ if (c != '\\' && c != '"') unescaped_string += '\\';
+ unescaped_string += c;
+ previous_was_backslash = false;
+ } else {
+ if (c == '\\') {
+ previous_was_backslash = true;
+ // Defer emission to the point where we can check if it was an escape.
+ } else {
+ unescaped_string += c;
+ }
+ }
+ }
+ return unescaped_string;
+}
+
void ExtractSnippetsFromStream(std::vector<std::string>* snippet_list,
+ ProgramOptions* options,
std::istream& body_stream, // NOLINT
bool read_raw_js_snippet) {
if (read_raw_js_snippet) {
snippet_list->push_back(ReadRawJSSnippet(body_stream));
} else {
+ ParseOptionsHeader(options, body_stream);
rmcilroy 2016/02/16 16:43:48 This should only happen for --rebaseline I think (
Stefano Sanfilippo 2016/02/16 20:28:26 Please see below.
std::string snippet;
while (ReadNextSnippet(body_stream, &snippet)) {
- snippet_list->push_back(snippet);
+ snippet_list->push_back(UnescapeString(snippet));
}
}
}
bool ExtractSnippets(std::vector<std::string>* snippet_list,
- const ProgramOptions& options) {
- if (options.read_from_stdin()) {
- ExtractSnippetsFromStream(snippet_list, std::cin,
- options.read_raw_js_snippet());
+ ProgramOptions* options) {
rmcilroy 2016/02/16 16:43:48 This should still be const, right? Any reason to m
Stefano Sanfilippo 2016/02/16 20:28:26 The reason is that ParseOptionsHeader called insid
rmcilroy 2016/02/17 10:16:59 I think the solution is to split out the opening o
rmcilroy 2016/02/17 10:16:59 I think the solution is to split out the opening o
Stefano Sanfilippo 2016/02/17 14:32:29 I've rewritten the piece using a better approach w
+ if (options->read_from_stdin()) {
+ ExtractSnippetsFromStream(snippet_list, options, std::cin,
+ options->read_raw_js_snippet());
} else {
- std::ifstream body_file(options.filename().c_str());
+ std::ifstream body_file(options->input_filename().c_str());
if (!body_file.is_open()) {
- std::cerr << "ERROR: Could not open '" << options.filename() << "'.\n";
+ std::cerr << "ERROR: Could not open '" << options->input_filename()
+ << "'.\n";
return false;
}
- ExtractSnippetsFromStream(snippet_list, body_file,
- options.read_raw_js_snippet());
+ ExtractSnippetsFromStream(snippet_list, options, body_file,
+ options->read_raw_js_snippet());
}
return true;
}
-void GenerateExpectationsFile(
- std::ostream& stream, // NOLINT
- const std::vector<std::string>& snippet_list,
- BytecodeExpectationsPrinter::ConstantPoolType const_pool_type,
- const char* exec_path) {
+void GenerateExpectationsFile(std::ostream& stream, // NOLINT
+ const std::vector<std::string>& snippet_list,
+ const ProgramOptions& options,
+ const char* exec_path) {
V8InitializationScope platform(exec_path);
{
v8::Isolate::Scope isolate_scope(platform.isolate());
@@ -236,9 +361,20 @@ void GenerateExpectationsFile(
v8::Local<v8::Context> context = v8::Context::New(platform.isolate());
v8::Context::Scope context_scope(context);
- stream << "#\n# Autogenerated by generate-bytecode-expectations\n#\n\n";
+ if (options.print_banner()) {
+ stream << "#\n# Autogenerated by generate-bytecode-expectations\n#\n\n";
+ }
+
+ EmitOptionsHeader(stream, options);
+
+ BytecodeExpectationsPrinter printer(platform.isolate(),
+ options.const_pool_type());
+ printer.set_wrap(options.wrap());
+ printer.set_execute(options.execute());
+ if (!options.top_function_name().empty()) {
+ printer.set_top_function_name(options.top_function_name());
+ }
- BytecodeExpectationsPrinter printer(platform.isolate(), const_pool_type);
for (const std::string& snippet : snippet_list) {
printer.PrintExpectation(stream, snippet);
}
@@ -253,8 +389,15 @@ void PrintUsage(const char* exec_path) {
" --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"
+ " --fix Overwrite input file.\n"
rmcilroy 2016/02/16 16:43:48 Call this "--rebaseline Rebaseline input snippet
Stefano Sanfilippo 2016/02/16 20:28:26 Done.
+ " --no-banner Do not add the \"generated by\" banner.\n"
rmcilroy 2016/02/16 16:43:48 Why is this one required? All golden files should
Stefano Sanfilippo 2016/02/16 20:28:26 ATM the banner is emitted by the utility before ca
rmcilroy 2016/02/17 10:17:00 I think the banner should be printed by this tool
Stefano Sanfilippo 2016/02/17 14:32:29 Done. I ended up changing the API from accepting a
+ " --no-wrap Do not wrap the snippet in a function.\n"
+ " --no-execute Do not execute after compilation.\n"
+ " --output=file.name Specify the output file. "
+ "If not specified, output goes to stdout.\n"
+ " --wrapper-name=foo Specify the name of the wrapper function.\n"
" --pool-type=(int|double|string|mixed)\n"
- " specify the type of the entries in the constant pool "
+ " 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"
@@ -274,10 +417,23 @@ int main(int argc, char** argv) {
}
std::vector<std::string> snippet_list;
- if (!ExtractSnippets(&snippet_list, options)) {
+ if (!ExtractSnippets(&snippet_list, &options)) {
return 2;
}
- GenerateExpectationsFile(std::cout, snippet_list, options.const_pool_type(),
- argv[0]);
+ CHECK(options.Validate());
+
+ if (!options.output_filename().empty() || options.fix()) {
+ std::ofstream output_file(options.fix()
+ ? options.input_filename().c_str()
+ : options.output_filename().c_str());
+ if (!output_file.is_open()) {
+ std::cerr << "ERROR: Could not open '" << options.output_filename()
+ << "' for writing.\n";
+ return 3;
+ }
+ GenerateExpectationsFile(output_file, snippet_list, options, argv[0]);
+ } else {
+ GenerateExpectationsFile(std::cout, snippet_list, options, argv[0]);
+ }
}

Powered by Google App Engine
This is Rietveld 408576698