Skip to content

Commit 487988e

Browse files
committed
Add WITH_LOCATION macros and expand test coverage.
- Add `XLA_RETURN_IF_ERROR_WITH_LOCATION` macro for external library status propagation - Add `XLA_ASSIGN_OR_RETURN_WITH_LOCATION` macro for external library status handling - Enhance test coverage with new test cases for location-specific macro variants - Improve macro documentation to clarify internal vs external usage patterns
1 parent d6ba41f commit 487988e

File tree

2 files changed

+98
-10
lines changed

2 files changed

+98
-10
lines changed

test/cpp/test_status_show_cpp_error_context.cpp

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ TEST(StatusWithErrorContextTest, MacroReturnIfErrorWithNestedError) {
135135

136136
TEST(StatusWithErrorContextTest, MacroReturnIfErrorWithErrorWithNewMessage) {
137137
int32_t errline = 0;
138+
138139
auto test_function = [&errline]() -> Status {
139140
Status error_status = absl::InvalidArgumentError(message);
140141
errline = __LINE__ + 1;
@@ -150,6 +151,23 @@ TEST(StatusWithErrorContextTest, MacroReturnIfErrorWithErrorWithNewMessage) {
150151
")\nFrom Error: Test error message"));
151152
}
152153

154+
TEST(StatusWithErrorContextTest, MacroReturnIfErrorWithLocationWithError) {
155+
int32_t errline = 0;
156+
157+
auto test_function = [&errline]() -> Status {
158+
Status error_status = absl::InvalidArgumentError(message);
159+
errline = __LINE__ + 1;
160+
XLA_RETURN_IF_ERROR_WITH_LOCATION(error_status);
161+
return absl::OkStatus();
162+
};
163+
164+
Status result = test_function();
165+
ASSERT_FALSE(result.ok());
166+
EXPECT_EQ(result.code(), StatusCode::kInvalidArgument);
167+
EXPECT_EQ(result.message(),
168+
StrCat("Test error message (at ", __FILE__, ":", errline, ")"));
169+
}
170+
153171
TEST(StatusWithErrorContextTest, MacroAssignOrReturn) {
154172
int initial_value = 42;
155173
int expected_value = initial_value * 2;
@@ -196,6 +214,23 @@ TEST(StatusWithErrorContextTest, MacroAssignOrReturnWithErrorWithNewMessage) {
196214
")\nFrom Error: Test error message"));
197215
}
198216

