Revived google-readability-casting (grpc#31607)

* Added google-readability-casting * Fix * Use brace init for constants * more * Fix * Fix 2 * More fix * Fixf
dmeister · Nov 12, 2022 · 250f9fc · 250f9fc
1 parent e8ac147
commit 250f9fc
Show file tree

Hide file tree

Showing 54 changed files with 185 additions and 175 deletions.
diff --git a/.clang-tidy b/.clang-tidy
@@ -31,7 +31,6 @@
 # - clang-diagnostic-deprecated-declarations
 # - clang-diagnostic-unused-function
 # - google-readability-avoid-underscore-in-googletest-name
-# - google-readability-casting
 # - google-runtime-int
 # - google-runtime-references
 # - modernize-avoid-bind
@@ -72,7 +71,6 @@ Checks: '-*,
   -bugprone-unchecked-optional-access,
   google-*,
   -google-readability-avoid-underscore-in-googletest-name,
-  -google-readability-casting,
   -google-runtime-int,
   -google-runtime-references,
   -google-upgrade-googletest-case,

diff --git a/include/grpcpp/completion_queue.h b/include/grpcpp/completion_queue.h
@@ -379,15 +379,14 @@ class CompletionQueue : private grpc::GrpcLibraryCodegen {
   /// registration must take place before CQ shutdown (which must be maintained
   /// elsehwere)
   void InitialAvalanching() {
-    gpr_atm_rel_store(&avalanches_in_flight_, static_cast<gpr_atm>(1));
+    gpr_atm_rel_store(&avalanches_in_flight_, gpr_atm{1});
   }
   void RegisterAvalanching() {
-    gpr_atm_no_barrier_fetch_add(&avalanches_in_flight_,
-                                 static_cast<gpr_atm>(1));
+    gpr_atm_no_barrier_fetch_add(&avalanches_in_flight_, gpr_atm{1});
   }
   void CompleteAvalanching() {
-    if (gpr_atm_no_barrier_fetch_add(&avalanches_in_flight_,
-                                     static_cast<gpr_atm>(-1)) == 1) {
+    if (gpr_atm_no_barrier_fetch_add(&avalanches_in_flight_, gpr_atm{-1}) ==
+        1) {
       grpc::g_core_codegen_interface->grpc_completion_queue_shutdown(cq_);
     }
   }

diff --git a/src/core/ext/filters/channel_idle/idle_filter_state.h b/src/core/ext/filters/channel_idle/idle_filter_state.h
@@ -58,7 +58,7 @@ class IdleFilterState {
   static constexpr uintptr_t kCallsInProgressShift = 2;
   // How much to increment/decrement the state_ when a call is started/stopped.
   // Ensures we don't clobber the preceding bits.
-  static constexpr uintptr_t kCallIncrement = uintptr_t(1)
+  static constexpr uintptr_t kCallIncrement = uintptr_t{1}
                                               << kCallsInProgressShift;
   std::atomic<uintptr_t> state_;
 };

diff --git a/src/core/ext/filters/client_channel/lb_policy/grpclb/load_balancer_api.cc b/src/core/ext/filters/client_channel/lb_policy/grpclb/load_balancer_api.cc
@@ -65,7 +65,7 @@ grpc_slice GrpcLbRequestCreate(const char* lb_service_name, upb_Arena* arena) {
   grpc_lb_v1_InitialLoadBalanceRequest* initial_request =
       grpc_lb_v1_LoadBalanceRequest_mutable_initial_request(req, arena);
   size_t name_len = std::min(strlen(lb_service_name),
-                             size_t(GRPC_GRPCLB_SERVICE_NAME_MAX_LENGTH));
+                             size_t{GRPC_GRPCLB_SERVICE_NAME_MAX_LENGTH});
   grpc_lb_v1_InitialLoadBalanceRequest_set_name(
       initial_request,
       upb_StringView_FromDataAndSize(lb_service_name, name_len));

diff --git a/src/core/ext/filters/client_channel/lb_policy/subchannel_list.h b/src/core/ext/filters/client_channel/lb_policy/subchannel_list.h
@@ -324,6 +324,7 @@ void SubchannelData<SubchannelListType,
   pending_watcher_ =
       new Watcher(this, subchannel_list()->WeakRef(DEBUG_LOCATION, "Watcher"));
   subchannel_->WatchConnectivityState(
+      // NOLINTNEXTLINE(google-readability-casting)
       std::unique_ptr<SubchannelInterface::ConnectivityStateWatcherInterface>(
           pending_watcher_));
 }

diff --git a/src/core/ext/filters/client_channel/retry_throttle.cc b/src/core/ext/filters/client_channel/retry_throttle.cc
@@ -88,8 +88,7 @@ bool ServerRetryThrottleData::RecordFailure() {
   // We decrement milli_tokens by 1000 (1 token) for each failure.
   const uintptr_t new_value =
       static_cast<uintptr_t>(gpr_atm_no_barrier_clamped_add(
-          &throttle_data->milli_tokens_, static_cast<gpr_atm>(-1000),
-          static_cast<gpr_atm>(0),
+          &throttle_data->milli_tokens_, gpr_atm{-1000}, gpr_atm{0},
           static_cast<gpr_atm>(throttle_data->max_milli_tokens_)));
   // Retries are allowed as long as the new value is above the threshold
   // (max_milli_tokens / 2).
@@ -103,8 +102,7 @@ void ServerRetryThrottleData::RecordSuccess() {
   // We increment milli_tokens by milli_token_ratio for each success.
   gpr_atm_no_barrier_clamped_add(
       &throttle_data->milli_tokens_,
-      static_cast<gpr_atm>(throttle_data->milli_token_ratio_),
-      static_cast<gpr_atm>(0),
+      static_cast<gpr_atm>(throttle_data->milli_token_ratio_), gpr_atm{0},
       static_cast<gpr_atm>(throttle_data->max_milli_tokens_));
 }
 

diff --git a/src/core/ext/filters/fault_injection/fault_injection_filter.cc b/src/core/ext/filters/fault_injection/fault_injection_filter.cc
@@ -206,7 +206,7 @@ FaultInjectionFilter::MakeInjectionDecision(
           initial_metadata->GetStringValue(fi_policy->delay_header, &buffer);
       if (value.has_value()) {
         delay = Duration::Milliseconds(
-            std::max(AsInt<int64_t>(*value).value_or(0), int64_t(0)));
+            std::max(AsInt<int64_t>(*value).value_or(0), int64_t{0}));
       }
     }
     if (!fi_policy->delay_percentage_header.empty()) {

diff --git a/src/core/ext/transport/binder/server/binder_server.cc b/src/core/ext/transport/binder/server/binder_server.cc
@@ -174,7 +174,7 @@ class BinderServerListener : public Server::ListenerInterface {
   absl::Status OnSetupTransport(transaction_code_t code,
                                 grpc_binder::ReadableParcel* parcel, int uid) {
     ExecCtx exec_ctx;
-    if (grpc_binder::BinderTransportTxCode(code) !=
+    if (static_cast<grpc_binder::BinderTransportTxCode>(code) !=
         grpc_binder::BinderTransportTxCode::SETUP_TRANSPORT) {
       return absl::InvalidArgumentError("Not a SETUP_TRANSPORT request");
     }

diff --git a/src/core/ext/transport/binder/wire_format/wire_reader_impl.cc b/src/core/ext/transport/binder/wire_format/wire_reader_impl.cc
@@ -156,7 +156,8 @@ absl::Status WireReaderImpl::ProcessTransaction(transaction_code_t code,
 
   {
     grpc_core::MutexLock lock(&mu_);
-    if (BinderTransportTxCode(code) != BinderTransportTxCode::SETUP_TRANSPORT &&
+    if (static_cast<BinderTransportTxCode>(code) !=
+            BinderTransportTxCode::SETUP_TRANSPORT &&
         !connected_) {
       return absl::InvalidArgumentError("Transports not connected yet");
     }
@@ -165,7 +166,7 @@ absl::Status WireReaderImpl::ProcessTransaction(transaction_code_t code,
   // TODO(mingcl): See if we want to check the security policy for every RPC
   // call or just during transport setup.
 
-  switch (BinderTransportTxCode(code)) {
+  switch (static_cast<BinderTransportTxCode>(code)) {
     case BinderTransportTxCode::SETUP_TRANSPORT: {
       grpc_core::MutexLock lock(&mu_);
       if (recvd_setup_transport_) {

diff --git a/src/core/ext/transport/binder/wire_format/wire_writer.cc b/src/core/ext/transport/binder/wire_format/wire_writer.cc
@@ -126,7 +126,7 @@ absl::Status WireWriterImpl::MakeBinderTransaction(
 
 absl::Status WireWriterImpl::RpcCallFastPath(std::unique_ptr<Transaction> tx) {
   return MakeBinderTransaction(
-      BinderTransportTxCode(tx->GetTxCode()),
+      static_cast<BinderTransportTxCode>(tx->GetTxCode()),
       [this, tx = tx.get()](
           WritableParcel* parcel) ABSL_EXCLUSIVE_LOCKS_REQUIRED(write_mu_) {
         RETURN_IF_ERROR(parcel->WriteInt32(tx->GetFlags()));
@@ -244,7 +244,7 @@ void WireWriterImpl::RunScheduledTxInternal(RunScheduledTxArgs* args) {
   }
   bool is_last_chunk = true;
   absl::Status result = MakeBinderTransaction(
-      BinderTransportTxCode(stream_tx->tx->GetTxCode()),
+      static_cast<BinderTransportTxCode>(stream_tx->tx->GetTxCode()),
       [stream_tx, &is_last_chunk, this](WritableParcel* parcel)
           ABSL_EXCLUSIVE_LOCKS_REQUIRED(write_mu_) {
             return RunStreamTx(stream_tx, parcel, &is_last_chunk);
@@ -305,12 +305,12 @@ absl::Status WireWriterImpl::SendAck(int64_t num_bytes) {
     return absl::OkStatus();
   }
   // Otherwise, we can directly send ack.
-  absl::Status result = MakeBinderTransaction(
-      BinderTransportTxCode(BinderTransportTxCode::ACKNOWLEDGE_BYTES),
-      [num_bytes](WritableParcel* parcel) {
-        RETURN_IF_ERROR(parcel->WriteInt64(num_bytes));
-        return absl::OkStatus();
-      });
+  absl::Status result =
+      MakeBinderTransaction((BinderTransportTxCode::ACKNOWLEDGE_BYTES),
+                            [num_bytes](WritableParcel* parcel) {
+                              RETURN_IF_ERROR(parcel->WriteInt64(num_bytes));
+                              return absl::OkStatus();
+                            });
   if (!result.ok()) {
     gpr_log(GPR_ERROR, "Failed to make binder transaction %s",
             result.ToString().c_str());

diff --git a/src/core/ext/transport/chttp2/transport/flow_control.cc b/src/core/ext/transport/chttp2/transport/flow_control.cc
@@ -122,7 +122,7 @@ uint32_t TransportFlowControl::MaybeSendUpdate(bool writing_anyway) {
       announced_window_ != target_announced_window) {
     const uint32_t announce =
         static_cast<uint32_t>(Clamp(target_announced_window - announced_window_,
-                                    int64_t(0), kMaxWindowUpdateSize));
+                                    int64_t{0}, kMaxWindowUpdateSize));
     announced_window_ += announce;
     return announce;
   }
@@ -255,7 +255,7 @@ TransportFlowControl::TargetInitialWindowSizeBasedOnMemoryPressureAndBdp()
   //                                                                pressure
   const double kAnythingGoesPressure = 0.2;
   const double kAdjustedToBdpPressure = 0.5;
-  const double kAnythingGoesWindow = std::max(double(1 << 24), bdp);
+  const double kAnythingGoesWindow = std::max(double{1 << 24}, bdp);
   if (memory_pressure < kAnythingGoesPressure) {
     return kAnythingGoesWindow;
   } else if (memory_pressure < kAdjustedToBdpPressure) {
@@ -347,23 +347,24 @@ FlowControlAction TransportFlowControl::PeriodicUpdate() {
       }
       // Though initial window 'could' drop to 0, we keep the floor at
       // kMinInitialWindowSize
-      UpdateSetting(
-          GRPC_CHTTP2_SETTINGS_INITIAL_WINDOW_SIZE,
-          &target_initial_window_size_,
-          static_cast<int32_t>(Clamp(target, double(kMinInitialWindowSize),
-                                     double(kMaxInitialWindowSize))),
-          &action, &FlowControlAction::set_send_initial_window_update);
+      UpdateSetting(GRPC_CHTTP2_SETTINGS_INITIAL_WINDOW_SIZE,
+                    &target_initial_window_size_,
+                    static_cast<int32_t>(Clamp(
+                        target, static_cast<double>(kMinInitialWindowSize),
+                        static_cast<double>(kMaxInitialWindowSize))),
+                    &action,
+                    &FlowControlAction::set_send_initial_window_update);
       // get bandwidth estimate and update max_frame accordingly.
       double bw_dbl = bdp_estimator_.EstimateBandwidth();
       // we target the max of BDP or bandwidth in microseconds.
       UpdateSetting(
           GRPC_CHTTP2_SETTINGS_MAX_FRAME_SIZE, &target_frame_size_,
-          static_cast<int32_t>(Clamp(
-              std::max(
-                  static_cast<int32_t>(Clamp(bw_dbl, 0.0, double(INT_MAX))) /
-                      1000,
-                  static_cast<int32_t>(target_initial_window_size_)),
-              16384, 16777215)),
+          static_cast<int32_t>(
+              Clamp(std::max(static_cast<int32_t>(Clamp(
+                                 bw_dbl, 0.0, static_cast<double>(INT_MAX))) /
+                                 1000,
+                             static_cast<int32_t>(target_initial_window_size_)),
+                    16384, 16777215)),
           &action, &FlowControlAction::set_send_max_frame_size_update);
     }
 
@@ -409,7 +410,7 @@ int64_t StreamFlowControl::DesiredAnnounceSize() const {
       return std::min(min_progress_size_, kMaxWindowDelta);
     }
   }();
-  return Clamp(desired_window_delta - announced_window_delta_, int64_t(0),
+  return Clamp(desired_window_delta - announced_window_delta_, int64_t{0},
                kMaxWindowUpdateSize);
 }
 
@@ -423,9 +424,8 @@ FlowControlAction StreamFlowControl::UpdateAction(FlowControlAction action) {
     // Currently set at half the initial window size or 8kb (whichever is
     // greater). 8kb means we don't send rapidly unnecessarily when the initial
     // window size is small.
-    const int64_t hurry_up_size =
-        std::max(static_cast<int64_t>(tfc_->sent_init_window()) / 2,
-                 static_cast<int64_t>(8192));
+    const int64_t hurry_up_size = std::max(
+        static_cast<int64_t>(tfc_->sent_init_window()) / 2, int64_t{8192});
     if (desired_announce_size > hurry_up_size) {
       urgency = FlowControlAction::Urgency::UPDATE_IMMEDIATELY;
     }

diff --git a/src/core/ext/transport/chttp2/transport/frame_ping.cc b/src/core/ext/transport/chttp2/transport/frame_ping.cc
@@ -118,7 +118,7 @@ grpc_error_handle grpc_chttp2_ping_parser_parse(void* parser,
       if (!g_disable_ping_ack) {
         if (t->ping_ack_count == t->ping_ack_capacity) {
           t->ping_ack_capacity =
-              std::max(t->ping_ack_capacity * 3 / 2, size_t(3));
+              std::max(t->ping_ack_capacity * 3 / 2, size_t{3});
           t->ping_acks = static_cast<uint64_t*>(gpr_realloc(
               t->ping_acks, t->ping_ack_capacity * sizeof(*t->ping_acks)));
         }

diff --git a/src/core/ext/transport/chttp2/transport/writing.cc b/src/core/ext/transport/chttp2/transport/writing.cc
@@ -220,7 +220,7 @@ static void report_stall(grpc_chttp2_transport* t, grpc_chttp2_stream* s,
                    [GRPC_CHTTP2_SETTINGS_INITIAL_WINDOW_SIZE],
         t->flow_control.remote_window(),
         static_cast<uint32_t>(std::max(
-            int64_t(0),
+            int64_t{0},
             s->flow_control.remote_window_delta() +
                 static_cast<int64_t>(
                     t->settings[GRPC_PEER_SETTINGS]
@@ -378,7 +378,7 @@ class DataSendContext {
 
   uint32_t stream_remote_window() const {
     return static_cast<uint32_t>(std::max(
-        int64_t(0),
+        int64_t{0},
         s_->flow_control.remote_window_delta() +
             static_cast<int64_t>(
                 t_->settings[GRPC_PEER_SETTINGS]
@@ -388,15 +388,17 @@ class DataSendContext {
   uint32_t max_outgoing() const {
     return static_cast<uint32_t>(std::min(
         t_->settings[GRPC_PEER_SETTINGS][GRPC_CHTTP2_SETTINGS_MAX_FRAME_SIZE],
-        static_cast<uint32_t>(std::min(int64_t(stream_remote_window()),
-                                       t_->flow_control.remote_window()))));
+        static_cast<uint32_t>(
+            std::min(static_cast<int64_t>(stream_remote_window()),
+                     t_->flow_control.remote_window()))));
   }
 
   bool AnyOutgoing() const { return max_outgoing() > 0; }
 
   void FlushBytes() {
-    uint32_t send_bytes = static_cast<uint32_t>(
-        std::min(size_t(max_outgoing()), s_->flow_controlled_buffer.length));
+    uint32_t send_bytes =
+        static_cast<uint32_t>(std::min(static_cast<size_t>(max_outgoing()),
+                                       s_->flow_controlled_buffer.length));
     is_last_frame_ = send_bytes == s_->flow_controlled_buffer.length &&
                      s_->send_trailing_metadata != nullptr &&
                      s_->send_trailing_metadata->empty();

diff --git a/src/core/ext/xds/xds_listener.cc b/src/core/ext/xds/xds_listener.cc
@@ -628,8 +628,8 @@ absl::optional<XdsListenerResource::FilterChainMap::CidrRange> CidrRangeParse(
         google_protobuf_UInt32Value_value(prefix_len_proto),
         (reinterpret_cast<const grpc_sockaddr*>(cidr_range.address.addr))
                     ->sa_family == GRPC_AF_INET
-            ? uint32_t(32)
-            : uint32_t(128));
+            ? uint32_t{32}
+            : uint32_t{128});
   }
   // Normalize the network address by masking it with prefix_len
   grpc_sockaddr_mask_bits(&cidr_range.address, cidr_range.prefix_len);

diff --git a/src/core/lib/address_utils/sockaddr_utils.cc b/src/core/lib/address_utils/sockaddr_utils.cc
@@ -371,7 +371,7 @@ void grpc_sockaddr_mask_bits(grpc_resolved_address* address,
     } else if (mask_bits >= 32) {
       return;
     }
-    uint32_t mask_ip_addr = (~(uint32_t(0))) << (32 - mask_bits);
+    uint32_t mask_ip_addr = (~(uint32_t{0})) << (32 - mask_bits);
     addr4->sin_addr.s_addr &= grpc_htonl(mask_ip_addr);
   } else if (addr->sa_family == GRPC_AF_INET6) {
     grpc_sockaddr_in6* addr6 = reinterpret_cast<grpc_sockaddr_in6*>(addr);
@@ -387,25 +387,25 @@ void grpc_sockaddr_mask_bits(grpc_resolved_address* address,
     GPR_ASSERT(sizeof(addr6->sin6_addr) == sizeof(address_parts));
     memcpy(address_parts, &addr6->sin6_addr, sizeof(grpc_in6_addr));
     if (mask_bits <= 32) {
-      uint32_t mask_ip_addr = (~(uint32_t(0))) << (32 - mask_bits);
+      uint32_t mask_ip_addr = (~(uint32_t{0})) << (32 - mask_bits);
       address_parts[0] &= grpc_htonl(mask_ip_addr);
       memset(&address_parts[1], 0, sizeof(uint32_t));
       memset(&address_parts[2], 0, sizeof(uint32_t));
       memset(&address_parts[3], 0, sizeof(uint32_t));
     } else if (mask_bits <= 64) {
       mask_bits -= 32;
-      uint32_t mask_ip_addr = (~(uint32_t(0))) << (32 - mask_bits);
+      uint32_t mask_ip_addr = (~(uint32_t{0})) << (32 - mask_bits);
       address_parts[1] &= grpc_htonl(mask_ip_addr);
       memset(&address_parts[2], 0, sizeof(uint32_t));
       memset(&address_parts[3], 0, sizeof(uint32_t));
     } else if (mask_bits <= 96) {
       mask_bits -= 64;
-      uint32_t mask_ip_addr = (~(uint32_t(0))) << (32 - mask_bits);
+      uint32_t mask_ip_addr = (~(uint32_t{0})) << (32 - mask_bits);
       address_parts[2] &= grpc_htonl(mask_ip_addr);
       memset(&address_parts[3], 0, sizeof(uint32_t));
     } else {
       mask_bits -= 96;
-      uint32_t mask_ip_addr = (~(uint32_t(0))) << (32 - mask_bits);
+      uint32_t mask_ip_addr = (~(uint32_t{0})) << (32 - mask_bits);
       address_parts[3] &= grpc_htonl(mask_ip_addr);
     }
     memcpy(&addr6->sin6_addr, address_parts, sizeof(grpc_in6_addr));

diff --git a/src/core/lib/compression/message_compress.cc b/src/core/lib/compression/message_compress.cc
@@ -40,7 +40,7 @@ static int zlib_body(z_stream* zs, grpc_slice_buffer* input,
   int flush;
   size_t i;
   grpc_slice outbuf = GRPC_SLICE_MALLOC(OUTPUT_BLOCK_SIZE);
-  const uInt uint_max = ~static_cast<uInt>(0);
+  const uInt uint_max = ~uInt{0};
 
   GPR_ASSERT(GRPC_SLICE_LENGTH(outbuf) <= uint_max);
   zs->avail_out = static_cast<uInt> GRPC_SLICE_LENGTH(outbuf);

diff --git a/src/core/lib/event_engine/posix_engine/ev_epoll1_linux.cc b/src/core/lib/event_engine/posix_engine/ev_epoll1_linux.cc
@@ -437,9 +437,8 @@ bool Epoll1Poller::ProcessEpollEvents(int max_epoll_events_to_handle,
       was_kicked = true;
     } else {
       Epoll1EventHandle* handle = reinterpret_cast<Epoll1EventHandle*>(
-          reinterpret_cast<intptr_t>(data_ptr) & ~static_cast<intptr_t>(1));
-      bool track_err =
-          reinterpret_cast<intptr_t>(data_ptr) & static_cast<intptr_t>(1);
+          reinterpret_cast<intptr_t>(data_ptr) & ~intptr_t{1});
+      bool track_err = reinterpret_cast<intptr_t>(data_ptr) & intptr_t{1};
       bool cancel = (ev->events & EPOLLHUP) != 0;
       bool error = (ev->events & EPOLLERR) != 0;
       bool read_ev = (ev->events & (EPOLLIN | EPOLLPRI)) != 0;

diff --git a/src/core/lib/gpr/string.cc b/src/core/lib/gpr/string.cc
@@ -89,7 +89,7 @@ static dump_out dump_out_create(void) {
 
 static void dump_out_append(dump_out* out, char c) {
   if (out->length == out->capacity) {
-    out->capacity = std::max(size_t(8), 2 * out->capacity);
+    out->capacity = std::max(size_t{8}, 2 * out->capacity);
     out->data = static_cast<char*>(gpr_realloc(out->data, out->capacity));
   }
   out->data[out->length++] = c;
@@ -290,7 +290,7 @@ static void add_string_to_split(const char* beg, const char* end, char*** strs,
   memcpy(out, beg, static_cast<size_t>(end - beg));
   out[end - beg] = 0;
   if (*nstrs == *capstrs) {
-    *capstrs = std::max(size_t(8), 2 * *capstrs);
+    *capstrs = std::max(size_t{8}, 2 * *capstrs);
     *strs = static_cast<char**>(gpr_realloc(*strs, sizeof(*strs) * *capstrs));
   }
   (*strs)[*nstrs] = out;