From e25ca2c063fdde8e0871ba749a4bd0e084a1459c Mon Sep 17 00:00:00 2001
From: reiniscirpons <rc234@st-andrews.ac.uk>
Date: Fri, 6 Dec 2024 13:20:48 +0000
Subject: [PATCH] Minor changes, more todos

---
 include/libsemigroups/sims.hpp | 14 +++-----------
 src/sims.cpp                   |  9 +++++++--
 tests/test-sims.cpp            | 27 +--------------------------
 3 files changed, 11 insertions(+), 39 deletions(-)

diff --git a/include/libsemigroups/sims.hpp b/include/libsemigroups/sims.hpp
index 3627e8496..c383588af 100644
--- a/include/libsemigroups/sims.hpp
+++ b/include/libsemigroups/sims.hpp
@@ -22,7 +22,6 @@
 
 // TODO(0):
 // * review the function aliases, and remove them if they are unnecessary
-// * doc
 // * const
 // * noexcept
 // * nodiscard
@@ -514,8 +513,7 @@ namespace libsemigroups {
     //! \throws LibsemigroupsException if `p` is not valid.
     //!
     //! \throws LibsemigroupsException if the alphabet of `p` is non-empty and
-    //! not equal to that of \ref long_rules or \ref presentation.
-    // TODO(1) review the previous exception
+    //! not compatible with \ref include or \ref exclude.
     //!
     //! \throws LibsemigroupsException if `p` has 0-generators and 0-relations.
     template <typename Word>
@@ -870,8 +868,6 @@ namespace libsemigroups {
     //! \warning
     //! This function replaces all previously set `exclude` pairs with
     //! those found in `[first, last)`.
-    // TODO(1) maybe should add instead of replacing similar to exclude of r
-    // pair? Replaces current exclude with [first, last)
     template <typename Iterator>
     Subclass& exclude(Iterator first, Iterator last) {
       add_exclude_pruner();
@@ -1065,10 +1061,10 @@ namespace libsemigroups {
 
        protected:
         using PendingDef = typename Sims1or2::PendingDef;
-        // TODO(1) (Sims1) ensure that _felsch_graph's settings are
-        // properly initialised
         using Definition = std::pair<node_type, label_type>;
 
+        // TODO(1) (Sims1) ensure that _felsch_graph's settings are
+        // properly initialised
         FelschGraph<word_type, node_type, std::vector<Definition>>
             _felsch_graph;
 
@@ -2177,9 +2173,6 @@ namespace libsemigroups {
                                                 WordGraph<Node> const&);
 
      public:
-      // TODO(0) add noexcept?
-      // NOTE(RC): Not sure if noexcept, initializing the _tree field of type
-      // Forest allocates memory
       //! Default constructor
       const_rcgp_iterator() = default;
       //! Default copy constructor.
@@ -2893,7 +2886,6 @@ namespace libsemigroups {
     //! \warning
     //! This method does not verify if \p forbid contains trivial pairs or not.
     // TODO(2): Construct from std::vector<std::string>
-    // TODO(2): Template constructor on typename Word
     explicit SimsRefinerFaithful(std::vector<word_type> const& forbid)
         : _forbid(forbid) {}
 
diff --git a/src/sims.cpp b/src/sims.cpp
index d5470c081..abf4194bf 100644
--- a/src/sims.cpp
+++ b/src/sims.cpp
@@ -931,8 +931,13 @@ namespace libsemigroups {
                           that._pending.cend());
     }
 
-    // TODO(0) expand to longer comment about thread sanitizer warnings and how
-    // we can gleefully ignore them
+    // NOTE: (reiniscirpons)
+    // When running the thread sanitizer we get the error
+    // ThreadSanitizer: lock-order-inversion (potential deadlock)
+    // This can be ignored here, despite the lock order inversion, since we only
+    // every try to steal when we are empty from a non-empty thread, it cannot
+    // be the case that two threads try to steal from each other simultaneously.
+    // In other words the thread sanitizer is overly cautious here.
     template <typename Sims1or2>
     bool SimsBase<Sims1or2>::thread_iterator::try_steal(thread_iterator& that) {
       std::lock_guard<std::mutex> lock(iterator_base::_mtx);
diff --git a/tests/test-sims.cpp b/tests/test-sims.cpp
index 0ecc4c874..90ed91bff 100644
--- a/tests/test-sims.cpp
+++ b/tests/test-sims.cpp
@@ -2348,7 +2348,7 @@ namespace libsemigroups {
                           "[extreme][low-index][plactic]") {
     std::array<uint64_t, 8> const num
         = {0, 1, 67, 2'794, 106'264, 4'795'980, 278'253'841, 20'855'970'290};
-    // Last value too 1h34m to compute so is not included.
+    // Last value took 1h34m to compute so is not included.
     auto rg = ReportGuard(true);
     auto p  = plactic_monoid(4);
     p.contains_empty_word(false);
@@ -4243,9 +4243,6 @@ namespace libsemigroups {
                           "119",
                           "2-sided ideals Jura's example",
                           "[quick][sims1][no-valgrind]") {
-    // TODO(0) change category back to quick, these fail
-    // currently because of changes to the api in
-    // ToddCoxeter
     Presentation<std::string> p;
     p.alphabet("ab");
     p.contains_empty_word(false);
@@ -4405,9 +4402,6 @@ namespace libsemigroups {
                           "123",
                           "Adding and removing pruners",
                           "[quick][low-index]") {
-    // TODO(0) change category back to quick, these fail
-    // currently because of changes to the api in
-    // ToddCoxeter
     auto                      rg = ReportGuard(true);
     Presentation<std::string> p;
     p.alphabet("ab");
@@ -5185,22 +5179,3 @@ namespace libsemigroups {
   }
 
 }  // namespace libsemigroups
-
-// [[[0, 0, 0]],            #1#
-// [[0, 0, 1], [1, 0, 1]],  #2#
-// [[0, 1, 0], [1, 1, 0]],
-// [[0, 1, 1], [1, 1, 1]]]  #3#
-
-// [[[0, 0, 0]],                        #1#
-//   [[1, 0, 1], [1, 1, 1]],
-//   [[1, 0, 2], [1, 1, 1], [1, 2, 1]],
-//   [[1, 1, 1], [1, 1, 1]],            #2#
-//   [[1, 1, 2], [1, 1, 1], [1, 1, 1]], #4#
-//   [[1, 0, 1], [2, 1, 2], [2, 2, 2]],
-//   [[1, 0, 2], [2, 1, 2], [2, 2, 2]],
-//   [[1, 1, 2], [2, 1, 2], [2, 2, 2]], #3#
-//   [[1, 2, 2], [2, 1, 2], [2, 2, 2]],
-//   [[1, 0, 2], [2, 2, 2], [2, 2, 2]],
-//   [[1, 2, 1], [2, 2, 2], [2, 2, 2]],
-//   [[1, 2, 2], [2, 2, 2], [2, 2, 2]],
-//   [[1, 2, 1], [1, 1, 1], [1, 2, 1]]]