refactor(doc): make the style registry immutable after construction

StyleRegistry now takes its font names and resolved styles in the
constructor (mirroring the xls registry); set_font_names/add_style are
gone, read_character_runs collects styles into a plain vector and no
longer depends on the registry, and the default style moved into a named
default_character_style() factory.

Also review cleanups across the branch:
- emplace_back for aggregate push_backs (C++20 parenthesized init)
- intern cache: single try_emplace instead of find + emplace with a
  second std::string(grpprl) allocation
- fixed-size integer types instead of plain int
- drop the local read_u16 in favor of util::byte::from_little_endian
- shared append_u16/append_u32 test helpers in oldms_test_util.hpp
- doc_helper.cpp: definitions moved into the namespace block

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WJRDZcksAtKnXyhDTeMkMK

Author: andiwand

src/odr/internal/oldms/spreadsheet/xls_parser.cpp

@@ -67,7 +67,7 @@ void parse_globals(BiffReader &reader, std::vector<BoundSheet> &sheets,
     case biff_font: {
       const auto font = reader.read<FontFixed>();
       std::string name = reader.read_short_xl_unicode_string();
-      styles.fonts.push_back({font, std::move(name)});
+      styles.fonts.emplace_back(font, std::move(name));
     } break;
     case biff_xf: {
       styles.xfs.push_back(reader.read<XfBody>());

src/odr/internal/oldms/text/doc_helper.cpp

@@ -13,35 +13,11 @@
 #include <string_view>
 #include <unordered_map>
 
-namespace odr::internal::oldms {
-
-text::CharacterIndex text::read_character_index(std::istream &in) {
-  CharacterIndex result;
-
-  read_Clx(in, skip_Prc, [&](std::istream &) {
-    if (const int c = in.get(); c != 0x2) {
-      throw std::runtime_error("Unexpected input: " + std::to_string(c));
-    }
-    const std::uint32_t lcb = util::byte_stream::read<std::uint32_t>(in);
-    std::string plcPcd = util::stream::read(in, lcb);
-    const PlcPcdMap plc_pcd_map(plcPcd.data(), plcPcd.size());
-
-    for (std::uint32_t i = 0; i < plc_pcd_map.n(); ++i) {
-      const bool is_compressed = plc_pcd_map.aData(i).fc.fCompressed != 0;
-      const std::size_t data_offset = is_compressed
-                                          ? plc_pcd_map.aData(i).fc.fc / 2
-                                          : plc_pcd_map.aData(i).fc.fc;
-      const std::size_t length_cp = plc_pcd_map.aCP(i + 1) - plc_pcd_map.aCP(i);
-      result.append(plc_pcd_map.aCP(i), length_cp, data_offset, is_compressed);
-    }
-  });
-
-  return result;
-}
+namespace odr::internal::oldms::text {
 
-void text::CharacterRuns::append_run(const std::uint32_t begin_fc,
-                                     const std::uint32_t end_fc,
-                                     const std::uint32_t style_index) {
+void CharacterRuns::append_run(const std::uint32_t begin_fc,
+                               const std::uint32_t end_fc,
+                               const std::uint32_t style_index) {
   if (begin_fc >= end_fc ||
       (!m_runs.empty() && begin_fc < m_runs.back().end_fc)) {
     throw std::runtime_error("doc: character runs must be ascending");
@@ -51,10 +27,10 @@ void text::CharacterRuns::append_run(const std::uint32_t begin_fc,
     m_runs.back().end_fc = end_fc;
     return;
   }
-  m_runs.push_back({begin_fc, end_fc, style_index});
+  m_runs.emplace_back(begin_fc, end_fc, style_index);
 }
 
-std::uint32_t text::CharacterRuns::index_at(const std::uint32_t fc) const {
+std::uint32_t CharacterRuns::index_at(const std::uint32_t fc) const {
   const auto it = std::ranges::upper_bound(m_runs, fc, {}, &Run::begin_fc);
   if (it == m_runs.begin()) {
     return 0;
@@ -63,7 +39,7 @@ std::uint32_t text::CharacterRuns::index_at(const std::uint32_t fc) const {
   return fc < run.end_fc ? run.style_index : 0;
 }
 
-std::uint32_t text::CharacterRuns::chunk_end(const std::uint32_t fc) const {
+std::uint32_t CharacterRuns::chunk_end(const std::uint32_t fc) const {
   const auto it = std::ranges::upper_bound(m_runs, fc, {}, &Run::begin_fc);
   if (it != m_runs.begin() && fc < std::prev(it)->end_fc) {
     return std::prev(it)->end_fc;
@@ -74,17 +50,46 @@ std::uint32_t text::CharacterRuns::chunk_end(const std::uint32_t fc) const {
   return std::numeric_limits<std::uint32_t>::max();
 }
 
-text::CharacterRuns text::read_character_runs(std::istream &document_stream,
-                                              std::istream &table_stream,
-                                              const FcLcb plcf_bte_chpx,
-                                              StyleRegistry &style_registry) {
+} // namespace odr::internal::oldms::text
+
+namespace odr::internal::oldms {
+
+text::CharacterIndex text::read_character_index(std::istream &in) {
+  CharacterIndex result;
+
+  read_Clx(in, skip_Prc, [&](std::istream &) {
+    if (const int c = in.get(); c != 0x2) {
+      throw std::runtime_error("Unexpected input: " + std::to_string(c));
+    }
+    const std::uint32_t lcb = util::byte_stream::read<std::uint32_t>(in);
+    std::string plcPcd = util::stream::read(in, lcb);
+    const PlcPcdMap plc_pcd_map(plcPcd.data(), plcPcd.size());
+
+    for (std::uint32_t i = 0; i < plc_pcd_map.n(); ++i) {
+      const bool is_compressed = plc_pcd_map.aData(i).fc.fCompressed != 0;
+      const std::size_t data_offset = is_compressed
+                                          ? plc_pcd_map.aData(i).fc.fc / 2
+                                          : plc_pcd_map.aData(i).fc.fc;
+      const std::size_t length_cp = plc_pcd_map.aCP(i + 1) - plc_pcd_map.aCP(i);
+      result.append(plc_pcd_map.aCP(i), length_cp, data_offset, is_compressed);
+    }
+  });
+
+  return result;
+}
+
+text::CharacterRuns
+text::read_character_runs(std::istream &document_stream,
+                          std::istream &table_stream, const FcLcb plcf_bte_chpx,
+                          std::vector<TextStyle> &styles,
+                          const std::span<const std::string> font_names) {
   CharacterRuns result;
   if (plcf_bte_chpx.lcb == 0) {
     return result;
   }
 
-  // Copy: `add_style` may reallocate the registry's style storage.
-  const TextStyle default_style = style_registry.text_style(0);
+  // Copy: appending to `styles` may reallocate.
+  const TextStyle default_style = styles.at(0);
 
   table_stream.seekg(plcf_bte_chpx.fc);
   std::string plc_bytes = util::stream::read(table_stream, plcf_bte_chpx.lcb);
@@ -96,14 +101,13 @@ text::CharacterRuns text::read_character_runs(std::istream &document_stream,
     if (grpprl.empty()) {
       return 0;
     }
-    const auto it = style_cache.find(std::string(grpprl));
-    if (it != style_cache.end()) {
-      return it->second;
+    const auto [it, inserted] = style_cache.try_emplace(std::string(grpprl));
+    if (inserted) {
+      styles.push_back(
+          apply_character_sprms(default_style, grpprl, font_names));
+      it->second = static_cast<std::uint32_t>(styles.size() - 1);
     }
-    const std::uint32_t index = style_registry.add_style(apply_character_sprms(
-        default_style, grpprl, style_registry.font_names()));
-    style_cache.emplace(std::string(grpprl), index);
-    return index;
+    return it->second;
   };
 
   for (std::uint32_t i = 0; i < plc.n(); ++i) {

src/odr/internal/oldms/text/doc_helper.hpp

@@ -1,17 +1,19 @@
 #pragma once
 
+#include <odr/style.hpp>
+
 #include <odr/internal/oldms/text/doc_structs.hpp>
 
 #include <algorithm>
 #include <cstdint>
 #include <iosfwd>
+#include <span>
 #include <stdexcept>
+#include <string>
 #include <vector>
 
 namespace odr::internal::oldms::text {
 
-class StyleRegistry;
-
 class CharacterIndex {
 public:
   [[nodiscard]] bool empty() const { return m_entries.empty(); }
@@ -111,8 +113,8 @@ class CharacterIndex {
 CharacterIndex read_character_index(std::istream &in);
 
 /// The document's character-formatting runs, keyed by WordDocument-stream
-/// offset (fc), each referencing a `StyleRegistry` style index. Index 0 is
-/// the default style; offsets outside any run resolve to it.
+/// offset (fc), each referencing a style index. Index 0 is the default style;
+/// offsets outside any run resolve to it.
 class CharacterRuns final {
 public:
   /// Appends a run; runs must be ascending and non-overlapping. Adjacent runs
@@ -138,12 +140,13 @@ class CharacterRuns final {
 
 /// Reads the PlcBteChpx at `fc` in the table stream and every referenced
 /// ChpxFkp page ([MS-DOC] 2.9.33) from the WordDocument stream, resolving
-/// each run's Chpx on top of the default style (index 0) into
-/// `style_registry` (fonts via its font names). Equal Chpx bytes share one
-/// style.
+/// each run's Chpx on top of `styles[0]` (the default style) and appending
+/// each distinct resolved style to `styles` (fonts via `font_names`). Equal
+/// Chpx bytes share one style.
 CharacterRuns read_character_runs(std::istream &document_stream,
                                   std::istream &table_stream,
                                   FcLcb plcf_bte_chpx,
-                                  StyleRegistry &style_registry);
+                                  std::vector<TextStyle> &styles,
+                                  std::span<const std::string> font_names);
 
 } // namespace odr::internal::oldms::text

src/odr/internal/oldms/text/doc_parser.cpp

@@ -96,7 +96,7 @@ class TextCleaner {
   // Open fields; entry is true while still in that field's instruction part.
   // m_instruction_depth counts those; text is hidden whenever it is > 0.
   std::vector<bool> m_field_in_instruction;
-  int m_instruction_depth{0};
+  std::int32_t m_instruction_depth{0};
 };
 
 /// A maximal piece of body text with uniform character formatting.
@@ -143,7 +143,7 @@ std::vector<StyledRun> decode_styled_runs(
       if (!runs.empty() && runs.back().style_index == style_index) {
         runs.back().text += chunk_text;
       } else {
-        runs.push_back({std::move(chunk_text), style_index});
+        runs.emplace_back(std::move(chunk_text), style_index);
       }
       cp += chunk_cp;
     }
@@ -169,18 +169,18 @@ ElementIdentifier text::parse_tree(ElementRegistry &registry,
       fib.base.fWhichTblStm == 1 ? "/1Table" : "/0Table";
   const auto table_stream = files.open(AbsPath(tableStreamPath))->stream();
 
-  // Font table; TextStyle::font_name points into the registry's copy.
-  style_registry.set_font_names(
-      read_font_names(*table_stream, fib.fibRgFcLcb->sttbfFfn));
+  // Font table; TextStyle::font_name points into the strings, which keep
+  // their buffers when the vector is moved into the registry below.
+  std::vector<std::string> font_names =
+      read_font_names(*table_stream, fib.fibRgFcLcb->sttbfFfn);
 
   // Direct character formatting ([MS-DOC] 2.4.6.2); Pcd.Prm modifications are
-  // not modelled. Without sprmCHps the size defaults to 20 half-points.
-  TextStyle default_style;
-  default_style.font_size = Measure(10.0, DynamicUnit("pt"));
-  style_registry.add_style(std::move(default_style)); // index 0
+  // not modelled.
+  std::vector<TextStyle> styles{default_character_style()}; // index 0
   const CharacterRuns character_runs =
       read_character_runs(*document_stream, *table_stream,
-                          fib.fibRgFcLcb->plcfBteChpx, style_registry);
+                          fib.fibRgFcLcb->plcfBteChpx, styles, font_names);
+  style_registry = StyleRegistry(std::move(font_names), std::move(styles));
 
   table_stream->seekg(fib.fibRgFcLcb->clx.fc);
   const CharacterIndex character_index = read_character_index(*table_stream);

src/odr/internal/oldms/text/doc_structs.hpp

@@ -308,7 +308,7 @@ struct Sprm {
   std::uint16_t sgc : 3;
   std::uint16_t spra : 3;
 
-  [[nodiscard]] int operand_size() const {
+  [[nodiscard]] std::int32_t operand_size() const {
     switch (spra) {
     case 0:
     case 1:

src/odr/internal/oldms/text/doc_style.cpp

@@ -2,10 +2,12 @@
 
 #include <odr/internal/oldms/text/doc_io.hpp>
 #include <odr/internal/util/byte_stream_util.hpp>
+#include <odr/internal/util/byte_util.hpp>
 #include <odr/internal/util/string_util.hpp>
 
 #include <array>
 #include <bit>
+#include <cstdint>
 #include <cstring>
 #include <istream>
 #include <stdexcept>
@@ -46,31 +48,22 @@ bool toggle_on(const std::uint8_t value) {
   }
 }
 
-std::uint16_t read_u16(const std::string_view bytes, const std::size_t at) {
-  std::uint16_t value;
-  std::memcpy(&value, bytes.data() + at, sizeof(value));
-  return value;
-}
-
 } // namespace
 
-void StyleRegistry::set_font_names(std::vector<std::string> names) {
-  m_font_names = std::move(names);
-}
-
-std::span<const std::string> StyleRegistry::font_names() const {
-  return m_font_names;
-}
-
-std::uint32_t StyleRegistry::add_style(TextStyle style) {
-  m_styles.push_back(std::move(style));
-  return static_cast<std::uint32_t>(m_styles.size() - 1);
-}
+StyleRegistry::StyleRegistry(std::vector<std::string> font_names,
+                             std::vector<TextStyle> styles)
+    : m_font_names(std::move(font_names)), m_styles(std::move(styles)) {}
 
 const TextStyle &StyleRegistry::text_style(const std::uint32_t index) const {
   return m_styles.at(index);
 }
 
+TextStyle default_character_style() {
+  TextStyle style;
+  style.font_size = Measure(10.0, DynamicUnit("pt"));
+  return style;
+}
+
 } // namespace odr::internal::oldms::text
 
 namespace odr::internal::oldms {
@@ -86,7 +79,7 @@ text::apply_character_sprms(TextStyle style, const std::string_view grpprl,
     at += sizeof(sprm);
 
     std::size_t operand_size;
-    if (const int fixed_size = sprm.operand_size(); fixed_size >= 0) {
+    if (const std::int32_t fixed_size = sprm.operand_size(); fixed_size >= 0) {
       operand_size = static_cast<std::size_t>(fixed_size);
     } else {
       // spra == 6: the first operand byte is the size of the remainder. The
@@ -132,7 +125,8 @@ text::apply_character_sprms(TextStyle style, const std::string_view grpprl,
       style.font_color = ico_color(static_cast<std::uint8_t>(operand[0]));
       break;
     case sprmCHps: {
-      const std::uint16_t half_points = read_u16(operand, 0);
+      const auto half_points =
+          util::byte::from_little_endian<std::uint16_t>(operand);
       if (half_points < 2 || half_points > 3276) {
         throw std::runtime_error("doc: sprmCHps value out of range");
       }
@@ -141,7 +135,8 @@ text::apply_character_sprms(TextStyle style, const std::string_view grpprl,
     case sprmCRgFtc0: {
       // SttbfFfn index ([MS-DOC] 2.6.1); with 0 entries the value MUST be 0
       // and the (unmodelled) style-sheet default font applies — leave unset.
-      const auto ftc = static_cast<std::int16_t>(read_u16(operand, 0));
+      const auto ftc = static_cast<std::int16_t>(
+          util::byte::from_little_endian<std::uint16_t>(operand));
       if (ftc < 0 || (font_names.empty() ? ftc != 0
                                          : static_cast<std::size_t>(ftc) >=
                                                font_names.size())) {

src/odr/internal/oldms/text/doc_style.hpp

@@ -15,26 +15,31 @@ namespace odr::internal::oldms::text {
 
 /// Owns the document's resolved character styles — indexed by the style index
 /// stored on paragraph/span elements, 0 being the default style — and the
-/// font names `TextStyle::font_name` points into.
+/// font names `TextStyle::font_name` points into. Immutable after
+/// construction.
 class StyleRegistry final {
 public:
-  /// Stores the font table (SttbfFfn names); must be set before any style
-  /// referencing a font is added.
-  void set_font_names(std::vector<std::string> names);
-  [[nodiscard]] std::span<const std::string> font_names() const;
+  StyleRegistry() = default;
+  /// `font_names` are the SttbfFfn names the styles' `font_name` point into;
+  /// `styles` are the resolved character styles, index 0 the default style.
+  StyleRegistry(std::vector<std::string> font_names,
+                std::vector<TextStyle> styles);
 
-  std::uint32_t add_style(TextStyle style);
   /// Throws if the index has no style.
   [[nodiscard]] const TextStyle &text_style(std::uint32_t index) const;
 
 private:
   /// Owns the font names: `TextStyle::font_name` (`const char *`) points into
-  /// them. Never modified after `set_font_names` (moving the registry is fine
-  /// — the strings themselves do not move).
+  /// them. Never modified after construction (moving the registry is fine —
+  /// the strings themselves do not move).
   std::vector<std::string> m_font_names;
   std::vector<TextStyle> m_styles;
 };
 
+/// The base character style Chpx grpprls apply on top of; without sprmCHps
+/// the font size defaults to 20 half-points ([MS-DOC] 2.6.1 sprmCHps).
+[[nodiscard]] TextStyle default_character_style();
+
 /// Applies the character SPRMs of a Chpx grpprl ([MS-DOC] 2.6.1) on top of
 /// `style`; non-character SPRMs are skipped via their operand size.
 /// `font_names` resolves sprmCRgFtc0 (the strings must outlive the style —