217+
TEST(StatusWithErrorContextTest, MacroAssignOrReturnWithLocationWithError) {
218+
int32_t errline = 0;
219+
220+
auto test_function = [&errline]() -> StatusOr<int> {
221+
StatusOr<int> status_or = absl::InvalidArgumentError(message);
222+
errline = __LINE__ + 1;
223+
XLA_ASSIGN_OR_RETURN_WITH_LOCATION(int value, status_or);
224+
return value * 2;
225+
};
226+
227+
StatusOr<int> result = test_function();
228+
ASSERT_FALSE(result.ok());
229+
EXPECT_EQ(result.status().code(), StatusCode::kInvalidArgument);
230+
EXPECT_EQ(result.status().message(),
231+
StrCat("Test error message (at ", __FILE__, ":", errline, ")"));
232+
}
233+
199234
TEST(StatusWithErrorContextTest, MacroErrorWithLocation) {
200235
Status error_status = absl::InvalidArgumentError(message);
201236
int32_t errline = __LINE__ + 1;

torch_xla/csrc/status.h

Lines changed: 63 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -39,21 +39,30 @@ namespace torch_xla {
3939
// Unique identifier for the status variable for the current line.
4040
#define XLA_STATUS_VAR_ XLA_CONCAT_(status_, __LINE__)
4141

42+
// Fake wrapper to `status`.
43+
//
44+
// This is used in place of `XLA_ERROR_WITH_LOCATION`, whenever we don't
45+
// want to append source code location information to the error message,
46+
// e.g. `XLA_RETURN_IF_ERROR` and `XLA_ASSIGN_OR_RETURN`.
47+
#define XLA_NO_WRAP_(status) status
48+
4249
// Provides a flexible way to handle error checking with optional message
4350
// modification. It evaluates `expr`, checks if it's OK, and either:
4451
// 1. Returns early with an error status (potentially modified by the provided
4552
// additional messages)
4653
// 2. Proceeds with the given `then` block if successful
47-
#define XLA_RETURN_IF_ERROR_IMPL_(expr, var, then, ...) \
48-
auto var = (expr); \
49-
if (!var.ok()) { \
50-
return ::torch_xla::MaybeWithNewMessage( \
51-
::torch_xla::GetStatus(var), __FILE__, __LINE__, ##__VA_ARGS__); \
52-
} \
54+
#define XLA_RETURN_IF_ERROR_IMPL_(expr, var, wrapper, then, ...) \
55+
auto var = (expr); \
56+
if (!var.ok()) { \
57+
return wrapper(::torch_xla::MaybeWithNewMessage( \
58+
::torch_xla::GetStatus(var), __FILE__, __LINE__, ##__VA_ARGS__)); \
59+
} \
5360
then
5461

5562
// Propagates `rexpr`, in case it's a non-ok status.
5663
//
64+
// This macro should be used for propagating status internally.
65+
//
5766
// Example:
5867
//
5968
// XLA_RETURN_IF_ERROR(
@@ -69,14 +78,37 @@ namespace torch_xla {
6978
// Previous error message. (at <cpp-source-file>:<line>)
7079
// ...
7180
//
72-
#define XLA_RETURN_IF_ERROR(rexpr, ...) \
73-
do { \
74-
XLA_RETURN_IF_ERROR_IMPL_(rexpr, XLA_STATUS_VAR_, {}, ##__VA_ARGS__) \
81+
#define XLA_RETURN_IF_ERROR(rexpr, ...) \
82+
do { \
83+
XLA_RETURN_IF_ERROR_IMPL_(rexpr, XLA_STATUS_VAR_, XLA_NO_WRAP_, {}, \
84+
##__VA_ARGS__) \
85+
} while (false)
86+
87+
// Propagates `rexpr`, in case it's a non-ok status, appending the source code
88+
// location to it.
89+
//
90+
// Note that while the macro above will append the source code location only if
91+
// a new message is given, this macro will append the source code location if
92+
// `XLA_SHOW_CPP_ERROR_CONTEXT` is set.
93+
//
94+
// This macro should be used whenever we are propagating some status that came
95+
// from some external library.
96+
//
97+
// Example:
98+
//
99+
// XLA_RETURN_IF_ERROR_WITH_LOCATION(FnThatReturnsStatus());
100+
//
101+
#define XLA_RETURN_IF_ERROR_WITH_LOCATION(rexpr) \
102+
do { \
103+
XLA_RETURN_IF_ERROR_IMPL_(rexpr, XLA_STATUS_VAR_, XLA_ERROR_WITH_LOCATION, \
104+
{}) \
75105
} while (false)
76106

77107
// Propagates `rexpr`, in case it's a non-ok status. Otherwise, assign
78108
// its result to `lhs`.
79109
//
110+
// This macro should be used for propagating status internally.
111+
//
80112
// Note 1: `lhs` might be a variable declarate, e.g:
81113
//
82114
// Note 2: this macro will be replaced by multiple statements that live on
@@ -100,10 +132,31 @@ namespace torch_xla {
100132
// ...
101133
//
102134
#define XLA_ASSIGN_OR_RETURN(lhs, rexpr, ...) \
103-
XLA_RETURN_IF_ERROR_IMPL_(rexpr, XLA_STATUS_VAR_, \
135+
XLA_RETURN_IF_ERROR_IMPL_(rexpr, XLA_STATUS_VAR_, XLA_NO_WRAP_, \
104136
lhs = std::move(XLA_STATUS_VAR_).value(), \
105137
##__VA_ARGS__)
106138

139+
// Propagates `rexpr`, in case it's a non-ok status. Otherwise, assign
140+
// its result to `lhs`.
141+
//
142+
// Note that while the macro above will append the source code location only if
143+
// a new message is given, this macro will append the source code location if
144+
// `XLA_SHOW_CPP_ERROR_CONTEXT` is set.
145+
//
146+
// This macro should be used whenever we are propagating some status that came
147+
// from some external library.
148+
//
149+
// Example:
150+
//
151+
// XLA_ASSIGN_OR_RETURN_WITH_LOCATION(
152+
// int result,
153+
// FnThatReturnsStatus(),
154+
// );
155+
//
156+
#define XLA_ASSIGN_OR_RETURN_WITH_LOCATION(lhs, rexpr) \
157+
XLA_RETURN_IF_ERROR_IMPL_(rexpr, XLA_STATUS_VAR_, XLA_ERROR_WITH_LOCATION, \
158+
lhs = std::move(XLA_STATUS_VAR_).value())
159+
107160
// Maybe shows location information in the status message.
108161
//
109162
// This function assumes that `status` is a non-ok status.

0 commit comments

Comments
 (0)