Use Iterator::reverse() everywhere in subdoc

Replace off-by-one or overflow bug-prone math with .reverse(). If
the iterator is not a DoubleEndedIterator then with
  .collect_vec().into_iter().reverse().

Author: danakj

subdoc/lib/database.h

@@ -496,12 +496,10 @@ struct Database {
                      "non-namespace.";
                 return sus::result::err(sus::move(s).str());
               }
-              if (!target.as_mut<Target::Namespace>().namespaces.contains(
-                      id)) {
+              if (!target.as_mut<Target::Namespace>().namespaces.contains(id)) {
                 std::ostringstream s;
                 s << "Inherited comment at " << c->begin_loc
-                  << " can't find namespace "
-                  << e.as<InheritPathNamespace>();
+                  << " can't find namespace " << e.as<InheritPathNamespace>();
                 return sus::result::err(sus::move(s).str());
               }
               target = sus::choice<Target::Namespace>(
@@ -526,8 +524,7 @@ struct Database {
                       find_record(target.as_mut<Target::Namespace>().records);
                   break;
                 case Target::Record:
-                  record =
-                      find_record(target.as_mut<Target::Record>().records);
+                  record = find_record(target.as_mut<Target::Record>().records);
                   break;
                 case Target::Function: {
                   std::ostringstream s;
@@ -648,11 +645,11 @@ struct Database {
     auto ns_cursor = [&]() -> sus::Option<const NamespaceElement&> {
       const NamespaceElement* cursor = &global;
 
-      auto it = iter_namespace_path(decl);
-      // TODO: Make Iterator::reverse() and use that.
-      auto v = sus::move(it).collect_vec();
-      for (usize i = 1u; i < v.len(); i += 1u) {
-        const Namespace& n = v[v.len() - i - 1u];
+      // Namespace path goes from the outside in to the global, we want the
+      // inverse, and then to skip the global namespace.
+      auto it = iter_namespace_path(decl).collect_vec().into_iter().reverse();
+      it.next();  // TODO: Use Iterator::skip().
+      for (const Namespace& n : it) {
         switch (n) {
           case Namespace::Tag::Global: {
             // We skipped the 1st namespace in the path which is the global
@@ -685,13 +682,12 @@ struct Database {
               clang::dyn_cast<clang::RecordDecl>(decl->getDeclContext())) {
         const RecordElement* cursor = nullptr;
 
-        auto it = iter_record_path(containing_record_decl);
-        // TODO: Make Iterator::reverse() and use that.
-        auto v = sus::move(it).collect_vec();
-        for (usize i; i < v.len(); i += 1u) {
-          std::string_view name = v[v.len() - i - 1u];
-
-          if (i == 0u) {
+        bool first = true;  // TODO: Use Iterator::enumerate().
+        for (std::string_view name : iter_record_path(containing_record_decl)
+                                         .collect_vec()
+                                         .into_iter()
+                                         .reverse()) {
+          if (first) {
             auto r_it = ns_cursor->records.find(RecordId(name));
             if (r_it == ns_cursor->records.end()) {
               return sus::none();
@@ -704,6 +700,7 @@ struct Database {
             }
             cursor = &r_it->second;
           }
+          first = false;
         }
         return sus::some(*cursor);
       } else {

subdoc/lib/gen/files.h

@@ -45,9 +45,7 @@ inline std::filesystem::path construct_html_file_path(
   std::filesystem::path p = sus::move(root);
 
   std::ostringstream fname;
-  // TODO: Add Iterator::reverse() and use that.
-  for (size_t i = 0; i < namespace_path.len(); ++i) {
-    const Namespace& n = namespace_path[namespace_path.len() - i - 1u];
+  for (const Namespace& n : namespace_path.iter().reverse()) {
     switch (n) {
       case Namespace::Tag::Global: break;
       case Namespace::Tag::Anonymous:
@@ -60,10 +58,8 @@ inline std::filesystem::path construct_html_file_path(
         break;
     }
   }
-  // TODO: Add Iterator::reverse.
-  for (size_t i = 0; i < record_path.len(); ++i) {
-    const std::string& n = record_path[record_path.len() - i - 1u];
-    fname << n + "-";
+  for (std::string_view n : record_path.iter().reverse()) {
+    fname << n << "-";
   }
   fname << name;
   fname << ".html";

subdoc/lib/gen/generate_record.cc

@@ -50,10 +50,7 @@ void generate_record_overview(HtmlWriter::OpenDiv& record_div,
       {
         // TODO: This code gets duplicated a lot, share it.
 
-        // TODO: Add Iterator::reverse() and use that.
-        for (size_t i = 0; i < element.namespace_path.len(); ++i) {
-          const Namespace& n =
-              element.namespace_path[element.namespace_path.len() - i - 1u];
+        for (const Namespace &n : element.namespace_path.iter().reverse()) {
           switch (n) {
             case Namespace::Tag::Global: break;
             case Namespace::Tag::Anonymous: {
@@ -71,10 +68,7 @@ void generate_record_overview(HtmlWriter::OpenDiv& record_div,
               break;
           }
         }
-        // TODO: Add Iterator::reverse() and use that.
-        for (size_t i = 0; i < element.record_path.len(); ++i) {
-          const std::string& record_name =
-              element.record_path[element.record_path.len() - i - 1u];
+        for (std::string_view record_name: element.record_path.iter().reverse()) {
           {
             auto record_anchor = full_type_span.open_a();
             record_anchor.write_text(record_name);
@@ -234,10 +228,7 @@ void generate_record(const RecordElement& element,
 
   {
     std::ostringstream title;
-    // TODO: Add Iterator::reverse() and use that.
-    for (size_t i = 0; i < element.namespace_path.len(); ++i) {
-      const Namespace& n =
-          element.namespace_path[element.namespace_path.len() - i - 1u];
+    for (const Namespace& n: element.namespace_path.iter().reverse()) {
       switch (n) {
         case Namespace::Tag::Global: break;
         case Namespace::Tag::Anonymous:
@@ -250,10 +241,7 @@ void generate_record(const RecordElement& element,
           break;
       }
     }
-    // TODO: Add Iterator::reverse() and use that.
-    for (size_t i = 0; i < element.record_path.len(); ++i) {
-      const std::string& record_name =
-          element.record_path[element.record_path.len() - i - 1u];
+    for (std::string_view record_name: element.record_path.iter().reverse()) {
       title << record_name;
       title << "::";
     }

subdoc/lib/path.h

@@ -43,10 +43,8 @@ inline std::string namespace_with_path_to_string(
   std::ostringstream s;
   bool add_colons = false;
 
-  // TODO: Add Iterator::reverse() and use that here.
-  for (usize i = path.len(); i > 0u; i -= 1u) {
+  for (const Namespace& n : path.iter().reverse()) {
     if (add_colons) s << "::";
-    const Namespace& n = path[i - 1u];
     switch (n) {
       case Namespace::Tag::Global: break;
       case Namespace::Tag::Anonymous:
@@ -64,9 +62,7 @@ inline std::string namespace_with_path_to_string(
   switch (tail) {
     case Namespace::Tag::Global: s << "Global namespace"; break;
     case Namespace::Tag::Anonymous: s << "(anonymous)"; break;
-    case Namespace::Tag::Named:
-      s << tail.as<Namespace::Tag::Named>();
-      break;
+    case Namespace::Tag::Named: s << tail.as<Namespace::Tag::Named>(); break;
   }
 
   return s.str();