From 90eb4a19d8593d366ddf7aed894d8bb1161da39c Mon Sep 17 00:00:00 2001 From: MediaPipe Team Date: Wed, 16 Nov 2022 18:11:00 -0800 Subject: [PATCH] Internal change PiperOrigin-RevId: 489088227 --- mediapipe/framework/deps/status_builder.cc | 23 ++--------- mediapipe/framework/deps/status_builder.h | 19 +-------- .../framework/deps/status_builder_test.cc | 39 ------------------- mediapipe/framework/deps/status_macros.h | 29 +++++++------- 4 files changed, 20 insertions(+), 90 deletions(-) diff --git a/mediapipe/framework/deps/status_builder.cc b/mediapipe/framework/deps/status_builder.cc index 70775949d..0202b8689 100644 --- a/mediapipe/framework/deps/status_builder.cc +++ b/mediapipe/framework/deps/status_builder.cc @@ -97,39 +97,24 @@ absl::Status StatusBuilder::Impl::JoinMessageToStatus() { }()); } -StatusBuilder::Impl::Impl(const absl::Status& status, const char* file, - int line) - : status(status), line(line), file(file), stream() {} - -StatusBuilder::Impl::Impl(absl::Status&& status, const char* file, int line) - : status(std::move(status)), line(line), file(file), stream() {} - StatusBuilder::Impl::Impl(const absl::Status& status, mediapipe::source_location location) - : status(status), - line(location.line()), - file(location.file_name()), - stream() {} + : status(status), location(location), stream() {} StatusBuilder::Impl::Impl(absl::Status&& status, mediapipe::source_location location) - : status(std::move(status)), - line(location.line()), - file(location.file_name()), - stream() {} + : status(std::move(status)), location(location), stream() {} StatusBuilder::Impl::Impl(const Impl& other) : status(other.status), - line(other.line), - file(other.file), + location(other.location), no_logging(other.no_logging), stream(other.stream.str()), join_style(other.join_style) {} StatusBuilder::Impl& StatusBuilder::Impl::operator=(const Impl& other) { status = other.status; - line = other.line; - file = other.file; + location = other.location; no_logging = other.no_logging; stream = std::ostringstream(other.stream.str()); join_style = other.join_style; diff --git a/mediapipe/framework/deps/status_builder.h b/mediapipe/framework/deps/status_builder.h index d2e40d575..ae11699d2 100644 --- a/mediapipe/framework/deps/status_builder.h +++ b/mediapipe/framework/deps/status_builder.h @@ -60,17 +60,6 @@ class ABSL_MUST_USE_RESULT StatusBuilder { ? nullptr : std::make_unique(absl::Status(code, ""), location)) {} - StatusBuilder(const absl::Status& original_status, const char* file, int line) - : impl_(original_status.ok() - ? nullptr - : std::make_unique(original_status, file, line)) {} - - StatusBuilder(absl::Status&& original_status, const char* file, int line) - : impl_(original_status.ok() - ? nullptr - : std::make_unique(std::move(original_status), file, - line)) {} - bool ok() const { return !impl_; } StatusBuilder& SetAppend() &; @@ -109,8 +98,6 @@ class ABSL_MUST_USE_RESULT StatusBuilder { kPrepend, }; - Impl(const absl::Status& status, const char* file, int line); - Impl(absl::Status&& status, const char* file, int line); Impl(const absl::Status& status, mediapipe::source_location location); Impl(absl::Status&& status, mediapipe::source_location location); Impl(const Impl&); @@ -120,10 +107,8 @@ class ABSL_MUST_USE_RESULT StatusBuilder { // The status that the result will be based on. absl::Status status; - // The line to record if this file is logged. - int line; - // Not-owned: The file to record if this status is logged. - const char* file; + // The source location to record if this file is logged. + mediapipe::source_location location; // Logging disabled if true. bool no_logging = false; // The additional messages added with `<<`. This is nullptr when status_ is diff --git a/mediapipe/framework/deps/status_builder_test.cc b/mediapipe/framework/deps/status_builder_test.cc index 560acd3c6..f517bb909 100644 --- a/mediapipe/framework/deps/status_builder_test.cc +++ b/mediapipe/framework/deps/status_builder_test.cc @@ -33,21 +33,6 @@ TEST(StatusBuilder, OkStatusRvalue) { ASSERT_EQ(status, absl::OkStatus()); } -TEST(StatusBuilder, OkStatusFileAndLineRvalueStatus) { - absl::Status status = StatusBuilder(absl::OkStatus(), "hello.cc", 1234) - << "annotated message1 " - << "annotated message2"; - ASSERT_EQ(status, absl::OkStatus()); -} - -TEST(StatusBuilder, OkStatusFileAndLineLvalueStatus) { - const auto original_status = absl::OkStatus(); - absl::Status status = StatusBuilder(original_status, "hello.cc", 1234) - << "annotated message1 " - << "annotated message2"; - ASSERT_EQ(status, absl::OkStatus()); -} - TEST(StatusBuilder, AnnotateMode) { absl::Status status = StatusBuilder(absl::Status(absl::StatusCode::kNotFound, "original message"), @@ -60,30 +45,6 @@ TEST(StatusBuilder, AnnotateMode) { "original message; annotated message1 annotated message2"); } -TEST(StatusBuilder, AnnotateModeFileAndLineRvalueStatus) { - absl::Status status = StatusBuilder(absl::Status(absl::StatusCode::kNotFound, - "original message"), - "hello.cc", 1234) - << "annotated message1 " - << "annotated message2"; - ASSERT_FALSE(status.ok()); - EXPECT_EQ(status.code(), absl::StatusCode::kNotFound); - EXPECT_EQ(status.message(), - "original message; annotated message1 annotated message2"); -} - -TEST(StatusBuilder, AnnotateModeFileAndLineLvalueStatus) { - const auto original_status = - absl::Status(absl::StatusCode::kNotFound, "original message"); - absl::Status status = StatusBuilder(original_status, "hello.cc", 1234) - << "annotated message1 " - << "annotated message2"; - ASSERT_FALSE(status.ok()); - EXPECT_EQ(status.code(), absl::StatusCode::kNotFound); - EXPECT_EQ(status.message(), - "original message; annotated message1 annotated message2"); -} - TEST(StatusBuilder, PrependModeLvalue) { StatusBuilder builder( absl::Status(absl::StatusCode::kInvalidArgument, "original message"), diff --git a/mediapipe/framework/deps/status_macros.h b/mediapipe/framework/deps/status_macros.h index 757d99392..92bbf0b84 100644 --- a/mediapipe/framework/deps/status_macros.h +++ b/mediapipe/framework/deps/status_macros.h @@ -81,11 +81,11 @@ // MP_RETURN_IF_ERROR(foo.Method(args...)); // return absl::OkStatus(); // } -#define MP_RETURN_IF_ERROR(expr) \ - STATUS_MACROS_IMPL_ELSE_BLOCKER_ \ - if (mediapipe::status_macro_internal::StatusAdaptorForMacros \ - status_macro_internal_adaptor = {(expr), __FILE__, __LINE__}) { \ - } else /* NOLINT */ \ +#define MP_RETURN_IF_ERROR(expr) \ + STATUS_MACROS_IMPL_ELSE_BLOCKER_ \ + if (mediapipe::status_macro_internal::StatusAdaptorForMacros \ + status_macro_internal_adaptor = {(expr), MEDIAPIPE_LOC}) { \ + } else /* NOLINT */ \ return status_macro_internal_adaptor.Consume() // Executes an expression `rexpr` that returns a `absl::StatusOr`. On @@ -156,14 +156,14 @@ return mediapipe::StatusBuilder( \ std::move(STATUS_MACROS_IMPL_CONCAT_(_status_or_value, __LINE__)) \ .status(), \ - __FILE__, __LINE__)) + MEDIAPIPE_LOC)) #define STATUS_MACROS_IMPL_ASSIGN_OR_RETURN_3_(lhs, rexpr, error_expression) \ STATUS_MACROS_IMPL_ASSIGN_OR_RETURN_( \ STATUS_MACROS_IMPL_CONCAT_(_status_or_value, __LINE__), lhs, rexpr, \ mediapipe::StatusBuilder _( \ std::move(STATUS_MACROS_IMPL_CONCAT_(_status_or_value, __LINE__)) \ .status(), \ - __FILE__, __LINE__); \ + MEDIAPIPE_LOC); \ (void)_; /* error_expression is allowed to not use this variable */ \ return (error_expression)) #define STATUS_MACROS_IMPL_ASSIGN_OR_RETURN_(statusor, lhs, rexpr, \ @@ -201,18 +201,17 @@ namespace status_macro_internal { // that declares a variable. class StatusAdaptorForMacros { public: - StatusAdaptorForMacros(const absl::Status& status, const char* file, int line) - : builder_(status, file, line) {} + StatusAdaptorForMacros(const absl::Status& status, source_location location) + : builder_(status, location) {} - StatusAdaptorForMacros(absl::Status&& status, const char* file, int line) - : builder_(std::move(status), file, line) {} + StatusAdaptorForMacros(absl::Status&& status, source_location location) + : builder_(std::move(status), location) {} - StatusAdaptorForMacros(const StatusBuilder& builder, const char* /* file */, - int /* line */) + StatusAdaptorForMacros(const StatusBuilder& builder, + source_location /*location*/) : builder_(builder) {} - StatusAdaptorForMacros(StatusBuilder&& builder, const char* /* file */, - int /* line */) + StatusAdaptorForMacros(StatusBuilder&& builder, source_location /*location*/) : builder_(std::move(builder)) {} StatusAdaptorForMacros(const StatusAdaptorForMacros&) = delete;