Skip to content

Commit 7ad72b3

Browse files
stragerstrager
committed
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.
1 parent 26b376b commit 7ad72b3

File tree

9 files changed

+44
-31
lines changed

9 files changed

+44
-31
lines changed

‎benchmark/benchmark-lint.cpp

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,13 @@ void benchmark_lint(benchmark::State &state) {
3434
Padded_String source = quick_lint_js::read_file_or_exit(source_path);
3535

3636
Configuration config;
37-
Parser_Options p_options;
38-
Parser p(&source, p_options);
3937
Buffering_Visitor visitor(new_delete_resource());
40-
p.parse_and_visit_module(visitor);
38+
{
39+
Parser_Options p_options;
40+
Monotonic_Allocator diag_memory("benchmark_lint diag_memory");
41+
Parser p(&source, &diag_memory, p_options);
42+
p.parse_and_visit_module(visitor);
43+
}
4144

4245
Variable_Analyzer_Options var_options;
4346

@@ -68,7 +71,8 @@ void benchmark_parse_and_lint(benchmark::State &state) {
6871
Variable_Analyzer_Options var_options;
6972
Configuration config;
7073
for (auto _ : state) {
71-
Parser p(&source, p_options);
74+
Monotonic_Allocator diag_memory("benchmark_parse_and_lint diag_memory");
75+
Parser p(&source, &diag_memory, p_options);
7276
Variable_Analyzer l(&p.diags(), &config.globals(), var_options);
7377
p.parse_and_visit_module(l);
7478
}

‎benchmark/benchmark-parse.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@ void benchmark_parse_file(benchmark::State &state) {
3131

3232
Parser_Options p_options;
3333
for (auto _ : state) {
34-
Parser p(&source, p_options);
34+
Monotonic_Allocator diag_memory("benchmark_parse_file diag_memory");
35+
Parser p(&source, &diag_memory, p_options);
3536
Null_Visitor visitor;
3637
p.parse_and_visit_module(visitor);
3738
}
@@ -43,7 +44,8 @@ void benchmark_parse(benchmark::State &state, String8_View raw_source) {
4344
Padded_String source(raw_source);
4445
Parser_Options p_options;
4546
for (auto _ : state) {
46-
Parser p(&source, p_options);
47+
Monotonic_Allocator diag_memory("benchmark_parse diag_memory");
48+
Parser p(&source, &diag_memory, p_options);
4749
Null_Visitor visitor;
4850
p.parse_and_visit_module(visitor);
4951
}

‎src/quick-lint-js/fe/linter.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ void parse_and_lint(Padded_String_View code, Diag_Reporter& reporter,
4343
break;
4444
}
4545

46-
Parser p(code, parser_options);
46+
Monotonic_Allocator diag_memory("parse_and_lint diag_memory");
47+
Parser p(code, &diag_memory, parser_options);
4748
Variable_Analyzer var_analyzer(
4849
&p.diags(), &options.configuration->globals(),
4950
Variable_Analyzer_Options{

‎src/quick-lint-js/fe/parse.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,14 @@ namespace quick_lint_js {
3939
Parser_Transaction::Parser_Transaction(Lexer* l)
4040
: lex_transaction(l->begin_transaction()) {}
4141

42-
Parser::Parser(Padded_String_View input, Parser_Options options)
43-
: lexer_(input, &this->diagnostic_memory_,
42+
Parser::Parser(Padded_String_View input, Monotonic_Allocator* diag_memory,
43+
Parser_Options options)
44+
: lexer_(input, diag_memory,
4445
Lexer_Options{
4546
.typescript = options.typescript,
4647
}),
47-
options_(options) {}
48+
options_(options),
49+
diagnostic_memory_(*diag_memory) {}
4850

4951
Parser::Function_Guard Parser::enter_function(Function_Attributes attributes) {
5052
bool was_in_top_level = this->in_top_level_;

‎src/quick-lint-js/fe/parse.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,8 @@ class Parser {
112112
class Depth_Guard;
113113
class Function_Guard;
114114

115-
explicit Parser(Padded_String_View input, Parser_Options options);
115+
explicit Parser(Padded_String_View input, Monotonic_Allocator *diag_memory,
116+
Parser_Options options);
116117

117118
quick_lint_js::Lexer &lexer() { return this->lexer_; }
118119

@@ -1092,14 +1093,13 @@ class Parser {
10921093
Parse_Expression_Cache_Key parse_expression_cache_key_for_current_state()
10931094
const;
10941095

1095-
// Memory used for strings in diagnostic messages.
1096-
// Must be initialized before lexer_.
1097-
Monotonic_Allocator diagnostic_memory_{"parser::diagnostic_memory_"};
1098-
10991096
quick_lint_js::Lexer lexer_;
11001097
Parser_Options options_;
11011098
Expression_Arena expressions_;
11021099

1100+
// Memory used for strings in diagnostic messages.
1101+
Monotonic_Allocator &diagnostic_memory_;
1102+
11031103
// Memory used for temporary memory allocations (e.g. vectors on the stack).
11041104
Monotonic_Allocator temporary_memory_{"parser::temporary_memory_"};
11051105

‎test/quick-lint-js/parse-support.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ class Test_Parser {
9393

9494
explicit Test_Parser(String8_View input, const Parser_Options& options,
9595
Capture_Diags_Tag)
96-
: code_(input), parser_(&this->code_, options) {}
96+
: code_(input), parser_(&this->code_, &this->diag_memory_, options) {}
9797

9898
// Fails the test if there are any diagnostics during parsing.
9999
explicit Test_Parser(String8_View input)
@@ -102,7 +102,7 @@ class Test_Parser {
102102
// Fails the test if there are any diagnostics during parsing.
103103
explicit Test_Parser(String8_View input, const Parser_Options& options)
104104
: code_(input),
105-
parser_(&this->code_, options),
105+
parser_(&this->code_, &this->diag_memory_, options),
106106
fail_on_any_diagnostic_(true) {}
107107

108108
Expression* parse_expression(
@@ -210,6 +210,9 @@ class Test_Parser {
210210
}
211211
}
212212

213+
Monotonic_Allocator diag_memory_ =
214+
Monotonic_Allocator("Test_Parser::diag_memory_");
215+
213216
Padded_String code_;
214217
Spy_Visitor errors_;
215218
Failing_Diag_Reporter failing_reporter_;

‎test/quick-lint-js/variable-analyzer-support.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,8 @@ void test_parse_and_analyze(String8_View input,
6060
Monotonic_Allocator memory("test");
6161
Padded_String code(input);
6262

63-
Parser p(&code, options.parse_options);
63+
Monotonic_Allocator diag_memory("test_parse_and_analyze diag_memory");
64+
Parser p(&code, &diag_memory, options.parse_options);
6465
Variable_Analyzer var_analyzer(&p.diags(), &globals, options.analyze_options);
6566
p.parse_and_visit_module(var_analyzer);
6667

‎test/test-parse-typescript.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,9 @@ TEST_F(Test_Parse_TypeScript, no_crash) {
8484
u8"export declare export"_sv,
8585
u8"export declare()"_sv,
8686
}) {
87-
Monotonic_Allocator memory("test");
87+
Monotonic_Allocator diag_memory("test");
8888
Padded_String code_string(code);
89-
Parser p(&code_string, typescript_options);
89+
Parser p(&code_string, &diag_memory, typescript_options);
9090
Spy_Visitor v;
9191
// Should not crash:
9292
p.parse_and_visit_module_catching_fatal_parse_errors(v);

‎test/test-parse.cpp

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,7 @@ Padded_String unimplemented_token_code(u8"]"_sv);
571571
#if defined(GTEST_HAS_DEATH_TEST) && GTEST_HAS_DEATH_TEST
572572
TEST_F(Test_Parse, unimplemented_token_crashes_SLOW) {
573573
auto check = [&] {
574-
Parser p(&unimplemented_token_code, javascript_options);
574+
Parser p(&unimplemented_token_code, &this->memory_, javascript_options);
575575
Spy_Visitor v;
576576
p.parse_and_visit_module(v);
577577
};
@@ -581,7 +581,7 @@ TEST_F(Test_Parse, unimplemented_token_crashes_SLOW) {
581581

582582
TEST_F(Test_Parse, unimplemented_token_doesnt_crash_if_caught) {
583583
{
584-
Parser p(&unimplemented_token_code, javascript_options);
584+
Parser p(&unimplemented_token_code, &this->memory_, javascript_options);
585585
Spy_Visitor v;
586586
bool ok = p.parse_and_visit_module_catching_fatal_parse_errors(v);
587587
EXPECT_FALSE(ok);
@@ -597,7 +597,7 @@ TEST_F(Test_Parse, unimplemented_token_doesnt_crash_if_caught) {
597597
TEST_F(Test_Parse, unimplemented_token_returns_to_innermost_handler) {
598598
{
599599
Padded_String code(u8"hello world"_sv);
600-
Parser p(&code, javascript_options);
600+
Parser p(&code, &this->memory_, javascript_options);
601601
volatile bool inner_catch_returned = false;
602602
bool outer_ok = p.catch_fatal_parse_errors([&] {
603603
bool inner_ok = p.catch_fatal_parse_errors(
@@ -618,7 +618,7 @@ TEST_F(Test_Parse,
618618
unimplemented_token_after_handler_ends_returns_to_outer_handler) {
619619
{
620620
Padded_String code(u8"hello world"_sv);
621-
Parser p(&code, javascript_options);
621+
Parser p(&code, &this->memory_, javascript_options);
622622
volatile bool inner_catch_returned = false;
623623
bool outer_ok = p.catch_fatal_parse_errors([&] {
624624
bool inner_ok = p.catch_fatal_parse_errors([] {
@@ -640,7 +640,7 @@ TEST_F(Test_Parse,
640640
TEST_F(Test_Parse, unimplemented_token_rolls_back_parser_depth) {
641641
{
642642
Padded_String code(u8"hello world"_sv);
643-
Parser p(&code, javascript_options);
643+
Parser p(&code, &this->memory_, javascript_options);
644644
volatile bool inner_catch_returned = false;
645645
bool outer_ok = p.catch_fatal_parse_errors([&] {
646646
Parser::Depth_Guard outer_g(&p);
@@ -662,7 +662,7 @@ TEST_F(Test_Parse, unimplemented_token_rolls_back_parser_depth) {
662662
TEST_F(Test_Parse, unimplemented_token_is_reported_on_outer_diag_reporter) {
663663
{
664664
Padded_String code(u8"hello world"_sv);
665-
Parser p(&code, javascript_options);
665+
Parser p(&code, &this->memory_, javascript_options);
666666

667667
Parser_Transaction transaction = p.begin_transaction();
668668
bool ok = p.catch_fatal_parse_errors(
@@ -771,7 +771,7 @@ TEST_F(Test_No_Overflow, parser_depth_limit_not_exceeded) {
771771
}) {
772772
Padded_String code(u8"return " + jsx);
773773
SCOPED_TRACE(code);
774-
Parser p(&code, jsx_options);
774+
Parser p(&code, &this->memory_, jsx_options);
775775
Spy_Visitor v;
776776
bool ok = p.parse_and_visit_module_catching_fatal_parse_errors(v);
777777
EXPECT_TRUE(ok);
@@ -783,7 +783,7 @@ TEST_F(Test_No_Overflow, parser_depth_limit_not_exceeded) {
783783
}) {
784784
Padded_String code(concat(u8"let x: "_sv, type, u8";"_sv));
785785
SCOPED_TRACE(code);
786-
Parser p(&code, typescript_options);
786+
Parser p(&code, &this->memory_, typescript_options);
787787
Spy_Visitor v;
788788
bool ok = p.parse_and_visit_module_catching_fatal_parse_errors(v);
789789
EXPECT_TRUE(ok);
@@ -838,7 +838,7 @@ TEST_F(Test_Overflow, parser_depth_limit_exceeded) {
838838
}) {
839839
Padded_String code(exps);
840840
SCOPED_TRACE(code);
841-
Parser p(&code, javascript_options);
841+
Parser p(&code, &this->memory_, javascript_options);
842842
Spy_Visitor v;
843843
bool ok = p.parse_and_visit_module_catching_fatal_parse_errors(v);
844844
EXPECT_FALSE(ok);
@@ -885,7 +885,7 @@ TEST_F(Test_Overflow, parser_depth_limit_exceeded) {
885885
}) {
886886
Padded_String code(concat(u8"return "_sv, jsx));
887887
SCOPED_TRACE(code);
888-
Parser p(&code, jsx_options);
888+
Parser p(&code, &this->memory_, jsx_options);
889889
Spy_Visitor v;
890890
bool ok = p.parse_and_visit_module_catching_fatal_parse_errors(v);
891891
EXPECT_FALSE(ok);
@@ -900,7 +900,7 @@ TEST_F(Test_Overflow, parser_depth_limit_exceeded) {
900900
}) {
901901
Padded_String code(concat(u8"let x: "_sv, type, u8";"_sv));
902902
SCOPED_TRACE(code);
903-
Parser p(&code, typescript_options);
903+
Parser p(&code, &this->memory_, typescript_options);
904904
Spy_Visitor v;
905905
bool ok = p.parse_and_visit_module_catching_fatal_parse_errors(v);
906906
EXPECT_FALSE(ok);

0 commit comments

Comments
 (0)