chore: fix some clang-tidy warnings (#5009)

Author: romange

.clang-tidy

@@ -12,6 +12,7 @@ Checks: >
   performance*,
   cert*,
   -cert-err58-cpp,
+  -cert-dcl58-cpp,  # Ignore std changes
   # Doesn't work with abseil flags
   clang-analyzer*,
   google-*,
@@ -81,4 +82,9 @@ Checks: >
 # modernize-use-nullptr,
 # modernize-use-equals-default,
 # readability-qualified-auto,
-cppcoreguidelines-narrowing-conversions.WarnOnIntegerNarrowingConversion: 'false'
+
+CheckOptions:
+  - key: bugprone-narrowing-conversions.WarnOnIntegerNarrowingConversion
+    value: false
+  - key: bugprone-narrowing-conversions.WarnOnEquivalentBitWidth
+    value: false

src/facade/facade_types.h

@@ -63,7 +63,8 @@ struct ArgRange {
   ArgRange(ArgRange& range) : ArgRange((const ArgRange&)range) {
   }
 
-  template <typename T> ArgRange(T&& span) : span(std::forward<T>(span)) {
+  template <typename T, std::enable_if_t<!std::is_same_v<ArgRange, T>, bool> = true>
+  ArgRange(T&& span) : span(std::forward<T>(span)) {  // NOLINT google-explicit-constructor)
   }
 
   size_t Size() const {
@@ -163,7 +164,9 @@ struct ErrorReply {
                       std::string_view kind = {})  // to resolve ambiguity of constructors above
       : message{std::string_view{msg}}, kind{kind} {
   }
-  ErrorReply(OpStatus status) : message{}, kind{}, status{status} {
+
+  ErrorReply(OpStatus status)  // NOLINT google-explicit-constructor)
+      : status{status} {
   }
 
   std::string_view ToSv() const {

src/server/list_family.cc

@@ -324,7 +324,7 @@ OpResult<string> Peek(const OpArgs& op_args, string_view key, ListDir dir, bool
 }
 
 OpResult<uint32_t> OpPush(const OpArgs& op_args, std::string_view key, ListDir dir,
-                          bool skip_notexist, facade::ArgRange vals, bool journal_rewrite) {
+                          bool skip_notexist, const facade::ArgRange& vals, bool journal_rewrite) {
   EngineShard* es = op_args.shard;
   DbSlice::ItAndUpdater res;
 
@@ -666,7 +666,7 @@ OpResult<int> OpInsert(const OpArgs& op_args, string_view key, string_view pivot
     QList* ql = GetQLV2(pv);
     QList::InsertOpt insert_opt = (insert_param == INSERT_BEFORE) ? QList::BEFORE : QList::AFTER;
     if (ql->Insert(pivot, elem, insert_opt)) {
-      res = ql->Size();
+      res = int(ql->Size());
     }
   } else {
     quicklist* ql = GetQL(pv);
@@ -688,7 +688,7 @@ OpResult<int> OpInsert(const OpArgs& op_args, string_view key, string_view pivot
         DCHECK_EQ(INSERT_BEFORE, insert_param);
         quicklistInsertBefore(qiter, &entry, elem.data(), elem.size());
       }
-      res = quicklistCount(ql);
+      res = int(quicklistCount(ql));
     }
     quicklistReleaseIterator(qiter);
   }
@@ -809,7 +809,7 @@ OpStatus OpTrim(const OpArgs& op_args, string_view key, long start, long end) {
 
   auto it = it_res->it;
 
-  long llen = it->second.Size();
+  long llen = long(it->second.Size());
 
   /* convert negative indexes */
   if (start < 0)
@@ -1324,27 +1324,27 @@ void ListFamily::LMPop(CmdArgList args, const CommandContext& cmd_cntx) {
 }
 
 void ListFamily::LPush(CmdArgList args, const CommandContext& cmd_cntx) {
-  return PushGeneric(ListDir::LEFT, false, std::move(args), cmd_cntx.tx, cmd_cntx.rb);
+  return PushGeneric(ListDir::LEFT, false, args, cmd_cntx.tx, cmd_cntx.rb);
 }
 
 void ListFamily::LPushX(CmdArgList args, const CommandContext& cmd_cntx) {
-  return PushGeneric(ListDir::LEFT, true, std::move(args), cmd_cntx.tx, cmd_cntx.rb);
+  return PushGeneric(ListDir::LEFT, true, args, cmd_cntx.tx, cmd_cntx.rb);
 }
 
 void ListFamily::LPop(CmdArgList args, const CommandContext& cmd_cntx) {
-  return PopGeneric(ListDir::LEFT, std::move(args), cmd_cntx.tx, cmd_cntx.rb);
+  return PopGeneric(ListDir::LEFT, args, cmd_cntx.tx, cmd_cntx.rb);
 }
 
 void ListFamily::RPush(CmdArgList args, const CommandContext& cmd_cntx) {
-  return PushGeneric(ListDir::RIGHT, false, std::move(args), cmd_cntx.tx, cmd_cntx.rb);
+  return PushGeneric(ListDir::RIGHT, false, args, cmd_cntx.tx, cmd_cntx.rb);
 }
 
 void ListFamily::RPushX(CmdArgList args, const CommandContext& cmd_cntx) {
-  return PushGeneric(ListDir::RIGHT, true, std::move(args), cmd_cntx.tx, cmd_cntx.rb);
+  return PushGeneric(ListDir::RIGHT, true, args, cmd_cntx.tx, cmd_cntx.rb);
 }
 
 void ListFamily::RPop(CmdArgList args, const CommandContext& cmd_cntx) {
-  return PopGeneric(ListDir::RIGHT, std::move(args), cmd_cntx.tx, cmd_cntx.rb);
+  return PopGeneric(ListDir::RIGHT, args, cmd_cntx.tx, cmd_cntx.rb);
 }
 
 void ListFamily::LLen(CmdArgList args, const CommandContext& cmd_cntx) {
@@ -1563,11 +1563,11 @@ void ListFamily::LSet(CmdArgList args, const CommandContext& cmd_cntx) {
 }
 
 void ListFamily::BLPop(CmdArgList args, const CommandContext& cmd_cntx) {
-  BPopGeneric(ListDir::LEFT, std::move(args), cmd_cntx.tx, cmd_cntx.rb, cmd_cntx.conn_cntx);
+  BPopGeneric(ListDir::LEFT, args, cmd_cntx.tx, cmd_cntx.rb, cmd_cntx.conn_cntx);
 }
 
 void ListFamily::BRPop(CmdArgList args, const CommandContext& cmd_cntx) {
-  BPopGeneric(ListDir::RIGHT, std::move(args), cmd_cntx.tx, cmd_cntx.rb, cmd_cntx.conn_cntx);
+  BPopGeneric(ListDir::RIGHT, args, cmd_cntx.tx, cmd_cntx.rb, cmd_cntx.conn_cntx);
 }
 
 void ListFamily::LMove(CmdArgList args, const CommandContext& cmd_cntx) {

src/server/main_service.cc

@@ -287,10 +287,10 @@ void DispatchMonitor(ConnectionContext* cntx, const CommandId* cid, CmdArgList t
 
 class InterpreterReplier : public RedisReplyBuilder {
  public:
-  InterpreterReplier(ObjectExplorer* explr) : RedisReplyBuilder(nullptr), explr_(explr) {
+  explicit InterpreterReplier(ObjectExplorer* explr) : RedisReplyBuilder(nullptr), explr_(explr) {
   }
 
-  void SendError(std::string_view str, std::string_view type = std::string_view{}) final;
+  void SendError(std::string_view str, std::string_view type) final;
 
   void SendBulkString(std::string_view str) final;
   void SendSimpleString(std::string_view str) final;
@@ -313,7 +313,7 @@ class InterpreterReplier : public RedisReplyBuilder {
 // Serialized result of script invocation to Redis protocol
 class EvalSerializer : public ObjectExplorer {
  public:
-  EvalSerializer(RedisReplyBuilder* rb) : rb_(rb) {
+  explicit EvalSerializer(RedisReplyBuilder* rb) : rb_(rb) {
   }
 
   void OnBool(bool b) final {
@@ -443,11 +443,8 @@ void InterpreterReplier::StartCollection(unsigned len, CollectionType type) {
 }
 
 bool IsSHA(string_view str) {
-  for (auto c : str) {
-    if (!absl::ascii_isxdigit(c))
-      return false;
-  }
-  return true;
+  return std::all_of(str.begin(), str.end(),
+                     [](unsigned char c) { return absl::ascii_isxdigit(c); });
 }
 
 optional<ErrorReply> EvalValidator(CmdArgList args) {
@@ -2102,12 +2099,8 @@ bool CheckWatchedKeyExpiry(ConnectionContext* cntx, const CommandId* exists_cid,
 
 // Check if exec_info watches keys on dbs other than db_indx.
 bool IsWatchingOtherDbs(DbIndex db_indx, const ConnectionState::ExecInfo& exec_info) {
-  for (const auto& [key_db, _] : exec_info.watched_keys) {
-    if (key_db != db_indx) {
-      return true;
-    }
-  }
-  return false;
+  return std::any_of(exec_info.watched_keys.begin(), exec_info.watched_keys.end(),
+                     [db_indx](const auto& pair) { return pair.first != db_indx; });
 }
 
 template <typename F> void IterateAllKeys(ConnectionState::ExecInfo* exec_info, F&& f) {
@@ -2275,7 +2268,7 @@ void SubscribeImpl(bool reject_cluster, CmdArgList args, const CommandContext& c
   if (reject_cluster && IsClusterEnabled()) {
     return cmd_cntx.rb->SendError("SUBSCRIBE is not supported in cluster mode yet");
   }
-  cmd_cntx.conn_cntx->ChangeSubscription(true /*add*/, true /* reply*/, std::move(args),
+  cmd_cntx.conn_cntx->ChangeSubscription(true /*add*/, true /* reply*/, args,
                                          static_cast<RedisReplyBuilder*>(cmd_cntx.rb));
 }