Commit c84fb46e authored by Tom Anderson's avatar Tom Anderson Committed by Commit Bot

Fix link error for certain usages of [D]CHECK_*

Background: as part of bug 1066670, we're removing usage of Xlib and
switching to our own X11 protocol bindings.  In Xlib, constants are
usually defined as macros:
    #define GenericEvent 35
Our approach is different:
    struct GeGenericEvent {
      static constexpr uint8_t opcode = 35;
    };

There are many instances of the following snippet:
    DCHECK_EQ(event.type, GenericEvent);
which now need to be changed to:
    DCHECK_EQ(event.type, GeGenericEvent::opcode);

However, this causes a subtle link failure:
    ld.lld: error: undefined symbol: x11::GeGenericEvent::opcode
This is because CheckEqImpl's parameters are passed as const&, but
the static constexpr value doesn't have an address.

Alternatives considered:
* Change uint8_t to int to get the existing int overload of
  CheckEqImpl.  We want to avoid this because using int would
  imply there are more opcodes than can actually exist in the
  protocol.
* Change *::opcode to be non-constexpr static, defined in source
  file.  This won't work because it's common to switch() over the
  opcode, and that won't work unless the cases are constexpr values.
* Create a separate source file that provides redundant definitions
  of these constants.  This is a hack.

This CL extends the existing int-int overload meant to solve this issue
to any fundamental type.

BUG=1066670
R=thestig

Change-Id: If62c845cf590cca484e13cf482a67ef4cdd51cbd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2236220
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Reviewed-by: default avatarLei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#776254}
parent 19bf6e08
...@@ -161,9 +161,13 @@ class CheckOpResult { ...@@ -161,9 +161,13 @@ class CheckOpResult {
#endif #endif
// The int-int overload avoids address-taking static int members. // The second overload avoids address-taking of static members for
// fundamental types.
#define DEFINE_CHECK_OP_IMPL(name, op) \ #define DEFINE_CHECK_OP_IMPL(name, op) \
template <typename T, typename U> \ template <typename T, typename U, \
std::enable_if_t<!std::is_fundamental<T>::value || \
!std::is_fundamental<U>::value, \
int> = 0> \
constexpr ::logging::CheckOpResult Check##name##Impl( \ constexpr ::logging::CheckOpResult Check##name##Impl( \
const T& v1, const U& v2, const char* expr_str) { \ const T& v1, const U& v2, const char* expr_str) { \
if (ANALYZER_ASSUME_TRUE(v1 op v2)) \ if (ANALYZER_ASSUME_TRUE(v1 op v2)) \
...@@ -171,7 +175,11 @@ class CheckOpResult { ...@@ -171,7 +175,11 @@ class CheckOpResult {
return ::logging::CheckOpResult(expr_str, CheckOpValueStr(v1), \ return ::logging::CheckOpResult(expr_str, CheckOpValueStr(v1), \
CheckOpValueStr(v2)); \ CheckOpValueStr(v2)); \
} \ } \
constexpr ::logging::CheckOpResult Check##name##Impl(int v1, int v2, \ template <typename T, typename U, \
std::enable_if_t<std::is_fundamental<T>::value && \
std::is_fundamental<U>::value, \
int> = 0> \
constexpr ::logging::CheckOpResult Check##name##Impl(T v1, U v2, \
const char* expr_str) { \ const char* expr_str) { \
if (ANALYZER_ASSUME_TRUE(v1 op v2)) \ if (ANALYZER_ASSUME_TRUE(v1 op v2)) \
return ::logging::CheckOpResult(); \ return ::logging::CheckOpResult(); \
......
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