Closed jamiesnape closed 4 years ago
jwnimmer-tri
commented
4 years ago Most look like size_t coming from std collection .size(). I suggest that the unsigned => int conversion be abstracted into a private helper method within the class, and that all of ==, !=, < etc. invoke the helper to convert the U-typed unsigned value into something to compare with this.
SeanCurtis-TRI
commented
4 years ago I don't think the solution is casting U to "this", but to convert the maximum value of this to U. For example:
template <typename U>
typename std::enable_if<
std::is_integral<U>::value && std::is_unsigned<U>::value, bool>::type
operator!=(const U& value) const {
DRAKE_ASSERT_VOID(AssertValid(index_, "Testing != with invalid index."));
return value > static_cast<U>(kMaxIndex) ||
index_ != static_cast<int>(value);
}
static constexpr int kMaxIndex = std::numeric_limits<int>::max();If sizeof(U) is bigger than sizeof(int), then things get nicely promoted there and we can detect size_t values that would otherwise truncate into a lie if we cast the other way.
If sizeof(U) is smaller, then we end up truncating 01111.1111 to the maximum unsigned value of that smaller type (16 1s, 8 1s, etc.) and, by definition the value stored in that smaller unsigned type cannot be larger than that type consisting of all 1s.
I'll go ahead and quickly PR this. I'll exercise the focal CI indicated above.
For example, from https://drake-jenkins.csail.mit.edu/job/linux-focal-unprovisioned-gcc-bazel-experimental-debug/1/:
Relates #13102. FYI @jwnimmer-tri @SeanCurtis-TRI.