Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(657)

Unified Diff: base/numerics/safe_math_impl.h

Issue 2554803002: Cleanup some undefined floating point behavior in base/numerics (Closed)
Patch Set: compile fix Created 4 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « base/numerics/safe_conversions.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: base/numerics/safe_math_impl.h
diff --git a/base/numerics/safe_math_impl.h b/base/numerics/safe_math_impl.h
index 679cfa4e5a0e73c84b541128ed9f9e963b3f2262..ae0a5b1d4f031d8228ee47172daf27153dd6ce77 100644
--- a/base/numerics/safe_math_impl.h
+++ b/base/numerics/safe_math_impl.h
@@ -635,6 +635,16 @@ class CheckedNumericState<T, NUMERIC_INTEGER> {
bool is_valid_;
T value_;
+ // Ensures that a type conversion does not trigger undefined behavior.
+ template <typename Src>
+ static constexpr T WellDefinedConversionOrZero(const Src value,
+ const bool is_valid) {
+ using SrcType = typename internal::UnderlyingType<Src>::type;
+ return (std::is_integral<SrcType>::value || is_valid)
+ ? static_cast<T>(value)
+ : static_cast<T>(0);
+ }
+
public:
template <typename Src, NumericRepresentation type>
friend class CheckedNumericState;
@@ -644,7 +654,7 @@ class CheckedNumericState<T, NUMERIC_INTEGER> {
template <typename Src>
constexpr CheckedNumericState(Src value, bool is_valid)
: is_valid_(is_valid && IsValueInRangeForNumericType<T>(value)),
- value_(is_valid_ ? static_cast<T>(value) : 0) {
+ value_(WellDefinedConversionOrZero(value, is_valid_)) {
static_assert(std::is_arithmetic<Src>::value, "Argument must be numeric.");
}
@@ -652,12 +662,12 @@ class CheckedNumericState<T, NUMERIC_INTEGER> {
template <typename Src>
constexpr CheckedNumericState(const CheckedNumericState<Src>& rhs)
: is_valid_(rhs.IsValid()),
- value_(is_valid_ ? static_cast<T>(rhs.value()) : 0) {}
+ value_(WellDefinedConversionOrZero(rhs.value(), is_valid_)) {}
template <typename Src>
constexpr explicit CheckedNumericState(Src value)
: is_valid_(IsValueInRangeForNumericType<T>(value)),
- value_(is_valid_ ? static_cast<T>(value) : 0) {}
+ value_(WellDefinedConversionOrZero(value, is_valid_)) {}
constexpr bool is_valid() const { return is_valid_; }
constexpr T value() const { return value_; }
@@ -669,6 +679,18 @@ class CheckedNumericState<T, NUMERIC_FLOATING> {
private:
T value_;
+ // Ensures that a type conversion does not trigger undefined behavior.
+ template <typename Src>
+ static constexpr T WellDefinedConversionOrNaN(const Src value,
+ const bool is_valid) {
+ using SrcType = typename internal::UnderlyingType<Src>::type;
+ return (StaticDstRangeRelationToSrcRange<T, SrcType>::value ==
+ NUMERIC_RANGE_CONTAINED ||
+ is_valid)
+ ? static_cast<T>(value)
+ : std::numeric_limits<T>::quiet_NaN();
+ }
+
public:
template <typename Src, NumericRepresentation type>
friend class CheckedNumericState;
@@ -677,18 +699,20 @@ class CheckedNumericState<T, NUMERIC_FLOATING> {
template <typename Src>
constexpr CheckedNumericState(Src value, bool is_valid)
- : value_((is_valid && IsValueInRangeForNumericType<T>(value))
- ? static_cast<T>(value)
- : std::numeric_limits<T>::quiet_NaN()) {}
+ : value_(WellDefinedConversionOrNaN(value, is_valid)) {}
template <typename Src>
constexpr explicit CheckedNumericState(Src value)
- : value_(static_cast<T>(value)) {}
+ : value_(WellDefinedConversionOrNaN(
+ value,
+ IsValueInRangeForNumericType<T>(value))) {}
// Copy constructor.
template <typename Src>
constexpr CheckedNumericState(const CheckedNumericState<Src>& rhs)
- : value_(static_cast<T>(rhs.value())) {}
+ : value_(WellDefinedConversionOrNaN(
+ rhs.value(),
+ rhs.is_valid() && IsValueInRangeForNumericType<T>(rhs.value()))) {}
constexpr bool is_valid() const {
// Written this way because std::isfinite is not reliably constexpr.
« no previous file with comments | « base/numerics/safe_conversions.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698