Skip to content

Commit

Permalink
[grpc][Gpr_To_Absl_Logging] Migrating from gpr to absl logging GPR_AS…
Browse files Browse the repository at this point in the history
…SERT (#36470)

[grpc][Gpr_To_Absl_Logging] Migrating from gpr to absl logging GPR_ASSERT
Replacing GPR_ASSERT with absl CHECK.
These changes have been made using string replacement and regex.
Will not be replacing all instances of CHECK with CHECK_EQ , CHECK_NE etc because there are too many callsites. Only ones which are doable using very simple regex with least chance of failure will be replaced.
Given that we have 5000+ instances of GPR_ASSERT to edit, Doing it manually is too much work for both the author and reviewer.

<!--

If you know who should review your pull request, please assign it to that
person, otherwise the pull request would get assigned randomly.

If your pull request is for a specific language, please add the appropriate
lang label.

-->

Closes #36470

COPYBARA_INTEGRATE_REVIEW=#36470 from tanvi-jagtap:tjagtap_src_core_lib_iomgr 1f209b3
PiperOrigin-RevId: 629292757
  • Loading branch information
tanvi-jagtap authored and Copybara-Service committed Apr 30, 2024
1 parent b73fdce commit 1e25193
Show file tree
Hide file tree
Showing 37 changed files with 251 additions and 175 deletions.
8 changes: 7 additions & 1 deletion BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -1423,6 +1423,7 @@ grpc_cc_library(
],
external_deps = [
"absl/container:inlined_vector",
"absl/log:check",
],
language = "c++",
deps = [
Expand Down Expand Up @@ -1624,6 +1625,7 @@ grpc_cc_library(
"absl/container:flat_hash_map",
"absl/container:flat_hash_set",
"absl/functional:any_invocable",
"absl/log:check",
"absl/status",
"absl/status:statusor",
"absl/strings",
Expand Down Expand Up @@ -3172,7 +3174,10 @@ grpc_cc_library(
"//src/core:lib/iomgr/executor.h",
"//src/core:lib/iomgr/iomgr_internal.h",
],
external_deps = ["absl/strings:str_format"],
external_deps = [
"absl/log:check",
"absl/strings:str_format",
],
visibility = [
"@grpc:alt_grpc_base_legacy",
"@grpc:exec_ctx",
Expand Down Expand Up @@ -3234,6 +3239,7 @@ grpc_cc_library(
"//src/core:lib/iomgr/iomgr.h",
],
external_deps = [
"absl/log:check",
"absl/strings",
"absl/strings:str_format",
],
Expand Down
7 changes: 7 additions & 0 deletions CMakeLists.txt

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions build_autogenerated.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 8 additions & 1 deletion src/core/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -1564,6 +1564,7 @@ grpc_cc_library(
"lib/iomgr/error.h",
],
external_deps = [
"absl/log:check",
"absl/status",
"absl/strings:str_format",
],
Expand All @@ -1589,7 +1590,10 @@ grpc_cc_library(
hdrs = [
"lib/iomgr/closure.h",
],
external_deps = ["absl/strings:str_format"],
external_deps = [
"absl/log:check",
"absl/strings:str_format",
],
visibility = ["@grpc:alt_grpc_base_legacy"],
deps = [
"error",
Expand Down Expand Up @@ -1648,6 +1652,9 @@ grpc_cc_library(
"lib/iomgr/sockaddr_windows.h",
"lib/iomgr/socket_utils.h",
],
external_deps = [
"absl/log:check",
],
deps = [
"iomgr_port",
"//:gpr",
Expand Down
6 changes: 4 additions & 2 deletions src/core/lib/iomgr/call_combiner.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@

#include <inttypes.h>

#include "absl/log/check.h"

#include <grpc/support/log.h>
#include <grpc/support/port_platform.h>

Expand Down Expand Up @@ -86,7 +88,7 @@ void CallCombiner::TsanClosure(void* arg, grpc_error_handle error) {
if (lock != nullptr) {
TSAN_ANNOTATE_RWLOCK_RELEASED(&lock->taken, true);
bool prev = true;
GPR_ASSERT(lock->taken.compare_exchange_strong(prev, false));
CHECK(lock->taken.compare_exchange_strong(prev, false));
}
}
#endif
Expand Down Expand Up @@ -154,7 +156,7 @@ void CallCombiner::Stop(DEBUG_ARGS const char* reason) {
gpr_log(GPR_INFO, " size: %" PRIdPTR " -> %" PRIdPTR, prev_size,
prev_size - 1);
}
GPR_ASSERT(prev_size >= 1);
CHECK_GE(prev_size, 1u);
if (prev_size > 1) {
while (true) {
if (GRPC_TRACE_FLAG_ENABLED(grpc_call_combiner_trace)) {
Expand Down
4 changes: 3 additions & 1 deletion src/core/lib/iomgr/closure.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
#include <assert.h>
#include <stdbool.h>

#include "absl/log/check.h"

#include <grpc/support/alloc.h>
#include <grpc/support/log.h>
#include <grpc/support/port_platform.h>
Expand Down Expand Up @@ -297,7 +299,7 @@ class Closure {
closure, closure->file_created, closure->line_created,
location.file(), location.line());
}
GPR_ASSERT(closure->cb != nullptr);
CHECK_NE(closure->cb, nullptr);
#endif
closure->cb(closure->cb_arg, error);
#ifndef NDEBUG
Expand Down
10 changes: 6 additions & 4 deletions src/core/lib/iomgr/combiner.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
#include <inttypes.h>
#include <string.h>

#include "absl/log/check.h"

#include <grpc/support/alloc.h>
#include <grpc/support/log.h>
#include <grpc/support/port_platform.h>
Expand Down Expand Up @@ -64,7 +66,7 @@ grpc_core::Combiner* grpc_combiner_create(

static void really_destroy(grpc_core::Combiner* lock) {
GRPC_COMBINER_TRACE(gpr_log(GPR_INFO, "C:%p really_destroy", lock));
GPR_ASSERT(gpr_atm_no_barrier_load(&lock->state) == 0);
CHECK_EQ(gpr_atm_no_barrier_load(&lock->state), 0);
delete lock;
}

Expand Down Expand Up @@ -148,7 +150,7 @@ static void combiner_exec(grpc_core::Combiner* lock, grpc_closure* cl,
gpr_atm_no_barrier_store(&lock->initiating_exec_ctx_or_null, 0);
}
}
GPR_ASSERT(last & STATE_UNORPHANED); // ensure lock has not been destroyed
CHECK(last & STATE_UNORPHANED); // ensure lock has not been destroyed
assert(cl->cb);
cl->error_data.error = grpc_core::internal::StatusAllocHeapPtr(error);
lock->queue.Push(cl->next_data.mpscq_node.get());
Expand Down Expand Up @@ -230,7 +232,7 @@ bool grpc_combiner_continue_exec_ctx() {
cl->cb(cl->cb_arg, std::move(cl_err));
} else {
grpc_closure* c = lock->final_list.head;
GPR_ASSERT(c != nullptr);
CHECK_NE(c, nullptr);
grpc_closure_list_init(&lock->final_list);
int loops = 0;
while (c != nullptr) {
Expand Down Expand Up @@ -293,7 +295,7 @@ static void enqueue_finally(void* closure, grpc_error_handle error);
static void combiner_finally_exec(grpc_core::Combiner* lock,
grpc_closure* closure,
grpc_error_handle error) {
GPR_ASSERT(lock != nullptr);
CHECK_NE(lock, nullptr);
GRPC_COMBINER_TRACE(gpr_log(
GPR_INFO, "C:%p grpc_combiner_execute_finally c=%p; ac=%p", lock, closure,
grpc_core::ExecCtx::Get()->combiner_data()->active_combiner));
Expand Down
12 changes: 7 additions & 5 deletions src/core/lib/iomgr/endpoint_cfstream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@

#import <CoreFoundation/CoreFoundation.h>

#include "absl/log/check.h"

#include <grpc/slice_buffer.h>
#include <grpc/support/alloc.h>
#include <grpc/support/string_util.h>
Expand Down Expand Up @@ -150,15 +152,15 @@ static void CallWriteCb(CFStreamEndpoint* ep, grpc_error_handle error) {

static void ReadAction(void* arg, grpc_error_handle error) {
CFStreamEndpoint* ep = static_cast<CFStreamEndpoint*>(arg);
GPR_ASSERT(ep->read_cb != nullptr);
CHECK_NE(ep->read_cb, nullptr);
if (!error.ok()) {
grpc_slice_buffer_reset_and_unref(ep->read_slices);
CallReadCb(ep, error);
EP_UNREF(ep, "read");
return;
}

GPR_ASSERT(ep->read_slices->count == 1);
CHECK_EQ(ep->read_slices->count, 1);
grpc_slice slice = ep->read_slices->slices[0];
size_t len = GRPC_SLICE_LENGTH(slice);
CFIndex read_size =
Expand Down Expand Up @@ -191,7 +193,7 @@ static void ReadAction(void* arg, grpc_error_handle error) {

static void WriteAction(void* arg, grpc_error_handle error) {
CFStreamEndpoint* ep = static_cast<CFStreamEndpoint*>(arg);
GPR_ASSERT(ep->write_cb != nullptr);
CHECK_NE(ep->write_cb, nullptr);
if (!error.ok()) {
grpc_slice_buffer_reset_and_unref(ep->write_slices);
CallWriteCb(ep, error);
Expand Down Expand Up @@ -247,7 +249,7 @@ static void CFStreamRead(grpc_endpoint* ep, grpc_slice_buffer* slices,
gpr_log(GPR_DEBUG, "CFStream endpoint:%p read (%p, %p) length:%zu", ep_impl,
slices, cb, slices->length);
}
GPR_ASSERT(ep_impl->read_cb == nullptr);
CHECK_EQ(ep_impl->read_cb, nullptr);
ep_impl->read_cb = cb;
ep_impl->read_slices = slices;
grpc_slice_buffer_reset_and_unref(slices);
Expand All @@ -265,7 +267,7 @@ static void CFStreamWrite(grpc_endpoint* ep, grpc_slice_buffer* slices,
gpr_log(GPR_DEBUG, "CFStream endpoint:%p write (%p, %p) length:%zu",
ep_impl, slices, cb, slices->length);
}
GPR_ASSERT(ep_impl->write_cb == nullptr);
CHECK_EQ(ep_impl->write_cb, nullptr);
ep_impl->write_cb = cb;
ep_impl->write_slices = slices;
EP_REF(ep_impl, "write");
Expand Down
9 changes: 5 additions & 4 deletions src/core/lib/iomgr/endpoint_pair_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

#include <string>

#include "absl/log/check.h"
#include "absl/strings/str_cat.h"

#include <grpc/support/alloc.h>
Expand All @@ -48,11 +49,11 @@ static void create_sockets(int sv[2]) {
int flags;
grpc_create_socketpair_if_unix(sv);
flags = fcntl(sv[0], F_GETFL, 0);
GPR_ASSERT(fcntl(sv[0], F_SETFL, flags | O_NONBLOCK) == 0);
CHECK_EQ(fcntl(sv[0], F_SETFL, flags | O_NONBLOCK), 0);
flags = fcntl(sv[1], F_GETFL, 0);
GPR_ASSERT(fcntl(sv[1], F_SETFL, flags | O_NONBLOCK) == 0);
GPR_ASSERT(grpc_set_socket_no_sigpipe_if_possible(sv[0]) == absl::OkStatus());
GPR_ASSERT(grpc_set_socket_no_sigpipe_if_possible(sv[1]) == absl::OkStatus());
CHECK_EQ(fcntl(sv[1], F_SETFL, flags | O_NONBLOCK), 0);
CHECK(grpc_set_socket_no_sigpipe_if_possible(sv[0]) == absl::OkStatus());
CHECK(grpc_set_socket_no_sigpipe_if_possible(sv[1]) == absl::OkStatus());
}

grpc_endpoint_pair grpc_iomgr_create_endpoint_pair(
Expand Down
21 changes: 11 additions & 10 deletions src/core/lib/iomgr/endpoint_pair_windows.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
#include <fcntl.h>
#include <string.h>

#include "absl/log/check.h"

#include <grpc/support/log.h>

#include "src/core/lib/address_utils/sockaddr_utils.h"
Expand All @@ -43,25 +45,24 @@ static void create_sockets(SOCKET sv[2]) {

lst_sock = WSASocket(AF_INET, SOCK_STREAM, IPPROTO_TCP, NULL, 0,
grpc_get_default_wsa_socket_flags());
GPR_ASSERT(lst_sock != INVALID_SOCKET);
CHECK(lst_sock != INVALID_SOCKET);

memset(&addr, 0, sizeof(addr));
addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
addr.sin_family = AF_INET;
GPR_ASSERT(bind(lst_sock, (grpc_sockaddr*)&addr, sizeof(addr)) !=
SOCKET_ERROR);
GPR_ASSERT(listen(lst_sock, SOMAXCONN) != SOCKET_ERROR);
GPR_ASSERT(getsockname(lst_sock, (grpc_sockaddr*)&addr, &addr_len) !=
SOCKET_ERROR);
CHECK(bind(lst_sock, (grpc_sockaddr*)&addr, sizeof(addr)) != SOCKET_ERROR);
CHECK(listen(lst_sock, SOMAXCONN) != SOCKET_ERROR);
CHECK(getsockname(lst_sock, (grpc_sockaddr*)&addr, &addr_len) !=
SOCKET_ERROR);

cli_sock = WSASocket(AF_INET, SOCK_STREAM, IPPROTO_TCP, NULL, 0,
grpc_get_default_wsa_socket_flags());
GPR_ASSERT(cli_sock != INVALID_SOCKET);
CHECK(cli_sock != INVALID_SOCKET);

GPR_ASSERT(WSAConnect(cli_sock, (grpc_sockaddr*)&addr, addr_len, NULL, NULL,
NULL, NULL) == 0);
CHECK(WSAConnect(cli_sock, (grpc_sockaddr*)&addr, addr_len, NULL, NULL, NULL,
NULL) == 0);
svr_sock = accept(lst_sock, (grpc_sockaddr*)&addr, &addr_len);
GPR_ASSERT(svr_sock != INVALID_SOCKET);
CHECK(svr_sock != INVALID_SOCKET);

closesocket(lst_sock);
grpc_error_handle error = grpc_tcp_prepare_socket(cli_sock);
Expand Down

0 comments on commit 1e25193

Please sign in to comment.