Skip to content

Commit 0647263

Browse files
committed
Refactor: store error codes in 16-bit int instead of string
The diagnostic table contains error code strings (e.g. "E1234"). Each string consumes 6 bytes in the table. Change the code to be stored as a 16-bit integer instead, saving four bytes per diagnostic table entry. This commit should not change behavior.
1 parent d1fcb39 commit 0647263

File tree

6 files changed

+56
-18
lines changed

6 files changed

+56
-18
lines changed

src/diagnostic.cpp

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// Copyright (C) 2020 Matthew "strager" Glazar
22
// See end of file for extended copyright information.
33

4+
#include <array>
5+
#include <cstdint>
46
#include <cstring>
57
#include <quick-lint-js/assert.h>
68
#include <quick-lint-js/char8.h>
@@ -31,19 +33,38 @@
3133

3234
namespace quick_lint_js {
3335
namespace {
34-
constexpr void strcpy(char* out, const char* in) noexcept {
35-
while ((*out++ = *in++) != '\0')
36-
;
36+
constexpr std::uint16_t parse_code_string(const char* code_string) noexcept {
37+
QLJS_CONSTEXPR_ASSERT(code_string[0] == 'E');
38+
QLJS_CONSTEXPR_ASSERT('0' <= code_string[1] && code_string[1] <= '9');
39+
QLJS_CONSTEXPR_ASSERT('0' <= code_string[2] && code_string[2] <= '9');
40+
QLJS_CONSTEXPR_ASSERT('0' <= code_string[3] && code_string[3] <= '9');
41+
QLJS_CONSTEXPR_ASSERT('0' <= code_string[4] && code_string[4] <= '9');
42+
QLJS_CONSTEXPR_ASSERT(code_string[5] == '\0');
43+
return static_cast<std::uint16_t>((code_string[1] - '0') * 1000 + //
44+
(code_string[2] - '0') * 100 + //
45+
(code_string[3] - '0') * 10 + //
46+
(code_string[4] - '0') * 1);
47+
}
48+
49+
std::array<char, 5> error_code_to_string(std::uint16_t error_code) noexcept {
50+
QLJS_ASSERT(error_code <= 9999);
51+
return std::array<char, 5>{
52+
'E',
53+
static_cast<char>('0' + ((error_code / 1000) % 10)),
54+
static_cast<char>('0' + ((error_code / 100) % 10)),
55+
static_cast<char>('0' + ((error_code / 10) % 10)),
56+
static_cast<char>('0' + ((error_code / 1) % 10)),
57+
};
3758
}
3859

3960
// Convert a QLJS_ERROR_TYPE user into a diagnostic_info.
4061
template <class Error>
4162
class diagnostic_info_builder {
4263
public:
43-
constexpr explicit diagnostic_info_builder(const char* code,
64+
constexpr explicit diagnostic_info_builder(const char* code_string,
4465
diagnostic_severity sev) {
4566
this->info_.severity = sev;
46-
strcpy(this->info_.code, code);
67+
this->info_.code = parse_code_string(code_string);
4768
}
4869

4970
// Each of Args must be a diagnostic_message_arg_info.
@@ -144,6 +165,10 @@ const diagnostic_info& get_diagnostic_info(error_type type) noexcept {
144165
return all_diagnostic_infos[static_cast<std::ptrdiff_t>(type)];
145166
}
146167

168+
std::array<char, 5> diagnostic_info::code_string() const noexcept {
169+
return error_code_to_string(this->code);
170+
}
171+
147172
QLJS_WARNING_PUSH
148173
// GCC thinks that all_diagnostic_infos[i].code is not null-terminated, but it
149174
// is.
@@ -152,7 +177,12 @@ QLJS_WARNING_IGNORE_GCC("-Wstringop-overflow")
152177
std::optional<error_type> error_type_from_code_slow(
153178
std::string_view code) noexcept {
154179
for (int i = 0; i < error_type_count; ++i) {
155-
if (all_diagnostic_infos[i].code == code) {
180+
// TODO(strager): Parse the incoming code instead of stringifying each code
181+
// in the table.
182+
auto diag_code_string = all_diagnostic_infos[i].code_string();
183+
std::string_view diag_code_string_view(diag_code_string.data(),
184+
diag_code_string.size());
185+
if (diag_code_string_view == code) {
156186
return static_cast<error_type>(i);
157187
}
158188
}

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include <quick-lint-js/language.h>
1414
#include <quick-lint-js/lex.h>
1515
#include <quick-lint-js/location.h>
16+
#include <quick-lint-js/narrow-cast.h>
1617
#include <quick-lint-js/translation.h>
1718
#include <quick-lint-js/unreachable.h>
1819
#include <type_traits>
@@ -143,10 +144,14 @@ class diagnostic_formatter : private diagnostic_formatter_base {
143144
template <class Derived>
144145
inline void diagnostic_formatter<Derived>::format(const diagnostic_info& info,
145146
const void* diagnostic) {
146-
this->format_message(info.code, info.severity, info.messages[0], diagnostic);
147+
auto code_string = info.code_string();
148+
std::string_view code_string_view(code_string.data(), code_string.size());
149+
150+
this->format_message(code_string_view, info.severity, info.messages[0],
151+
diagnostic);
147152
if (info.messages[1].format.valid()) {
148-
this->format_message(info.code, diagnostic_severity::note, info.messages[1],
149-
diagnostic);
153+
this->format_message(code_string_view, diagnostic_severity::note,
154+
info.messages[1], diagnostic);
150155
}
151156
}
152157

src/quick-lint-js/diagnostic.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#ifndef QUICK_LINT_JS_DIAGNOSTIC_H
55
#define QUICK_LINT_JS_DIAGNOSTIC_H
66

7+
#include <array>
78
#include <cstddef>
89
#include <cstdint>
910
#include <optional>
@@ -78,7 +79,9 @@ struct diagnostic_message_info {
7879
};
7980

8081
struct diagnostic_info {
81-
char code[6];
82+
std::array<char, 5> code_string() const noexcept;
83+
84+
std::uint16_t code;
8285
diagnostic_severity severity QLJS_WORK_AROUND_GCC_BUG_105191;
8386
// If we support more than two infos (i.e. more than one note), the VS Code
8487
// plugin needs to be updated. See NOTE(multiple notes).

test/test-diagnostic-formatter.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ TEST(test_diagnostic_formatter, single_span_simple_message) {
9494

9595
TEST(test_diagnostic_formatter, diagnostic_with_single_message) {
9696
constexpr diagnostic_info info = {
97-
.code = "E9999",
97+
.code = 9999,
9898
.severity = diagnostic_severity::error,
9999
.messages =
100100
{
@@ -115,7 +115,7 @@ TEST(test_diagnostic_formatter, diagnostic_with_single_message) {
115115

116116
TEST(test_diagnostic_formatter, diagnostic_with_two_messages) {
117117
constexpr diagnostic_info info = {
118-
.code = "E9999",
118+
.code = 9999,
119119
.severity = diagnostic_severity::error,
120120
.messages =
121121
{

test/test-diagnostic.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ TEST(test_diagnostic, diagnostic_info) {
2323
{
2424
const diagnostic_info& info = diagnostic_info_for_error<
2525
error_expected_parentheses_around_if_condition>;
26-
EXPECT_EQ(info.code, "E0017"sv);
26+
EXPECT_EQ(info.code, 17);
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");
@@ -38,7 +38,7 @@ TEST(test_diagnostic, diagnostic_info) {
3838
{
3939
const diagnostic_info& info = diagnostic_info_for_error<
4040
error_expected_parenthesis_around_if_condition>;
41-
EXPECT_EQ(info.code, "E0018"sv);
41+
EXPECT_EQ(info.code, 18);
4242
EXPECT_EQ(info.severity, diagnostic_severity::error);
4343
EXPECT_STREQ(source_code_messages.translate(info.messages[0].format),
4444
"if statement is missing '{1}' around condition");
@@ -55,7 +55,7 @@ TEST(test_diagnostic, diagnostic_info) {
5555
{
5656
const diagnostic_info& info = diagnostic_info_for_error<
5757
error_function_call_before_declaration_in_block_scope>;
58-
EXPECT_EQ(info.code, "E0077"sv);
58+
EXPECT_EQ(info.code, 77);
5959
EXPECT_EQ(info.severity, diagnostic_severity::warning);
6060
EXPECT_STREQ(source_code_messages.translate(info.messages[0].format),
6161
"function called before declaration in block scope: {0}");
@@ -74,7 +74,7 @@ TEST(test_diagnostic, diagnostic_info) {
7474
{
7575
const diagnostic_info& info =
7676
diagnostic_info_for_error<error_class_statement_not_allowed_in_body>;
77-
EXPECT_EQ(info.code, "E0149"sv);
77+
EXPECT_EQ(info.code, 149);
7878
EXPECT_EQ(info.severity, diagnostic_severity::error);
7979
EXPECT_STREQ(source_code_messages.translate(info.messages[0].format),
8080
"missing body for {1:headlinese}");

test/test-vim-qflist-json-error-reporter.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ TEST_F(test_vim_qflist_json_error_reporter, use_of_undeclared_variable) {
282282

283283
TEST(test_vim_qflist_json_error_formatter, single_span_simple_message) {
284284
constexpr diagnostic_info diag_info = {
285-
.code = "E9999",
285+
.code = 9999,
286286
.severity = diagnostic_severity::error,
287287
.messages =
288288
{
@@ -321,7 +321,7 @@ TEST(test_vim_qflist_json_error_formatter, message_with_note_ignores_note) {
321321
source_code_span world_span;
322322
};
323323
constexpr diagnostic_info diag_info = {
324-
.code = "E9999",
324+
.code = 9999,
325325
.severity = diagnostic_severity::error,
326326
.messages =
327327
{

0 commit comments

Comments
 (0)