Add SafeMin() and SafeMax(), which accept args of different types
Specifically, they handle all combinations of two integer and two
floating-point arguments by picking a result type that is guaranteed
to be able to hold the result. This means callers no longer have to
deal with potentially dangerous casting to make all the arguments have
the same type, like they have to with std::min() and std::max().
Also, they're constexpr.
Mostly for illustrative purposes, this CL replaces a few std::min()
and std::max() calls with SafeMin() and SafeMax().
BUG=webrtc:7459
Review-Url: https://codereview.webrtc.org/2810483002
Cr-Commit-Position: refs/heads/master@{#17869}
diff --git a/webrtc/base/BUILD.gn b/webrtc/base/BUILD.gn
index 6014809..a3ee03f 100644
--- a/webrtc/base/BUILD.gn
+++ b/webrtc/base/BUILD.gn
@@ -165,6 +165,7 @@
"safe_compare.h",
"safe_conversions.h",
"safe_conversions_impl.h",
+ "safe_minmax.h",
"sanitizer.h",
"scoped_ref_ptr.h",
"string_to_number.cc",
@@ -813,6 +814,7 @@
"ratetracker_unittest.cc",
"refcountedobject_unittest.cc",
"safe_compare_unittest.cc",
+ "safe_minmax_unittest.cc",
"string_to_number_unittest.cc",
"stringencode_unittest.cc",
"stringutils_unittest.cc",
diff --git a/webrtc/base/safe_minmax.h b/webrtc/base/safe_minmax.h
new file mode 100644
index 0000000..8d1fc87
--- /dev/null
+++ b/webrtc/base/safe_minmax.h
@@ -0,0 +1,188 @@
+/*
+ * Copyright 2017 The WebRTC Project Authors. All rights reserved.
+ *
+ * Use of this source code is governed by a BSD-style license
+ * that can be found in the LICENSE file in the root of the source
+ * tree. An additional intellectual property rights grant can be found
+ * in the file PATENTS. All contributing project authors may
+ * be found in the AUTHORS file in the root of the source tree.
+ */
+
+// Minimum and maximum
+// ===================
+//
+// rtc::SafeMin(x, y)
+// rtc::SafeMax(x, y)
+//
+// Accept two arguments of either any two integral or any two floating-point
+// types, and return the smaller and larger value, respectively, with no
+// truncation or wrap-around. If only one of the input types is statically
+// guaranteed to be able to represent the result, the return type is that type;
+// if either one would do, the result type is the smaller type. (One of these
+// two cases always applies.)
+//
+// (The case with one floating-point and one integral type is not allowed,
+// because the floating-point type will have greater range, but may not have
+// sufficient precision to represent the integer value exactly.)
+//
+// Requesting a specific return type
+// =================================
+//
+// Both functions allow callers to explicitly specify the return type as a
+// template parameter, overriding the default return type. E.g.
+//
+// rtc::SafeMin<int>(x, y) // returns an int
+//
+// If the requested type is statically guaranteed to be able to represent the
+// result, then everything's fine, and the return type is as requested. But if
+// the requested type is too small, a static_assert is triggered.
+
+#ifndef WEBRTC_BASE_SAFE_MINMAX_H_
+#define WEBRTC_BASE_SAFE_MINMAX_H_
+
+#include <limits>
+#include <type_traits>
+
+#include "webrtc/base/checks.h"
+#include "webrtc/base/safe_compare.h"
+#include "webrtc/base/type_traits.h"
+
+namespace rtc {
+
+namespace safe_minmax_impl {
+
+// Make the range of a type available via something other than a constexpr
+// function, to work around MSVC limitations. See
+// https://blogs.msdn.microsoft.com/vcblog/2015/12/02/partial-support-for-expression-sfinae-in-vs-2015-update-1/
+template <typename T>
+struct Limits {
+ static constexpr T lowest = std::numeric_limits<T>::lowest();
+ static constexpr T max = std::numeric_limits<T>::max();
+};
+
+template <typename T, bool is_enum = std::is_enum<T>::value>
+struct UnderlyingType;
+
+template <typename T>
+struct UnderlyingType<T, false> {
+ using type = T;
+};
+
+template <typename T>
+struct UnderlyingType<T, true> {
+ using type = typename std::underlying_type<T>::type;
+};
+
+// Given two types T1 and T2, find types that can hold the smallest (in
+// ::min_t) and the largest (in ::max_t) of the two values.
+template <typename T1,
+ typename T2,
+ bool int1 = IsIntlike<T1>::value,
+ bool int2 = IsIntlike<T2>::value>
+struct MType {
+ static_assert(int1 == int2,
+ "You may not mix integral and floating-point arguments");
+};
+
+// Specialization for when neither type is integral (and therefore presumably
+// floating-point).
+template <typename T1, typename T2>
+struct MType<T1, T2, false, false> {
+ using min_t = typename std::common_type<T1, T2>::type;
+ static_assert(std::is_same<min_t, T1>::value ||
+ std::is_same<min_t, T2>::value,
+ "");
+
+ using max_t = typename std::common_type<T1, T2>::type;
+ static_assert(std::is_same<max_t, T1>::value ||
+ std::is_same<max_t, T2>::value,
+ "");
+};
+
+// Specialization for when both types are integral.
+template <typename T1, typename T2>
+struct MType<T1, T2, true, true> {
+ // The type with the lowest minimum value. In case of a tie, the type with
+ // the lowest maximum value. In case that too is a tie, the types have the
+ // same range, and we arbitrarily pick T1.
+ using min_t = typename std::conditional<
+ safe_cmp::Lt(Limits<T1>::lowest, Limits<T2>::lowest),
+ T1,
+ typename std::conditional<
+ safe_cmp::Gt(Limits<T1>::lowest, Limits<T2>::lowest),
+ T2,
+ typename std::conditional<safe_cmp::Le(Limits<T1>::max,
+ Limits<T2>::max),
+ T1,
+ T2>::type>::type>::type;
+ static_assert(std::is_same<min_t, T1>::value ||
+ std::is_same<min_t, T2>::value,
+ "");
+
+ // The type with the highest maximum value. In case of a tie, the types have
+ // the same range (because in C++, integer types with the same maximum also
+ // have the same minimum).
+ static_assert(safe_cmp::Ne(Limits<T1>::max, Limits<T2>::max) ||
+ safe_cmp::Eq(Limits<T1>::lowest, Limits<T2>::lowest),
+ "integer types with the same max should have the same min");
+ using max_t = typename std::
+ conditional<safe_cmp::Ge(Limits<T1>::max, Limits<T2>::max), T1, T2>::type;
+ static_assert(std::is_same<max_t, T1>::value ||
+ std::is_same<max_t, T2>::value,
+ "");
+};
+
+// A dummy type that we pass around at compile time but never actually use.
+// Declared but not defined.
+struct DefaultType;
+
+// ::type is A, except we fall back to B if A is DefaultType. We static_assert
+// that the chosen type can hold all values that B can hold.
+template <typename A, typename B>
+struct TypeOr {
+ using type = typename std::
+ conditional<std::is_same<A, DefaultType>::value, B, A>::type;
+ static_assert(safe_cmp::Le(Limits<type>::lowest, Limits<B>::lowest) &&
+ safe_cmp::Ge(Limits<type>::max, Limits<B>::max),
+ "The specified type isn't large enough");
+ static_assert(IsIntlike<type>::value == IsIntlike<B>::value &&
+ std::is_floating_point<type>::value ==
+ std::is_floating_point<type>::value,
+ "float<->int conversions not allowed");
+};
+
+} // namespace safe_minmax_impl
+
+template <typename R = safe_minmax_impl::DefaultType,
+ typename T1 = safe_minmax_impl::DefaultType,
+ typename T2 = safe_minmax_impl::DefaultType,
+ typename R2 = typename safe_minmax_impl::TypeOr<
+ R,
+ typename safe_minmax_impl::UnderlyingType<
+ typename safe_minmax_impl::MType<T1, T2>::min_t>::type>::type>
+constexpr R2 SafeMin(T1 a, T2 b) {
+ static_assert(IsIntlike<T1>::value || std::is_floating_point<T1>::value,
+ "The first argument must be integral or floating-point");
+ static_assert(IsIntlike<T2>::value || std::is_floating_point<T2>::value,
+ "The second argument must be integral or floating-point");
+ return safe_cmp::Lt(a, b) ? static_cast<R2>(a) : static_cast<R2>(b);
+}
+
+template <typename R = safe_minmax_impl::DefaultType,
+ typename T1 = safe_minmax_impl::DefaultType,
+ typename T2 = safe_minmax_impl::DefaultType,
+ typename R2 = typename safe_minmax_impl::TypeOr<
+ R,
+ typename safe_minmax_impl::UnderlyingType<
+ typename safe_minmax_impl::MType<T1, T2>::max_t>::type>::type>
+constexpr R2 SafeMax(T1 a, T2 b) {
+ static_assert(IsIntlike<T1>::value || std::is_floating_point<T1>::value,
+ "The first argument must be integral or floating-point");
+ static_assert(IsIntlike<T2>::value || std::is_floating_point<T2>::value,
+ "The second argument must be integral or floating-point");
+ return safe_cmp::Gt(a, b) ? static_cast<R2>(a) : static_cast<R2>(b);
+}
+
+} // namespace rtc
+
+#endif // WEBRTC_BASE_SAFE_MINMAX_H_
diff --git a/webrtc/base/safe_minmax_unittest.cc b/webrtc/base/safe_minmax_unittest.cc
new file mode 100644
index 0000000..6fcbf00
--- /dev/null
+++ b/webrtc/base/safe_minmax_unittest.cc
@@ -0,0 +1,141 @@
+/*
+ * Copyright 2017 The WebRTC Project Authors. All rights reserved.
+ *
+ * Use of this source code is governed by a BSD-style license
+ * that can be found in the LICENSE file in the root of the source
+ * tree. An additional intellectual property rights grant can be found
+ * in the file PATENTS. All contributing project authors may
+ * be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include <algorithm>
+#include <limits>
+
+#include "webrtc/base/safe_minmax.h"
+#include "webrtc/test/gtest.h"
+
+namespace rtc {
+
+namespace {
+
+// Functions that check that SafeMin() and SafeMax() return the specified type.
+// The functions that end in "R" use an explicitly given return type.
+
+template <typename T1, typename T2, typename Tmin, typename Tmax>
+constexpr bool TypeCheckMinMax() {
+ return std::is_same<decltype(SafeMin(std::declval<T1>(), std::declval<T2>())),
+ Tmin>::value &&
+ std::is_same<decltype(SafeMax(std::declval<T1>(), std::declval<T2>())),
+ Tmax>::value;
+}
+
+template <typename T1, typename T2, typename R>
+constexpr bool TypeCheckMinR() {
+ return std::is_same<
+ decltype(SafeMin<R>(std::declval<T1>(), std::declval<T2>())), R>::value;
+}
+
+template <typename T1, typename T2, typename R>
+constexpr bool TypeCheckMaxR() {
+ return std::is_same<
+ decltype(SafeMax<R>(std::declval<T1>(), std::declval<T2>())), R>::value;
+}
+
+// clang-format off
+
+// SafeMin/SafeMax: Check that all combinations of signed/unsigned 8/64 bits
+// give the correct default result type.
+static_assert(TypeCheckMinMax< int8_t, int8_t, int8_t, int8_t>(), "");
+static_assert(TypeCheckMinMax< int8_t, uint8_t, int8_t, uint8_t>(), "");
+static_assert(TypeCheckMinMax< int8_t, int64_t, int64_t, int64_t>(), "");
+static_assert(TypeCheckMinMax< int8_t, uint64_t, int8_t, uint64_t>(), "");
+static_assert(TypeCheckMinMax< uint8_t, int8_t, int8_t, uint8_t>(), "");
+static_assert(TypeCheckMinMax< uint8_t, uint8_t, uint8_t, uint8_t>(), "");
+static_assert(TypeCheckMinMax< uint8_t, int64_t, int64_t, int64_t>(), "");
+static_assert(TypeCheckMinMax< uint8_t, uint64_t, uint8_t, uint64_t>(), "");
+static_assert(TypeCheckMinMax< int64_t, int8_t, int64_t, int64_t>(), "");
+static_assert(TypeCheckMinMax< int64_t, uint8_t, int64_t, int64_t>(), "");
+static_assert(TypeCheckMinMax< int64_t, int64_t, int64_t, int64_t>(), "");
+static_assert(TypeCheckMinMax< int64_t, uint64_t, int64_t, uint64_t>(), "");
+static_assert(TypeCheckMinMax<uint64_t, int8_t, int8_t, uint64_t>(), "");
+static_assert(TypeCheckMinMax<uint64_t, uint8_t, uint8_t, uint64_t>(), "");
+static_assert(TypeCheckMinMax<uint64_t, int64_t, int64_t, uint64_t>(), "");
+static_assert(TypeCheckMinMax<uint64_t, uint64_t, uint64_t, uint64_t>(), "");
+
+// SafeMin/SafeMax: Check that we can use enum types.
+enum DefaultE { kFoo = -17 };
+enum UInt8E : uint8_t { kBar = 17 };
+static_assert(TypeCheckMinMax<unsigned, DefaultE, int, unsigned>(), "");
+static_assert(TypeCheckMinMax<unsigned, UInt8E, uint8_t, unsigned>(), "");
+static_assert(TypeCheckMinMax<DefaultE, UInt8E, int, int>(), "");
+
+using ld = long double;
+
+// SafeMin/SafeMax: Check that all floating-point combinations give the
+// correct result type.
+static_assert(TypeCheckMinMax< float, float, float, float>(), "");
+static_assert(TypeCheckMinMax< float, double, double, double>(), "");
+static_assert(TypeCheckMinMax< float, ld, ld, ld>(), "");
+static_assert(TypeCheckMinMax<double, float, double, double>(), "");
+static_assert(TypeCheckMinMax<double, double, double, double>(), "");
+static_assert(TypeCheckMinMax<double, ld, ld, ld>(), "");
+static_assert(TypeCheckMinMax< ld, float, ld, ld>(), "");
+static_assert(TypeCheckMinMax< ld, double, ld, ld>(), "");
+static_assert(TypeCheckMinMax< ld, ld, ld, ld>(), "");
+
+// clang-format on
+
+// SafeMin/SafeMax: Check some cases of explicitly specified return type. The
+// commented-out lines give compilation errors due to the requested return type
+// being too small or requiring an int<->float conversion.
+static_assert(TypeCheckMinR<int8_t, int8_t, int16_t>(), "");
+// static_assert(TypeCheckMinR<int8_t, int8_t, float>(), "");
+static_assert(TypeCheckMinR<uint32_t, uint64_t, uint32_t>(), "");
+// static_assert(TypeCheckMaxR<uint64_t, float, float>(), "");
+// static_assert(TypeCheckMaxR<uint64_t, double, float>(), "");
+static_assert(TypeCheckMaxR<uint32_t, int32_t, uint32_t>(), "");
+// static_assert(TypeCheckMaxR<uint32_t, int32_t, int32_t>(), "");
+
+template <typename T1, typename T2, typename Tmin, typename Tmax>
+constexpr bool CheckMinMax(T1 a, T2 b, Tmin min, Tmax max) {
+ return TypeCheckMinMax<T1, T2, Tmin, Tmax>() && SafeMin(a, b) == min &&
+ SafeMax(a, b) == max;
+}
+
+// SafeMin/SafeMax: Check a few values.
+static_assert(CheckMinMax(int8_t{1}, int8_t{-1}, int8_t{-1}, int8_t{1}), "");
+static_assert(CheckMinMax(uint8_t{1}, int8_t{-1}, int8_t{-1}, uint8_t{1}), "");
+static_assert(CheckMinMax(uint8_t{5}, uint64_t{2}, uint8_t{2}, uint64_t{5}),
+ "");
+static_assert(CheckMinMax(std::numeric_limits<int32_t>::min(),
+ std::numeric_limits<uint32_t>::max(),
+ std::numeric_limits<int32_t>::min(),
+ std::numeric_limits<uint32_t>::max()),
+ "");
+static_assert(CheckMinMax(std::numeric_limits<int32_t>::min(),
+ std::numeric_limits<uint16_t>::max(),
+ std::numeric_limits<int32_t>::min(),
+ int32_t{std::numeric_limits<uint16_t>::max()}),
+ "");
+// static_assert(CheckMinMax(1.f, 2, 1.f, 2.f), "");
+static_assert(CheckMinMax(1.f, 0.0, 0.0, 1.0), "");
+
+} // namespace
+
+// clang-format off
+
+// These functions aren't used in the tests, but it's useful to look at the
+// compiler output for them, and verify that (1) the same-signedness *Safe
+// functions result in exactly the same code as their *Ref counterparts, and
+// that (2) the mixed-signedness *Safe functions have just a few extra
+// arithmetic and logic instructions (but no extra control flow instructions).
+int32_t TestMinRef( int32_t a, int32_t b) { return std::min(a, b); }
+uint32_t TestMinRef( uint32_t a, uint32_t b) { return std::min(a, b); }
+int32_t TestMinSafe( int32_t a, int32_t b) { return SafeMin(a, b); }
+int32_t TestMinSafe( int32_t a, uint32_t b) { return SafeMin(a, b); }
+int32_t TestMinSafe(uint32_t a, int32_t b) { return SafeMin(a, b); }
+uint32_t TestMinSafe(uint32_t a, uint32_t b) { return SafeMin(a, b); }
+
+// clang-format on
+
+} // namespace rtc
diff --git a/webrtc/common_video/h264/sps_vui_rewriter.cc b/webrtc/common_video/h264/sps_vui_rewriter.cc
index 2e118d5..534e75c 100644
--- a/webrtc/common_video/h264/sps_vui_rewriter.cc
+++ b/webrtc/common_video/h264/sps_vui_rewriter.cc
@@ -18,6 +18,7 @@
#include "webrtc/base/bitbuffer.h"
#include "webrtc/base/checks.h"
#include "webrtc/base/logging.h"
+#include "webrtc/base/safe_minmax.h"
#include "webrtc/common_video/h264/h264_common.h"
#include "webrtc/common_video/h264/sps_parser.h"
@@ -351,8 +352,7 @@
COPY_BITS(source, destination, bits_tmp, misaligned_bits);
}
while (source->RemainingBitCount() > 0) {
- size_t count = std::min(static_cast<size_t>(32u),
- static_cast<size_t>(source->RemainingBitCount()));
+ auto count = rtc::SafeMin<size_t>(32u, source->RemainingBitCount());
COPY_BITS(source, destination, bits_tmp, count);
}
// TODO(noahric): The last byte could be all zeroes now, which we should just
diff --git a/webrtc/modules/audio_coding/neteq/merge.cc b/webrtc/modules/audio_coding/neteq/merge.cc
index 299682f..d50b665 100644
--- a/webrtc/modules/audio_coding/neteq/merge.cc
+++ b/webrtc/modules/audio_coding/neteq/merge.cc
@@ -16,6 +16,8 @@
#include <algorithm> // min, max
#include <memory>
+#include "webrtc/base/safe_conversions.h"
+#include "webrtc/base/safe_minmax.h"
#include "webrtc/common_audio/signal_processing/include/signal_processing_library.h"
#include "webrtc/modules/audio_coding/neteq/audio_multi_vector.h"
#include "webrtc/modules/audio_coding/neteq/cross_correlation.h"
@@ -209,8 +211,8 @@
int16_t Merge::SignalScaling(const int16_t* input, size_t input_length,
const int16_t* expanded_signal) const {
// Adjust muting factor if new vector is more or less of the BGN energy.
- const size_t mod_input_length =
- std::min(static_cast<size_t>(64 * fs_mult_), input_length);
+ const auto mod_input_length = rtc::SafeMin<size_t>(
+ 64 * rtc::dchecked_cast<size_t>(fs_mult_), input_length);
const int16_t expanded_max =
WebRtcSpl_MaxAbsValueW16(expanded_signal, mod_input_length);
int32_t factor = (expanded_max * expanded_max) /
diff --git a/webrtc/modules/audio_processing/audio_processing_unittest.cc b/webrtc/modules/audio_processing/audio_processing_unittest.cc
index 814eea9..c56a57b 100644
--- a/webrtc/modules/audio_processing/audio_processing_unittest.cc
+++ b/webrtc/modules/audio_processing/audio_processing_unittest.cc
@@ -21,6 +21,7 @@
#include "webrtc/base/gtest_prod_util.h"
#include "webrtc/base/ignore_wundef.h"
#include "webrtc/base/protobuf_utils.h"
+#include "webrtc/base/safe_minmax.h"
#include "webrtc/common_audio/include/audio_util.h"
#include "webrtc/common_audio/resampler/include/push_resampler.h"
#include "webrtc/common_audio/resampler/push_sinc_resampler.h"
@@ -678,7 +679,7 @@
// Calculate expected delay estimate and acceptable regions. Further,
// limit them w.r.t. AEC delay estimation support.
const size_t samples_per_ms =
- std::min(static_cast<size_t>(16), frame_->samples_per_channel_ / 10);
+ rtc::SafeMin<size_t>(16u, frame_->samples_per_channel_ / 10);
int expected_median = std::min(std::max(delay_ms - system_delay_ms,
delay_min), delay_max);
int expected_median_high = std::min(
diff --git a/webrtc/modules/remote_bitrate_estimator/aimd_rate_control.cc b/webrtc/modules/remote_bitrate_estimator/aimd_rate_control.cc
index 71415c7..e645090 100644
--- a/webrtc/modules/remote_bitrate_estimator/aimd_rate_control.cc
+++ b/webrtc/modules/remote_bitrate_estimator/aimd_rate_control.cc
@@ -15,6 +15,7 @@
#include <cmath>
#include "webrtc/base/checks.h"
+#include "webrtc/base/safe_minmax.h"
#include "webrtc/modules/remote_bitrate_estimator/overuse_detector.h"
#include "webrtc/modules/remote_bitrate_estimator/include/bwe_defines.h"
@@ -250,8 +251,8 @@
int64_t now_ms, int64_t last_ms, uint32_t current_bitrate_bps) const {
double alpha = 1.08;
if (last_ms > -1) {
- int time_since_last_update_ms = std::min(static_cast<int>(now_ms - last_ms),
- 1000);
+ auto time_since_last_update_ms =
+ rtc::SafeMin<int64_t>(now_ms - last_ms, 1000);
alpha = pow(alpha, time_since_last_update_ms / 1000.0);
}
uint32_t multiplicative_increase_bps = std::max(