refactor(fe): move Parser::diagnostic_memory_ into caller

Currently, Parser owns the diagnostic_memory_ allocator. This ties the lifetime of Diag_List to Parser, preventing a refactor I want to perform. Have users of Parser pass their own diag_memory allocator into the Parser.
quick-lint · Sep 22, 2024 · 4550cae · 4550cae
1 parent 7ce8b65
commit 4550cae
Show file tree

Hide file tree

Showing 9 changed files with 44 additions and 31 deletions.
diff --git a/benchmark/benchmark-lint.cpp b/benchmark/benchmark-lint.cpp
@@ -34,10 +34,13 @@ void benchmark_lint(benchmark::State &state) {
   Padded_String source = quick_lint_js::read_file_or_exit(source_path);
 
   Configuration config;
-  Parser_Options p_options;
-  Parser p(&source, p_options);
   Buffering_Visitor visitor(new_delete_resource());
-  p.parse_and_visit_module(visitor);
+  {
+    Parser_Options p_options;
+    Monotonic_Allocator diag_memory("benchmark_lint diag_memory");
+    Parser p(&source, &diag_memory, p_options);
+    p.parse_and_visit_module(visitor);
+  }
 
   Variable_Analyzer_Options var_options;
 
@@ -68,7 +71,8 @@ void benchmark_parse_and_lint(benchmark::State &state) {
   Variable_Analyzer_Options var_options;
   Configuration config;
   for (auto _ : state) {
-    Parser p(&source, p_options);
+    Monotonic_Allocator diag_memory("benchmark_parse_and_lint diag_memory");
+    Parser p(&source, &diag_memory, p_options);
     Variable_Analyzer l(&p.diags(), &config.globals(), var_options);
     p.parse_and_visit_module(l);
   }

diff --git a/benchmark/benchmark-parse.cpp b/benchmark/benchmark-parse.cpp
@@ -31,7 +31,8 @@ void benchmark_parse_file(benchmark::State &state) {
 
   Parser_Options p_options;
   for (auto _ : state) {
-    Parser p(&source, p_options);
+    Monotonic_Allocator diag_memory("benchmark_parse_file diag_memory");
+    Parser p(&source, &diag_memory, p_options);
     Null_Visitor visitor;
     p.parse_and_visit_module(visitor);
   }
@@ -43,7 +44,8 @@ void benchmark_parse(benchmark::State &state, String8_View raw_source) {
   Padded_String source(raw_source);
   Parser_Options p_options;
   for (auto _ : state) {
-    Parser p(&source, p_options);
+    Monotonic_Allocator diag_memory("benchmark_parse diag_memory");
+    Parser p(&source, &diag_memory, p_options);
     Null_Visitor visitor;
     p.parse_and_visit_module(visitor);
   }

diff --git a/src/quick-lint-js/fe/linter.cpp b/src/quick-lint-js/fe/linter.cpp
@@ -43,7 +43,8 @@ void parse_and_lint(Padded_String_View code, Diag_Reporter& reporter,
     break;
   }
 
-  Parser p(code, parser_options);
+  Monotonic_Allocator diag_memory("parse_and_lint diag_memory");
+  Parser p(code, &diag_memory, parser_options);
   Variable_Analyzer var_analyzer(
       &p.diags(), &options.configuration->globals(),
       Variable_Analyzer_Options{

diff --git a/src/quick-lint-js/fe/parse.cpp b/src/quick-lint-js/fe/parse.cpp
@@ -39,12 +39,14 @@ namespace quick_lint_js {
 Parser_Transaction::Parser_Transaction(Lexer* l)
     : lex_transaction(l->begin_transaction()) {}
 
-Parser::Parser(Padded_String_View input, Parser_Options options)
-    : lexer_(input, &this->diagnostic_memory_,
+Parser::Parser(Padded_String_View input, Monotonic_Allocator* diag_memory,
+               Parser_Options options)
+    : lexer_(input, diag_memory,
              Lexer_Options{
                  .typescript = options.typescript,
              }),
-      options_(options) {}
+      options_(options),
+      diagnostic_memory_(*diag_memory) {}
 
 Parser::Function_Guard Parser::enter_function(Function_Attributes attributes) {
   bool was_in_top_level = this->in_top_level_;

diff --git a/src/quick-lint-js/fe/parse.h b/src/quick-lint-js/fe/parse.h
@@ -112,7 +112,8 @@ class Parser {
   class Depth_Guard;
   class Function_Guard;
 
-  explicit Parser(Padded_String_View input, Parser_Options options);
+  explicit Parser(Padded_String_View input, Monotonic_Allocator *diag_memory,
+                  Parser_Options options);
 
   quick_lint_js::Lexer &lexer() { return this->lexer_; }
 
@@ -1092,14 +1093,13 @@ class Parser {
   Parse_Expression_Cache_Key parse_expression_cache_key_for_current_state()
       const;
 
-  // Memory used for strings in diagnostic messages.
-  // Must be initialized before lexer_.
-  Monotonic_Allocator diagnostic_memory_{"parser::diagnostic_memory_"};
-
   quick_lint_js::Lexer lexer_;
   Parser_Options options_;
   Expression_Arena expressions_;
 
+  // Memory used for strings in diagnostic messages.
+  Monotonic_Allocator &diagnostic_memory_;
+
   // Memory used for temporary memory allocations (e.g. vectors on the stack).
   Monotonic_Allocator temporary_memory_{"parser::temporary_memory_"};
 

diff --git a/test/quick-lint-js/parse-support.h b/test/quick-lint-js/parse-support.h
@@ -93,7 +93,7 @@ class Test_Parser {
 
   explicit Test_Parser(String8_View input, const Parser_Options& options,
                        Capture_Diags_Tag)
-      : code_(input), parser_(&this->code_, options) {}
+      : code_(input), parser_(&this->code_, &this->diag_memory_, options) {}
 
   // Fails the test if there are any diagnostics during parsing.
   explicit Test_Parser(String8_View input)
@@ -102,7 +102,7 @@ class Test_Parser {
   // Fails the test if there are any diagnostics during parsing.
   explicit Test_Parser(String8_View input, const Parser_Options& options)
       : code_(input),
-        parser_(&this->code_, options),
+        parser_(&this->code_, &this->diag_memory_, options),
         fail_on_any_diagnostic_(true) {}
 
   Expression* parse_expression(
@@ -210,6 +210,9 @@ class Test_Parser {
     }
   }
 
+  Monotonic_Allocator diag_memory_ =
+      Monotonic_Allocator("Test_Parser::diag_memory_");
+
   Padded_String code_;
   Spy_Visitor errors_;
   Failing_Diag_Reporter failing_reporter_;

diff --git a/test/quick-lint-js/variable-analyzer-support.cpp b/test/quick-lint-js/variable-analyzer-support.cpp
@@ -60,7 +60,8 @@ void test_parse_and_analyze(String8_View input,
   Monotonic_Allocator memory("test");
   Padded_String code(input);
 
-  Parser p(&code, options.parse_options);
+  Monotonic_Allocator diag_memory("test_parse_and_analyze diag_memory");
+  Parser p(&code, &diag_memory, options.parse_options);
   Variable_Analyzer var_analyzer(&p.diags(), &globals, options.analyze_options);
   p.parse_and_visit_module(var_analyzer);
 

diff --git a/test/test-parse-typescript.cpp b/test/test-parse-typescript.cpp
@@ -84,9 +84,9 @@ TEST_F(Test_Parse_TypeScript, no_crash) {
            u8"export declare export"_sv,
            u8"export declare()"_sv,
        }) {
-    Monotonic_Allocator memory("test");
+    Monotonic_Allocator diag_memory("test");
     Padded_String code_string(code);
-    Parser p(&code_string, typescript_options);
+    Parser p(&code_string, &diag_memory, typescript_options);
     Spy_Visitor v;
     // Should not crash:
     p.parse_and_visit_module_catching_fatal_parse_errors(v);

diff --git a/test/test-parse.cpp b/test/test-parse.cpp
@@ -571,7 +571,7 @@ Padded_String unimplemented_token_code(u8"]"_sv);
 #if defined(GTEST_HAS_DEATH_TEST) && GTEST_HAS_DEATH_TEST
 TEST_F(Test_Parse, unimplemented_token_crashes_SLOW) {
   auto check = [&] {
-    Parser p(&unimplemented_token_code, javascript_options);
+    Parser p(&unimplemented_token_code, &this->memory_, javascript_options);
     Spy_Visitor v;
     p.parse_and_visit_module(v);
   };
@@ -581,7 +581,7 @@ TEST_F(Test_Parse, unimplemented_token_crashes_SLOW) {
 
 TEST_F(Test_Parse, unimplemented_token_doesnt_crash_if_caught) {
   {
-    Parser p(&unimplemented_token_code, javascript_options);
+    Parser p(&unimplemented_token_code, &this->memory_, javascript_options);
     Spy_Visitor v;
     bool ok = p.parse_and_visit_module_catching_fatal_parse_errors(v);
     EXPECT_FALSE(ok);
@@ -597,7 +597,7 @@ TEST_F(Test_Parse, unimplemented_token_doesnt_crash_if_caught) {
 TEST_F(Test_Parse, unimplemented_token_returns_to_innermost_handler) {
   {
     Padded_String code(u8"hello world"_sv);
-    Parser p(&code, javascript_options);
+    Parser p(&code, &this->memory_, javascript_options);
     volatile bool inner_catch_returned = false;
     bool outer_ok = p.catch_fatal_parse_errors([&] {
       bool inner_ok = p.catch_fatal_parse_errors(
@@ -618,7 +618,7 @@ TEST_F(Test_Parse,
        unimplemented_token_after_handler_ends_returns_to_outer_handler) {
   {
     Padded_String code(u8"hello world"_sv);
-    Parser p(&code, javascript_options);
+    Parser p(&code, &this->memory_, javascript_options);
     volatile bool inner_catch_returned = false;
     bool outer_ok = p.catch_fatal_parse_errors([&] {
       bool inner_ok = p.catch_fatal_parse_errors([] {
@@ -640,7 +640,7 @@ TEST_F(Test_Parse,
 TEST_F(Test_Parse, unimplemented_token_rolls_back_parser_depth) {
   {
     Padded_String code(u8"hello world"_sv);
-    Parser p(&code, javascript_options);
+    Parser p(&code, &this->memory_, javascript_options);
     volatile bool inner_catch_returned = false;
     bool outer_ok = p.catch_fatal_parse_errors([&] {
       Parser::Depth_Guard outer_g(&p);
@@ -662,7 +662,7 @@ TEST_F(Test_Parse, unimplemented_token_rolls_back_parser_depth) {
 TEST_F(Test_Parse, unimplemented_token_is_reported_on_outer_diag_reporter) {
   {
     Padded_String code(u8"hello world"_sv);
-    Parser p(&code, javascript_options);
+    Parser p(&code, &this->memory_, javascript_options);
 
     Parser_Transaction transaction = p.begin_transaction();
     bool ok = p.catch_fatal_parse_errors(
@@ -771,7 +771,7 @@ TEST_F(Test_No_Overflow, parser_depth_limit_not_exceeded) {
        }) {
     Padded_String code(u8"return " + jsx);
     SCOPED_TRACE(code);
-    Parser p(&code, jsx_options);
+    Parser p(&code, &this->memory_, jsx_options);
     Spy_Visitor v;
     bool ok = p.parse_and_visit_module_catching_fatal_parse_errors(v);
     EXPECT_TRUE(ok);
@@ -783,7 +783,7 @@ TEST_F(Test_No_Overflow, parser_depth_limit_not_exceeded) {
        }) {
     Padded_String code(concat(u8"let x: "_sv, type, u8";"_sv));
     SCOPED_TRACE(code);
-    Parser p(&code, typescript_options);
+    Parser p(&code, &this->memory_, typescript_options);
     Spy_Visitor v;
     bool ok = p.parse_and_visit_module_catching_fatal_parse_errors(v);
     EXPECT_TRUE(ok);
@@ -838,7 +838,7 @@ TEST_F(Test_Overflow, parser_depth_limit_exceeded) {
        }) {
     Padded_String code(exps);
     SCOPED_TRACE(code);
-    Parser p(&code, javascript_options);
+    Parser p(&code, &this->memory_, javascript_options);
     Spy_Visitor v;
     bool ok = p.parse_and_visit_module_catching_fatal_parse_errors(v);
     EXPECT_FALSE(ok);
@@ -885,7 +885,7 @@ TEST_F(Test_Overflow, parser_depth_limit_exceeded) {
        }) {
     Padded_String code(concat(u8"return "_sv, jsx));
     SCOPED_TRACE(code);
-    Parser p(&code, jsx_options);
+    Parser p(&code, &this->memory_, jsx_options);
     Spy_Visitor v;
     bool ok = p.parse_and_visit_module_catching_fatal_parse_errors(v);
     EXPECT_FALSE(ok);
@@ -900,7 +900,7 @@ TEST_F(Test_Overflow, parser_depth_limit_exceeded) {
        }) {
     Padded_String code(concat(u8"let x: "_sv, type, u8";"_sv));
     SCOPED_TRACE(code);
-    Parser p(&code, typescript_options);
+    Parser p(&code, &this->memory_, typescript_options);
     Spy_Visitor v;
     bool ok = p.parse_and_visit_module_catching_fatal_parse_errors(v);
     EXPECT_FALSE(ok);