Refactor: optimize diagnostic table storage

strager · strager · commit 81f5fd4ac41f · 2022-04-17T19:42:33.000-07:00
diagnostic_message_arg_info takes two bytes: one for the offset and one
for the type. The diagnostic table contains a lot of
diagnostic_message_arg_info-s, so these bytes add up.

Shrink diagnostic_message_arg_info by storing the offset in 5 bits and
the type in 3 bits (totalling 8 bits aka one byte).

This commit should not change behavior.
diff --git a/src/diagnostic.cpp b/src/diagnostic.cpp
@@ -98,10 +98,8 @@ get_diagnostic_message_arg_type<string8_view>() noexcept {
 template <class ArgType>
 constexpr diagnostic_message_arg_info make_diagnostic_message_arg_info(
     std::uint8_t offset) noexcept {
-  return diagnostic_message_arg_info{
-      .offset = offset,
-      .type = get_diagnostic_message_arg_type<ArgType>(),
-  };
+  return diagnostic_message_arg_info(
+      offset, get_diagnostic_message_arg_type<ArgType>());
 }
 
 template <class Error>
diff --git a/src/quick-lint-js/diagnostic-formatter.h b/src/quick-lint-js/diagnostic-formatter.h
@@ -36,6 +36,7 @@ class diagnostic_formatter_base {
       return *reinterpret_cast<const source_code_span*>(arg_data);
 
     case diagnostic_arg_type::char8:
+    case diagnostic_arg_type::invalid:
     case diagnostic_arg_type::statement_kind:
     case diagnostic_arg_type::string8_view:
     case diagnostic_arg_type::variable_kind:
@@ -63,6 +64,7 @@ class diagnostic_formatter_base {
     case diagnostic_arg_type::string8_view:
       return *reinterpret_cast<const string8_view*>(arg_data);
 
+    case diagnostic_arg_type::invalid:
     case diagnostic_arg_type::statement_kind:
     case diagnostic_arg_type::variable_kind:
       QLJS_UNREACHABLE();
@@ -81,6 +83,7 @@ class diagnostic_formatter_base {
 
     case diagnostic_arg_type::char8:
     case diagnostic_arg_type::identifier:
+    case diagnostic_arg_type::invalid:
     case diagnostic_arg_type::source_code_span:
     case diagnostic_arg_type::string8_view:
     case diagnostic_arg_type::variable_kind:
@@ -100,6 +103,7 @@ class diagnostic_formatter_base {
 
     case diagnostic_arg_type::char8:
     case diagnostic_arg_type::identifier:
+    case diagnostic_arg_type::invalid:
     case diagnostic_arg_type::source_code_span:
     case diagnostic_arg_type::string8_view:
     case diagnostic_arg_type::variable_kind:
@@ -114,7 +118,7 @@ class diagnostic_formatter_base {
       int arg_index) noexcept {
     const diagnostic_message_arg_info& arg_info = message_info.args[arg_index];
     const void* arg_data =
-        reinterpret_cast<const char*>(diagnostic) + arg_info.offset;
+        reinterpret_cast<const char*>(diagnostic) + arg_info.offset();
     return std::make_pair(arg_data, arg_info.type);
   }
 };
diff --git a/src/quick-lint-js/diagnostic.h b/src/quick-lint-js/diagnostic.h
@@ -4,10 +4,13 @@
 #ifndef QUICK_LINT_JS_DIAGNOSTIC_H
 #define QUICK_LINT_JS_DIAGNOSTIC_H
 
+#include <cstddef>
 #include <cstdint>
 #include <optional>
+#include <quick-lint-js/assert.h>
 #include <quick-lint-js/language.h>
 #include <quick-lint-js/translation.h>
+#include <quick-lint-js/warning.h>
 #include <string_view>
 
 // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105191
@@ -27,6 +30,8 @@ enum class diagnostic_severity : std::uint8_t {
 };
 
 enum class diagnostic_arg_type : std::uint8_t {
+  invalid = 0,
+
   char8,
   identifier,
   source_code_span,
@@ -36,8 +41,35 @@ enum class diagnostic_arg_type : std::uint8_t {
 };
 
 struct diagnostic_message_arg_info {
-  std::uint8_t offset QLJS_WORK_AROUND_GCC_BUG_105191;
-  diagnostic_arg_type type QLJS_WORK_AROUND_GCC_BUG_105191;
+  // offset_shift is how many bits are removed in compact_offset.
+  //
+  // For example, if offset_shift is 3, then an arg must be 8-byte aligned.
+  static constexpr int offset_shift = 1;
+  static constexpr int offset_mask = (1 << offset_shift) - 1;
+  static constexpr int offset_bits = 5;
+
+  /*implicit*/ constexpr diagnostic_message_arg_info() noexcept
+      : compact_offset(0), type(diagnostic_arg_type::invalid) {}
+
+  QLJS_WARNING_PUSH
+  QLJS_WARNING_IGNORE_CLANG("-Wimplicit-int-conversion")
+  QLJS_WARNING_IGNORE_GCC("-Wconversion")
+  /*implicit*/ constexpr diagnostic_message_arg_info(
+      std::size_t offset, diagnostic_arg_type type) noexcept
+      : compact_offset(offset >> offset_shift), type(type) {
+    // offset should be small.
+    QLJS_CONSTEXPR_ASSERT((offset >> offset_shift) < (1 << offset_bits));
+    // offset should be aligned.
+    QLJS_CONSTEXPR_ASSERT((offset & offset_mask) == 0);
+  }
+  QLJS_WARNING_POP
+
+  constexpr std::size_t offset() const noexcept {
+    return static_cast<std::size_t>(this->compact_offset << offset_shift);
+  }
+
+  std::uint8_t compact_offset : offset_bits QLJS_WORK_AROUND_GCC_BUG_105191;
+  diagnostic_arg_type type : (8 - offset_bits) QLJS_WORK_AROUND_GCC_BUG_105191;
 };
 
 struct diagnostic_message_info {
diff --git a/test/test-diagnostic.cpp b/test/test-diagnostic.cpp
@@ -28,7 +28,7 @@ TEST(test_diagnostic, diagnostic_info) {
     EXPECT_STREQ(source_code_messages.translate(info.messages[0].format),
                  "if statement needs parentheses around condition");
     EXPECT_EQ(
-        info.messages[0].args[0].offset,
+        info.messages[0].args[0].offset(),
         offsetof(error_expected_parentheses_around_if_condition, condition));
     EXPECT_EQ(info.messages[0].args[0].type,
               diagnostic_arg_type::source_code_span);
@@ -42,11 +42,11 @@ TEST(test_diagnostic, diagnostic_info) {
     EXPECT_EQ(info.severity, diagnostic_severity::error);
     EXPECT_STREQ(source_code_messages.translate(info.messages[0].format),
                  "if statement is missing '{1}' around condition");
-    EXPECT_EQ(info.messages[0].args[0].offset,
+    EXPECT_EQ(info.messages[0].args[0].offset(),
               offsetof(error_expected_parenthesis_around_if_condition, where));
     EXPECT_EQ(info.messages[0].args[0].type,
               diagnostic_arg_type::source_code_span);
-    EXPECT_EQ(info.messages[0].args[1].offset,
+    EXPECT_EQ(info.messages[0].args[1].offset(),
               offsetof(error_expected_parenthesis_around_if_condition, token));
     EXPECT_EQ(info.messages[0].args[1].type, diagnostic_arg_type::char8);
     EXPECT_FALSE(info.messages[1].format.valid());
@@ -60,12 +60,12 @@ TEST(test_diagnostic, diagnostic_info) {
     EXPECT_STREQ(source_code_messages.translate(info.messages[0].format),
                  "function called before declaration in block scope: {0}");
     EXPECT_EQ(
-        info.messages[0].args[0].offset,
+        info.messages[0].args[0].offset(),
         offsetof(error_function_call_before_declaration_in_block_scope, use));
     EXPECT_EQ(info.messages[0].args[0].type, diagnostic_arg_type::identifier);
     EXPECT_STREQ(source_code_messages.translate(info.messages[1].format),
                  "function declared here");
-    EXPECT_EQ(info.messages[1].args[0].offset,
+    EXPECT_EQ(info.messages[1].args[0].offset(),
               offsetof(error_function_call_before_declaration_in_block_scope,
                        declaration));
     EXPECT_EQ(info.messages[1].args[0].type, diagnostic_arg_type::identifier);
@@ -79,25 +79,25 @@ TEST(test_diagnostic, diagnostic_info) {
     EXPECT_STREQ(source_code_messages.translate(info.messages[0].format),
                  "missing body for {1:headlinese}");
     EXPECT_EQ(
-        info.messages[0].args[0].offset,
+        info.messages[0].args[0].offset(),
         offsetof(error_class_statement_not_allowed_in_body, expected_body));
     EXPECT_EQ(info.messages[0].args[0].type,
               diagnostic_arg_type::source_code_span);
     EXPECT_EQ(
-        info.messages[0].args[1].offset,
+        info.messages[0].args[1].offset(),
         offsetof(error_class_statement_not_allowed_in_body, kind_of_statement));
     EXPECT_EQ(info.messages[0].args[1].type,
               diagnostic_arg_type::statement_kind);
     EXPECT_STREQ(
         source_code_messages.translate(info.messages[1].format),
         "a class statement is not allowed as the body of {1:singular}");
     EXPECT_EQ(
-        info.messages[1].args[0].offset,
+        info.messages[1].args[0].offset(),
         offsetof(error_class_statement_not_allowed_in_body, class_keyword));
     EXPECT_EQ(info.messages[1].args[0].type,
               diagnostic_arg_type::source_code_span);
     EXPECT_EQ(
-        info.messages[1].args[1].offset,
+        info.messages[1].args[1].offset(),
         offsetof(error_class_statement_not_allowed_in_body, kind_of_statement));
     EXPECT_EQ(info.messages[1].args[1].type,
               diagnostic_arg_type::statement_kind);
