Rename XLA_SHOW_CPP_ERROR_CONTEXT to TORCH_SHOW_CPP_STACKTRACES

.github/scripts/run_tests.sh

@@ -57,8 +57,8 @@ function run_torch_xla_cpp_tests() {
                #"test_xla_backend_intf"
                "test_xla_sharding"
                "test_runtime"
-               "test_status_dont_show_cpp_error_context"
-               "test_status_show_cpp_error_context"
+               "test_status_dont_show_cpp_stacktraces"
+               "test_status_show_cpp_stacktraces"
                "test_debug_macros")
   for name in "${test_names[@]}"; do
     echo "Running $name cpp test..."

BUILD

@@ -78,8 +78,8 @@ test_suite(
         "//test/cpp:test_tensor",
         "//test/cpp:test_xla_sharding",
         "//test/cpp:test_runtime",
-        "//test/cpp:test_status_dont_show_cpp_error_context",
-        "//test/cpp:test_status_show_cpp_error_context",
+        "//test/cpp:test_status_dont_show_cpp_stacktraces",
+        "//test/cpp:test_status_show_cpp_stacktraces",
         "//test/cpp:test_debug_macros",
         "//torch_xla/csrc/runtime:pjrt_computation_client_test",
         # "//torch_xla/csrc/runtime:ifrt_computation_client_test",

test/cpp/BUILD

@@ -175,17 +175,17 @@ cc_library(
 )
 
 ptxla_cc_test(
-    name = "test_status_dont_show_cpp_error_context",
-    srcs = ["test_status_dont_show_cpp_error_context.cpp"],
+    name = "test_status_dont_show_cpp_stacktraces",
+    srcs = ["test_status_dont_show_cpp_stacktraces.cpp"],
     deps = [
         ":test_status_common",
         "@com_google_googletest//:gtest_main",
     ],
 )
 
 ptxla_cc_test(
-    name = "test_status_show_cpp_error_context",
-    srcs = ["test_status_show_cpp_error_context.cpp"],
+    name = "test_status_show_cpp_stacktraces",
+    srcs = ["test_status_show_cpp_stacktraces.cpp"],
     deps = [
         ":test_status_common",
         "@com_google_googletest//:gtest_main",

test/cpp/run_tests.sh

@@ -101,8 +101,8 @@ if [[ "$RUN_CPP_TESTS" == "cpp_tests" ]]; then
               #"test_xla_backend_intf"
               "test_xla_sharding"
               "test_runtime"
-              "test_status_dont_show_cpp_error_context"
-              "test_status_show_cpp_error_context"
+              "test_status_dont_show_cpp_stacktraces"
+              "test_status_show_cpp_stacktraces"
               "test_debug_macros")
 fi
 for name in "${test_names[@]}"; do

test/cpp/test_debug_macros.cpp

@@ -56,8 +56,7 @@ TEST_XLA_CHECK_GREATER(GT, <, 5, 8)
 }  // namespace torch_xla
 
 static void SetUp() {
-  setenv(torch_xla::runtime::env::kEnvShowCppErrorContext, /* value= */ "true",
-         /* replace= */ 1);
+  setenv("TORCH_SHOW_CPP_STACKTRACES", /* value= */ "1", /* replace= */ 1);
 }
 
 int main(int argc, char** argv) {

test/cpp/test_status_common.h

@@ -4,11 +4,11 @@
 // in a parameterized way, so as to avoid duplicating tests for both
 // configurations:
 //
-//   1. `XLA_SHOW_CPP_ERROR_CONTEXT=true`: instantiated in
-//      test_status_show_cpp_error_context.cpp
+//   1. `TORCH_SHOW_CPP_STACKTRACES=true`: instantiated in
+//      test_status_show_cpp_stacktraces.cpp
 //
-//   2. `XLA_SHOW_CPP_ERROR_CONTEXT=false`: instantiated in
-//      test_status_dont_show_cpp_error_context.cpp
+//   2. `TORCH_SHOW_CPP_STACKTRACES=false`: instantiated in
+//      test_status_dont_show_cpp_stacktraces.cpp
 //
 // In order to easily instantiate the tests, this file also defines the macro
 // `INSTANTIATE_TEST_SUITE_WITH_MODE(mode)`, where `mode` is either `kShow` or
@@ -31,32 +31,32 @@
 namespace torch_xla {
 
 // Enum to control whether C++ error context is shown in status messages
-enum class CppErrorContextMode {
+enum class CppStacktracesMode {
   kShow,
   kHide,
 };
 
-// Converts CppErrorContextMode enum to string for test parameter naming
-inline const char* const ToString(CppErrorContextMode mode) {
+// Converts CppStacktracesMode enum to string for test parameter naming
+inline const char* const ToString(CppStacktracesMode mode) {
   switch (mode) {
-    case CppErrorContextMode::kShow:
-      return "ShowCppErrorContext";
-    case CppErrorContextMode::kHide:
-      return "DontShowCppErrorContext";
+    case CppStacktracesMode::kShow:
+      return "ShowCppStacktraces";
+    case CppStacktracesMode::kHide:
+      return "DontShowCppStacktraces";
   }
 }
 
 // Base test class for parameterized status tests with C++ error context control
-class StatusTest : public testing::TestWithParam<CppErrorContextMode> {
+class StatusTest : public testing::TestWithParam<CppStacktracesMode> {
  public:
   StatusTest() {
-    const char* const value = IsShowCppErrorContextMode() ? "true" : "false";
-    setenv(runtime::env::kEnvShowCppErrorContext, value, /* replace= */ 1);
+    const char* const value = IsShowCppStacktracesMode() ? "1" : "0";
+    setenv("TORCH_SHOW_CPP_STACKTRACES", value, /* replace= */ 1);
   }
 
  protected:
-  bool IsShowCppErrorContextMode() {
-    return GetParam() == CppErrorContextMode::kShow;
+  bool IsShowCppStacktracesMode() {
+    return GetParam() == CppStacktracesMode::kShow;
   }
 };
 
@@ -66,10 +66,10 @@ class StatusTest : public testing::TestWithParam<CppErrorContextMode> {
 // non-qualified identifier. That's because the underlying
 // `INSTANTIATE_TEST_SUITE_P` GTest macro will concatenate it with other
 // things for creating a unique identifier.
-#define INSTANTIATE_WITH_CPP_ERROR_CONTEXT_MODE(suite, test, mode)            \
-  INSTANTIATE_TEST_SUITE_P(                                                   \
-      suite, test, ::testing::Values(::torch_xla::CppErrorContextMode::mode), \
-      [](const ::testing::TestParamInfo<::torch_xla::CppErrorContextMode>&    \
+#define INSTANTIATE_WITH_CPP_STACKTRACES_MODE(suite, test, mode)             \
+  INSTANTIATE_TEST_SUITE_P(                                                  \
+      suite, test, ::testing::Values(::torch_xla::CppStacktracesMode::mode), \
+      [](const ::testing::TestParamInfo<::torch_xla::CppStacktracesMode>&    \
              info) { return ToString(info.param); })
 
 namespace testing {
@@ -104,7 +104,7 @@ TEST_P(StatusTest, GetValueOrThrowWithErrorStatusOr) {
 TEST_P(StatusTest, MaybeWithLocationPropagatesErrorStatus) {
   absl::Status error_status = absl::InvalidArgumentError(message);
   absl::Status result = MaybeWithLocation(error_status, test_file, line);
-  if (IsShowCppErrorContextMode()) {
+  if (IsShowCppStacktracesMode()) {
     ASSERT_NE(result, error_status);
     EXPECT_FALSE(result.ok());
     EXPECT_EQ(result.code(), error_status.code());
@@ -129,7 +129,7 @@ TEST_P(StatusTest, MaybeWithNewMessageNonEmptyNewMessage) {
   ASSERT_FALSE(result.ok());
   EXPECT_EQ(result.code(), error_status.code());
 
-  if (IsShowCppErrorContextMode()) {
+  if (IsShowCppStacktracesMode()) {
     EXPECT_EQ(result.message(),
               absl::StrCat("New test error message (at test_file.cpp:42)\n"
                            "From Error: Test error message"));
@@ -186,7 +186,7 @@ TEST_P(StatusTest, MacroReturnIfErrorWithNestedError) {
   ASSERT_FALSE(result.ok());
   EXPECT_EQ(result.code(), absl::StatusCode::kInvalidArgument);
 
-  if (IsShowCppErrorContextMode()) {
+  if (IsShowCppStacktracesMode()) {
     EXPECT_EQ(result.message(), absl::StrCat("Test error message (at ",
                                              __FILE__, ":", errline, ")"));
   } else {
@@ -207,7 +207,7 @@ TEST_P(StatusTest, MacroReturnIfErrorWithErrorWithNewMessage) {
   ASSERT_FALSE(result.ok());
   EXPECT_EQ(result.code(), absl::StatusCode::kInvalidArgument);
 
-  if (IsShowCppErrorContextMode()) {
+  if (IsShowCppStacktracesMode()) {
     EXPECT_EQ(result.message(),
               absl::StrCat("New test error message (at ", __FILE__, ":",
                            errline, ")\nFrom Error: Test error message"));
@@ -258,7 +258,7 @@ TEST_P(StatusTest, MacroAssignOrReturnWithErrorWithNewMessage) {
   ASSERT_FALSE(result.ok());
   EXPECT_EQ(result.status().code(), absl::StatusCode::kInvalidArgument);
 
-  if (IsShowCppErrorContextMode()) {
+  if (IsShowCppStacktracesMode()) {
     EXPECT_EQ(result.status().message(),
               absl::StrCat("New test error message (at ", __FILE__, ":",
                            errline, ")\nFrom Error: Test error message"));
@@ -273,7 +273,7 @@ TEST_P(StatusTest, MacroErrorWithLocation) {
   absl::Status result = XLA_ERROR_WITH_LOCATION(error_status);
   ASSERT_FALSE(result.ok());
   EXPECT_EQ(result.code(), absl::StatusCode::kInvalidArgument);
-  if (IsShowCppErrorContextMode()) {
+  if (IsShowCppStacktracesMode()) {
     EXPECT_EQ(result.message(), absl::StrCat("Test error message (at ",
                                              __FILE__, ":", errline, ")"));
   } else {

test/cpp/test_status_dont_show_cpp_error_context.cpp → test/cpp/test_status_dont_show_cpp_stacktraces.cpp

@@ -4,10 +4,10 @@ using torch_xla::StatusTest;
 
 // This file instantiates the parameterized tests defined in
 // `test_status_common.h`. It specifically configures the test environment by
-// explicitly setting the `XLA_SHOW_CPP_ERROR_CONTEXT` environment variable to
+// explicitly setting the `TORCH_SHOW_CPP_STACKTRACES` environment variable to
 // 'false' in the test fixture's `SetUp` method.
 //
 // Any new `TEST_P` test cases added to `test_status_common.h` will
 // automatically be run in this mode (without C++ error context).
 //
-INSTANTIATE_WITH_CPP_ERROR_CONTEXT_MODE(StatusTest, StatusTest, kHide);
+INSTANTIATE_WITH_CPP_STACKTRACES_MODE(StatusTest, StatusTest, kHide);

test/cpp/test_status_show_cpp_error_context.cpp → test/cpp/test_status_show_cpp_stacktraces.cpp

@@ -4,10 +4,10 @@ using torch_xla::StatusTest;
 
 // This file instantiates the parameterized tests defined in
 // `test_status_common.h`. It specifically configures the test environment by
-// explicitly setting the `XLA_SHOW_CPP_ERROR_CONTEXT` environment variable to
+// explicitly setting the `TORCH_SHOW_CPP_STACKTRACES` environment variable to
 // 'true' in the test fixture's `SetUp` method.
 //
 // Any new `TEST_P` test cases added to `test_status_common.h` will
 // automatically be run in this mode (with C++ error context).
-INSTANTIATE_WITH_CPP_ERROR_CONTEXT_MODE(StatusWithCppErrorContextTest,
-                                        StatusTest, kShow);
+INSTANTIATE_WITH_CPP_STACKTRACES_MODE(StatusWithCppErrorContextTest, StatusTest,
+                                      kShow);

torch_xla/csrc/BUILD

@@ -376,8 +376,7 @@ cc_library(
     srcs = ["status.cpp"],
     hdrs = ["status.h"],
     deps = [
-        "//torch_xla/csrc/runtime:sys_util",
-        "//torch_xla/csrc/runtime:env_vars",
+        "@torch//:headers",
         "@com_google_absl//absl/log:absl_check",
         "@com_google_absl//absl/status:statusor",
     ],

torch_xla/csrc/runtime/BUILD

@@ -165,6 +165,7 @@ cc_library(
     hdrs = ["debug_macros.h"],
     deps = [
         ":tf_logging",
+        "@torch//:headers",
         "@tsl//tsl/platform:statusor",
         "@tsl//tsl/platform:macros",
     ],

torch_xla/csrc/runtime/debug_macros.h

@@ -6,11 +6,11 @@
 #include "tsl/platform/stacktrace.h"
 #include "tsl/platform/statusor.h"
 
-// XLA_SHOW_CPP_ERROR_CONTEXT environment variable changes the behavior of the
+// TORCH_SHOW_CPP_STACKTRACES environment variable changes the behavior of the
 // macros below, such as XLA_CHECK(), XLA_CHECK_EQ(), etc. (except for
 // XLA_CHECK_OK) in the following way:
 //
-// If XLA_SHOW_CPP_ERROR_CONTEXT environment variable is set, error messages
+// If TORCH_SHOW_CPP_STACKTRACES environment variable is set, error messages
 // generated by these macros will include detailed source location information
 // (file name and line number) and a C++ stacktrace, providing a comprehensive
 // context for debugging.

torch_xla/csrc/runtime/env_vars.h

@@ -38,10 +38,6 @@ inline constexpr char kEnvDistSvcMaxMissingHeartbeats[] =
 inline constexpr char kEnvDistSvcShutdownTimeoutInMin[] =
     "DIST_SERVICE_SHUTDOWN_TIMEOUT_IN_MIN";
 
-// When set to true, this enables showing C++ error context in backtraces,
-// which can be helpful for debugging but may clutter logs when not needed
-inline constexpr char kEnvShowCppErrorContext[] = "XLA_SHOW_CPP_ERROR_CONTEXT";
-
 }  // namespace env
 }  // namespace runtime
 }  // namespace torch_xla

torch_xla/csrc/runtime/tf_logging.cpp

@@ -1,5 +1,7 @@
 #include "torch_xla/csrc/runtime/tf_logging.h"
 
+#include <torch/csrc/utils/cpp_stacktraces.h>
+
 #include <stdexcept>
 
 #include "torch_xla/csrc/status.h"
@@ -15,7 +17,7 @@ void ErrorGenerator::operator&(const std::basic_ostream<char>& oss) const {
   std::stringstream ess;
   ess << sink.str();
 
-  if (ShouldShowCppErrorContext()) {
+  if (torch::get_cpp_stacktraces_enabled()) {
     ess << " (at " << file_ << ":" << line_ << ")\n";
     ess << tsl::CurrentStackTrace();
   }

torch_xla/csrc/status.cpp

@@ -1,17 +1,11 @@
 #include "torch_xla/csrc/status.h"
 
+#include <torch/csrc/utils/cpp_stacktraces.h>
+
 #include "absl/log/absl_check.h"
-#include "torch_xla/csrc/runtime/env_vars.h"
-#include "torch_xla/csrc/runtime/sys_util.h"
 
 namespace torch_xla {
 
-bool ShouldShowCppErrorContext() {
-  static const bool show_cpp_error_context = runtime::sys_util::GetEnvBool(
-      runtime::env::kEnvShowCppErrorContext, false);
-  return show_cpp_error_context;
-}
-
 // Common function for generating file location information with a space in the
 // beginning.
 static std::string LocationStrWithSpace(const char* file, const int32_t line) {
@@ -23,7 +17,7 @@ absl::Status MaybeWithLocation(const absl::Status& status, const char* file,
   ABSL_CHECK(!status.ok());
 
   // Return the same status if we don't need to add the C++ source location.
-  if (!ShouldShowCppErrorContext()) {
+  if (!torch::get_cpp_stacktraces_enabled()) {
     return status;
   }
 
@@ -40,7 +34,7 @@ absl::Status MaybeWithNewMessage(const absl::Status& status, const char* file,
   // Return the same status if:
   //   1. we don't need to add the C++ source location.
   //   2. there's no new message to replace the old one.
-  if (!ShouldShowCppErrorContext() && new_message.empty()) {
+  if (!torch::get_cpp_stacktraces_enabled() && new_message.empty()) {
     return status;
   }
 
@@ -52,7 +46,7 @@ absl::Status MaybeWithNewMessage(const absl::Status& status, const char* file,
   // context to give a better error message to the user.
   std::string_view message = new_message.empty() ? old_message : new_message;
 
-  // If `XLA_SHOW_CPP_ERROR_CONTEXT` is set, show the context of this error.
+  // If `TORCH_SHOW_CPP_STACKTRACES` is set, show the context of this error.
   // In other words, show:
   //   1. The error location
   //   2. The old messages that were replaced by `new_message`.
@@ -65,7 +59,7 @@ absl::Status MaybeWithNewMessage(const absl::Status& status, const char* file,
   // a stacktrace. Instead, we show only the history of error messages that
   // has led to the current error.
   const std::string context =
-      (ShouldShowCppErrorContext() && !new_message.empty())
+      (torch::get_cpp_stacktraces_enabled() && !new_message.empty())
           ? absl::StrCat(LocationStrWithSpace(file, line),
                          "\nFrom Error: ", old_message)
           : "";

torch_xla/csrc/status.h

@@ -14,13 +14,7 @@
 
 namespace torch_xla {
 
-// Returns whether we should show C++ error context.
-//
-// More specifically, whether the `XLA_SHOW_CPP_ERROR_CONTEXT` environment
-// variable is set or not.
-[[nodiscard]] bool ShouldShowCppErrorContext();
-
-// If `XLA_SHOW_CPP_ERROR_CONTEXT` is set, creates a new Status instance,
+// If `TORCH_SHOW_CPP_STACKTRACES` is set, creates a new Status instance,
 // appending the current location (e.g. file and line information) to the
 // status message.
 //
@@ -32,7 +26,7 @@ namespace torch_xla {
 //         absl::InvalidArgumentError("Error message.")
 //     );
 //
-// If `XLA_SHOW_CPP_ERROR_CONTEXT` is set, the error shown will be:
+// If `TORCH_SHOW_CPP_STACKTRACES` is set, the error shown will be:
 //
 //     Error message. (at <cpp-source-file>:<line>)
 //
@@ -68,7 +62,7 @@ namespace torch_xla {
 //     );
 //
 // If the function call results in an ok status, execution continues. Otherwise,
-// we early return a non-ok status. Then, if `XLA_SHOW_CPP_ERROR_CONTEXT` is
+// we early return a non-ok status. Then, if `TORCH_SHOW_CPP_STACKTRACES` is
 // set, the error shown will be:
 //
 //     New error message. (at <cpp-source-file>:<line>)
@@ -99,7 +93,7 @@ namespace torch_xla {
 // If the function call results in an ok status, execution continues with
 // `result` set to `ret.value()`, where `ret` is the returned value of the
 // function. Otherwise, we early return a non-ok status. Then, if
-// `XLA_SHOW_CPP_ERROR_CONTEXT` is set, the error shown will be:
+// `TORCH_SHOW_CPP_STACKTRACES` is set, the error shown will be:
 //
 //     New error message. (at <cpp-source-file>:<line>)
 //     Previous error message. (at <cpp-source-file>:<line>)
@@ -114,7 +108,7 @@ namespace torch_xla {
 //
 // This function assumes that `status` is a non-ok status.
 //
-// If `XLA_SHOW_CPP_ERROR_CONTEXT` is set, appends the current source
+// If `TORCH_SHOW_CPP_STACKTRACES` is set, appends the current source
 // location information to the status message. Otherwise, it simply returns
 // `status`.
 absl::Status MaybeWithLocation(const absl::Status& status, const char* file,
@@ -144,7 +138,7 @@ const absl::Status& GetStatus(const absl::StatusOr<T>& status) {
 // to construct better error messages to the user.
 //
 // This function also appends file location information to the error message, if
-// `XLA_SHOW_CPP_ERROR_CONTEXT` is set.
+// `TORCH_SHOW_CPP_STACKTRACES` is set.
 absl::Status MaybeWithNewMessage(const absl::Status& status, const char* file,
                                  int32_t line,
                                  std::string_view new_message = "");