Move helper struct from logging.h to template-utils.h

I want to reuse the PassType helper in another CL, thus move it from
logging.h to template-utils.h, and rename it to pass_value_or_ref to
match other helpers there.
Also, add a boolean template parameter to declare whether array
dimensions should be removed. The default is to do so, which helps to
reduce the number of template instantiations by always passing arrays
as pointers.

Also, fix the usages in logging.h to actually use that helper when
instantiating other template functions. This will reduce the number of
instantiations.

And finally, we now have unit tests for the template utils, to document
what we expect, and test that this works on all architectures.

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

Change-Id: I1ef5d2a489a5cfc7601c5ab13748674e3aa86cd6
Reviewed-on: https://chromium-review.googlesource.com/594247
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47191}

src/base/logging.h

@@ -12,6 +12,7 @@
 #include "src/base/base-export.h"
 #include "src/base/build_config.h"
 #include "src/base/compiler-specific.h"
+#include "src/base/template-utils.h"
 
 [[noreturn]] PRINTF_FORMAT(3, 4) V8_BASE_EXPORT
     void V8_Fatal(const char* file, int line, const char* format, ...);
@@ -66,21 +67,23 @@ V8_BASE_EXPORT void SetPrintStackTrace(void (*print_stack_trace_)());
 
 // 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<decltype(lhs), decltype(rhs)>( \
