Skip to content

Commit e471f73

Browse files
stragerstrager
committed
Refactor: only store severity once per diagnostic
Each diagnostic can contain one or two messages. We store severity (error/warning/note) in each diagnostic *message*. This is wasteful because we every message after the first is always a note. Store severity once per diagnostic instead of once per message. This commit should not change behavior.
1 parent 7bca020 commit e471f73

File tree

6 files changed

+50
-50
lines changed

6 files changed

+50
-50
lines changed

‎src/diagnostic.cpp‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@ class diagnostic_info_builder {
6464
diagnostic_message_info& message_info =
6565
this->info_.messages[this->current_message_index_++];
6666
message_info.format = message;
67-
message_info.severity = sev;
6867

6968
int current_arg_index = 0;
7069
((message_info.args[current_arg_index++] = arg_infos), ...);

‎src/quick-lint-js/diagnostic-formatter.h‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -131,32 +131,33 @@ class diagnostic_formatter : private diagnostic_formatter_base {
131131

132132
void format(const diagnostic_info& info, const void* diagnostic);
133133

134-
void format_message(std::string_view code,
134+
void format_message(std::string_view code, diagnostic_severity severity,
135135
const diagnostic_message_info& info,
136136
const void* diagnostic);
137137
};
138138

139139
template <class Derived>
140140
inline void diagnostic_formatter<Derived>::format(const diagnostic_info& info,
141141
const void* diagnostic) {
142-
this->format_message(info.code, info.messages[0], diagnostic);
142+
this->format_message(info.code, info.severity, info.messages[0], diagnostic);
143143
if (info.messages[1].format.valid()) {
144-
this->format_message(info.code, info.messages[1], diagnostic);
144+
this->format_message(info.code, diagnostic_severity::note, info.messages[1],
145+
diagnostic);
145146
}
146147
}
147148

148149
template <class Derived>
149150
inline void diagnostic_formatter<Derived>::format_message(
150-
std::string_view code, const diagnostic_message_info& info,
151-
const void* diagnostic) {
151+
std::string_view code, diagnostic_severity severity,
152+
const diagnostic_message_info& info, const void* diagnostic) {
152153
static constexpr auto npos = string8_view::npos;
153154
using string8_pos = string8_view::size_type;
154155

155156
Derived* self = static_cast<Derived*>(this);
156157

157158
source_code_span origin_span =
158159
get_argument_source_code_span(info, diagnostic, 0);
159-
self->write_before_message(code, info.severity, origin_span);
160+
self->write_before_message(code, severity, origin_span);
160161

161162
string8_view remaining_message(translate(info.format));
162163
string8_pos left_curly_index;
@@ -167,13 +168,12 @@ inline void diagnostic_formatter<Derived>::format_message(
167168
if (remaining_message[left_curly_index + 1] == '{') {
168169
// "{{"; the '{' is escaped.
169170
self->write_message_part(
170-
code, info.severity,
171-
remaining_message.substr(0, left_curly_index + 1));
171+
code, severity, remaining_message.substr(0, left_curly_index + 1));
172172
remaining_message = remaining_message.substr(left_curly_index + 2);
173173
continue;
174174
}
175175

176-
self->write_message_part(code, info.severity,
176+
self->write_message_part(code, severity,
177177
remaining_message.substr(0, left_curly_index));
178178

179179
string8_pos right_curly_index =
@@ -200,12 +200,12 @@ inline void diagnostic_formatter<Derived>::format_message(
200200
QLJS_UNREACHABLE();
201201
}
202202

203-
self->write_message_part(code, info.severity, expanded_parameter);
203+
self->write_message_part(code, severity, expanded_parameter);
204204
remaining_message = remaining_message.substr(right_curly_index + 1);
205205
}
206-
self->write_message_part(code, info.severity, remaining_message);
206+
self->write_message_part(code, severity, remaining_message);
207207

208-
self->write_after_message(code, info.severity, origin_span);
208+
self->write_after_message(code, severity, origin_span);
209209
}
210210
}
211211

‎src/quick-lint-js/diagnostic.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ struct diagnostic_message_arg_info {
4242

4343
struct diagnostic_message_info {
4444
translatable_message format;
45-
diagnostic_severity severity QLJS_WORK_AROUND_GCC_BUG_105191;
4645
diagnostic_message_arg_info args[3];
4746
};
4847

‎test/test-diagnostic-formatter.cpp‎

Lines changed: 36 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -63,15 +63,15 @@ TEST(test_diagnostic_formatter, origin_span) {
6363

6464
constexpr diagnostic_message_info message_info = {
6565
.format = QLJS_TRANSLATABLE("something happened"),
66-
.severity = diagnostic_severity::error,
6766
.args =
6867
{
6968
{0, diagnostic_arg_type::source_code_span},
7069
},
7170
};
7271

7372
test_diagnostic_formatter formatter;
74-
formatter.format_message("E9999"sv, message_info, &span);
73+
formatter.format_message("E9999"sv, diagnostic_severity::error, message_info,
74+
&span);
7575

7676
EXPECT_EQ(formatter.write_before_message_call_count, 1);
7777
EXPECT_EQ(formatter.write_after_message_call_count, 1);
@@ -80,26 +80,26 @@ TEST(test_diagnostic_formatter, origin_span) {
8080
TEST(test_diagnostic_formatter, single_span_simple_message) {
8181
constexpr diagnostic_message_info message_info = {
8282
.format = QLJS_TRANSLATABLE("something happened"),
83-
.severity = diagnostic_severity::error,
8483
.args =
8584
{
8685
{0, diagnostic_arg_type::source_code_span},
8786
},
8887
};
8988

9089
string_diagnostic_formatter formatter;
91-
formatter.format_message("E9999"sv, message_info, &empty_span);
90+
formatter.format_message("E9999"sv, diagnostic_severity::error, message_info,
91+
&empty_span);
9292
EXPECT_EQ(formatter.message, u8"something happened\n");
9393
}
9494

9595
TEST(test_diagnostic_formatter, diagnostic_with_single_message) {
9696
constexpr diagnostic_info info = {
9797
.code = "E9999",
98+
.severity = diagnostic_severity::error,
9899
.messages =
99100
{
100101
diagnostic_message_info{
101102
.format = QLJS_TRANSLATABLE("something happened"),
102-
.severity = diagnostic_severity::error,
103103
.args =
104104
{
105105
{0, diagnostic_arg_type::source_code_span},
@@ -116,19 +116,18 @@ TEST(test_diagnostic_formatter, diagnostic_with_single_message) {
116116
TEST(test_diagnostic_formatter, diagnostic_with_two_messages) {
117117
constexpr diagnostic_info info = {
118118
.code = "E9999",
119+
.severity = diagnostic_severity::error,
119120
.messages =
120121
{
121122
diagnostic_message_info{
122123
.format = QLJS_TRANSLATABLE("something happened"),
123-
.severity = diagnostic_severity::error,
124124
.args =
125125
{
126126
{0, diagnostic_arg_type::source_code_span},
127127
},
128128
},
129129
diagnostic_message_info{
130130
.format = QLJS_TRANSLATABLE("see here"),
131-
.severity = diagnostic_severity::note,
132131
.args =
133132
{
134133
{0, diagnostic_arg_type::source_code_span},
@@ -147,7 +146,6 @@ TEST(test_diagnostic_formatter, diagnostic_with_two_messages) {
147146
TEST(test_diagnostic_formatter, message_with_zero_placeholder) {
148147
constexpr diagnostic_message_info message_info = {
149148
.format = QLJS_TRANSLATABLE("this {0} looks fishy"),
150-
.severity = diagnostic_severity::error,
151149
.args =
152150
{
153151
{0, diagnostic_arg_type::source_code_span},
@@ -158,7 +156,8 @@ TEST(test_diagnostic_formatter, message_with_zero_placeholder) {
158156
source_code_span hello_span(&code[0], &code[5]);
159157

160158
string_diagnostic_formatter formatter;
161-
formatter.format_message("E9999"sv, message_info, &hello_span);
159+
formatter.format_message("E9999"sv, diagnostic_severity::error, message_info,
160+
&hello_span);
162161
EXPECT_EQ(formatter.message, u8"this hello looks fishy\n");
163162
}
164163

@@ -169,7 +168,6 @@ TEST(test_diagnostic_formatter, message_with_extra_identifier_placeholder) {
169168
};
170169
constexpr diagnostic_message_info message_info = {
171170
.format = QLJS_TRANSLATABLE("this {1} looks fishy"),
172-
.severity = diagnostic_severity::error,
173171
.args =
174172
{
175173
{offsetof(test_diag, hello),
@@ -185,7 +183,8 @@ TEST(test_diagnostic_formatter, message_with_extra_identifier_placeholder) {
185183
};
186184

187185
string_diagnostic_formatter formatter;
188-
formatter.format_message("E9999"sv, message_info, &diag);
186+
formatter.format_message("E9999"sv, diagnostic_severity::error, message_info,
187+
&diag);
189188
EXPECT_EQ(formatter.message, u8"this world looks fishy\n");
190189
}
191190

@@ -197,7 +196,6 @@ TEST(test_diagnostic_formatter, message_with_multiple_span_placeholders) {
197196
};
198197
constexpr diagnostic_message_info message_info = {
199198
.format = QLJS_TRANSLATABLE("free {1} and {0} {1} {2}"),
200-
.severity = diagnostic_severity::error,
201199
.args =
202200
{
203201
{offsetof(test_diag, let_span),
@@ -220,7 +218,8 @@ TEST(test_diagnostic_formatter, message_with_multiple_span_placeholders) {
220218
ASSERT_EQ(diag.be_span.string_view(), u8"be");
221219

222220
string_diagnostic_formatter formatter;
223-
formatter.format_message("E9999"sv, message_info, &diag);
221+
formatter.format_message("E9999"sv, diagnostic_severity::error, message_info,
222+
&diag);
224223
EXPECT_EQ(formatter.message, u8"free me and let me be\n");
225224
}
226225

@@ -231,7 +230,6 @@ TEST(test_diagnostic_formatter, message_with_char_placeholder) {
231230
};
232231
constexpr diagnostic_message_info message_info = {
233232
.format = QLJS_TRANSLATABLE("what is this '{1}' nonsense?"),
234-
.severity = diagnostic_severity::error,
235233
.args =
236234
{
237235
{offsetof(test_diag, span),
@@ -245,14 +243,14 @@ TEST(test_diagnostic_formatter, message_with_char_placeholder) {
245243
.c = u8'Q',
246244
};
247245
string_diagnostic_formatter formatter;
248-
formatter.format_message("E9999"sv, message_info, &diag);
246+
formatter.format_message("E9999"sv, diagnostic_severity::error, message_info,
247+
&diag);
249248
EXPECT_EQ(formatter.message, u8"what is this 'Q' nonsense?\n");
250249
}
251250

252251
TEST(test_diagnostic_formatter, message_with_escaped_curlies) {
253252
constexpr diagnostic_message_info message_info = {
254253
.format = QLJS_TRANSLATABLE("a {{0} b }} c"),
255-
.severity = diagnostic_severity::error,
256254
.args =
257255
{
258256
{0, diagnostic_arg_type::source_code_span},
@@ -263,7 +261,8 @@ TEST(test_diagnostic_formatter, message_with_escaped_curlies) {
263261
source_code_span code_span(&code[0], &code[3]);
264262

265263
string_diagnostic_formatter formatter;
266-
formatter.format_message("E9999"sv, message_info, &code_span);
264+
formatter.format_message("E9999"sv, diagnostic_severity::error, message_info,
265+
&code_span);
267266
EXPECT_EQ(formatter.message, u8"a {0} b }} c\n");
268267
}
269268

@@ -274,7 +273,6 @@ TEST(test_diagnostic_formatter, statement_kind_placeholder) {
274273
};
275274
constexpr diagnostic_message_info headlinese_message_info = {
276275
.format = QLJS_TRANSLATABLE("expected {1:headlinese}"),
277-
.severity = diagnostic_severity::error,
278276
.args =
279277
{
280278
{offsetof(test_diag, empty_span),
@@ -285,7 +283,6 @@ TEST(test_diagnostic_formatter, statement_kind_placeholder) {
285283
};
286284
constexpr diagnostic_message_info singular_message_info = {
287285
.format = QLJS_TRANSLATABLE("expected {1:singular}"),
288-
.severity = diagnostic_severity::error,
289286
.args =
290287
{
291288
{offsetof(test_diag, empty_span),
@@ -301,7 +298,8 @@ TEST(test_diagnostic_formatter, statement_kind_placeholder) {
301298
.statement = statement_kind::do_while_loop,
302299
};
303300
string_diagnostic_formatter formatter;
304-
formatter.format_message("E9999"sv, headlinese_message_info, &diag);
301+
formatter.format_message("E9999"sv, diagnostic_severity::error,
302+
headlinese_message_info, &diag);
305303
EXPECT_EQ(formatter.message, u8"expected 'do-while' loop\n");
306304
}
307305

@@ -311,7 +309,8 @@ TEST(test_diagnostic_formatter, statement_kind_placeholder) {
311309
.statement = statement_kind::do_while_loop,
312310
};
313311
string_diagnostic_formatter formatter;
314-
formatter.format_message("E9999"sv, singular_message_info, &diag);
312+
formatter.format_message("E9999"sv, diagnostic_severity::error,
313+
singular_message_info, &diag);
315314
EXPECT_EQ(formatter.message, u8"expected a 'do-while' loop\n");
316315
}
317316

@@ -321,7 +320,8 @@ TEST(test_diagnostic_formatter, statement_kind_placeholder) {
321320
.statement = statement_kind::for_loop,
322321
};
323322
string_diagnostic_formatter formatter;
324-
formatter.format_message("E9999"sv, headlinese_message_info, &diag);
323+
formatter.format_message("E9999"sv, diagnostic_severity::error,
324+
headlinese_message_info, &diag);
325325
EXPECT_EQ(formatter.message, u8"expected 'for' loop\n");
326326
}
327327

@@ -331,7 +331,8 @@ TEST(test_diagnostic_formatter, statement_kind_placeholder) {
331331
.statement = statement_kind::for_loop,
332332
};
333333
string_diagnostic_formatter formatter;
334-
formatter.format_message("E9999"sv, singular_message_info, &diag);
334+
formatter.format_message("E9999"sv, diagnostic_severity::error,
335+
singular_message_info, &diag);
335336
EXPECT_EQ(formatter.message, u8"expected a 'for' loop\n");
336337
}
337338

@@ -341,7 +342,8 @@ TEST(test_diagnostic_formatter, statement_kind_placeholder) {
341342
.statement = statement_kind::if_statement,
342343
};
343344
string_diagnostic_formatter formatter;
344-
formatter.format_message("E9999"sv, headlinese_message_info, &diag);
345+
formatter.format_message("E9999"sv, diagnostic_severity::error,
346+
headlinese_message_info, &diag);
345347
EXPECT_EQ(formatter.message, u8"expected 'if' statement\n");
346348
}
347349

@@ -351,7 +353,8 @@ TEST(test_diagnostic_formatter, statement_kind_placeholder) {
351353
.statement = statement_kind::if_statement,
352354
};
353355
string_diagnostic_formatter formatter;
354-
formatter.format_message("E9999"sv, singular_message_info, &diag);
356+
formatter.format_message("E9999"sv, diagnostic_severity::error,
357+
singular_message_info, &diag);
355358
EXPECT_EQ(formatter.message, u8"expected an 'if' statement\n");
356359
}
357360

@@ -361,7 +364,8 @@ TEST(test_diagnostic_formatter, statement_kind_placeholder) {
361364
.statement = statement_kind::while_loop,
362365
};
363366
string_diagnostic_formatter formatter;
364-
formatter.format_message("E9999"sv, headlinese_message_info, &diag);
367+
formatter.format_message("E9999"sv, diagnostic_severity::error,
368+
headlinese_message_info, &diag);
365369
EXPECT_EQ(formatter.message, u8"expected 'while' loop\n");
366370
}
367371

@@ -371,7 +375,8 @@ TEST(test_diagnostic_formatter, statement_kind_placeholder) {
371375
.statement = statement_kind::while_loop,
372376
};
373377
string_diagnostic_formatter formatter;
374-
formatter.format_message("E9999"sv, singular_message_info, &diag);
378+
formatter.format_message("E9999"sv, diagnostic_severity::error,
379+
singular_message_info, &diag);
375380
EXPECT_EQ(formatter.message, u8"expected a 'while' loop\n");
376381
}
377382

@@ -381,7 +386,8 @@ TEST(test_diagnostic_formatter, statement_kind_placeholder) {
381386
.statement = statement_kind::with_statement,
382387
};
383388
string_diagnostic_formatter formatter;
384-
formatter.format_message("E9999"sv, headlinese_message_info, &diag);
389+
formatter.format_message("E9999"sv, diagnostic_severity::error,
390+
headlinese_message_info, &diag);
385391
EXPECT_EQ(formatter.message, u8"expected 'with' statement\n");
386392
}
387393

@@ -391,7 +397,8 @@ TEST(test_diagnostic_formatter, statement_kind_placeholder) {
391397
.statement = statement_kind::with_statement,
392398
};
393399
string_diagnostic_formatter formatter;
394-
formatter.format_message("E9999"sv, singular_message_info, &diag);
400+
formatter.format_message("E9999"sv, diagnostic_severity::error,
401+
singular_message_info, &diag);
395402
EXPECT_EQ(formatter.message, u8"expected a 'with' statement\n");
396403
}
397404
}

‎test/test-diagnostic.cpp‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ TEST(test_diagnostic, diagnostic_info) {
2727
EXPECT_EQ(info.severity, diagnostic_severity::error);
2828
EXPECT_STREQ(source_code_messages.translate(info.messages[0].format),
2929
"if statement needs parentheses around condition");
30-
EXPECT_EQ(info.messages[0].severity, diagnostic_severity::error);
3130
EXPECT_EQ(
3231
info.messages[0].args[0].offset,
3332
offsetof(error_expected_parentheses_around_if_condition, condition));
@@ -43,7 +42,6 @@ TEST(test_diagnostic, diagnostic_info) {
4342
EXPECT_EQ(info.severity, diagnostic_severity::error);
4443
EXPECT_STREQ(source_code_messages.translate(info.messages[0].format),
4544
"if statement is missing '{1}' around condition");
46-
EXPECT_EQ(info.messages[0].severity, diagnostic_severity::error);
4745
EXPECT_EQ(info.messages[0].args[0].offset,
4846
offsetof(error_expected_parenthesis_around_if_condition, where));
4947
EXPECT_EQ(info.messages[0].args[0].type,
@@ -61,7 +59,6 @@ TEST(test_diagnostic, diagnostic_info) {
6159
EXPECT_EQ(info.severity, diagnostic_severity::warning);
6260
EXPECT_STREQ(source_code_messages.translate(info.messages[0].format),
6361
"function called before declaration in block scope: {0}");
64-
EXPECT_EQ(info.messages[0].severity, diagnostic_severity::warning);
6562
EXPECT_EQ(
6663
info.messages[0].args[0].offset,
6764
offsetof(error_function_call_before_declaration_in_block_scope, use));
@@ -81,7 +78,6 @@ TEST(test_diagnostic, diagnostic_info) {
8178
EXPECT_EQ(info.severity, diagnostic_severity::error);
8279
EXPECT_STREQ(source_code_messages.translate(info.messages[0].format),
8380
"missing body for {1:headlinese}");
84-
EXPECT_EQ(info.messages[0].severity, diagnostic_severity::error);
8581
EXPECT_EQ(
8682
info.messages[0].args[0].offset,
8783
offsetof(error_class_statement_not_allowed_in_body, expected_body));

0 commit comments

Comments
 (0)