Commit eb18a514 authored by Clemens Hammacher's avatar Clemens Hammacher Committed by Commit Bot

[base] Fix integer check in CHECK/DCHECK macros

The current implementation failed when comparing an integral type to a
reference to an integral type of different signedness (see updated
unittest).
This CL fixes the checks to actually test the std::decay<T>::type,
i.e. with all references, const or volatile modifiers stripped.

R=jochen@chromium.org, ishell@chromium.org
TEST=unittests/LoggingTest.CompareWithReferenceType

Change-Id: Ib0ac077a91e0409ada7a80b68150cb98cbdd32f1
Reviewed-on: https://chromium-review.googlesource.com/502814Reviewed-by: 's avatarJochen Eisinger <jochen@chromium.org>
Reviewed-by: 's avatarIgor Sheludko <ishell@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45271}
parent ca370361
......@@ -124,8 +124,10 @@ DEFINE_MAKE_CHECK_OP_STRING(void const*)
template <typename Lhs, typename Rhs>
struct is_signed_vs_unsigned {
enum : bool {
value = std::is_integral<Lhs>::value && std::is_integral<Rhs>::value &&
std::is_signed<Lhs>::value && std::is_unsigned<Rhs>::value
value = std::is_integral<typename std::decay<Lhs>::type>::value &&
std::is_integral<typename std::decay<Rhs>::type>::value &&
std::is_signed<typename std::decay<Lhs>::type>::value &&
std::is_unsigned<typename std::decay<Rhs>::type>::value
};
};
// Same thing, other way around: Lhs is unsigned, Rhs signed.
......@@ -135,8 +137,10 @@ struct is_unsigned_vs_signed : public is_signed_vs_unsigned<Rhs, Lhs> {};
// Specialize the compare functions for signed vs. unsigned comparisons.
// std::enable_if ensures that this template is only instantiable if both Lhs
// and Rhs are integral types, and their signedness does not match.
#define MAKE_UNSIGNED(Type, value) \
static_cast<typename std::make_unsigned<Type>::type>(value)
#define MAKE_UNSIGNED(Type, value) \
static_cast< \
typename std::make_unsigned<typename std::decay<Type>::type>::type>( \
value)
#define DEFINE_SIGNED_MISMATCH_COMP(CHECK, NAME, IMPL) \
template <typename Lhs, typename Rhs> \
V8_INLINE typename std::enable_if<CHECK<Lhs, Rhs>::value, bool>::type \
......
......@@ -63,11 +63,11 @@ TEST(LoggingTest, CompareAgainstStaticConstPointer) {
CHECK_SUCCEED(GT, 0, v8::internal::Smi::kMinValue);
}
TEST(LoggingTest, CompareWithDifferentSignedness) {
#define CHECK_BOTH(name, lhs, rhs) \
CHECK_##name(lhs, rhs); \
DCHECK_##name(lhs, rhs)
TEST(LoggingTest, CompareWithDifferentSignedness) {
int32_t i32 = 10;
uint32_t u32 = 20;
int64_t i64 = 30;
......@@ -82,5 +82,18 @@ TEST(LoggingTest, CompareWithDifferentSignedness) {
CHECK_BOTH(IMPLIES, !u32, !i64);
}
TEST(LoggingTest, CompareWithReferenceType) {
int32_t i32 = 10;
uint32_t u32 = 20;
int64_t i64 = 30;
uint64_t u64 = 40;
// All these checks should compile (!) and succeed.
CHECK_BOTH(EQ, i32 + 10, *&u32);
CHECK_BOTH(LT, *&i32, u64);
CHECK_BOTH(IMPLIES, *&i32, i64);
CHECK_BOTH(IMPLIES, *&i32, u64);
}
} // namespace base
} // namespace v8
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment