refactor: make reader invariants and fallbacks explicit

src/odr/document.cpp

@@ -626,6 +626,7 @@ void Document::recalculate_edits_() const {
   try {
     recalculate();
   } catch (const UnsupportedOperation &) {
+    return;
   }
 }
 

src/odr/document_element.hpp

@@ -25,7 +25,6 @@ struct PageLayout;
 } // namespace odr
 
 namespace odr::internal::abstract {
-class Document;
 
 class ElementAdapter;
 class TextRootAdapter;

src/odr/internal/cfb/cfb_util.cpp

@@ -190,11 +190,7 @@ void Archive::Iterator::enter_(Entry entry) {
 }
 
 void Archive::Iterator::dig_left_() {
-  if (!m_entry.has_value()) {
-    return;
-  }
-
-  while (true) {
+  while (m_entry.has_value()) {
     const std::optional<Entry> left = m_entry->left();
     if (!left.has_value()) {
       break;

src/odr/internal/common/sheet_recalculation.cpp

@@ -355,6 +355,7 @@ internal::recalculate(const abstract::Document &document) {
                                 position.cell.row,
                                 CellValue(ValueType::unknown));
       } catch (const UnsupportedOperation &) {
+        continue;
       }
       continue;
     }

src/odr/internal/html/filesystem.cpp

@@ -65,7 +65,7 @@ std::optional<std::string> entry_data_url(const File &file,
 /// a path that escapes the output directory or collides with the listing gets
 /// none, and stays inline instead.
 std::optional<RelPath> entry_location(const Path &path) {
-  const RelPath relative = path.make_relative();
+  RelPath relative = path.make_relative();
   if (relative.escaping() || relative.empty() ||
       relative.string() == listing_path) {
     return std::nullopt;

src/odr/internal/odf/odf_sheet_references.cpp

@@ -92,7 +92,7 @@ constexpr std::array<std::string_view, 3> condition_attributes{
 
 /// The attributes stating the cell a condition is relative to, whose sheet
 /// is the one a reference of the condition names by stating none.
-constexpr std::array<std::string_view, 3> base_cell_attributes{
+constexpr std::array<const char *, 3> base_cell_attributes{
     "table:base-cell-address", "calcext:base-cell-address",
     "style:base-cell-address"};
 
@@ -165,8 +165,8 @@ void move_subtree(const pugi::xml_node node, std::string sheet,
   // Where a delete removes that cell, it reads from the first one that stays.
   std::string own = sheet;
   std::int64_t shift = 0;
-  for (const std::string_view base : base_cell_attributes) {
-    pugi::xml_attribute address = node.attribute(base.data());
+  for (const char *const base : base_cell_attributes) {
+    pugi::xml_attribute address = node.attribute(base);
     std::optional<formula::Node> cell =
         address ? formula::parse("[" + std::string(address.value()) + "]",
                                  formula::Syntax::opendocument)

src/odr/internal/ooxml/ooxml_util.cpp

@@ -583,6 +583,7 @@ std::vector<AbsPath> ooxml::parse_relationship_targets(
           resolve_part_path(path, e.node().attribute("Target").value()));
     } catch (const std::invalid_argument &) {
       // An invalid optional relationship contributes no part.
+      continue;
     }
   }
   return result;

src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_references.cpp

@@ -245,9 +245,10 @@ void move_shared_group(const std::vector<Member> &members,
         shifted(*expression, master->position, member.position);
     formula::move_references(expected, edit, sheet);
     std::string text = formula::to_string(expected, syntax);
-    kept = kept &&
-           formula::to_string(shifted(moved_master, *master_at, *member_at),
-                              syntax) == text;
+    if (kept && master_at.has_value()) {
+      kept = formula::to_string(shifted(moved_master, *master_at, *member_at),
+                                syntax) == text;
+    }
     written.emplace_back(member.formula, std::move(text));
   }
 
@@ -324,7 +325,7 @@ void move_calc_chain(pugi::xml_node calc_chain,
 /// An element stating a range list, which goes when a delete takes the list.
 struct RangeAttribute final {
   std::string_view element;
-  std::string_view attribute;
+  const char *attribute;
 };
 
 constexpr std::array<RangeAttribute, 8> removable_ranges{{
@@ -427,7 +428,7 @@ void collect_ranges(const pugi::xml_node node, const formula::SheetEdit &edit,
     }
     bool removed = false;
     for (const RangeAttribute &range : removable_ranges) {
-      pugi::xml_attribute attribute = child.attribute(range.attribute.data());
+      pugi::xml_attribute attribute = child.attribute(range.attribute);
       if (name != range.element || !attribute) {
         continue;
       }

src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_style.cpp

@@ -482,6 +482,7 @@ void StyleRegistry::generate_indices_(const pugi::xml_node styles_root) {
           number_format::Format(format.attribute("formatCode").value()));
     } catch (const std::invalid_argument &) {
       // a code this cannot read shows its value as `General` does
+      continue;
     }
   }
 }

src/odr/internal/pdf/pdf_page_extractor.cpp

@@ -842,7 +842,7 @@ void show_type3(std::vector<PageElement> &out, const Resources &resources,
                 GraphicsState &state, const Logger &logger,
                 std::set<std::string> &warned, ActiveForms &active,
                 MarkedContentStack &marked, std::optional<Pen> &pen,
-                const std::string &codes, Font *font) {
+                const std::string &codes, Font *font, const Type3Data &type3) {
   // `show` advances the text matrix by the whole segment; snapshot `Tm` so the
   // per-glyph loop below can replay the advance to place each char proc.
   const util::math::Transform2D saved_matrix = state.current().text.matrix;
@@ -857,7 +857,7 @@ void show_type3(std::vector<PageElement> &out, const Resources &resources,
     return;
   }
 
-  if (font->type3->char_procs.empty() || state.type3_depth > 8) {
+  if (type3.char_procs.empty() || state.type3_depth > 8) {
     return; // nothing to draw, or pathological Type3 recursion
   }
   ++state.type3_depth;
@@ -874,10 +874,9 @@ void show_type3(std::vector<PageElement> &out, const Resources &resources,
         font->encoding
             ? font->encoding->glyph_name(static_cast<std::uint8_t>(code))
             : std::string_view{};
-    const auto it = name.empty()
-                        ? font->type3->char_procs.end()
-                        : font->type3->char_procs.find(std::string(name));
-    if (it != font->type3->char_procs.end()) {
+    const auto it = name.empty() ? type3.char_procs.end()
+                                 : type3.char_procs.find(std::string(name));
+    if (it != type3.char_procs.end()) {
       const auto &current = state.current();
       const GraphicsState::Text &text = current.text;
       // glyph space -> user space = /FontMatrix x [size params] x Tm x CTM.
@@ -886,11 +885,10 @@ void show_type3(std::vector<PageElement> &out, const Resources &resources,
               text.size * text.horizontal_scaling / 100.0, text.size, 0,
               text.rise);
       const util::math::Transform2D glyph_to_user =
-          font->type3->font_matrix * size_params * text.matrix *
+          type3.font_matrix * size_params * text.matrix *
           current.general.transform_matrix;
-      const Resources &scope = font->type3->resources != nullptr
-                                   ? *font->type3->resources
-                                   : resources;
+      const Resources &scope =
+          type3.resources != nullptr ? *type3.resources : resources;
 
       // Scoped like `q`/`Q`, but pinned: the char proc's own `q`/`Q` cannot
       // escape it and disturb the text state driving the glyph loop.
@@ -919,7 +917,7 @@ void run_content(const std::string &content, const Resources &resources,
   const auto show_run = [&](const std::string &codes, Font *font) {
     if (font != nullptr && font->type3) {
       show_type3(out, resources, state, logger, warned, active, marked, pen,
-                 codes, font);
+                 codes, font, *font->type3);
     } else {
       show(out, state, marked, pen, codes, font);
     }

src/odr/internal/png/png_util.cpp

@@ -60,7 +60,7 @@ std::string assemble(const std::string &rows, const std::size_t stride,
   const std::string compressed = crypto::util::zlib_deflate(raw);
   std::string_view remaining = compressed;
   // PNG 5.3 limits chunk lengths to 2^31-1; small chunks avoid narrowing.
-  constexpr std::size_t chunk_size = 64 * 1024;
+  constexpr std::size_t chunk_size = std::size_t{64} * 1024;
   while (!remaining.empty()) {
     const std::size_t count = std::min(remaining.size(), chunk_size);
     write_chunk(out, "IDAT", remaining.substr(0, count));

src/odr/style.hpp

@@ -125,7 +125,7 @@ template <typename T> struct DirectionalStyle final {
   std::optional<T> bottom;
 
   DirectionalStyle() = default;
-  explicit DirectionalStyle(std::optional<T> all)
+  explicit DirectionalStyle(const std::optional<T> &all)
       : right{all}, top{all}, left{all}, bottom{all} {}
 
   void override(const DirectionalStyle &other) {