backport from main

Author: Mizux

ortools/algorithms/binary_search.h

@@ -19,29 +19,37 @@
 #include <functional>
 #include <utility>
 
+#include "absl/base/log_severity.h"
 #include "absl/functional/function_ref.h"
 #include "absl/log/check.h"
 #include "absl/numeric/int128.h"
 #include "absl/types/span.h"
 #include "ortools/base/logging.h"
 
 namespace operations_research {
-
+// Finds a point in [x_true, x_false) where f changes from true to false.
+// If check_bounds is true, it will CHECK that f(x_true) = true and
+// f(x_false) = false.
+//
 // EXAMPLE:
 //   // Finds the value x in [0,Pi/2] such that cos(x)=2sin(x):
 //   BinarySearch<double>(/*x_true=*/0.0, /*x_false=*/M_PI/2,
 //                        [](double x) { return cos(x) >= 2*sin(x); });
 //
-// Note that x_true > x_false is supported: it works either way.
+// MONOTONIC FUNCTIONS: Suppose f is a monotonic boolean function. See below for
+// the NON-MONOTONIC case.
 //
-// Ideally, f is a monotonic boolean function, such that:
-// - f(x_true) = true
-// - f(x_false) = false
-// - there exists X such that f(x)=true for all x between x_true and X, and
-//   f(x)=false for all x between X and x_false.
+// If x_true < x_false, this returns X such that:
+// - x_true < X < x_false,
+// - f((x_true, X]) = true (i.e. for all x in (x_true, X], f(x) = true),
+// - f((X, x_false)) = false (i.e. for all x in (X, x_false), f(x) = false)
+// or this returns x_true if such an X does not exist.
 //
-// In those conditions, this returns that value X (note that f(X) is true).
-// See below for the NON-MONOTONIC case.
+// If x_true > x_false, this function returns X such that:
+// - x_false < X < x_true
+// - f((x_false, X)) = false
+// - f([X, x_true)) = true
+// or this return x_true if such an X does not exist.
 //
 // Also note that 'Point' may be floating-point types: the function will still
 // converge when the midpoint can't be distinguished from one of the limits,
@@ -68,7 +76,7 @@ namespace operations_research {
 // Note also that even if f() is non-deterministic, i.e. f(X) can sometimes
 // return true and sometimes false for the same X, then the binary search will
 // still finish, but it's hard to say anything about the returned X.
-template <class Point>
+template <class Point, bool check_bounds = DEBUG_MODE>
 Point BinarySearch(Point x_true, Point x_false, std::function<bool(Point)> f);
 
 // Used by BinarySearch(). This is just (x+y)/2, with some DCHECKs to catch
@@ -207,10 +215,12 @@ Point BinarySearchMidpoint(Point x, Point y) {
   return midpoint;
 }
 
-template <class Point>
+template <class Point, bool check_bounds>
 Point BinarySearch(Point x_true, Point x_false, std::function<bool(Point)> f) {
-  DCHECK(f(x_true)) << x_true;
-  DCHECK(!f(x_false)) << x_false;
+  if constexpr (check_bounds) {
+    CHECK(f(x_true)) << x_true;
+    CHECK(!f(x_false)) << x_false;
+  }
   int num_iterations = 0;
   constexpr int kMaxNumIterations = 1000000;
   while (true) {

ortools/algorithms/binary_search_test.cc

@@ -175,6 +175,53 @@ TEST(BinarySearchDeathTest, DiesIfEitherBoundaryConditionViolatedInFastbuild) {
                "");
 }
 
+TEST(BinarySearchDeathTest, DiesIfBoundCheckIsEnabledAndABoundIsViolated) {
+  // EXPECT_DEATH does not work with 2-parameter templates.
+  auto bs = [](int x_true, int x_false, auto f) {
+    return BinarySearch<int, true>(x_true, x_false, f);
+  };
+  EXPECT_DEATH(bs(/*x_true=*/0, /*x_false=*/42, [](int x) { return x < 999; }),
+               "");
+  EXPECT_DEATH(bs(/*x_true=*/0, /*x_false=*/42, [](int x) { return x < 0; }),
+               "");
+  EXPECT_DEATH(bs(/*x_true=*/0, /*x_false=*/42, [](int x) { return x > 20; }),
+               "");
+}
+
+TEST(BinarySearchTest, NoDeathIfBoundCheckIsDisabled) {
+  // EXPECT_EQ does not work with 2-parameter templates.
+  auto bs = [](int x_true, int x_false, auto f) {
+    return BinarySearch<int, false>(x_true, x_false, f);
+  };
+
+  // f is true on the whole interval ]0, 42[: return last value 41.
+  EXPECT_EQ(bs(/*x_true=*/0, /*x_false=*/42, [](int x) { return x < 999; }),
+            41);
+  // f is true on reversed interval ]42, 0[: return last value 1.
+  EXPECT_EQ(bs(/*x_true=*/42, /*x_false=*/0, [](int x) { return x < 999; }), 1);
+  // f is false on the whole interval ]0, 42[: return x_true bound 0.
+  EXPECT_EQ(bs(/*x_true=*/0, /*x_false=*/42, [](int x) { return x < 0; }), 0);
+  // f is false on reversed interval ]42, 0[: return x_true bound 42.
+  EXPECT_EQ(bs(/*x_true=*/0, /*x_false=*/42, [](int x) { return x < 0; }), 0);
+  // f is false on x_true, true on x_false.
+  // No DCHECK trigger, instead return a transition point of the function
+  // f': x_true -> true, x_false -> false, x_true < x < x_false -> f(x).
+  // The transitions are: 0 (true) -> 1 (false) and 41 (true) -> 42 (false).
+  {
+    const int x_transition =
+        bs(/*x_true=*/0, /*x_false=*/42, [](int x) { return x > 20; });
+    EXPECT_TRUE(x_transition == 0 || x_transition == 41);
+  }
+  // f is false on x_true, true on x_false, with a reversed interval.
+  // No DCHECK trigger, return one of the transition points.
+  // The transitions are: 42 (true) -> 41 (false) and 1 (true) -> 0 (false).
+  {
+    const int x_transition =
+        bs(/*x_true=*/42, /*x_false=*/0, [](int x) { return x < 20; });
+    EXPECT_TRUE(x_transition == 42 || x_transition == 1);
+  }
+}
+
 }  // namespace
 
 // Examples of cases where one needs to override BinarySearchMidpoint() to get

ortools/algorithms/dynamic_partition_test.cc

@@ -19,8 +19,10 @@
 #include <random>
 #include <vector>
 
+#include "absl/base/log_severity.h"
 #include "absl/random/bit_gen_ref.h"
 #include "absl/random/random.h"
+#include "absl/types/span.h"
 #include "gtest/gtest.h"
 #include "ortools/base/gmock.h"
 #include "ortools/base/stl_util.h"

ortools/algorithms/hungarian_test.cc

@@ -20,13 +20,12 @@
 #include <random>
 #include <vector>
 
+#include "absl/base/macros.h"
 #include "absl/container/flat_hash_map.h"
 #include "absl/random/distributions.h"
 #include "absl/types/span.h"
 #include "gtest/gtest.h"
-#include "ortools/base/macros.h"
 #include "ortools/base/map_util.h"
-#include "ortools/base/types.h"
 
 namespace operations_research {
 
@@ -94,24 +93,24 @@ TEST(LinearAssignmentTest, InvalidMatrix) {
   TestMinimization(cost_nan, 0, expected_agents, expected_tasks);
 }
 
-#define MATRIX_TEST                                                  \
-  {                                                                  \
-    std::vector<std::vector<double>> cost(kMatrixHeight);            \
-    for (int row = 0; row < kMatrixHeight; ++row) {                  \
-      cost[row].resize(kMatrixWidth);                                \
-      for (int col = 0; col < kMatrixWidth; ++col) {                 \
-        cost[row][col] = kCost[row][col];                            \
-      }                                                              \
-    }                                                                \
-    EXPECT_EQ(arraysize(expected_agents_for_min),                    \
-              arraysize(expected_tasks_for_min));                    \
-    EXPECT_EQ(arraysize(expected_agents_for_max),                    \
-              arraysize(expected_tasks_for_max));                    \
-    const int assignment_size = arraysize(expected_agents_for_max);  \
-    TestMinimization(cost, assignment_size, expected_agents_for_min, \
-                     expected_tasks_for_min);                        \
-    TestMaximization(cost, assignment_size, expected_agents_for_max, \
-                     expected_tasks_for_max);                        \
+#define MATRIX_TEST                                                      \
+  {                                                                      \
+    std::vector<std::vector<double>> cost(kMatrixHeight);                \
+    for (int row = 0; row < kMatrixHeight; ++row) {                      \
+      cost[row].resize(kMatrixWidth);                                    \
+      for (int col = 0; col < kMatrixWidth; ++col) {                     \
+        cost[row][col] = kCost[row][col];                                \
+      }                                                                  \
+    }                                                                    \
+    EXPECT_EQ(ABSL_ARRAYSIZE(expected_agents_for_min),                   \
+              ABSL_ARRAYSIZE(expected_tasks_for_min));                   \
+    EXPECT_EQ(ABSL_ARRAYSIZE(expected_agents_for_max),                   \
+              ABSL_ARRAYSIZE(expected_tasks_for_max));                   \
+    const int assignment_size = ABSL_ARRAYSIZE(expected_agents_for_max); \
+    TestMinimization(cost, assignment_size, expected_agents_for_min,     \
+                     expected_tasks_for_min);                            \
+    TestMaximization(cost, assignment_size, expected_agents_for_max,     \
+                     expected_tasks_for_max);                            \
   }
 
 // Test on a 1x1 matrix

ortools/algorithms/knapsack_solver_test.cc

@@ -17,8 +17,8 @@
 #include <limits>
 #include <vector>
 
+#include "absl/base/macros.h"
 #include "gtest/gtest.h"
-#include "ortools/base/macros.h"
 #include "ortools/util/time_limit.h"
 
 namespace operations_research {

ortools/base/BUILD.bazel

@@ -44,7 +44,6 @@ cc_library(
     hdrs = [
         "commandlineflags.h",
         "init_google.h",
-        "logging.h",
         "stl_logging.h",
         "types.h",
         "version.h",
@@ -63,7 +62,6 @@ cc_library(
     deps = [
         ":commandlineflags",
         ":logging",
-        ":macros",
         ":types",
         "@com_google_absl//absl/base",
         "@com_google_absl//absl/container:flat_hash_map",
@@ -73,7 +71,10 @@ cc_library(
         "@com_google_absl//absl/flags:flag",
         "@com_google_absl//absl/flags:parse",
         "@com_google_absl//absl/flags:usage",
+        "@com_google_absl//absl/log",
+        "@com_google_absl//absl/log:check",
         "@com_google_absl//absl/log:die_if_null",
+        "@com_google_absl//absl/log:globals",
         "@com_google_absl//absl/log:initialize",
         "@com_google_absl//absl/strings",
         "@com_google_absl//absl/synchronization",
@@ -617,7 +618,6 @@ cc_library(
     srcs = ["timer.cc"],
     hdrs = ["timer.h"],
     deps = [
-        ":macros",
         "@com_google_absl//absl/log:check",
         "@com_google_absl//absl/time",
     ],

ortools/base/adjustable_priority_queue-inl.h

@@ -14,6 +14,6 @@
 #ifndef OR_TOOLS_BASE_ADJUSTABLE_PRIORITY_QUEUE_INL_H_
 #define OR_TOOLS_BASE_ADJUSTABLE_PRIORITY_QUEUE_INL_H_
 
-#include "ortools/base/adjustable_priority_queue.h"
+#include "ortools/base/adjustable_priority_queue.h"  // IWYU pragma: export
 
 #endif  // OR_TOOLS_BASE_ADJUSTABLE_PRIORITY_QUEUE_INL_H_

ortools/base/adjustable_priority_queue.h

@@ -20,8 +20,6 @@
 #include <list>
 #include <vector>
 
-#include "ortools/base/macros.h"
-
 template <typename T, typename Comparator>
 class LowerPriorityThan {
  public:

ortools/base/constant_divisor_test.cc

@@ -68,7 +68,7 @@ TEST(ConstantDivisorTemplateTest, Simple) {
 }
 
 TEST(ConstantDivisorUint64Test, Bugs) {
-  // If forumula (27) from p231 is ever implemented, these divisors will break
+  // If formula (27) from p231 is ever implemented, these divisors will break
   // if a >= is accidentally used instead of >.
   EXPECT_EQ(uint64_t{828560257293048160},
             ConstantDivisor<uint64_t>(21).div(uint64_t{17399765403154011380u}));

ortools/base/gzipfile.h

@@ -28,7 +28,7 @@ enum class AppendedStreams {
   kIgnoreAppendedData,
 };
 
-// Return a readonly file that contains a uncompressed version of
+// Return a read-only file that contains a uncompressed version of
 // another File.
 //
 // If "ownership == TAKE_OWNERSHIP", the file takes ownership of