Refactor constraint handling: replace raw pointers with std::optional<std::reference_wrapper<constraint>> for improved safety and consistency in method signatures.

Author: riccardodebenedictis

include/linspire.hpp

@@ -126,7 +126,7 @@ namespace linspire
      * @param reason An optional constraint that serves as the reason for adding this new constraint.
      * @return true if the constraint was successfully added; false if it leads to inconsistency.
      */
-    [[nodiscard]] bool new_lt(const utils::lin &lhs, const utils::lin &rhs, bool strict = false, constraint *reason = nullptr) noexcept;
+    [[nodiscard]] bool new_lt(const utils::lin &lhs, const utils::lin &rhs, bool strict = false, std::optional<std::reference_wrapper<constraint>> reason = std::nullopt) noexcept;
     /**
      * @brief Adds a new equality constraint to the solver.
      *
@@ -138,7 +138,7 @@ namespace linspire
      * @param reason An optional constraint that serves as the reason for adding this new equality constraint.
      * @return true if the equality constraint was successfully added; false if it leads to inconsistency.
      */
-    [[nodiscard]] bool new_eq(const utils::lin &lhs, const utils::lin &rhs, constraint *reason = nullptr) noexcept;
+    [[nodiscard]] bool new_eq(const utils::lin &lhs, const utils::lin &rhs, std::optional<std::reference_wrapper<constraint>> reason = std::nullopt) noexcept;
     /**
      * @brief Adds a new greater-than or greater-than-or-equal-to constraint to the solver.
      *
@@ -152,7 +152,7 @@ namespace linspire
      * @param reason An optional constraint that serves as the reason for adding this new constraint.
      * @return true if the constraint was successfully added; false if it leads to inconsistency.
      */
-    [[nodiscard]] bool new_gt(const utils::lin &lhs, const utils::lin &rhs, bool strict = false, constraint *reason = nullptr) noexcept { return new_lt(rhs, lhs, strict, reason); }
+    [[nodiscard]] bool new_gt(const utils::lin &lhs, const utils::lin &rhs, bool strict = false, std::optional<std::reference_wrapper<constraint>> reason = std::nullopt) noexcept;
 
     /**
      * @brief Retracts a previously added constraint from the solver.
@@ -185,7 +185,7 @@ namespace linspire
      *
      * @return A constant reference to the vector of constraints representing the last conflict explanation.
      */
