-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cleanup: replace ad-hoc [0, 1] value types with UnitFloat #14081
Changes from 3 commits
f22a548
4c457c3
72d8755
f95e89b
69a6de1
64b8c37
ac126f1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
#pragma once | ||
|
||
#include <algorithm> | ||
|
||
namespace Envoy { | ||
|
||
// Template helper type that represents a closed interval with the given minimum and maximum values. | ||
template <typename T, T MinValue, T MaxValue> struct Interval { | ||
static_assert(MinValue <= MaxValue, "min must be <= max"); | ||
static constexpr T min_value = MinValue; | ||
static constexpr T max_value = MaxValue; | ||
}; | ||
|
||
// Utility type that represents a value of type T in the given interval. | ||
template <typename T, typename Interval> class ClosedIntervalValue { | ||
antoniovicente marked this conversation as resolved.
Show resolved
Hide resolved
|
||
public: | ||
static ClosedIntervalValue min() { return ClosedIntervalValue(Interval::min_value); } | ||
static ClosedIntervalValue max() { return ClosedIntervalValue(Interval::max_value); } | ||
|
||
constexpr explicit ClosedIntervalValue(T value) | ||
: value_(std::max<T>(Interval::min_value, std::min<T>(Interval::max_value, value))) {} | ||
|
||
T value() const { return value_; } | ||
|
||
private: | ||
T value_; | ||
}; | ||
|
||
// C++17 doesn't allow templating on floating point values, otherwise that's | ||
// what we should do here instead of relying on a int => float implicit | ||
// conversion. TODO(akonradi): when Envoy is using C++20, switch these template | ||
// parameters to floats. | ||
|
||
// Floating point value in the range [0, 1]. | ||
using UnitFloat = ClosedIntervalValue<float, Interval<int, 0, 1>>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you avoid mixing float and int in this definition? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No; float template parameters aren't allowed until C++20. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Darn. Ok. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
|
||
} // namespace Envoy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
super nit: Could call this closedInterval since it represents a closed interval in particular
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the name is still "Interval"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I opted to make the change to the value type since it didn't seem helpful to give the interval type an opinion on whether it is open or closed - that is, the value type is templated over a thing with a min and a max, which sounds like it could apply to both an open or closed interval (though "bounds" might be better for an open interval).