Skip to content

V
V8
Commit 76723502 authored by clemensh's avatar clemensh Committed by Commit bot
Browse files
Options

[base] Pass scalar arguments by value in CHECK/DCHECK 

This not only potentially improves performance, but also avoids weird
linker errors, like the one below, where I used Smi::kMinValue in a
DCHECK_EQ.

> [421/649] LINK ./mksnapshot
> FAILED: mksnapshot
> src/base/logging.h|178| error: undefined reference to
  'v8::internal::Smi::kMinValue'

R=bmeurer@chromium.org, ishell@chromium.org

Review-Url: https://codereview.chromium.org/2524093002
Cr-Commit-Position: refs/heads/master@{#41273}
parent 9be74766
Hide whitespace changes
Inline Side-by-side
Showing with 47 additions and 33 deletions
src/base/logging.cc
View file @ 76723502
......@@ -14,9 +14,8 @@ namespace v8 {
namespace base {
// Explicit instantiations for commonly used comparisons.
#define DEFINE_MAKE_CHECK_OP_STRING(type) \
template std::string* MakeCheckOpString<type, type>( \
type const&, type const&, char const*);
#define DEFINE_MAKE_CHECK_OP_STRING(type) \
template std::string* MakeCheckOpString<type, type>(type, type, char const*);
DEFINE_MAKE_CHECK_OP_STRING(int)
DEFINE_MAKE_CHECK_OP_STRING(long) // NOLINT(runtime/int)
DEFINE_MAKE_CHECK_OP_STRING(long long) // NOLINT(runtime/int)
......@@ -29,11 +28,11 @@ DEFINE_MAKE_CHECK_OP_STRING(void const*)
// Explicit instantiations for floating point checks.
#define DEFINE_CHECK_OP_IMPL(NAME) \
template std::string* Check##NAME##Impl<float, float>( \
float const& lhs, float const& rhs, char const* msg); \
template std::string* Check##NAME##Impl<double, double>( \
double const& lhs, double const& rhs, char const* msg);
#define DEFINE_CHECK_OP_IMPL(NAME) \
template std::string* Check##NAME##Impl<float, float>(float lhs, float rhs, \
char const* msg); \
template std::string* Check##NAME##Impl<double, double>( \
double lhs, double rhs, char const* msg);
DEFINE_CHECK_OP_IMPL(EQ)
DEFINE_CHECK_OP_IMPL(NE)
DEFINE_CHECK_OP_IMPL(LE)
......
src/base/logging.h
View file @ 76723502
......@@ -55,13 +55,14 @@ namespace base {
// Helper macro for binary operators.
// Don't use this macro directly in your code, use CHECK_EQ et al below.
#define CHECK_OP(name, op, lhs, rhs) \
do { \
if (std::string* _msg = ::v8::base::Check##name##Impl( \
(lhs), (rhs), #lhs " " #op " " #rhs)) { \
V8_Fatal(__FILE__, __LINE__, "Check failed: %s.", _msg->c_str()); \
delete _msg; \
} \
#define CHECK_OP(name, op, lhs, rhs) \
do { \
if (std::string* _msg = \
::v8::base::Check##name##Impl<decltype(lhs), decltype(rhs)>( \
(lhs), (rhs), #lhs " " #op " " #rhs)) { \
V8_Fatal(__FILE__, __LINE__, "Check failed: %s.", _msg->c_str()); \
delete _msg; \
} \
} while (0)
#else
......@@ -73,13 +74,26 @@ namespace base {
#endif
// Helpers to determine how to pass values: Pass scalars by value, others by
// const reference.
template <typename Lhs, typename Rhs>
struct PassByValue {
enum : bool {
value = (std::is_scalar<Lhs>::value || std::is_array<Lhs>::value) &&
(std::is_scalar<Rhs>::value || std::is_array<Rhs>::value)
};
};
template <typename T>
struct PassType
: public std::conditional<PassByValue<T, T>::value, T, T const&> {};
// Build the error message string. This is separate from the "Impl"
// function template because it is not performance critical and so can
// be out of line, while the "Impl" code should be inline. Caller
// takes ownership of the returned string.
template <typename Lhs, typename Rhs>
std::string* MakeCheckOpString(Lhs const& lhs, Rhs const& rhs,
std::string* MakeCheckOpString(typename PassType<Lhs>::type lhs,
typename PassType<Rhs>::type rhs,
char const* msg) {
std::ostringstream ss;
ss << msg << " (" << lhs << " vs. " << rhs << ")";
......@@ -90,7 +104,7 @@ std::string* MakeCheckOpString(Lhs const& lhs, Rhs const& rhs,
// in logging.cc.
#define DEFINE_MAKE_CHECK_OP_STRING(type) \
extern template V8_BASE_EXPORT std::string* MakeCheckOpString<type, type>( \
type const&, type const&, char const*);
type, type, char const*);
DEFINE_MAKE_CHECK_OP_STRING(int)
DEFINE_MAKE_CHECK_OP_STRING(long) // NOLINT(runtime/int)
DEFINE_MAKE_CHECK_OP_STRING(long long) // NOLINT(runtime/int)
......@@ -101,27 +115,21 @@ DEFINE_MAKE_CHECK_OP_STRING(char const*)
DEFINE_MAKE_CHECK_OP_STRING(void const*)
#undef DEFINE_MAKE_CHECK_OP_STRING
// Helper functions for CHECK_OP macro.
// The (int, int) specialization works around the issue that the compiler
// will not instantiate the template version of the function on values of
// unnamed enum type - see comment below.
// The (float, float) and (double, double) instantiations are explicitly
// externialized to ensure proper 32/64-bit comparisons on x86.
#define DEFINE_CHECK_OP_IMPL(NAME, op) \
template <typename Lhs, typename Rhs> \
V8_INLINE std::string* Check##NAME##Impl(Lhs const& lhs, Rhs const& rhs, \
char const* msg) { \
return V8_LIKELY(lhs op rhs) ? nullptr : MakeCheckOpString(lhs, rhs, msg); \
} \
V8_INLINE std::string* Check##NAME##Impl(int lhs, int rhs, \
V8_INLINE std::string* Check##NAME##Impl(typename PassType<Lhs>::type lhs, \
typename PassType<Rhs>::type rhs, \
char const* msg) { \
return V8_LIKELY(lhs op rhs) ? nullptr : MakeCheckOpString(lhs, rhs, msg); \
return V8_LIKELY(lhs op rhs) ? nullptr \
: MakeCheckOpString<Lhs, Rhs>(lhs, rhs, msg); \
} \
extern template V8_BASE_EXPORT std::string* Check##NAME##Impl<float, float>( \
float const& lhs, float const& rhs, char const* msg); \
const float lhs, const float rhs, char const* msg); \
extern template V8_BASE_EXPORT std::string* \
Check##NAME##Impl<double, double>(double const& lhs, double const& rhs, \
Check##NAME##Impl<double, double>(const double lhs, const double rhs, \
char const* msg);
DEFINE_CHECK_OP_IMPL(EQ, ==)
DEFINE_CHECK_OP_IMPL(NE, !=)
......
test/unittests/base/logging-unittest.cc
View file @ 76723502
......@@ -8,11 +8,18 @@
namespace v8 {
namespace base {
namespace {
template <typename Lhs, typename Rhs>
std::string *CallCheckEQ(Lhs lhs, Rhs rhs, const char *msg) {
return CheckEQImpl<Lhs, Rhs>(lhs, rhs, msg);
}
} // namespace
TEST(LoggingTest, CheckEQImpl) {
EXPECT_EQ(nullptr, CheckEQImpl(0.0, 0.0, ""));
EXPECT_EQ(nullptr, CheckEQImpl(0.0, -0.0, ""));
EXPECT_EQ(nullptr, CheckEQImpl(-0.0, 0.0, ""));
EXPECT_EQ(nullptr, CheckEQImpl(-0.0, -0.0, ""));
EXPECT_EQ(nullptr, CallCheckEQ(0.0, 0.0, ""));
EXPECT_EQ(nullptr, CallCheckEQ(0.0, -0.0, ""));
EXPECT_EQ(nullptr, CallCheckEQ(-0.0, 0.0, ""));
EXPECT_EQ(nullptr, CallCheckEQ(-0.0, -0.0, ""));
}
} // namespace base
......
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