Skip to content

Commit 8fb90c8

Browse files
authored
Remove functions that throw status error. (#9602)
Follow-up: #9580 This PR finalizes the work that started with #9580 for replacing `OkOrThrow()` and `GetValueOrThrow()` in favor of the macros introduced in #9588 (which were also part of that work). In summary, is the last one after: - #9580: initial work that was broken down into the PRs below - #9588: actual first PR merged - #9590 - #9591 - #9592 - #9593 - #9594 - #9595 - #9596 - #9602: last PR **Key Changes:** - (`test/cpp/test_status_common.h`) Remove tests for `OkOrThrow()` and `GetValueOrThrow()` - (`torch_xla/csrc/status.{h,cpp}`) Remove the implementation of those functions
1 parent 763e5b7 commit 8fb90c8

File tree

3 files changed

+0
-137
lines changed

3 files changed

+0
-137
lines changed

test/cpp/test_status_common.h

Lines changed: 0 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,6 @@ class StatusTest : public testing::TestWithParam<CppStacktracesMode> {
8080
namespace cpp_test {
8181

8282
// Prefix of the C++ stacktrace PyTorch adds to the error message.
83-
constexpr inline char kTorchCppStacktracePrefixDeprecated[] =
84-
"Exception raised from OkOrThrow at torch_xla/csrc/status.cpp:";
8583
constexpr inline char kTorchCppStacktracePrefix[] =
8684
"Exception raised from ThrowStatusError at torch_xla/csrc/status.cpp:";
8785

@@ -102,50 +100,6 @@ inline std::string GetStatusPropagationTrace(const absl::Status& status) {
102100
: "";
103101
}
104102

105-
TEST_P(StatusTest, OkOrThrowWithOkStatus) {
106-
absl::Status ok_status = absl::OkStatus();
107-
EXPECT_NO_THROW(OkOrThrow(ok_status));
108-
}
109-
110-
TEST_P(StatusTest, OkOrThrowWithErrorStatus) {
111-
try {
112-
absl::Status error_status = absl::InvalidArgumentError(kMessage);
113-
OkOrThrow(error_status);
114-
} catch (const c10::Error& error) {
115-
if (IsShowCppStacktracesMode()) {
116-
EXPECT_THAT(std::string_view(error.what()),
117-
::testing::StartsWith(absl::StrCat(
118-
kMessage, "\n\n", kTorchCppStacktracePrefixDeprecated)));
119-
} else {
120-
EXPECT_EQ(std::string_view(error.what_without_backtrace()),
121-
std::string_view(kMessage));
122-
}
123-
}
124-
}
125-
126-
TEST_P(StatusTest, GetValueOrThrowWithOkStatusOr) {
127-
int value = 42;
128-
absl::StatusOr<int> status_or = value;
129-
int result = GetValueOrThrow(std::move(status_or));
130-
EXPECT_EQ(result, value);
131-
}
132-
133-
TEST_P(StatusTest, GetValueOrThrowWithErrorStatusOr) {
134-
try {
135-
absl::StatusOr<int> error_status = absl::InvalidArgumentError(kMessage);
136-
int value = GetValueOrThrow(error_status);
137-
} catch (const c10::Error& error) {
138-
if (IsShowCppStacktracesMode()) {
139-
EXPECT_THAT(std::string_view(error.what()),
140-
::testing::StartsWith(absl::StrCat(
141-
kMessage, "\n\n", kTorchCppStacktracePrefixDeprecated)));
142-
} else {
143-
EXPECT_EQ(std::string_view(error.what_without_backtrace()),
144-
std::string_view(kMessage));
145-
}
146-
}
147-
}
148-
149103
TEST_P(StatusTest, MaybeWithLocationPropagatesErrorStatus) {
150104
absl::Status error_status = absl::InvalidArgumentError(kMessage);
151105
absl::Status result =
@@ -345,61 +299,6 @@ TEST_P(StatusTest, MacroErrorWithLocation) {
345299
}
346300
}
347301

348-
TEST_P(StatusTest, OkOrThrowWithErrorPropagationWithNewMessage) {
349-
int32_t errline0 = __LINE__ + 2;
350-
auto innerfn = [&]() -> absl::Status {
351-
return XLA_ERROR_WITH_LOCATION(absl::InvalidArgumentError(kMessage));
352-
};
353-
354-
int32_t errline1 = __LINE__ + 2;
355-
auto midfn = [&]() -> absl::Status {
356-
XLA_RETURN_IF_ERROR(innerfn(), kNewMessage);
357-
return absl::OkStatus();
358-
};
359-
360-
int32_t errline2 = __LINE__ + 2;
361-
auto outerfn = [&]() -> absl::Status {
362-
XLA_RETURN_IF_ERROR(midfn());
363-
return absl::OkStatus();
364-
};
365-
366-
try {
367-
OkOrThrow(outerfn());
368-
} catch (const c10::Error& error) {
369-
if (IsShowCppStacktracesMode()) {
370-
// Expected Error Message Prefix
371-
// =============================
372-
//
373-
// New test error kMessage
374-
//
375-
// Status Propagation Stacktrace:
376-
// From: ./test/cpp/test_status_common.h:329 (error: Test error
377-
// kMessage) From: ./test/cpp/test_status_common.h:335 (error: New
378-
// test error kMessage) From: ./test/cpp/test_status_common.h:342
379-
//
380-
// C++ Stacktrace:
381-
//
382-
std::ostringstream oss;
383-
oss << kNewMessage;
384-
oss << "\n\n";
385-
oss << "Status Propagation Trace:";
386-
oss << kEntryPrefix << "From: operator() at " << __FILE__ << ":"
387-
<< errline0 << " (error: " << kMessage << ")";
388-
oss << kEntryPrefix << "From: operator() at " << __FILE__ << ":"
389-
<< errline1 << " (error: " << kNewMessage << ")";
390-
oss << kEntryPrefix << "From: operator() at " << __FILE__ << ":"
391-
<< errline2;
392-
oss << "\n\n";
393-
oss << kTorchCppStacktracePrefixDeprecated;
394-
EXPECT_THAT(std::string_view(error.what()),
395-
::testing::StartsWith(oss.str()));
396-
} else {
397-
EXPECT_EQ(std::string_view(error.what_without_backtrace()),
398-
std::string_view(kNewMessage));
399-
}
400-
}
401-
}
402-
403302
TEST_P(StatusTest, MacroThrowIfErrorWithErrorPropagationWithNewMessage) {
404303
int32_t errline0 = __LINE__ + 2;
405304
auto innerfn = [&]() -> absl::Status {

torch_xla/csrc/status.cpp

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -129,13 +129,6 @@ void status_internal::ThrowStatusError(const absl::Status& status,
129129
LineBreakIfCppStacktracesEnabled()));
130130
}
131131

132-
void OkOrThrow(const absl::Status& status) {
133-
TORCH_CHECK(status.ok(), absl::StrCat(BuildStatusErrorMessage(status),
134-
LineBreakIfCppStacktracesEnabled()));
135-
}
136-
137-
void GetValueOrThrow(const absl::Status& status) { OkOrThrow(status); }
138-
139132
void status_internal::OkOrDie(const absl::Status& status, const char* file,
140133
const int32_t line, const char* function,
141134
std::string_view message) {

torch_xla/csrc/status.h

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -297,35 +297,6 @@ void OkOrDie(const absl::Status& status, const char* file, const int32_t line,
297297
// It doesn't add a trailing line break.
298298
std::string BuildStatusErrorMessage(const absl::Status& status);
299299

300-
// Throws an exception if `status` has a non-ok code.
301-
//
302-
// Ideally, this function should be used only used in the project's
303-
// boundary, e.g. when we need to throw an exception for the user to see.
304-
void OkOrThrow(const absl::Status& status);
305-
306-
// Either returns the value `status` holds, if it's an ok-status, or throw an
307-
// exception from its error status.
308-
template <class T>
309-
T& GetValueOrThrow(absl::StatusOr<T>& status) {
310-
OkOrThrow(status.status());
311-
return status.value();
312-
}
313-
314-
template <class T>
315-
const T& GetValueOrThrow(const absl::StatusOr<T>& status) {
316-
OkOrThrow(status.status());
317-
return status.value();
318-
}
319-
320-
template <class T>
321-
T GetValueOrThrow(absl::StatusOr<T>&& status) {
322-
OkOrThrow(status.status());
323-
return std::move(status).value();
324-
}
325-
326-
// `GetValueOrThrow` overload for `Status`.
327-
void GetValueOrThrow(const absl::Status& status);
328-
329300
} // namespace torch_xla
330301

331302
#endif // XLA_TORCH_XLA_CSRC_STATUS_H_

0 commit comments

Comments
 (0)