-    [[nodiscard]] const std::vector<constraint *> &get_conflict() const noexcept { return cnfl; }
+    [[nodiscard]] const std::vector<std::reference_wrapper<constraint>> &get_conflict() const noexcept { return cnfl; }
 
     /**
      * @brief Checks if two linear expressions can be made equal.
@@ -226,8 +226,8 @@ namespace linspire
   private:
     [[nodiscard]] bool is_basic(const utils::var v) const noexcept { return tableau.count(v); }
 
-    [[nodiscard]] bool set_lb(const utils::var x_i, const utils::inf_rational &v, constraint *reason = nullptr) noexcept;
-    [[nodiscard]] bool set_ub(const utils::var x_i, const utils::inf_rational &v, constraint *reason = nullptr) noexcept;
+    [[nodiscard]] bool set_lb(const utils::var x_i, const utils::inf_rational &v, std::optional<std::reference_wrapper<constraint>> reason = std::nullopt) noexcept;
+    [[nodiscard]] bool set_ub(const utils::var x_i, const utils::inf_rational &v, std::optional<std::reference_wrapper<constraint>> reason = std::nullopt) noexcept;
 
     void update(const utils::var x_i, const utils::inf_rational &v) noexcept;
 
@@ -237,11 +237,11 @@ namespace linspire
 
     void new_row(const utils::var x, utils::lin &&l) noexcept;
 
-    std::vector<var> vars;                             // index is the variable id
-    std::unordered_map<std::string, utils::var> exprs; // the expressions (string to numeric variable) for which already exist slack variables..
-    std::map<utils::var, utils::lin> tableau;          // basic variable -> expression
-    std::vector<std::set<utils::var>> t_watches;       // for each variable `v`, a set of tableau rows watching `v`..
-    std::vector<constraint *> cnfl;                    // the last conflict explanation..
+    std::vector<var> vars;                                // index is the variable id
+    std::unordered_map<std::string, utils::var> exprs;    // the expressions (string to numeric variable) for which already exist slack variables..
+    std::map<utils::var, utils::lin> tableau;             // basic variable -> expression
+    std::vector<std::set<utils::var>> t_watches;          // for each variable `v`, a set of tableau rows watching `v`..
+    std::vector<std::reference_wrapper<constraint>> cnfl; // the last conflict explanation..
 #ifdef LINSPIRE_ENABLE_LISTENERS
     std::unordered_map<utils::var, std::set<listener *>> listening; // for each variable, the listeners listening to it..
     std::set<listener *> listeners;                                 // the collection of listeners..

include/var.hpp

@@ -5,6 +5,7 @@
 #include "json.hpp"
 #include <set>
 #include <optional>
+#include <functional>
 
 namespace linspire
 {
@@ -25,9 +26,9 @@ namespace linspire
     friend json::json to_json(const var &x) noexcept;
 
   private:
-    void set_lb(const utils::inf_rational &v, constraint *reason = nullptr) noexcept;
+    void set_lb(const utils::inf_rational &v, std::optional<std::reference_wrapper<constraint>> reason = std::nullopt) noexcept;
     void unset_lb(const utils::inf_rational &v, constraint &reason) noexcept;
-    void set_ub(const utils::inf_rational &v, constraint *reason = nullptr) noexcept;
+    void set_ub(const utils::inf_rational &v, std::optional<std::reference_wrapper<constraint>> reason = std::nullopt) noexcept;
     void unset_ub(const utils::inf_rational &v, constraint &reason) noexcept;
 
   private:

src/linspire.cpp

@@ -41,7 +41,7 @@ namespace linspire
     utils::inf_rational solver::ub(const utils::var x) const noexcept { return vars[x].get_ub(); }
     utils::inf_rational solver::val(const utils::var x) const noexcept { return vars[x].val; }
 
-    bool solver::new_eq(const utils::lin &lhs, const utils::lin &rhs, constraint *reason) noexcept
+    bool solver::new_eq(const utils::lin &lhs, const utils::lin &rhs, std::optional<std::reference_wrapper<constraint>> reason) noexcept
     {
         LOG_TRACE(utils::to_string(lhs) + " == " + utils::to_string(rhs));
         utils::lin expr = lhs - rhs;
@@ -68,18 +68,17 @@ namespace linspire
             assert(c != 0);
             const utils::inf_rational c_right = utils::inf_rational(-expr.known_term) / c; // the right-hand side of the constraint is the division of the negation of the known term by the coefficient..
             // we can set both the lower and upper bound of the variable to the right-hand side of the constraint..
-            return set_lb(x, c_right, reason) && set_ub(x, c_right, reason);
+            return reason ? set_lb(x, c_right, reason.value()) && set_ub(x, c_right, reason.value()) : set_lb(x, c_right) && set_ub(x, c_right);
         }
         default: // the expression is still a general linear expression..
             const utils::inf_rational c_right = utils::inf_rational(-expr.known_term);
             expr.known_term = utils::rational::zero;
             // we add the expression to the tableau, associating it with a new (slack) variable
             utils::var slack = new_var(std::move(expr));
-            return set_lb(slack, c_right, reason) && set_ub(slack, c_right, reason);
+            return reason ? set_lb(slack, c_right, reason.value()) && set_ub(slack, c_right, reason.value()) : set_lb(slack, c_right) && set_ub(slack, c_right);
         }
     }
-
-    bool solver::new_lt(const utils::lin &lhs, const utils::lin &rhs, bool strict, constraint *reason) noexcept
+    bool solver::new_lt(const utils::lin &lhs, const utils::lin &rhs, bool strict, std::optional<std::reference_wrapper<constraint>> reason) noexcept
     {
         LOG_TRACE(utils::to_string(lhs) + (strict ? " < " : " <= ") + utils::to_string(rhs));
         utils::lin expr = lhs - rhs;
@@ -106,18 +105,25 @@ namespace linspire
             assert(c != 0);
             const utils::inf_rational c_right = utils::inf_rational(-expr.known_term, strict ? -1 : 0) / c; // the right-hand side of the constraint is the division of the negation of the known term minus an infinitesimal by the coefficient..
             if (is_positive(c))
-                return set_ub(x, c_right, reason); // we are in the case `c * v < c_right`..
+                return reason ? set_ub(x, c_right, reason.value()) : set_ub(x, c_right); // we are in the case `c * v < c_right`..
             else
-                return set_lb(x, c_right, reason); // we are in the case `c * v > c_right`..
+                return reason ? set_lb(x, c_right, reason.value()) : set_lb(x, c_right); // we are in the case `c * v > c_right`..
         }
         default: // the expression is still a general linear expression..
             const utils::inf_rational c_right = utils::inf_rational(-expr.known_term, strict ? -1 : 0);
             expr.known_term = utils::rational::zero;
             // we add the expression to the tableau, associating it with a new (slack) variable
             utils::var slack = new_var(std::move(expr));
-            return set_ub(slack, c_right, reason); // we are in the case `expr < c_right`..
+            return reason ? set_ub(slack, c_right, reason.value()) : set_ub(slack, c_right); // we are in the case `expr < c_right`..
         }
     }
+    bool solver::new_gt(const utils::lin &lhs, const utils::lin &rhs, bool strict, std::optional<std::reference_wrapper<constraint>> reason) noexcept
+    {
+        if (reason)
+            return new_lt(rhs, lhs, strict, *reason);
+        else
+            return new_lt(rhs, lhs, strict);
+    }
 
     void solver::retract(constraint &c) noexcept
     {
@@ -152,12 +158,12 @@ namespace linspire
                     for (const auto &[v, c] : l.vars)
                         if (is_positive(c)) // we use the most restrictive upper bound of v
                             for (const auto &w : vars.at(v).ubs.begin()->second)
-                                cnfl.push_back(w);
+                                cnfl.push_back(*w);
                         else if (is_negative(c)) // we use the most restrictive lower bound of v
                             for (const auto &w : vars.at(v).lbs.rbegin()->second)
-                                cnfl.push_back(w);
+                                cnfl.push_back(*w);
                     for (const auto &w : vars.at(x_i).lbs.rbegin()->second) // we use the most restrictive lower bound of x_i
-                        cnfl.push_back(w);
+                        cnfl.push_back(*w);
                     return false;
                 }
             }
@@ -173,12 +179,12 @@ namespace linspire
                     for (const auto &[v, c] : l.vars)
                         if (is_positive(c)) // we use the most restrictive lower bound of v
                             for (const auto &w : vars.at(v).lbs.rbegin()->second)
-                                cnfl.push_back(w);
+                                cnfl.push_back(*w);
                         else if (is_negative(c)) // we use the most restrictive upper bound of v
                             for (const auto &w : vars.at(v).ubs.begin()->second)
-                                cnfl.push_back(w);
+                                cnfl.push_back(*w);
                     for (const auto &w : vars.at(x_i).ubs.begin()->second) // we use the most restrictive upper bound of x_i
-                        cnfl.push_back(w);
+                        cnfl.push_back(*w);
                     return false;
                 }
             }
@@ -187,7 +193,7 @@ namespace linspire
 
     bool solver::match(const utils::lin &l0, const utils::lin &l1) const noexcept { return lb(l0) <= ub(l1) && ub(l0) >= lb(l1); }
 
-    bool solver::set_lb(const utils::var x, const utils::inf_rational &v, constraint *reason) noexcept
+    bool solver::set_lb(const utils::var x, const utils::inf_rational &v, std::optional<std::reference_wrapper<constraint>> reason) noexcept
     {
         assert(x < vars.size());
         assert(v > utils::rational::negative_infinite);
@@ -196,14 +202,14 @@ namespace linspire
         { // inconsistent bound..
             cnfl.clear();
             if (reason)
-                cnfl.push_back(reason);
+                cnfl.push_back(reason.value());
             for (const auto &w : vars.at(x).ubs.begin()->second) // we use the most restrictive upper bound of x
-                cnfl.push_back(w);
+                cnfl.push_back(*w);
             return false;
         }
         if (reason)
         { // we have a reason for this bound..
-            if (auto it = reason->lbs.find(x); it != reason->lbs.end())
+            if (auto it = reason->get().lbs.find(x); it != reason->get().lbs.end())
             { // we already have a lower bound for this variable in the reason..
                 if (it->second < v)
                 { // we update the lower bound only if the new one is more restrictive..
@@ -212,14 +218,14 @@ namespace linspire
                 }
             }
             else
-                reason->lbs.emplace(x, v);
+                reason->get().lbs.emplace(x, v);
         }
         vars.at(x).set_lb(v, reason);
         if (val(x) < v && !is_basic(x))
             update(x, v);
         return true;
     }
-    bool solver::set_ub(const utils::var x, const utils::inf_rational &v, constraint *reason) noexcept
+    bool solver::set_ub(const utils::var x, const utils::inf_rational &v, std::optional<std::reference_wrapper<constraint>> reason) noexcept
     {
         assert(x < vars.size());
         assert(v < utils::rational::positive_infinite);
@@ -228,14 +234,14 @@ namespace linspire
         { // inconsistent bound..
             cnfl.clear();
             if (reason)
-                cnfl.push_back(reason);
+                cnfl.push_back(reason.value());
             for (const auto &w : vars.at(x).lbs.rbegin()->second) // we use the most restrictive lower bound of x
-                cnfl.push_back(w);
+                cnfl.push_back(*w);
             return false;
         }
         if (reason)
         { // we have a reason for this bound..
-            if (auto it = reason->ubs.find(x); it != reason->ubs.end())
+            if (auto it = reason->get().ubs.find(x); it != reason->get().ubs.end())
             { // we already have an upper bound for this variable in the reason..
                 if (it->second > v)
                 { // we update the upper bound only if the new one is more restrictive..
@@ -244,7 +250,7 @@ namespace linspire
                 }
             }
             else
-                reason->ubs.emplace(x, v);
+                reason->get().ubs.emplace(x, v);
         }
         vars.at(x).set_ub(v, reason);
         if (val(x) > v && !is_basic(x))

src/var.cpp

@@ -9,15 +9,15 @@ namespace linspire
     utils::inf_rational var::get_lb() const noexcept { return lbs.empty() ? utils::rational::negative_infinite : lbs.rbegin()->first; }
     utils::inf_rational var::get_ub() const noexcept { return ubs.empty() ? utils::rational::positive_infinite : ubs.begin()->first; }
 
-    void var::set_lb(const utils::inf_rational &v, constraint *reason) noexcept
+    void var::set_lb(const utils::inf_rational &v, std::optional<std::reference_wrapper<constraint>> reason) noexcept
     {
         assert(v <= get_ub()); // we cannot set a lower bound greater than the current upper bound..
         if (reason)
         { // we add a new lower bound `v` with the given reason..
             if (auto it = lbs.find(v); it != lbs.end())
-                it->second.insert(reason); // we add the reason to the existing lower bound `v`..
+                it->second.insert(&reason->get()); // we add the reason to the existing lower bound `v`..
             else
-                lbs.emplace(v, std::set<constraint *>{reason}); // we create a new lower bound `v`..
+                lbs.emplace(v, std::set<constraint *>{&reason->get()}); // we create a new lower bound `v`..
         }
         else
         { // we remove all the lower bounds that are less than `v`..
@@ -35,15 +35,15 @@ namespace linspire
             lbs.erase(it);
     }
 
-    void var::set_ub(const utils::inf_rational &v, constraint *reason) noexcept
+    void var::set_ub(const utils::inf_rational &v, std::optional<std::reference_wrapper<constraint>> reason) noexcept
     {
         assert(v >= get_lb()); // we cannot set an upper bound less than the current lower bound..
         if (reason)
         { // we add a new upper bound `v` with the given reason..
             if (auto it = ubs.find(v); it != ubs.end())
-                it->second.insert(reason); // we add the reason to the existing upper bound `v`..
+                it->second.insert(&reason->get()); // we add the reason to the existing upper bound `v`..
             else
-                ubs.emplace(v, std::set<constraint *>{reason}); // we create a new upper bound `v`..
+                ubs.emplace(v, std::set<constraint *>{&reason->get()}); // we create a new upper bound `v`..
         }
         else
         { // we remove all the upper bounds that are greater than `v`..

tests/test_linspire.cpp

@@ -86,10 +86,10 @@ void test2()
     linspire::constraint c0;
 
     // x >= 0
-    bool res0 = s.new_gt({{x, 1}}, 0, false, &c0);
+    bool res0 = s.new_gt({{x, 1}}, 0, false, c0);
     assert(res0);
     // x >= 1 (we add this constraint with the same reason as the previous one)
-    bool res1 = s.new_gt({{x, 1}}, 1, false, &c0);
+    bool res1 = s.new_gt({{x, 1}}, 1, false, c0);
     assert(res1);
 
     auto cons = s.check();
@@ -121,10 +121,10 @@ void test3()
     linspire::constraint c1;
 
     // y >= x + 1
-    bool res0 = s.new_gt({{y, 1}, {x, -1}}, 1, false, &c0);
+    bool res0 = s.new_gt({{y, 1}, {x, -1}}, 1, false, c0);
     assert(res0);
     // z >= y + 1
-    bool res1 = s.new_gt({{z, 1}, {y, -1}}, 1, false, &c1);
+    bool res1 = s.new_gt({{z, 1}, {y, -1}}, 1, false, c1);
     assert(res1);
     bool cons = s.check();
     assert(cons);
@@ -161,22 +161,22 @@ void test4()
     linspire::constraint c2;
 
     // x + y >= 1
-    bool res0 = s.new_gt({{x, 1}, {y, 1}}, 1, false, &c0);
+    bool res0 = s.new_gt({{x, 1}, {y, 1}}, 1, false, c0);
     assert(res0);
     // x >= 2
-    bool res1 = s.new_gt({{x, 1}}, 2, false, &c1);
+    bool res1 = s.new_gt({{x, 1}}, 2, false, c1);
     assert(res1);
     bool cons = s.check();
     assert(cons);
 
     // x + y <= 0
-    bool res2 = s.new_lt({{x, 1}, {y, 1}}, 0, false, &c2);
+    bool res2 = s.new_lt({{x, 1}, {y, 1}}, 0, false, c2);
     assert(res2);
     cons = s.check();
     assert(!cons);
     auto expl = s.get_conflict();
     assert(expl.size() == 2);
-    assert((expl[0] == &c0 && expl[1] == &c2) || (expl[0] == &c2 && expl[1] == &c0));
+    assert((&expl[0].get() == &c0 && &expl[1].get() == &c2) || (&expl[0].get() == &c2 && &expl[1].get() == &c0));
 
     s.retract(c0);
     cons = s.check();