Internal Change

Use internal absl::SourceLocation and absl::Status apis when possible.

PiperOrigin-RevId: 609791364

xls/common/BUILD

@@ -199,9 +199,7 @@ cc_test(
 cc_library(
     name = "source_location",
     hdrs = ["source_location.h"],
-    deps = [
-        "@com_google_absl//absl/base:config",
-    ],
+    deps = ["@com_google_absl//absl/base:config"],
 )
 
 cc_test(

xls/common/source_location.h

@@ -33,6 +33,20 @@
 
 #include "absl/base/config.h"
 
+#ifdef XLS_USE_ABSL_SOURCE_LOCATION
+#include "absl/types/source_location.h"
+
+// Use absl::SourceLocation
+namespace xabsl {
+using SourceLocation = absl::SourceLocation;
+#define XABSL_LOC ABSL_LOC
+#define XABSL_LOC_CURRENT_DEFAULT_ARG ABSL_LOC_CURRENT_DEFAULT_ARG
+}  // namespace xabsl
+
+#else
+
+// OSS Version of source_location
+
 #define XABSL_HAVE_SOURCE_LOCATION_CURRENT 1
 
 // The xabsl namespace has types that are anticipated to become available in
@@ -52,14 +66,12 @@ class SourceLocation {
 
  public:
   // Avoid this constructor; it populates the object with dummy values.
-  constexpr SourceLocation()
-      : line_(0),
-        file_name_(nullptr) {}
+  constexpr SourceLocation() : line_(0), file_name_(nullptr) {}
 
   // Wrapper to invoke the private constructor below. This should only be used
   // by the `XABSL_LOC` macro, hence the name.
   static constexpr SourceLocation DoNotInvokeDirectly(std::uint_least32_t line,
-                                                       const char* file_name) {
+                                                      const char* file_name) {
     return SourceLocation(line, file_name);
   }
 
@@ -109,8 +121,7 @@ class SourceLocation {
   // `file_name` must outlive all copies of the `xabsl::SourceLocation` object,
   // so in practice it should be a string literal.
   constexpr SourceLocation(std::uint_least32_t line, const char* file_name)
-      : line_(line),
-        file_name_(file_name) {}
+      : line_(line), file_name_(file_name) {}
 
   friend constexpr int UseUnused() {
     static_assert(SourceLocation(0, nullptr).unused_column_ == 0,
@@ -150,5 +161,6 @@ class SourceLocation {
 #else
 #define XABSL_LOC_CURRENT_DEFAULT_ARG
 #endif
+#endif  // XLS_USE_ABSL_SOURCE_LOCATION
 
 #endif  // XLS_COMMON_SOURCE_LOCATION_H_

xls/common/status/BUILD

@@ -77,7 +77,10 @@ cc_test(
 
 cc_library(
     name = "status_builder",
-    srcs = ["status_builder.cc"],
+    srcs = [
+        "status_builder.cc",
+        "status_builder_oss.cc",
+    ],
     hdrs = ["status_builder.h"],
     deps = [
         "@com_google_absl//absl/base:log_severity",

xls/common/status/status_builder.h

@@ -15,6 +15,7 @@
 #ifndef XLS_COMMON_STATUS_STATUS_BUILDER_H_
 #define XLS_COMMON_STATUS_STATUS_BUILDER_H_
 
+#include <algorithm>
 #include <limits>
 #include <memory>
 #include <ostream>
@@ -142,10 +143,8 @@ class ABSL_MUST_USE_RESULT StatusBuilder {
   // logging mutator functions.  Returns `*this` to allow method chaining.
   // If period is absl::ZeroDuration() or less, then this is equivalent to
   // calling the Log() method.
-  StatusBuilder& LogEvery(absl::LogSeverity level,
-                          absl::Duration period) &;
-  StatusBuilder&& LogEvery(absl::LogSeverity level,
-                           absl::Duration period) &&;
+  StatusBuilder& LogEvery(absl::LogSeverity level, absl::Duration period) &;
+  StatusBuilder&& LogEvery(absl::LogSeverity level, absl::Duration period) &&;
 
   // Mutates the builder so that the result status will be VLOGged (without a
   // stack trace) when this builder is converted to a Status.  `verbose_level`
@@ -268,13 +267,13 @@ class ABSL_MUST_USE_RESULT StatusBuilder {
   // Style guide exception for Ref qualified methods and rvalue refs.  This
   // allows us to avoid a copy in the common case.
   template <typename Adaptor>
-  auto With(Adaptor&& adaptor) & -> decltype(
-      std::forward<Adaptor>(adaptor)(*this)) {
+  auto With(
+      Adaptor&& adaptor) & -> decltype(std::forward<Adaptor>(adaptor)(*this)) {
     return std::forward<Adaptor>(adaptor)(*this);
   }
   template <typename Adaptor>
-  auto With(Adaptor&& adaptor) && -> decltype(
-      std::forward<Adaptor>(adaptor)(std::move(*this))) {
+  auto With(Adaptor&& adaptor) && -> decltype(std::forward<Adaptor>(adaptor)(
+                                      std::move(*this))) {
     return std::forward<Adaptor>(adaptor)(std::move(*this));
   }
 
@@ -412,6 +411,13 @@ class ABSL_MUST_USE_RESULT StatusBuilder {
   // nullptr if the result status will be OK.  Extra fields moved to the heap to
   // minimize stack space.
   std::unique_ptr<Rep> rep_;
+
+ private:
+  // Update this status to include the given source location (if supported by
+  // Status implementation).
+  //
+  // This is only for internal use by the StatusBuilder.
+  static void AddSourceLocation(absl::Status& status, SourceLocation loc);
 };
 
 // Implicitly converts `builder` to `Status` and write it to `os`.
@@ -658,13 +664,17 @@ inline absl::StatusCode StatusBuilder::code() const { return status_.code(); }
 
 inline StatusBuilder::operator absl::Status() const& {
   if (rep_ == nullptr) {
-    return status_;
+    absl::Status result = status_;
+    StatusBuilder::AddSourceLocation(result, loc_);
+    return result;
   }
   return StatusBuilder(*this).CreateStatusAndConditionallyLog();
 }
 inline StatusBuilder::operator absl::Status() && {
   if (rep_ == nullptr) {
-    return std::move(status_);
+    absl::Status result = std::move(status_);
+    StatusBuilder::AddSourceLocation(result, loc_);
+    return result;
   }
   return std::move(*this).CreateStatusAndConditionallyLog();
 }

xls/common/status/status_builder_oss.cc

@@ -0,0 +1,25 @@
+// Copyright 2024 The XLS Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "xls/common/source_location.h"
+#include "xls/common/status/status_builder.h"
+
+namespace xabsl {
+
+/* static */ void StatusBuilder::AddSourceLocation(absl::Status& status,
+                                                 xabsl::SourceLocation loc) {
+  // Not supported in OSS absl::Status at the moment.
+}
+
+}  // namespace xabsl

xls/common/status/status_macros_test.cc

@@ -30,6 +30,7 @@
 namespace {
 
 using ::testing::AllOf;
+using ::testing::ContainsRegex;
 using ::testing::Eq;
 using ::testing::HasSubstr;
 
@@ -261,6 +262,33 @@ TEST(AssignOrReturn, UniquePtrWorksForExistingVariable) {
   EXPECT_THAT(func().message(), Eq("EXPECTED"));
 }
 
+namespace assign_or_return {
+absl::Status CallThree() { return absl::InternalError("foobar"); }
+absl::StatusOr<int> CallTwo() {
+  XLS_RETURN_IF_ERROR(CallThree());
+  return 1;
+}
+absl::Status CallOne() {
+  XLS_ASSIGN_OR_RETURN(auto abc, CallTwo());
+  (void)abc;
+  return absl::OkStatus();
+}
+}  // namespace assign_or_return
+TEST(AssignOrReturn, KeepsBackTrace) {
+#ifndef XLS_USE_ABSL_SOURCE_LOCATION
+  GTEST_SKIP() << "Back trace not recorded";
+#endif
+  auto result = assign_or_return::CallOne();
+  RecordProperty("result", testing::PrintToString(result));
+  EXPECT_THAT(result.message(), Eq("foobar"));
+  // Expect 3 lines in the stack trace with status_macros_test.cc in them for
+  // the three deep call stack.
+  EXPECT_THAT(
+      result.ToString(absl::StatusToStringMode::kWithEverything),
+      ContainsRegex(
+          "(.*xls/common/status/status_macros_test.cc:[0-9]+.*\n?){3}"));
+}
+
 TEST(ReturnIfError, Works) {
   auto func = []() -> absl::Status {
     XLS_RETURN_IF_ERROR(ReturnOk());
@@ -325,6 +353,33 @@ TEST(ReturnIfError, WorksWithVoidReturnAdaptor) {
   EXPECT_EQ(code, 2);
 }
 
+namespace return_if_error {
+absl::Status CallThree() { return absl::InternalError("foobar"); }
+absl::Status CallTwo() {
+  XLS_RETURN_IF_ERROR(CallThree());
+  return absl::OkStatus();
+}
+absl::Status CallOne() {
+  XLS_RETURN_IF_ERROR(CallTwo());
+  return absl::OkStatus();
+}
+}  // namespace return_if_error
+
+TEST(ReturnIfError, KeepsBackTrace) {
+#ifndef XLS_USE_ABSL_SOURCE_LOCATION
+  GTEST_SKIP() << "Back trace not recorded";
+#endif
+  auto result = return_if_error::CallOne();
+  RecordProperty("result", testing::PrintToString(result));
+  EXPECT_THAT(result.message(), Eq("foobar"));
+  // Expect 3 lines in the stack trace with status_macros_test.cc in them for
+  // the three deep call stack.
+  EXPECT_THAT(
+      result.ToString(absl::StatusToStringMode::kWithEverything),
+      ContainsRegex(
+          "(.*xls/common/status/status_macros_test.cc:[0-9]+.*\n?){3}"));
+}
+
 // Basis for XLS_RETURN_IF_ERROR and XLS_ASSIGN_OR_RETURN benchmarks.  Derived
 // classes override LoopAgain() with the macro invocation(s).
 template <class T>