Skip to content

Commit

Permalink
Invert the "is inlined" bit of absl::Status
Browse files Browse the repository at this point in the history
This change makes  RepToPointer/PointerToRep have 0 instructions.
This makes IsMovedFrom simpler (although this could always have left out the IsInlined check since that bit can never be set on the aligned pointer)

In exchange, it makes CodeToInlinedRep slower, but does not inhibit replacing it with a constant.
InlinedRepToCode is unaffected.

PiperOrigin-RevId: 562826801
Change-Id: I2732f04ab293b773edc2efdec546b3a287b980c2
  • Loading branch information
Abseil Team authored and copybara-github committed Sep 5, 2023
1 parent 461f1e4 commit 5c9f72f
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 10 deletions.
4 changes: 4 additions & 0 deletions absl/status/status.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@
namespace absl {
ABSL_NAMESPACE_BEGIN

static_assert(
alignof(status_internal::StatusRep) >= 4,
"absl::Status assumes it can use the bottom 2 bits of a StatusRep*.");

std::string StatusCodeToString(StatusCode code) {
switch (code) {
case StatusCode::kOk:
Expand Down
23 changes: 13 additions & 10 deletions absl/status/status.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,15 @@
#ifndef ABSL_STATUS_STATUS_H_
#define ABSL_STATUS_STATUS_H_

#include <cassert>
#include <cstdint>
#include <ostream>
#include <string>
#include <utility>

#include "absl/base/attributes.h"
#include "absl/base/config.h"
#include "absl/base/optimization.h"
#include "absl/functional/function_ref.h"
#include "absl/status/internal/status_internal.h"
#include "absl/strings/cord.h"
Expand Down Expand Up @@ -644,13 +649,13 @@ class Status final {
std::string ToStringSlow(StatusToStringMode mode) const;

// Status supports two different representations.
// - When the low bit is off it is an inlined representation.
// - When the low bit is set it is an inlined representation.
// It uses the canonical error space, no message or payload.
// The error code is (rep_ >> 2).
// The (rep_ & 2) bit is the "moved from" indicator, used in IsMovedFrom().
// - When the low bit is on it is an external representation.
// - When the low bit is off it is an external representation.
// In this case all the data comes from a heap allocated Rep object.
// (rep_ - 1) is a status_internal::StatusRep* pointer to that structure.
// rep_ is a status_internal::StatusRep* pointer to that structure.
uintptr_t rep_;
};

Expand Down Expand Up @@ -839,18 +844,16 @@ inline status_internal::Payloads* Status::GetPayloads() {
return IsInlined(rep_) ? nullptr : RepToPointer(rep_)->payloads.get();
}

inline bool Status::IsInlined(uintptr_t rep) { return (rep & 1) == 0; }
inline bool Status::IsInlined(uintptr_t rep) { return (rep & 1) != 0; }

inline bool Status::IsMovedFrom(uintptr_t rep) {
return IsInlined(rep) && (rep & 2) != 0;
}
inline bool Status::IsMovedFrom(uintptr_t rep) { return (rep & 2) != 0; }

inline uintptr_t Status::MovedFromRep() {
return CodeToInlinedRep(absl::StatusCode::kInternal) | 2;
}

inline uintptr_t Status::CodeToInlinedRep(absl::StatusCode code) {
return static_cast<uintptr_t>(code) << 2;
return (static_cast<uintptr_t>(code) << 2) + 1;
}

inline absl::StatusCode Status::InlinedRepToCode(uintptr_t rep) {
Expand All @@ -860,11 +863,11 @@ inline absl::StatusCode Status::InlinedRepToCode(uintptr_t rep) {

inline status_internal::StatusRep* Status::RepToPointer(uintptr_t rep) {
assert(!IsInlined(rep));
return reinterpret_cast<status_internal::StatusRep*>(rep - 1);
return reinterpret_cast<status_internal::StatusRep*>(rep);
}

inline uintptr_t Status::PointerToRep(status_internal::StatusRep* rep) {
return reinterpret_cast<uintptr_t>(rep) + 1;
return reinterpret_cast<uintptr_t>(rep);
}

inline void Status::Ref(uintptr_t rep) {
Expand Down

0 comments on commit 5c9f72f

Please sign in to comment.