Skip to content

Commit bed6f4d

Browse files
ysiraichiysiraichi
committed
Add __FUNCTION__ to the status propagation trace.
1 parent 1486045 commit bed6f4d

File tree

3 files changed

+42
-34
lines changed

3 files changed

+42
-34
lines changed

‎test/cpp/test_status_common.h

Lines changed: 27 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ namespace testing {
7979
constexpr inline char kNewMessage[] = "New test error message";
8080
constexpr inline char kMessage[] = "Test error message";
8181
constexpr inline char kFile[] = "test_file.cpp";
82+
constexpr inline char kFunction[] = "foo";
8283
constexpr inline char kEntryPrefix[] = "\n ";
8384
constexpr inline int32_t kLine = 42;
8485

@@ -128,7 +129,7 @@ TEST_P(StatusTest, GetValueOrThrowWithErrorStatusOr) {
128129
TEST_P(StatusTest, MaybeWithLocationPropagatesErrorStatus) {
129130
absl::Status error_status = absl::InvalidArgumentError(kMessage);
130131
absl::Status result =
131-
status_internal::MaybeWithLocation(error_status, kFile, kLine);
132+
status_internal::MaybeWithLocation(error_status, kFile, kLine, kFunction);
132133

133134
ASSERT_FALSE(result.ok());
134135
EXPECT_EQ(result.code(), error_status.code());
@@ -137,17 +138,17 @@ TEST_P(StatusTest, MaybeWithLocationPropagatesErrorStatus) {
137138
if (IsShowCppStacktracesMode()) {
138139
EXPECT_NE(result, error_status);
139140
EXPECT_EQ(GetStatusPropagationTrace(result),
140-
absl::StrCat(kEntryPrefix, "From: ", kFile, ":", kLine,
141-
" (error: ", kMessage, ")"));
141+
absl::StrCat(kEntryPrefix, "From: ", kFunction, " at ", kFile,
142+
":", kLine, " (error: ", kMessage, ")"));
142143
} else {
143144
EXPECT_EQ(result, error_status);
144145
}
145146
}
146147

147148
TEST_P(StatusTest, MaybeWithNewMessageEmptyNewMessage) {
148149
absl::Status error_status = absl::InvalidArgumentError(kMessage);
149-
absl::Status result =
150-
status_internal::MaybeWithNewMessage(error_status, kFile, kLine);
150+
absl::Status result = status_internal::MaybeWithNewMessage(
151+
error_status, kFile, kLine, kFunction);
151152

152153
ASSERT_FALSE(result.ok());
153154
EXPECT_EQ(result.code(), error_status.code());
@@ -156,7 +157,8 @@ TEST_P(StatusTest, MaybeWithNewMessageEmptyNewMessage) {
156157
if (IsShowCppStacktracesMode()) {
157158
EXPECT_NE(result, error_status);
158159
EXPECT_EQ(GetStatusPropagationTrace(result),
159-
absl::StrCat(kEntryPrefix, "From: ", kFile, ":", kLine));
160+
absl::StrCat(kEntryPrefix, "From: ", kFunction, " at ", kFile,
161+
":", kLine));
160162
} else {
161163
EXPECT_EQ(result, error_status);
162164
}
@@ -165,7 +167,7 @@ TEST_P(StatusTest, MaybeWithNewMessageEmptyNewMessage) {
165167
TEST_P(StatusTest, MaybeWithNewMessageNonEmptyNewMessage) {
166168
absl::Status error_status = absl::InvalidArgumentError(kMessage);
167169
absl::Status result = status_internal::MaybeWithNewMessage(
168-
error_status, kFile, kLine, kNewMessage);
170+
error_status, kFile, kLine, kFunction, kNewMessage);
169171

170172
ASSERT_FALSE(result.ok());
171173
EXPECT_EQ(result.code(), error_status.code());
@@ -174,8 +176,8 @@ TEST_P(StatusTest, MaybeWithNewMessageNonEmptyNewMessage) {
174176

175177
if (IsShowCppStacktracesMode()) {
176178
EXPECT_EQ(GetStatusPropagationTrace(result),
177-
absl::StrCat(kEntryPrefix, "From: ", kFile, ":", kLine,
178-
" (error: ", kNewMessage, ")"));
179+
absl::StrCat(kEntryPrefix, "From: ", kFunction, " at ", kFile,
180+
":", kLine, " (error: ", kNewMessage, ")"));
179181
}
180182
}
181183

@@ -230,10 +232,12 @@ TEST_P(StatusTest, MacroReturnIfErrorWithNestedError) {
230232
EXPECT_EQ(result.message(), std::string_view(kMessage));
231233

232234
if (IsShowCppStacktracesMode()) {
233-
auto frame0 = absl::StrCat(kEntryPrefix, "From: ", __FILE__, ":", errline0,
234-
" (error: ", kMessage, ")");
235-
auto frame1 = absl::StrCat(kEntryPrefix, "From: ", __FILE__, ":", errline1);
236-
auto frame2 = absl::StrCat(kEntryPrefix, "From: ", __FILE__, ":", errline2);
235+
auto frame0 = absl::StrCat(kEntryPrefix, "From: operator() at ", __FILE__,
236+
":", errline0, " (error: ", kMessage, ")");
237+
auto frame1 = absl::StrCat(kEntryPrefix, "From: operator() at ", __FILE__,
238+
":", errline1);
239+
auto frame2 = absl::StrCat(kEntryPrefix, "From: operator() at ", __FILE__,
240+
":", errline2);
237241
EXPECT_EQ(GetStatusPropagationTrace(result),
238242
absl::StrCat(frame0, frame1, frame2));
239243
}
@@ -254,8 +258,8 @@ TEST_P(StatusTest, MacroReturnIfErrorWithErrorWithNewMessage) {
254258

255259
if (IsShowCppStacktracesMode()) {
256260
EXPECT_EQ(GetStatusPropagationTrace(result),
257-
absl::StrCat(kEntryPrefix, "From: ", __FILE__, ":", errline,
258-
" (error: ", kNewMessage, ")"));
261+
absl::StrCat(kEntryPrefix, "From: operator() at ", __FILE__, ":",
262+
errline, " (error: ", kNewMessage, ")"));
259263
}
260264
}
261265

@@ -302,8 +306,8 @@ TEST_P(StatusTest, MacroAssignOrReturnWithErrorWithNewMessage) {
302306

303307
if (IsShowCppStacktracesMode()) {
304308
EXPECT_EQ(GetStatusPropagationTrace(result.status()),
305-
absl::StrCat(kEntryPrefix, "From: ", __FILE__, ":", errline,
306-
" (error: ", kNewMessage, ")"));
309+
absl::StrCat(kEntryPrefix, "From: operator() at ", __FILE__, ":",
310+
errline, " (error: ", kNewMessage, ")"));
307311
}
308312
}
309313

@@ -316,8 +320,8 @@ TEST_P(StatusTest, MacroErrorWithLocation) {
316320
EXPECT_EQ(result.message(), std::string_view(kMessage));
317321
if (IsShowCppStacktracesMode()) {
318322
EXPECT_EQ(GetStatusPropagationTrace(result),
319-
absl::StrCat(kEntryPrefix, "From: ", __FILE__, ":", errline,
320-
" (error: ", kMessage, ")"));
323+
absl::StrCat(kEntryPrefix, "From: ", __FUNCTION__, " at ",
324+
__FILE__, ":", errline, " (error: ", kMessage, ")"));
321325
}
322326
}
323327

@@ -356,10 +360,10 @@ TEST_P(StatusTest, MaybeThrowWithErrorPropagationWithNewMessage) {
356360
//
357361
std::string expected_prefix = absl::StrCat(
358362
kNewMessage, "\n\nStatus Propagation Trace:", kEntryPrefix,
359-
"From: ", __FILE__, ":", errline0, " (error: ", kMessage, ")",
360-
kEntryPrefix, "From: ", __FILE__, ":", errline1,
361-
" (error: ", kNewMessage, ")", kEntryPrefix, "From: ", __FILE__, ":",
362-
errline2, "\n\nC++ Stacktrace:\n");
363+
"From: operator() at ", __FILE__, ":", errline0, " (error: ", kMessage,
364+
")", kEntryPrefix, "From: operator() at ", __FILE__, ":", errline1,
365+
" (error: ", kNewMessage, ")", kEntryPrefix, "From: operator() at ",
366+
__FILE__, ":", errline2, "\n\nC++ Stacktrace:\n");
363367

364368
EXPECT_THAT(throw_exception, ::testing::ThrowsMessage<std::runtime_error>(
365369
::testing::StartsWith(expected_prefix)));

‎torch_xla/csrc/status.cpp

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,15 @@ constexpr char kFramePrefix[] = "\n ";
2121
// trace of the status currently being processed.
2222
//
2323
// Example:
24-
// \n From: <file>:<line> [(error: <message>)]
24+
// \n From: <function> at <file>:<line> [(error: <message>)]
2525
//
2626
static std::string GetStackFrame(const char* file, const int32_t line,
27+
const char* function,
2728
const std::string_view new_message) {
2829
auto error_suffix =
2930
new_message.empty() ? "" : absl::StrCat(" (error: ", new_message, ")");
30-
return absl::StrCat(kEntryPrefix, "From: ", file, ":", line, error_suffix);
31+
return absl::StrCat(kFramePrefix, "From: ", function, " at ", file, ":", line,
32+
error_suffix);
3133
}
3234

3335
// Convenient function that retrieves the status propagation trace payload
@@ -39,7 +41,8 @@ static absl::Cord GetStatusPropagationTraceOrEmpty(const absl::Status& status) {
3941

4042
absl::Status status_internal::MaybeWithLocation(const absl::Status& status,
4143
const char* file,
42-
const int32_t line) {
44+
const int32_t line,
45+
const char* function) {
4346
ABSL_CHECK(!status.ok());
4447

4548
// Return the same status if we don't need to add the C++ source location.
@@ -54,12 +57,12 @@ absl::Status status_internal::MaybeWithLocation(const absl::Status& status,
5457
// the status message:
5558
// 1. An stack frame will be added to the status propagation trace
5659
// 2. The status' message will be the same
57-
return MaybeWithNewMessage(status, file, line, status.message());
60+
return MaybeWithNewMessage(status, file, line, function, status.message());
5861
}
5962

6063
absl::Status status_internal::MaybeWithNewMessage(
6164
const absl::Status& status, const char* file, const int32_t line,
62-
const std::string_view new_message) {
65+
const char* function, const std::string_view new_message) {
6366
ABSL_CHECK(!status.ok());
6467

6568
// Return the same status if:
@@ -84,7 +87,7 @@ absl::Status status_internal::MaybeWithNewMessage(
8487
// 2. append the new error message, if not empty
8588
if (torch::get_cpp_stacktraces_enabled()) {
8689
auto status_propagation_trace = GetStatusPropagationTraceOrEmpty(status);
87-
status_propagation_trace.Append(GetStackFrame(file, line, new_message));
90+
status_propagation_trace.Append(GetStackFrame(file, line, function, new_message));
8891
new_status.SetPayload(kStatusPropagationTraceKey, status_propagation_trace);
8992
}
9093

‎torch_xla/csrc/status.h

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,9 @@ constexpr char kStatusPropagationTraceKey[] = "status-propagation-trace";
4848
// RuntimeError: Error message.
4949
// From: <cpp-source-file>:<line> (error: Error message.)
5050
//
51-
#define XLA_ERROR_WITH_LOCATION(status) \
52-
::torch_xla::status_internal::MaybeWithLocation(status, __FILE__, __LINE__)
51+
#define XLA_ERROR_WITH_LOCATION(status) \
52+
::torch_xla::status_internal::MaybeWithLocation(status, __FILE__, __LINE__, \
53+
__FUNCTION__)
5354

5455
#define XLA_CONCAT_(a, b) XLA_CONCAT_IMPL_(a, b)
5556
#define XLA_CONCAT_IMPL_(a, b) a##b
@@ -66,7 +67,7 @@ constexpr char kStatusPropagationTraceKey[] = "status-propagation-trace";
6667
if (!var.ok()) { \
6768
return ::torch_xla::status_internal::MaybeWithNewMessage( \
6869
::torch_xla::status_internal::GetStatus(var), __FILE__, __LINE__, \
69-
##__VA_ARGS__); \
70+
__FUNCTION__, ##__VA_ARGS__); \
7071
} \
7172
then
7273

@@ -137,7 +138,7 @@ namespace status_internal {
137138
// whole program.
138139
//
139140
absl::Status MaybeWithLocation(const absl::Status& status, const char* file,
140-
int32_t line);
141+
int32_t line, const char* function);
141142

142143
// Returns an `absl::Status` from an `absl::Status`.
143144
// In this case, this function is a no-op. It simply returns the argument.
@@ -167,7 +168,7 @@ const absl::Status& GetStatus(const absl::StatusOr<T>& status) {
167168
// propagation trace payload (creates a new one if needed), if
168169
// `TORCH_SHOW_CPP_STACKTRACES` is set.
169170
absl::Status MaybeWithNewMessage(const absl::Status& status, const char* file,
170-
int32_t line,
171+
int32_t line, const char* function,
171172
std::string_view new_message = "");
172173

173174
} // namespace status_internal

0 commit comments

Comments
 (0)