From 960394231344f13c4f838cc4b292c90b0bc279ed Mon Sep 17 00:00:00 2001 From: Tanvi Jagtap <139093547+tanvi-jagtap@users.noreply.github.com> Date: Thu, 2 May 2024 02:23:25 -0700 Subject: [PATCH] [grpc][Gpr_To_Absl_Logging] Migrating from gpr to absl logging GPR_ASSERT (#36467) [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. Closes #36467 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/36467 from tanvi-jagtap:tjagtap_src_core_lib 30d3ff5bbba3e60b2bd6b3961b6d46cbf375383f PiperOrigin-RevId: 629995895 --- BUILD | 2 +- Package.swift | 1 - build_autogenerated.yaml | 1 - gRPC-C++.podspec | 2 - gRPC-Core.podspec | 2 - grpc.gemspec | 1 - package.xml | 1 - src/core/BUILD | 24 ++++---- src/core/handshaker/handshaker.cc | 5 +- .../tcp_connect/tcp_connect_handshaker.cc | 5 +- src/core/lib/config/load_config.cc | 1 - src/core/lib/gpr/log_internal.h | 55 ------------------- src/core/lib/transport/batch_builder.cc | 8 ++- src/core/lib/transport/bdp_estimator.cc | 4 +- src/core/lib/transport/bdp_estimator.h | 5 +- src/core/lib/transport/call_destination.h | 2 +- src/core/lib/transport/call_filters.cc | 20 ++++--- src/core/lib/transport/call_filters.h | 54 +++++++++--------- src/core/lib/transport/call_spine.h | 26 +++++---- src/core/lib/transport/metadata_batch.h | 5 +- src/core/lib/transport/promise_endpoint.cc | 5 +- src/core/lib/transport/promise_endpoint.h | 20 +++---- src/core/lib/transport/timeout_encoding.cc | 7 ++- tools/doxygen/Doxyfile.c++.internal | 1 - tools/doxygen/Doxyfile.core.internal | 1 - 25 files changed, 106 insertions(+), 152 deletions(-) delete mode 100644 src/core/lib/gpr/log_internal.h diff --git a/BUILD b/BUILD index fff2695d653..2c38f3990db 100644 --- a/BUILD +++ b/BUILD @@ -2983,7 +2983,6 @@ grpc_cc_library( deps = [ "gpr_platform", "//src/core:env", - "//src/core:gpr_log_internal", ], ) @@ -3100,6 +3099,7 @@ grpc_cc_library( external_deps = [ "absl/base:core_headers", "absl/container:inlined_vector", + "absl/log:check", "absl/status", "absl/strings:str_format", ], diff --git a/Package.swift b/Package.swift index c4eacbae00d..9afca3494ca 100644 --- a/Package.swift +++ b/Package.swift @@ -1316,7 +1316,6 @@ let package = Package( "src/core/lib/gpr/linux/cpu.cc", "src/core/lib/gpr/linux/log.cc", "src/core/lib/gpr/log.cc", - "src/core/lib/gpr/log_internal.h", "src/core/lib/gpr/msys/tmpfile.cc", "src/core/lib/gpr/posix/cpu.cc", "src/core/lib/gpr/posix/log.cc", diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index 636d5ea958c..bd949a4560a 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -56,7 +56,6 @@ libs: - src/core/lib/config/load_config.h - src/core/lib/event_engine/thread_local.h - src/core/lib/gpr/alloc.h - - src/core/lib/gpr/log_internal.h - src/core/lib/gpr/string.h - src/core/lib/gpr/time_precise.h - src/core/lib/gpr/tmpfile.h diff --git a/gRPC-C++.podspec b/gRPC-C++.podspec index fca100b28d2..57179a085a3 100644 --- a/gRPC-C++.podspec +++ b/gRPC-C++.podspec @@ -993,7 +993,6 @@ Pod::Spec.new do |s| 'src/core/lib/experiments/config.h', 'src/core/lib/experiments/experiments.h', 'src/core/lib/gpr/alloc.h', - 'src/core/lib/gpr/log_internal.h', 'src/core/lib/gpr/spinlock.h', 'src/core/lib/gpr/string.h', 'src/core/lib/gpr/time_precise.h', @@ -2263,7 +2262,6 @@ Pod::Spec.new do |s| 'src/core/lib/experiments/config.h', 'src/core/lib/experiments/experiments.h', 'src/core/lib/gpr/alloc.h', - 'src/core/lib/gpr/log_internal.h', 'src/core/lib/gpr/spinlock.h', 'src/core/lib/gpr/string.h', 'src/core/lib/gpr/time_precise.h', diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index c1ad3a41850..154ea290e80 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -1433,7 +1433,6 @@ Pod::Spec.new do |s| 'src/core/lib/gpr/linux/cpu.cc', 'src/core/lib/gpr/linux/log.cc', 'src/core/lib/gpr/log.cc', - 'src/core/lib/gpr/log_internal.h', 'src/core/lib/gpr/msys/tmpfile.cc', 'src/core/lib/gpr/posix/cpu.cc', 'src/core/lib/gpr/posix/log.cc', @@ -3041,7 +3040,6 @@ Pod::Spec.new do |s| 'src/core/lib/experiments/config.h', 'src/core/lib/experiments/experiments.h', 'src/core/lib/gpr/alloc.h', - 'src/core/lib/gpr/log_internal.h', 'src/core/lib/gpr/spinlock.h', 'src/core/lib/gpr/string.h', 'src/core/lib/gpr/time_precise.h', diff --git a/grpc.gemspec b/grpc.gemspec index 05b8d202eca..1247adbae75 100644 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -1322,7 +1322,6 @@ Gem::Specification.new do |s| s.files += %w( src/core/lib/gpr/linux/cpu.cc ) s.files += %w( src/core/lib/gpr/linux/log.cc ) s.files += %w( src/core/lib/gpr/log.cc ) - s.files += %w( src/core/lib/gpr/log_internal.h ) s.files += %w( src/core/lib/gpr/msys/tmpfile.cc ) s.files += %w( src/core/lib/gpr/posix/cpu.cc ) s.files += %w( src/core/lib/gpr/posix/log.cc ) diff --git a/package.xml b/package.xml index 62f4ba023b7..f95df49ac37 100644 --- a/package.xml +++ b/package.xml @@ -1304,7 +1304,6 @@ - diff --git a/src/core/BUILD b/src/core/BUILD index ef1fcfe8a4c..b28f3b72064 100644 --- a/src/core/BUILD +++ b/src/core/BUILD @@ -288,15 +288,6 @@ grpc_cc_library( ], ) -grpc_cc_library( - name = "gpr_log_internal", - hdrs = [ - "lib/gpr/log_internal.h", - ], - language = "c++", - deps = ["//:gpr_platform"], -) - grpc_cc_library( name = "env", srcs = [ @@ -1276,6 +1267,7 @@ grpc_cc_library( ], external_deps = [ "absl/base:core_headers", + "absl/log:check", "absl/status", "absl/status:statusor", "absl/types:optional", @@ -2865,7 +2857,10 @@ grpc_cc_library( "lib/transport/bdp_estimator.cc", ], hdrs = ["lib/transport/bdp_estimator.h"], - external_deps = ["absl/strings"], + external_deps = [ + "absl/log:check", + "absl/strings", + ], deps = [ "time", "//:gpr", @@ -7097,6 +7092,7 @@ grpc_cc_library( ], external_deps = [ "absl/base:core_headers", + "absl/log:check", "absl/status", "absl/status:statusor", "absl/types:optional", @@ -7298,6 +7294,9 @@ grpc_cc_library( hdrs = [ "lib/transport/call_filters.h", ], + external_deps = [ + "absl/log:check", + ], deps = [ "call_final_info", "latch", @@ -7405,6 +7404,9 @@ grpc_cc_library( hdrs = [ "lib/transport/call_spine.h", ], + external_deps = [ + "absl/log:check", + ], deps = [ "1999", "call_arena_allocator", @@ -7439,6 +7441,7 @@ grpc_cc_library( "absl/container:flat_hash_set", "absl/container:inlined_vector", "absl/functional:function_ref", + "absl/log:check", "absl/meta:type_traits", "absl/strings", "absl/strings:str_format", @@ -7473,6 +7476,7 @@ grpc_cc_library( ], external_deps = [ "absl/base:core_headers", + "absl/log:check", "absl/types:optional", ], deps = [ diff --git a/src/core/handshaker/handshaker.cc b/src/core/handshaker/handshaker.cc index efd73de2de0..9be4fd4383f 100644 --- a/src/core/handshaker/handshaker.cc +++ b/src/core/handshaker/handshaker.cc @@ -23,6 +23,7 @@ #include #include +#include "absl/log/check.h" #include "absl/status/status.h" #include "absl/strings/str_format.h" @@ -100,7 +101,7 @@ bool HandshakeManager::CallNextHandshakerLocked(grpc_error_handle error) { this, StatusToString(error).c_str(), is_shutdown_, index_, HandshakerArgsString(&args_).c_str()); } - GPR_ASSERT(index_ <= handshakers_.size()); + CHECK(index_ <= handshakers_.size()); // If we got an error or we've been shut down or we're exiting early or // we've finished the last handshaker, invoke the on_handshake_done // callback. Otherwise, call the next handshaker. @@ -177,7 +178,7 @@ void HandshakeManager::DoHandshake(grpc_endpoint* endpoint, bool done; { MutexLock lock(&mu_); - GPR_ASSERT(index_ == 0); + CHECK_EQ(index_, 0u); // Construct handshaker args. These will be passed through all // handshakers and eventually be freed by the on_handshake_done callback. args_.endpoint = endpoint; diff --git a/src/core/handshaker/tcp_connect/tcp_connect_handshaker.cc b/src/core/handshaker/tcp_connect/tcp_connect_handshaker.cc index 9db2cb81bd8..fa10c544d05 100644 --- a/src/core/handshaker/tcp_connect/tcp_connect_handshaker.cc +++ b/src/core/handshaker/tcp_connect/tcp_connect_handshaker.cc @@ -21,6 +21,7 @@ #include #include "absl/base/thread_annotations.h" +#include "absl/log/check.h" #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/types/optional.h" @@ -124,7 +125,7 @@ void TCPConnectHandshaker::DoHandshake(grpc_tcp_server_acceptor* /*acceptor*/, MutexLock lock(&mu_); on_handshake_done_ = on_handshake_done; } - GPR_ASSERT(args->endpoint == nullptr); + CHECK_EQ(args->endpoint, nullptr); args_ = args; absl::StatusOr uri = URI::Parse( args->args.GetString(GRPC_ARG_TCP_HANDSHAKER_RESOLVED_ADDRESS).value()); @@ -179,7 +180,7 @@ void TCPConnectHandshaker::Connected(void* arg, grpc_error_handle error) { } return; } - GPR_ASSERT(self->endpoint_to_destroy_ != nullptr); + CHECK_NE(self->endpoint_to_destroy_, nullptr); self->args_->endpoint = self->endpoint_to_destroy_; self->endpoint_to_destroy_ = nullptr; if (self->bind_endpoint_to_pollset_) { diff --git a/src/core/lib/config/load_config.cc b/src/core/lib/config/load_config.cc index a961a24a81d..1e9c3170a74 100644 --- a/src/core/lib/config/load_config.cc +++ b/src/core/lib/config/load_config.cc @@ -24,7 +24,6 @@ #include -#include "src/core/lib/gpr/log_internal.h" #include "src/core/lib/gprpp/env.h" namespace grpc_core { diff --git a/src/core/lib/gpr/log_internal.h b/src/core/lib/gpr/log_internal.h deleted file mode 100644 index 0d7c9108880..00000000000 --- a/src/core/lib/gpr/log_internal.h +++ /dev/null @@ -1,55 +0,0 @@ -// Copyright 2022 gRPC authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -#ifndef GRPC_SRC_CORE_LIB_GPR_LOG_INTERNAL_H -#define GRPC_SRC_CORE_LIB_GPR_LOG_INTERNAL_H - -#include - -#include -#include - -#include - -/// abort() the process if x is zero, with rudimentary logging to prevent -/// circular dependencies with gpr_log. - -/// Intended for internal invariants. If the error can be recovered from, -/// without the possibility of corruption, or might best be reflected via -/// an exception in a higher-level language, consider returning error code. -#define GPR_ASSERT_INTERNAL(x) \ - do { \ - if (GPR_UNLIKELY(!(x))) { \ - fprintf(stderr, "assertion failed: %s", #x); \ - abort(); \ - } \ - } while (0) - -#ifndef NDEBUG -#define GPR_DEBUG_ASSERT_INTERNAL(x) GPR_ASSERT_INTERNAL(x) -#else -#define GPR_DEBUG_ASSERT_INTERNAL(x) -#endif - -#define GPR_LOG_ERROR_INTERNAL(format, ...) \ - do { \ - char f[] = __FILE__; \ - char* display_file = f; \ - char* slash_pos = strrchr(f, '/'); \ - if (slash_pos != nullptr) display_file = slash_pos + 1; \ - char prefix[60]; \ - sprintf(prefix, "INTERNAL %37s:%d]", display_file, __LINE__); \ - fprintf(stderr, "%-60s " format "\n", prefix, __VA_ARGS__); \ - } while (0) - -#endif // GRPC_SRC_CORE_LIB_GPR_LOG_INTERNAL_H diff --git a/src/core/lib/transport/batch_builder.cc b/src/core/lib/transport/batch_builder.cc index 81c0b2fa5e9..a16f5a164df 100644 --- a/src/core/lib/transport/batch_builder.cc +++ b/src/core/lib/transport/batch_builder.cc @@ -16,6 +16,8 @@ #include +#include "absl/log/check.h" + #include #include "src/core/lib/promise/poll.h" @@ -96,13 +98,13 @@ BatchBuilder::Batch* BatchBuilder::GetBatch(Target target) { batch_ = GetContext()->NewPooled(payload_, target_->stream_refcount); } - GPR_ASSERT(batch_ != nullptr); + CHECK_NE(batch_, nullptr); return batch_; } void BatchBuilder::FlushBatch() { - GPR_ASSERT(batch_ != nullptr); - GPR_ASSERT(target_.has_value()); + CHECK_NE(batch_, nullptr); + CHECK(target_.has_value()); if (grpc_call_trace.enabled()) { gpr_log( GPR_DEBUG, "%sPerform transport stream op batch: %p %s", diff --git a/src/core/lib/transport/bdp_estimator.cc b/src/core/lib/transport/bdp_estimator.cc index cb5fc537754..d81e047cbc0 100644 --- a/src/core/lib/transport/bdp_estimator.cc +++ b/src/core/lib/transport/bdp_estimator.cc @@ -23,6 +23,8 @@ #include +#include "absl/log/check.h" + #include grpc_core::TraceFlag grpc_bdp_estimator_trace(false, "bdp_estimator"); @@ -53,7 +55,7 @@ Timestamp BdpEstimator::CompletePing() { std::string(name_).c_str(), accumulator_, estimate_, dt, bw / 125000.0, bw_est_ / 125000.0); } - GPR_ASSERT(ping_state_ == PingState::STARTED); + CHECK(ping_state_ == PingState::STARTED); if (accumulator_ > 2 * estimate_ / 3 && bw > bw_est_) { estimate_ = std::max(accumulator_, estimate_ * 2); bw_est_ = bw; diff --git a/src/core/lib/transport/bdp_estimator.h b/src/core/lib/transport/bdp_estimator.h index 571cee0028d..6c953bd4054 100644 --- a/src/core/lib/transport/bdp_estimator.h +++ b/src/core/lib/transport/bdp_estimator.h @@ -23,6 +23,7 @@ #include +#include "absl/log/check.h" #include "absl/strings/string_view.h" #include @@ -54,7 +55,7 @@ class BdpEstimator { gpr_log(GPR_INFO, "bdp[%s]:sched acc=%" PRId64 " est=%" PRId64, std::string(name_).c_str(), accumulator_, estimate_); } - GPR_ASSERT(ping_state_ == PingState::UNSCHEDULED); + CHECK(ping_state_ == PingState::UNSCHEDULED); ping_state_ = PingState::SCHEDULED; accumulator_ = 0; } @@ -67,7 +68,7 @@ class BdpEstimator { gpr_log(GPR_INFO, "bdp[%s]:start acc=%" PRId64 " est=%" PRId64, std::string(name_).c_str(), accumulator_, estimate_); } - GPR_ASSERT(ping_state_ == PingState::SCHEDULED); + CHECK(ping_state_ == PingState::SCHEDULED); ping_state_ = PingState::STARTED; ping_start_time_ = gpr_now(GPR_CLOCK_MONOTONIC); } diff --git a/src/core/lib/transport/call_destination.h b/src/core/lib/transport/call_destination.h index 2cf0f446d24..77683e230f5 100644 --- a/src/core/lib/transport/call_destination.h +++ b/src/core/lib/transport/call_destination.h @@ -36,7 +36,7 @@ class UnstartedCallDestination // and started. // Must be called from the party owned by the call, eg the following must // hold: - // GPR_ASSERT(GetContext() == unstarted_call_handler.party()); + // CHECK(GetContext() == unstarted_call_handler.party()); virtual void StartCall(UnstartedCallHandler unstarted_call_handler) = 0; }; diff --git a/src/core/lib/transport/call_filters.cc b/src/core/lib/transport/call_filters.cc index c40ca8609e8..d4792c02bfe 100644 --- a/src/core/lib/transport/call_filters.cc +++ b/src/core/lib/transport/call_filters.cc @@ -14,6 +14,8 @@ #include "src/core/lib/transport/call_filters.h" +#include "absl/log/check.h" + #include #include "src/core/lib/gprpp/crash.h" @@ -43,7 +45,7 @@ Poll> OperationExecutor::Start( if (layout->promise_size == 0) { // No call state ==> instantaneously ready auto r = InitStep(std::move(input), call_data); - GPR_ASSERT(r.ready()); + CHECK(r.ready()); return r; } promise_data_ = @@ -53,7 +55,7 @@ Poll> OperationExecutor::Start( template Poll> OperationExecutor::InitStep(T input, void* call_data) { - GPR_ASSERT(input != nullptr); + CHECK(input != nullptr); while (true) { if (ops_ == end_ops_) { return ResultOr{std::move(input), nullptr}; @@ -73,7 +75,7 @@ Poll> OperationExecutor::InitStep(T input, void* call_data) { template Poll> OperationExecutor::Step(void* call_data) { - GPR_DEBUG_ASSERT(promise_data_ != nullptr); + DCHECK_NE(promise_data_, nullptr); auto p = ContinueStep(call_data); if (p.ready()) { gpr_free_aligned(promise_data_); @@ -109,7 +111,7 @@ Poll InfallibleOperationExecutor::Start( if (layout->promise_size == 0) { // No call state ==> instantaneously ready auto r = InitStep(std::move(input), call_data); - GPR_ASSERT(r.ready()); + CHECK(r.ready()); return r; } promise_data_ = @@ -137,7 +139,7 @@ Poll InfallibleOperationExecutor::InitStep(T input, void* call_data) { template Poll InfallibleOperationExecutor::Step(void* call_data) { - GPR_DEBUG_ASSERT(promise_data_ != nullptr); + DCHECK_NE(promise_data_, nullptr); auto p = ContinueStep(call_data); if (p.ready()) { gpr_free_aligned(promise_data_); @@ -182,7 +184,7 @@ CallFilters::~CallFilters() { } void CallFilters::SetStack(RefCountedPtr stack) { - GPR_ASSERT(call_data_ == nullptr); + CHECK_EQ(call_data_, nullptr); stack_ = std::move(stack); call_data_ = gpr_malloc_aligned(stack_->data_.call_data_size, stack_->data_.call_data_alignment); @@ -217,13 +219,13 @@ void CallFilters::CancelDueToFailedPipeOperation(SourceLocation but_where) { } void CallFilters::PushServerTrailingMetadata(ServerMetadataHandle md) { - GPR_ASSERT(md != nullptr); + CHECK(md != nullptr); if (GRPC_TRACE_FLAG_ENABLED(grpc_trace_promise_primitives)) { gpr_log(GPR_INFO, "%s PushServerTrailingMetadata[%p]: %s into %s", GetContext()->DebugTag().c_str(), this, md->DebugString().c_str(), DebugString().c_str()); } - GPR_ASSERT(md != nullptr); + CHECK(md != nullptr); if (server_trailing_metadata_ != nullptr) return; server_trailing_metadata_ = std::move(md); client_initial_metadata_state_.CloseWithError(); @@ -286,7 +288,7 @@ RefCountedPtr CallFilters::StackBuilder::Build() { // CallFilters::PipeState void filters_detail::PipeState::Start() { - GPR_DEBUG_ASSERT(!started_); + DCHECK(!started_); started_ = true; wait_recv_.Wake(); } diff --git a/src/core/lib/transport/call_filters.h b/src/core/lib/transport/call_filters.h index 043bf6434da..a9e3a1c5b98 100644 --- a/src/core/lib/transport/call_filters.h +++ b/src/core/lib/transport/call_filters.h @@ -19,6 +19,8 @@ #include #include +#include "absl/log/check.h" + #include #include "src/core/lib/gprpp/ref_counted.h" @@ -157,7 +159,7 @@ template struct ResultOr { ResultOr(T ok, ServerMetadataHandle error) : ok(std::move(ok)), error(std::move(error)) { - GPR_ASSERT((this->ok == nullptr) ^ (this->error == nullptr)); + CHECK((this->ok == nullptr) ^ (this->error == nullptr)); } T ok; ServerMetadataHandle error; @@ -990,13 +992,13 @@ struct StackData { template void AddFinalizer(FilterType*, size_t, const NoInterceptor* p) { - GPR_DEBUG_ASSERT(p == &FilterType::Call::OnFinalize); + DCHECK(p == &FilterType::Call::OnFinalize); } template void AddFinalizer(FilterType* channel_data, size_t call_offset, void (FilterType::Call::*p)(const grpc_call_final_info*)) { - GPR_DEBUG_ASSERT(p == &FilterType::Call::OnFinalize); + DCHECK(p == &FilterType::Call::OnFinalize); finalizers.push_back(Finalizer{ channel_data, call_offset, @@ -1011,7 +1013,7 @@ struct StackData { void AddFinalizer(FilterType* channel_data, size_t call_offset, void (FilterType::Call::*p)(const grpc_call_final_info*, FilterType*)) { - GPR_DEBUG_ASSERT(p == &FilterType::Call::OnFinalize); + DCHECK(p == &FilterType::Call::OnFinalize); finalizers.push_back(Finalizer{ channel_data, call_offset, @@ -1041,11 +1043,11 @@ class OperationExecutor { OperationExecutor(OperationExecutor&& other) noexcept : ops_(other.ops_), end_ops_(other.end_ops_) { // Movable iff we're not running. - GPR_DEBUG_ASSERT(other.promise_data_ == nullptr); + DCHECK_EQ(other.promise_data_, nullptr); } OperationExecutor& operator=(OperationExecutor&& other) noexcept { - GPR_DEBUG_ASSERT(other.promise_data_ == nullptr); - GPR_DEBUG_ASSERT(promise_data_ == nullptr); + DCHECK_EQ(other.promise_data_, nullptr); + DCHECK_EQ(promise_data_, nullptr); ops_ = other.ops_; end_ops_ = other.end_ops_; return *this; @@ -1094,12 +1096,12 @@ class InfallibleOperationExecutor { InfallibleOperationExecutor(InfallibleOperationExecutor&& other) noexcept : ops_(other.ops_), end_ops_(other.end_ops_) { // Movable iff we're not running. - GPR_DEBUG_ASSERT(other.promise_data_ == nullptr); + DCHECK_EQ(other.promise_data_, nullptr); } InfallibleOperationExecutor& operator=( InfallibleOperationExecutor&& other) noexcept { - GPR_DEBUG_ASSERT(other.promise_data_ == nullptr); - GPR_DEBUG_ASSERT(promise_data_ == nullptr); + DCHECK_EQ(other.promise_data_, nullptr); + DCHECK_EQ(promise_data_, nullptr); ops_ = other.ops_; end_ops_ = other.end_ops_; return *this; @@ -1388,12 +1390,12 @@ class CallFilters { public: Push(CallFilters* filters, T x) : filters_(filters), value_(std::move(x)) { - GPR_ASSERT(value_ != nullptr); + CHECK(value_ != nullptr); if (GRPC_TRACE_FLAG_ENABLED(grpc_trace_promise_primitives)) { gpr_log(GPR_INFO, "BeginPush[%p|%p]: %s", &state(), this, state().DebugString().c_str()); } - GPR_ASSERT(push_slot() == nullptr); + CHECK_EQ(push_slot(), nullptr); state().BeginPush(); push_slot() = this; } @@ -1402,7 +1404,7 @@ class CallFilters { if (value_ != nullptr) { state().DropPush(); } - GPR_ASSERT(push_slot() == this); + CHECK(push_slot() == this); push_slot() = nullptr; } } @@ -1413,7 +1415,7 @@ class CallFilters { : filters_(std::exchange(other.filters_, nullptr)), value_(std::move(other.value_)) { if (filters_ != nullptr) { - GPR_DEBUG_ASSERT(push_slot() == &other); + DCHECK(push_slot() == &other); push_slot() = this; } } @@ -1422,7 +1424,7 @@ class CallFilters { Poll operator()() { if (value_ == nullptr) { - GPR_ASSERT(filters_ == nullptr); + CHECK_EQ(filters_, nullptr); if (GRPC_TRACE_FLAG_ENABLED(grpc_trace_promise_primitives)) { gpr_log(GPR_INFO, "Push[|%p]: already done", this); } @@ -1457,8 +1459,8 @@ class CallFilters { gpr_log(GPR_INFO, "Push[%p|%p]: take value; %s", &state(), this, state().DebugString().c_str()); } - GPR_ASSERT(value_ != nullptr); - GPR_ASSERT(filters_ != nullptr); + CHECK(value_ != nullptr); + CHECK(filters_ != nullptr); push_slot() = nullptr; filters_ = nullptr; return std::move(value_); @@ -1536,7 +1538,7 @@ class CallFilters { Poll> p) { auto* r = p.value_if_ready(); if (r == nullptr) return Pending{}; - GPR_DEBUG_ASSERT(!executor_.IsRunning()); + DCHECK(!executor_.IsRunning()); state().AckPull(); if (r->ok != nullptr) return std::move(r->ok); filters_->PushServerTrailingMetadata(std::move(r->error)); @@ -1564,7 +1566,7 @@ class CallFilters { PullMessage& operator=(PullMessage&&) = delete; Poll>> operator()() { - GPR_ASSERT(filters_ != nullptr); + CHECK(filters_ != nullptr); if (GRPC_TRACE_FLAG_ENABLED(grpc_trace_promise_primitives)) { gpr_log(GPR_INFO, "PullMessage[%p|%p]: %s executor:%d", &state(), this, state().DebugString().c_str(), executor_.IsRunning()); @@ -1592,7 +1594,7 @@ class CallFilters { return Failure{}; } if (!**r) return absl::nullopt; - GPR_ASSERT(filters_ != nullptr); + CHECK(filters_ != nullptr); return FinishOperationExecutor(executor_.Start( layout(), push()->TakeValue(), filters_->call_data_)); } @@ -1609,7 +1611,7 @@ class CallFilters { FinishOperationExecutor(Poll> p) { auto* r = p.value_if_ready(); if (r == nullptr) return Pending{}; - GPR_DEBUG_ASSERT(!executor_.IsRunning()); + DCHECK(!executor_.IsRunning()); if (GRPC_TRACE_FLAG_ENABLED(grpc_trace_promise_primitives)) { gpr_log(GPR_INFO, "PullMessage[%p|%p] executor done: %s", &state(), this, state().DebugString().c_str()); @@ -1658,7 +1660,7 @@ class CallFilters { filters_->CancelDueToFailedPipeOperation(); return Failure{}; } - GPR_ASSERT(filters_->client_initial_metadata_ != nullptr); + CHECK(filters_->client_initial_metadata_ != nullptr); return FinishOperationExecutor(executor_.Start( &filters_->stack_->data_.client_initial_metadata, std::move(filters_->client_initial_metadata_), filters_->call_data_)); @@ -1673,7 +1675,7 @@ class CallFilters { Poll> p) { auto* r = p.value_if_ready(); if (r == nullptr) return Pending{}; - GPR_DEBUG_ASSERT(!executor_.IsRunning()); + DCHECK(!executor_.IsRunning()); state().AckPull(); if (r->ok != nullptr) return std::move(r->ok); filters_->PushServerTrailingMetadata(std::move(r->error)); @@ -1803,7 +1805,7 @@ inline auto CallFilters::PullClientInitialMetadata() { } inline auto CallFilters::PushServerInitialMetadata(ServerMetadataHandle md) { - GPR_ASSERT(md != nullptr); + CHECK(md != nullptr); return [p = ServerInitialMetadataPromises::Push{ this, std::move(md)}]() mutable { return p(); }; } @@ -1813,7 +1815,7 @@ inline auto CallFilters::PullServerInitialMetadata() { } inline auto CallFilters::PushClientToServerMessage(MessageHandle message) { - GPR_ASSERT(message != nullptr); + CHECK(message != nullptr); return [p = ClientToServerMessagePromises::Push{ this, std::move(message)}]() mutable { return p(); }; } @@ -1823,7 +1825,7 @@ inline auto CallFilters::PullClientToServerMessage() { } inline auto CallFilters::PushServerToClientMessage(MessageHandle message) { - GPR_ASSERT(message != nullptr); + CHECK(message != nullptr); return [p = ServerToClientMessagePromises::Push{ this, std::move(message)}]() mutable { return p(); }; } diff --git a/src/core/lib/transport/call_spine.h b/src/core/lib/transport/call_spine.h index 2c81c2ff927..8593e47e9c8 100644 --- a/src/core/lib/transport/call_spine.h +++ b/src/core/lib/transport/call_spine.h @@ -15,6 +15,8 @@ #ifndef GRPC_SRC_CORE_LIB_TRANSPORT_CALL_SPINE_H #define GRPC_SRC_CORE_LIB_TRANSPORT_CALL_SPINE_H +#include "absl/log/check.h" + #include #include @@ -88,7 +90,7 @@ class CallSpineInterface { // The resulting (returned) promise will resolve to Empty. template auto CancelIfFails(Promise promise) { - GPR_DEBUG_ASSERT(GetContext() == &party()); + DCHECK(GetContext() == &party()); using P = promise_detail::PromiseLike; using ResultType = typename P::Result; return Map(std::move(promise), [this](ResultType r) { @@ -151,7 +153,7 @@ class PipeBasedCallSpine : public CallSpineInterface { Promise>> PullServerInitialMetadata() final { - GPR_DEBUG_ASSERT(GetContext() == &party()); + DCHECK(GetContext() == &party()); return Map(server_initial_metadata().receiver.Next(), [](NextResult md) -> ValueOrFailure> { @@ -164,41 +166,41 @@ class PipeBasedCallSpine : public CallSpineInterface { } Promise PullServerTrailingMetadata() final { - GPR_DEBUG_ASSERT(GetContext() == &party()); + DCHECK(GetContext() == &party()); return cancel_latch().Wait(); } Promise>> PullServerToClientMessage() final { - GPR_DEBUG_ASSERT(GetContext() == &party()); + DCHECK(GetContext() == &party()); return Map(server_to_client_messages().receiver.Next(), MapNextMessage); } Promise PushClientToServerMessage(MessageHandle message) final { - GPR_DEBUG_ASSERT(GetContext() == &party()); + DCHECK(GetContext() == &party()); return Map(client_to_server_messages().sender.Push(std::move(message)), [](bool r) { return StatusFlag(r); }); } Promise>> PullClientToServerMessage() final { - GPR_DEBUG_ASSERT(GetContext() == &party()); + DCHECK(GetContext() == &party()); return Map(client_to_server_messages().receiver.Next(), MapNextMessage); } Promise PushServerToClientMessage(MessageHandle message) final { - GPR_DEBUG_ASSERT(GetContext() == &party()); + DCHECK(GetContext() == &party()); return Map(server_to_client_messages().sender.Push(std::move(message)), [](bool r) { return StatusFlag(r); }); } void FinishSends() final { - GPR_DEBUG_ASSERT(GetContext() == &party()); + DCHECK(GetContext() == &party()); client_to_server_messages().sender.Close(); } void PushServerTrailingMetadata(ServerMetadataHandle metadata) final { - GPR_DEBUG_ASSERT(GetContext() == &party()); + DCHECK(GetContext() == &party()); auto& c = cancel_latch(); if (c.is_set()) return; const bool was_cancelled = @@ -213,13 +215,13 @@ class PipeBasedCallSpine : public CallSpineInterface { } Promise WasCancelled() final { - GPR_DEBUG_ASSERT(GetContext() == &party()); + DCHECK(GetContext() == &party()); return was_cancelled_latch().Wait(); } Promise> PullClientInitialMetadata() final { - GPR_DEBUG_ASSERT(GetContext() == &party()); + DCHECK(GetContext() == &party()); return Map(client_initial_metadata().receiver.Next(), [](NextResult md) -> ValueOrFailure { @@ -230,7 +232,7 @@ class PipeBasedCallSpine : public CallSpineInterface { Promise PushServerInitialMetadata( absl::optional md) final { - GPR_DEBUG_ASSERT(GetContext() == &party()); + DCHECK(GetContext() == &party()); return If( md.has_value(), [&md, this]() { diff --git a/src/core/lib/transport/metadata_batch.h b/src/core/lib/transport/metadata_batch.h index d7d0185a66e..7df578396bc 100644 --- a/src/core/lib/transport/metadata_batch.h +++ b/src/core/lib/transport/metadata_batch.h @@ -28,6 +28,7 @@ #include "absl/container/inlined_vector.h" #include "absl/functional/function_ref.h" +#include "absl/log/check.h" #include "absl/meta/type_traits.h" #include "absl/strings/numbers.h" #include "absl/strings/string_view.h" @@ -107,7 +108,7 @@ struct TeMetadata { MetadataParseErrorFn on_error); static ValueType MementoToValue(MementoType te) { return te; } static StaticSlice Encode(ValueType x) { - GPR_ASSERT(x == kTrailers); + CHECK(x == kTrailers); return StaticSlice::FromStaticString("trailers"); } static const char* DisplayValue(ValueType te); @@ -210,7 +211,7 @@ struct CompressionAlgorithmBasedMetadata { MetadataParseErrorFn on_error); static ValueType MementoToValue(MementoType x) { return x; } static Slice Encode(ValueType x) { - GPR_ASSERT(x != GRPC_COMPRESS_ALGORITHMS_COUNT); + CHECK(x != GRPC_COMPRESS_ALGORITHMS_COUNT); return Slice::FromStaticString(CompressionAlgorithmAsString(x)); } static const char* DisplayValue(ValueType x) { diff --git a/src/core/lib/transport/promise_endpoint.cc b/src/core/lib/transport/promise_endpoint.cc index 9f8ced08ef8..031f5194e02 100644 --- a/src/core/lib/transport/promise_endpoint.cc +++ b/src/core/lib/transport/promise_endpoint.cc @@ -19,6 +19,7 @@ #include #include +#include "absl/log/check.h" #include "absl/status/status.h" #include "absl/types/optional.h" @@ -38,7 +39,7 @@ PromiseEndpoint::PromiseEndpoint( endpoint, SliceBuffer already_received) : endpoint_(std::move(endpoint)) { - GPR_ASSERT(endpoint_ != nullptr); + CHECK_NE(endpoint_, nullptr); read_state_->endpoint = endpoint_; // TODO(ladynana): Replace this with `SliceBufferCast<>` when it is // available. @@ -72,7 +73,7 @@ void PromiseEndpoint::ReadState::Complete(absl::Status status, // Appends `pending_buffer` to `buffer`. pending_buffer.MoveFirstNBytesIntoSliceBuffer(pending_buffer.Length(), buffer); - GPR_DEBUG_ASSERT(pending_buffer.Count() == 0u); + DCHECK(pending_buffer.Count() == 0u); if (buffer.Length() < num_bytes_requested) { // A further read is needed. // Set read args with number of bytes needed as hint. diff --git a/src/core/lib/transport/promise_endpoint.h b/src/core/lib/transport/promise_endpoint.h index be49a1fccb0..12659aff583 100644 --- a/src/core/lib/transport/promise_endpoint.h +++ b/src/core/lib/transport/promise_endpoint.h @@ -25,6 +25,7 @@ #include #include "absl/base/thread_annotations.h" +#include "absl/log/check.h" #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/types/optional.h" @@ -75,7 +76,7 @@ class PromiseEndpoint { // Start write and assert previous write finishes. auto prev = write_state_->state.exchange(WriteState::kWriting, std::memory_order_relaxed); - GPR_ASSERT(prev == WriteState::kIdle); + CHECK(prev == WriteState::kIdle); bool completed; if (data.Length() == 0) { completed = true; @@ -102,7 +103,7 @@ class PromiseEndpoint { return [write_state = write_state_]() { auto prev = write_state->state.exchange(WriteState::kIdle, std::memory_order_relaxed); - GPR_ASSERT(prev == WriteState::kWriting); + CHECK(prev == WriteState::kWriting); return absl::OkStatus(); }; }, @@ -120,7 +121,7 @@ class PromiseEndpoint { } // State was not Written; since we're polling it must be // Writing. Assert that and return Pending. - GPR_ASSERT(expected == WriteState::kWriting); + CHECK(expected == WriteState::kWriting); return Pending(); }; }); @@ -134,9 +135,9 @@ class PromiseEndpoint { // undefined behavior. auto Read(size_t num_bytes) { // Assert previous read finishes. - GPR_ASSERT(!read_state_->complete.load(std::memory_order_relaxed)); + CHECK(!read_state_->complete.load(std::memory_order_relaxed)); // Should not have pending reads. - GPR_ASSERT(read_state_->pending_buffer.Count() == 0u); + CHECK(read_state_->pending_buffer.Count() == 0u); bool complete = true; while (read_state_->buffer.Length() < num_bytes) { // Set read args with hinted bytes. @@ -156,7 +157,7 @@ class PromiseEndpoint { read_state_->waker = Waker(); read_state_->pending_buffer.MoveFirstNBytesIntoSliceBuffer( read_state_->pending_buffer.Length(), read_state_->buffer); - GPR_DEBUG_ASSERT(read_state_->pending_buffer.Count() == 0u); + DCHECK(read_state_->pending_buffer.Count() == 0u); } else { complete = false; break; @@ -233,8 +234,7 @@ class PromiseEndpoint { // Copy everything from read_state_->buffer into a single slice and // replace the contents of read_state_->buffer with that slice. grpc_slice slice = grpc_slice_malloc_large(read_state_->buffer.Length()); - GPR_ASSERT( - reinterpret_cast(GRPC_SLICE_START_PTR(slice)) % 64 == 0); + CHECK(reinterpret_cast(GRPC_SLICE_START_PTR(slice)) % 64 == 0); size_t ofs = 0; for (size_t i = 0; i < read_state_->buffer.Count(); i++) { memcpy( @@ -249,7 +249,7 @@ class PromiseEndpoint { read_state_->buffer.Clear(); read_state_->buffer.AppendIndexed( grpc_event_engine::experimental::Slice(slice)); - GPR_DEBUG_ASSERT(read_state_->buffer.Length() == ofs); + DCHECK(read_state_->buffer.Length() == ofs); } } @@ -305,7 +305,7 @@ class PromiseEndpoint { auto prev = state.exchange(kWritten, std::memory_order_release); // Previous state should be Writing. If we got anything else we've entered // the callback path twice. - GPR_ASSERT(prev == kWriting); + CHECK(prev == kWriting); w.Wakeup(); } }; diff --git a/src/core/lib/transport/timeout_encoding.cc b/src/core/lib/transport/timeout_encoding.cc index 265081aac34..deb82cb2cdc 100644 --- a/src/core/lib/transport/timeout_encoding.cc +++ b/src/core/lib/transport/timeout_encoding.cc @@ -21,6 +21,7 @@ #include #include "absl/base/attributes.h" +#include "absl/log/check.h" #include #include @@ -182,7 +183,7 @@ Timeout Timeout::FromMillis(int64_t millis) { } Timeout Timeout::FromSeconds(int64_t seconds) { - GPR_DEBUG_ASSERT(seconds != 0); + DCHECK_NE(seconds, 0); if (seconds < 1000) { if (seconds % kSecondsPerMinute != 0) { return Timeout(seconds, Unit::kSeconds); @@ -202,7 +203,7 @@ Timeout Timeout::FromSeconds(int64_t seconds) { } Timeout Timeout::FromMinutes(int64_t minutes) { - GPR_DEBUG_ASSERT(minutes != 0); + DCHECK_NE(minutes, 0); if (minutes < 1000) { if (minutes % kMinutesPerHour != 0) { return Timeout(minutes, Unit::kMinutes); @@ -222,7 +223,7 @@ Timeout Timeout::FromMinutes(int64_t minutes) { } Timeout Timeout::FromHours(int64_t hours) { - GPR_DEBUG_ASSERT(hours != 0); + DCHECK_NE(hours, 0); if (hours < kMaxHours) { return Timeout(hours, Unit::kHours); } diff --git a/tools/doxygen/Doxyfile.c++.internal b/tools/doxygen/Doxyfile.c++.internal index 87f43bfb161..435df91432a 100644 --- a/tools/doxygen/Doxyfile.c++.internal +++ b/tools/doxygen/Doxyfile.c++.internal @@ -2321,7 +2321,6 @@ src/core/lib/gpr/iphone/cpu.cc \ src/core/lib/gpr/linux/cpu.cc \ src/core/lib/gpr/linux/log.cc \ src/core/lib/gpr/log.cc \ -src/core/lib/gpr/log_internal.h \ src/core/lib/gpr/msys/tmpfile.cc \ src/core/lib/gpr/posix/cpu.cc \ src/core/lib/gpr/posix/log.cc \ diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index 06fbb0305b5..c05ab1fc01d 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -2094,7 +2094,6 @@ src/core/lib/gpr/iphone/cpu.cc \ src/core/lib/gpr/linux/cpu.cc \ src/core/lib/gpr/linux/log.cc \ src/core/lib/gpr/log.cc \ -src/core/lib/gpr/log_internal.h \ src/core/lib/gpr/msys/tmpfile.cc \ src/core/lib/gpr/posix/cpu.cc \ src/core/lib/gpr/posix/log.cc \