Fix cppcoreguidelines-macro-usage warning reported by clang-tidy (#1155)

.clang-tidy

@@ -1,7 +1,7 @@
 # refer to https://clang.llvm.org/extra/clang-tidy/checks/list.html
 Checks: -*, clang-analyzer-core.*, clang-analyzer-cplusplus.*, clang-analyzer-deadcode.*, clang-analyzer-nullability.*, clang-analyzer-security.*, clang-analyzer-unix.*, clang-analyzer-valist.*, cppcoreguidelines-init-variables, cppcoreguidelines-macro-usage, cppcoreguidelines-interfaces-global-init, cppcoreguidelines-narrowing-conversions, cppcoreguidelines-no-malloc, cppcoreguidelines-prefer-member-initializer, cppcoreguidelines-special-member-functions, cppcoreguidelines-slicing, google-build-explicit-make-pair, google-default-arguments, google-explicit-constructor, modernize-avoid-bind, modernize-loop-convert, modernize-macro-to-enum, modernize-make-shared, modernize-make-unique, modernize-pass-by-value, modernize-redundant-void-arg, modernize-return-braced-init-list, modernize-use-auto, modernize-use-bool-literals, modernize-use-emplace, modernize-use-equals-default, modernize-use-equals-delete, modernize-use-nullptr, modernize-use-override, modernize-use-using, performance-faster-string-find, performance-for-range-copy, performance-implicit-conversion-in-loop, performance-inefficient-algorithm, performance-inefficient-vector-operation, performance-move-const-arg, performance-move-constructor-init, performance-no-automatic-move, performance-trivially-destructible, performance-type-promotion-in-math-fn, performance-unnecessary-copy-initialization, performance-unnecessary-value-param
 
-WarningsAsErrors: clang-analyzer-*, -clang-analyzer-security.insecureAPI.rand, cppcoreguidelines-interfaces-global-init, cppcoreguidelines-no-malloc, cppcoreguidelines-slicing, google-*, modernize-use-emplace, modernize-use-equals-default, modernize-use-equals-delete, performance-implicit-conversion-in-loop, performance-inefficient-algorithm, performance-move-constructor-init, performance-no-automatic-move, performance-trivially-destructible, performance-type-promotion-in-math-fn, performance-unnecessary-copy-initialization, modernize-use-bool-literals, performance-unnecessary-value-param, modernize-make-unique, performance-for-range-copy, performance-faster-string-find, modernize-redundant-void-arg, modernize-avoid-bind, modernize-use-auto, modernize-use-using, performance-inefficient-vector-operation, cppcoreguidelines-special-member-functions, modernize-loop-convert, cppcoreguidelines-init-variables, modernize-use-nullptr
+WarningsAsErrors: clang-analyzer-*, -clang-analyzer-security.insecureAPI.rand, cppcoreguidelines-interfaces-global-init, cppcoreguidelines-no-malloc, cppcoreguidelines-slicing, google-*, modernize-use-emplace, modernize-use-equals-default, modernize-use-equals-delete, performance-implicit-conversion-in-loop, performance-inefficient-algorithm, performance-move-constructor-init, performance-no-automatic-move, performance-trivially-destructible, performance-type-promotion-in-math-fn, performance-unnecessary-copy-initialization, modernize-use-bool-literals, performance-unnecessary-value-param, modernize-make-unique, performance-for-range-copy, performance-faster-string-find, modernize-redundant-void-arg, modernize-avoid-bind, modernize-use-auto, modernize-use-using, performance-inefficient-vector-operation, cppcoreguidelines-special-member-functions, modernize-loop-convert, cppcoreguidelines-init-variables, modernize-use-nullptr, cppcoreguidelines-macro-usage
 
 CheckOptions:
   - key:           cppcoreguidelines-special-member-functions.AllowSoleDefaultDtor

src/common/io_util.cc

@@ -44,10 +44,10 @@
 #define POLLNVAL 0x0020 /* Invalid request: fd not open */
 #endif
 
-#define AE_READABLE 1
-#define AE_WRITABLE 2
-#define AE_ERROR 4
-#define AE_HUP 8
+#define AE_READABLE 1  // NOLINT
+#define AE_WRITABLE 2  // NOLINT
+#define AE_ERROR 4     // NOLINT
+#define AE_HUP 8       // NOLINT
 
 namespace Util {
 Status SockConnect(const std::string &host, uint32_t port, int *fd) {

src/common/sha1.cc

@@ -43,34 +43,43 @@ A million repetitions of "a"
 
 #define SHA1HANDSOFF
 
+// NOLINTNEXTLINE
 #define rol(value, bits) (((value) << (bits)) | ((value) >> (32 - (bits))))
 
 /* blk0() and blk() perform the initial expand. */
 /* I got the idea of expanding during the round function from SSLeay */
 #if BYTE_ORDER == LITTLE_ENDIAN
+// NOLINTNEXTLINE
 #define blk0(i) (block->l[i] = (rol(block->l[i], 24) & 0xFF00FF00) | (rol(block->l[i], 8) & 0x00FF00FF))
 #elif BYTE_ORDER == BIG_ENDIAN
+// NOLINTNEXTLINE
 #define blk0(i) block->l[i]
 #else
 #error "Endianness not defined!"
 #endif
+// NOLINTNEXTLINE
 #define blk(i)        \
   (block->l[i & 15] = \
        rol(block->l[(i + 13) & 15] ^ block->l[(i + 8) & 15] ^ block->l[(i + 2) & 15] ^ block->l[i & 15], 1))
 
 /* (R0+R1), R2, R3, R4 are the different operations used in SHA1 */
+// NOLINTNEXTLINE
 #define R0(v, w, x, y, z, i)                                   \
   z += ((w & (x ^ y)) ^ y) + blk0(i) + 0x5A827999 + rol(v, 5); \
   w = rol(w, 30);
+// NOLINTNEXTLINE
 #define R1(v, w, x, y, z, i)                                  \
   z += ((w & (x ^ y)) ^ y) + blk(i) + 0x5A827999 + rol(v, 5); \
   w = rol(w, 30);
+// NOLINTNEXTLINE
 #define R2(v, w, x, y, z, i)                          \
   z += (w ^ x ^ y) + blk(i) + 0x6ED9EBA1 + rol(v, 5); \
   w = rol(w, 30);
+// NOLINTNEXTLINE
 #define R3(v, w, x, y, z, i)                                            \
   z += (((w | x) & y) | ((w) & (x))) + blk(i) + 0x8F1BBCDC + rol(v, 5); \
   w = rol(w, 30);
+// NOLINTNEXTLINE
 #define R4(v, w, x, y, z, i)                          \
   z += (w ^ x ^ y) + blk(i) + 0xCA62C1D6 + rol(v, 5); \
   w = rol(w, 30);

src/common/status.h

@@ -121,7 +121,7 @@ template <typename T>
 struct IsStatusOr<StatusOr<T>> : std::integral_constant<bool, true> {};
 
 template <typename T>
-struct StatusOr {
+struct StatusOr {  // NOLINT
   static_assert(!std::is_same<T, Status>::value, "value_type cannot be Status");
   static_assert(!std::is_same<T, Status::Code>::value, "value_type cannot be Status::Code");
   static_assert(!IsStatusOr<T>::value, "value_type cannot be StatusOr");
@@ -150,14 +150,14 @@ struct StatusOr {
                                      !std::is_same<Code, remove_cvref_t<first_element<Ts...>>>::value &&
                                      !std::is_same<StatusOr, remove_cvref_t<first_element<Ts...>>>::value),
                                     int>::type = 0>  // NOLINT
-  StatusOr(Ts&&... args) : code_(Code::cOK) {
+  StatusOr(Ts&&... args) : code_(Code::cOK) {        // NOLINT
     new (&value_) value_type(std::forward<Ts>(args)...);
   }
 
   StatusOr(const StatusOr&) = delete;
 
   template <typename U, typename std::enable_if<std::is_convertible<U, T>::value, int>::type = 0>
-  StatusOr(StatusOr<U>&& other) : code_(other.code_) {
+  StatusOr(StatusOr<U>&& other) : code_(other.code_) {  // NOLINT
     if (code_ == Code::cOK) {
       new (&value_) value_type(std::move(other.value_));
     } else {
@@ -166,7 +166,7 @@ struct StatusOr {
   }
 
   template <typename U, typename std::enable_if<!std::is_convertible<U, T>::value, int>::type = 0>
-  StatusOr(StatusOr<U>&& other) : code_(other.code_) {
+  StatusOr(StatusOr<U>&& other) : code_(other.code_) {  // NOLINT
     CHECK(code_ != Code::cOK);
     new (&error_) error_type(std::move(other.error_));
   }
@@ -191,9 +191,9 @@ struct StatusOr {
     return Status(code_, std::move(*error_));
   }
 
-  operator Status() const& { return ToStatus(); }
+  operator Status() const& { return ToStatus(); }  // NOLINT
 
-  operator Status() && { return std::move(*this).ToStatus(); }
+  operator Status() && { return std::move(*this).ToStatus(); }  // NOLINT
 
   Code GetCode() const { return code_; }
 
@@ -283,6 +283,7 @@ struct StatusOr {
   friend struct StatusOr;
 };
 
+// NOLINTNEXTLINE
 #define GET_OR_RET(...)                                         \
   ({                                                            \
     auto&& status = (__VA_ARGS__);                              \

src/main.cc

@@ -27,7 +27,7 @@
 #include <iomanip>
 #include <ostream>
 #ifdef __linux__
-#define _XOPEN_SOURCE 700
+#define _XOPEN_SOURCE 700  // NOLINT
 #else
 #define _XOPEN_SOURCE
 #endif

src/storage/scripting.cc

@@ -66,7 +66,7 @@
 /* The maximum number of characters needed to represent a long double
  * as a string (long double has a huge range).
  * This should be the size of the buffer given to doule to string */
-#define MAX_LONG_DOUBLE_CHARS 5 * 1024
+constexpr size_t MAX_LONG_DOUBLE_CHARS = 5 * 1024;
 
 enum {
   LL_DEBUG = 0,
@@ -315,13 +315,13 @@ Status evalGenericCommand(Redis::Connection *conn, const std::vector<std::string
     lua_pop(lua, 1);
   }
 
-/* Call the Lua garbage collector from time to time to avoid a
- * full cycle performed by Lua, which adds too latency.
- *
- * The call is performed every LUA_GC_CYCLE_PERIOD executed commands
- * (and for LUA_GC_CYCLE_PERIOD collection steps) because calling it
- * for every command uses too much CPU. */
-#define LUA_GC_CYCLE_PERIOD 50
+  /* Call the Lua garbage collector from time to time to avoid a
+   * full cycle performed by Lua, which adds too latency.
+   *
+   * The call is performed every LUA_GC_CYCLE_PERIOD executed commands
+   * (and for LUA_GC_CYCLE_PERIOD collection steps) because calling it
+   * for every command uses too much CPU. */
+  constexpr int64_t LUA_GC_CYCLE_PERIOD = 50;
   {
     static int64_t gc_count = 0;
 

src/types/geohash.cc

@@ -59,12 +59,7 @@
 
 #include <math.h>
 
-#define D_R (M_PI / 180.0)
-#define R_MAJOR 6378137.0
-#define R_MINOR 6356752.3142
-#define RATIO (R_MINOR / R_MAJOR)
-#define ECCENT (sqrt(1.0 - (RATIO * RATIO)))
-#define COM (0.5 * ECCENT)
+constexpr double D_R = M_PI / 180.0;
 
 // @brief The usual PI/180 constant
 // const double DEG_TO_RAD = 0.017453292519943295769236907684886;

src/types/geohash.h

@@ -60,21 +60,13 @@
 #include <stddef.h>
 #include <stdint.h>
 
-#define HASHISZERO(r) (!(r).bits && !(r).step)
-#define RANGEISZERO(r) (!(r).max && !(r).min)
-#define RANGEPISZERO(r) (r == NULL || RANGEISZERO(*r))
-
-#define GEO_STEP_MAX 26 /* 26*2 = 52 bits. */
+constexpr uint8_t GEO_STEP_MAX = 26; /* 26*2 = 52 bits. */
 
 /* Limits from EPSG:900913 / EPSG:3785 / OSGEO:41001 */
-#define GEO_LAT_MIN -85.05112878
-#define GEO_LAT_MAX 85.05112878
-#define GEO_LONG_MIN -180
-#define GEO_LONG_MAX 180
-
-#define GZERO(s) s.bits = s.step = 0;
-#define GISZERO(s) (!s.bits && !s.step)
-#define GISNOTZERO(s) (s.bits || s.step)
+constexpr double GEO_LAT_MIN = -85.05112878;
+constexpr double GEO_LAT_MAX = 85.05112878;
+constexpr double GEO_LONG_MIN = -180;
+constexpr double GEO_LONG_MAX = 180;
 
 enum GeoDirection {
   GEOHASH_NORTH = 0,
@@ -122,6 +114,14 @@ struct GeoHashRadius {
   GeoHashNeighbors neighbors;
 };
 
+inline constexpr bool HASHISZERO(const GeoHashBits &r) { return !r.bits && !r.step; }
+inline constexpr bool RANGEISZERO(const GeoHashRange &r) { return !r.max && !r.min; }
+inline constexpr bool RANGEPISZERO(const GeoHashRange *r) { return !r || RANGEISZERO(*r); }
+
+inline constexpr void GZERO(GeoHashBits &s) { s.bits = s.step = 0; }
+inline constexpr bool GISZERO(const GeoHashBits &s) { return (!s.bits && !s.step); }
+inline constexpr bool GISNOTZERO(const GeoHashBits &s) { return (s.bits || s.step); }
+
 /*
  * 0:success
  * -1:failed