-                (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<              \
+            typename v8::base::pass_value_or_ref<decltype(lhs)>::type,  \
+            typename v8::base::pass_value_or_ref<decltype(rhs)>::type>( \
+            (lhs), (rhs), #lhs " " #op " " #rhs)) {                     \
+      V8_Fatal(__FILE__, __LINE__, "Check failed: %s.", _msg->c_str()); \
+      delete _msg;                                                      \
+    }                                                                   \
   } while (0)
 
 #define DCHECK_OP(name, op, lhs, rhs)                                         \
   do {                                                                        \
-    if (std::string* _msg =                                                   \
-            ::v8::base::Check##name##Impl<decltype(lhs), decltype(rhs)>(      \
-                (lhs), (rhs), #lhs " " #op " " #rhs)) {                       \
+    if (std::string* _msg = ::v8::base::Check##name##Impl<                    \
+            typename v8::base::pass_value_or_ref<decltype(lhs)>::type,        \
+            typename v8::base::pass_value_or_ref<decltype(rhs)>::type>(       \
+            (lhs), (rhs), #lhs " " #op " " #rhs)) {                           \
       V8_Fatal(__FILE__, __LINE__, "Debug check failed: %s.", _msg->c_str()); \
       delete _msg;                                                            \
     }                                                                         \
@@ -91,24 +94,17 @@ V8_BASE_EXPORT void SetPrintStackTrace(void (*print_stack_trace_)());
 // Make all CHECK functions discard their log strings to reduce code
 // bloat for official release builds.
 
-#define CHECK_OP(name, op, lhs, rhs)                                         \
-  do {                                                                       \
-    bool _cmp =                                                              \
-        ::v8::base::Cmp##name##Impl<decltype(lhs), decltype(rhs)>(lhs, rhs); \
-    CHECK_WITH_MSG(_cmp, #lhs " " #op " " #rhs);                             \
+#define CHECK_OP(name, op, lhs, rhs)                                       \
+  do {                                                                     \
+    bool _cmp = ::v8::base::Cmp##name##Impl<                               \
+        typename v8::base::pass_value_or_ref<decltype(lhs)>::type,         \
+        typename v8::base::pass_value_or_ref<decltype(rhs)>::type>((lhs),  \
+                                                                   (rhs)); \
+    CHECK_WITH_MSG(_cmp, #lhs " " #op " " #rhs);                           \
   } while (0)
 
 #endif
 
-// Helper to determine how to pass values: Pass scalars and arrays by value,
-// others by const reference. std::decay<T> provides the type which should be
-// used to pass T by value, e.g. converts array to pointer and removes const,
-// volatile and reference.
-template <typename T>
-struct PassType : public std::conditional<
-                      std::is_scalar<typename std::decay<T>::type>::value,
-                      typename std::decay<T>::type, T const&> {};
-
 template <typename Op>
 void PrintCheckOperand(std::ostream& os, Op op) {
   os << op;
@@ -135,9 +131,7 @@ DEFINE_PRINT_CHECK_OPERAND_CHAR(unsigned char)
 // 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(typename PassType<Lhs>::type lhs,
-                               typename PassType<Rhs>::type rhs,
-                               char const* msg) {
+std::string* MakeCheckOpString(Lhs lhs, Rhs rhs, char const* msg) {
   std::ostringstream ss;
   ss << msg << " (";
   PrintCheckOperand(ss, lhs);
@@ -235,18 +229,18 @@ DEFINE_SIGNED_MISMATCH_COMP(is_unsigned_vs_signed, GE, CmpLEImpl(rhs, lhs))
   V8_INLINE                                                                    \
       typename std::enable_if<!is_signed_vs_unsigned<Lhs, Rhs>::value &&       \
                                   !is_unsigned_vs_signed<Lhs, Rhs>::value,     \
-                              bool>::type                                      \
-          Cmp##NAME##Impl(typename PassType<Lhs>::type lhs,                    \
-                          typename PassType<Rhs>::type rhs) {                  \
+                              bool>::type Cmp##NAME##Impl(Lhs lhs, Rhs rhs) {  \
     return lhs op rhs;                                                         \
   }                                                                            \
   template <typename Lhs, typename Rhs>                                        \
-  V8_INLINE std::string* Check##NAME##Impl(typename PassType<Lhs>::type lhs,   \
-                                           typename PassType<Rhs>::type rhs,   \
+  V8_INLINE std::string* Check##NAME##Impl(Lhs lhs, Rhs rhs,                   \
                                            char const* msg) {                  \
-    bool cmp = Cmp##NAME##Impl<Lhs, Rhs>(lhs, rhs);                            \
-    return V8_LIKELY(cmp) ? nullptr                                            \
-                          : MakeCheckOpString<Lhs, Rhs>(lhs, rhs, msg);        \
+    using LhsPassT = typename pass_value_or_ref<Lhs>::type;                    \
+    using RhsPassT = typename pass_value_or_ref<Rhs>::type;                    \
+    bool cmp = Cmp##NAME##Impl<LhsPassT, RhsPassT>(lhs, rhs);                  \
+    return V8_LIKELY(cmp)                                                      \
+               ? nullptr                                                       \
+               : MakeCheckOpString<LhsPassT, RhsPassT>(lhs, rhs, msg);         \
   }                                                                            \
   extern template V8_BASE_EXPORT std::string* Check##NAME##Impl<float, float>( \
       float lhs, float rhs, char const* msg);                                  \

src/base/template-utils.h

@@ -62,6 +62,22 @@ A implicit_cast(A x) {
   return x;
 }
 
+// Helper to determine how to pass values: Pass scalars and arrays by value,
+// others by const reference (even if it was a non-const ref before; this is
+// disallowed by the style guide anyway).
+// The default is to also remove array extends (int[5] -> int*), but this can be
+// disabled by setting {remove_array_extend} to false.
+template <typename T, bool remove_array_extend = true>
+struct pass_value_or_ref {
+  using noref_t = typename std::remove_reference<T>::type;
+  using decay_t = typename std::conditional<
+      std::is_array<noref_t>::value && !remove_array_extend, noref_t,
+      typename std::decay<noref_t>::type>::type;
+  using type = typename std::conditional<std::is_scalar<decay_t>::value ||
+                                             std::is_array<decay_t>::value,
+                                         decay_t, const decay_t&>::type;
+};
+
 }  // namespace base
 }  // namespace v8
 

test/unittests/BUILD.gn

@@ -37,6 +37,7 @@ v8_executable("unittests") {
     "base/platform/semaphore-unittest.cc",
     "base/platform/time-unittest.cc",
     "base/sys-info-unittest.cc",
+    "base/template-utils-unittest.cc",
     "base/utils/random-number-generator-unittest.cc",
     "cancelable-tasks-unittest.cc",
     "char-predicates-unittest.cc",

test/unittests/base/template-utils-unittest.cc

+// Copyright 2017 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "src/base/template-utils.h"
+
+#include "test/unittests/test-utils.h"
+
+namespace v8 {
+namespace base {
+
+////////////////////////////
+// Test make_array.
+////////////////////////////
+
+namespace {
+template <typename T, size_t Size>
+void CheckArrayEquals(const std::array<T, Size>& arr1,
+                      const std::array<T, Size>& arr2) {
+  for (size_t i = 0; i < Size; ++i) {
+    CHECK_EQ(arr1[i], arr2[i]);
+  }
+}
+}  // namespace
+
+TEST(TemplateUtilsTest, MakeArraySimple) {
+  auto computed_array = base::make_array<3>([](int i) { return 1 + (i * 3); });
+  std::array<int, 3> expected{{1, 4, 7}};
+  CheckArrayEquals(computed_array, expected);
+}
+
+namespace {
+constexpr int doubleIntValue(int i) { return i * 2; }
+};  // namespace
+
+TEST(TemplateUtilsTest, MakeArrayConstexpr) {
+  constexpr auto computed_array = base::make_array<3>(doubleIntValue);
+  constexpr std::array<int, 3> expected{{0, 2, 4}};
+  CheckArrayEquals(computed_array, expected);
+}
+
+////////////////////////////
+// Test pass_value_or_ref.
+////////////////////////////
+
+// Wrap into this helper struct, such that the type is printed on errors.
+template <typename T1, typename T2>
+struct CheckIsSame {
+  static_assert(std::is_same<T1, T2>::value, "test failure");
+};
+
+#define TEST_PASS_VALUE_OR_REF0(remove_extend, expected, given)               \
+  static_assert(                                                              \
+      sizeof(CheckIsSame<expected,                                            \
+                         pass_value_or_ref<given, remove_extend>::type>) > 0, \
+      "check")
+
+#define TEST_PASS_VALUE_OR_REF(expected, given)                          \
+  static_assert(                                                         \
+      sizeof(CheckIsSame<expected, pass_value_or_ref<given>::type>) > 0, \
+      "check")
+
+TEST_PASS_VALUE_OR_REF(int, int&);
+TEST_PASS_VALUE_OR_REF(int, int&&);
+TEST_PASS_VALUE_OR_REF(const char*, const char[14]);
+TEST_PASS_VALUE_OR_REF(const char*, const char*&&);
+TEST_PASS_VALUE_OR_REF(const char*, const char (&)[14]);
+TEST_PASS_VALUE_OR_REF(const std::string&, std::string);
+TEST_PASS_VALUE_OR_REF(const std::string&, std::string&);
+TEST_PASS_VALUE_OR_REF(const std::string&, const std::string&);
+TEST_PASS_VALUE_OR_REF(int, const int);
+TEST_PASS_VALUE_OR_REF(int, const int&);
+TEST_PASS_VALUE_OR_REF(const int*, const int*);
+TEST_PASS_VALUE_OR_REF(const int*, const int* const);
+TEST_PASS_VALUE_OR_REF0(false, const char[14], const char[14]);
+TEST_PASS_VALUE_OR_REF0(false, const char[14], const char (&)[14]);
+TEST_PASS_VALUE_OR_REF0(false, const std::string&, std::string);
+TEST_PASS_VALUE_OR_REF0(false, const std::string&, std::string&);
+TEST_PASS_VALUE_OR_REF0(false, const std::string&, const std::string&);
+TEST_PASS_VALUE_OR_REF0(false, int, const int);
+TEST_PASS_VALUE_OR_REF0(false, int, const int&);
+
+}  // namespace base
+}  // namespace v8

test/unittests/unittests.gyp

@@ -34,6 +34,7 @@
       'base/platform/semaphore-unittest.cc',
       'base/platform/time-unittest.cc',
       'base/sys-info-unittest.cc',
+      'base/template-utils-unittest.cc',
       'base/utils/random-number-generator-unittest.cc',
       'cancelable-tasks-unittest.cc',
       'char-predicates-unittest.cc',