Skip to content

Commit cdad3a1

Browse files
jckingcopybara-github
authored andcommitted
Allow updating cel::UnknownValue in-place
PiperOrigin-RevId: 999588834
1 parent 234f0b8 commit cdad3a1

3 files changed

Lines changed: 43 additions & 17 deletions

File tree

‎common/internal/unknowns.cc‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ std::string UnknownTree::ToString(
159159
return result;
160160
}
161161

162-
UnknownSet UnknownTree::GraftImpl(const UnknownSet& in) {
162+
void UnknownTree::GraftImpl(const UnknownSet& in, UnknownSet& out) {
163163
ABSL_DCHECK_NE(in.root, RootIfPresent());
164164
// This should only happen in rare circumstances where somebody passed an
165165
// UnknownValue from one expression evaluation into another.
@@ -184,11 +184,9 @@ UnknownSet UnknownTree::GraftImpl(const UnknownSet& in) {
184184
std::sort(unknown_function_set->nodes,
185185
unknown_function_set->nodes + unknown_function_set->size);
186186
}
187-
return UnknownSet{
188-
.root = Root(),
189-
.attributes = unknown_attribute_set,
190-
.functions = unknown_function_set,
191-
};
187+
out.root = Root();
188+
out.attributes = unknown_attribute_set;
189+
out.functions = unknown_function_set;
192190
}
193191

194192
UnknownAttributeNode* UnknownTree::GraftImpl(const UnknownAttributeNode* in) {

‎common/internal/unknowns.h‎

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include <string>
2323
#include <variant>
2424

25+
#include "absl/base/attributes.h"
2526
#include "absl/base/nullability.h"
2627
#include "absl/base/optimization.h"
2728
#include "absl/container/flat_hash_set.h"
@@ -258,7 +259,9 @@ class UnknownTree {
258259
UnknownTree(const UnknownTree&) = delete;
259260
UnknownTree& operator=(const UnknownTree&) = delete;
260261

261-
explicit UnknownTree(google::protobuf::Arena* absl_nonnull arena) : arena_(arena) {}
262+
explicit UnknownTree(google::protobuf::Arena* absl_nonnull arena) : arena_(arena) {
263+
ABSL_DCHECK(arena != nullptr);
264+
}
262265

263266
void MaybeReset(google::protobuf::Arena* absl_nonnull arena) {
264267
ABSL_DCHECK(arena != nullptr);
@@ -345,21 +348,41 @@ class UnknownTree {
345348
// address for efficiency. This function does that grafting when it is
346349
// required.
347350
[[nodiscard]]
348-
UnknownSet Graft(const UnknownSet& in) {
351+
ABSL_ATTRIBUTE_ALWAYS_INLINE UnknownSet Graft(const UnknownSet& in) {
349352
// Fast path, should be taken almost always.
353+
UnknownSet out;
354+
if (ABSL_PREDICT_TRUE(in.root == nullptr || in.root == RootIfPresent())) {
355+
ABSL_DCHECK(in.root != nullptr || in.attributes == nullptr);
356+
ABSL_DCHECK(in.root != nullptr || in.functions == nullptr);
357+
out = in;
358+
} else {
359+
// Slow path, should be taken extremely rarely.
360+
GraftImpl(in, out);
361+
}
362+
return out;
363+
}
364+
365+
// In the rare event that somebody manages to pass an unknown set from one
366+
// expression into another, we need to be able to graft those nodes into our
367+
// tree. The unknown attribute set and unknown function set sort nodes by
368+
// address for efficiency. This function does that grafting when it is
369+
// required.
370+
ABSL_ATTRIBUTE_ALWAYS_INLINE void GraftAt(UnknownSet& in) {
350371
if (ABSL_PREDICT_TRUE(in.root == nullptr || in.root == RootIfPresent())) {
351-
return in;
372+
// Fast path, should be taken almost always.
373+
ABSL_DCHECK(in.root != nullptr || in.attributes == nullptr);
374+
ABSL_DCHECK(in.root != nullptr || in.functions == nullptr);
375+
} else {
376+
// Slow path, should be taken extremely rarely.
377+
GraftImpl(in, in);
352378
}
353-
// Slow path, should be taken extremely rarely.
354-
return GraftImpl(in);
355379
}
356380

357381
private:
358382
friend class UnknownAttributeNode;
359383
friend class UnknownRoot;
360384

361-
[[nodiscard]]
362-
UnknownSet GraftImpl(const UnknownSet& in);
385+
ABSL_ATTRIBUTE_NOINLINE void GraftImpl(const UnknownSet& in, UnknownSet& out);
363386

364387
[[nodiscard]]
365388
UnknownAttributeNode* GraftImpl(const UnknownAttributeNode* in);

‎common/value.h‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1020,9 +1020,7 @@ class Value final : private common_internal::ValueMixin<Value> {
10201020
// Performs a checked cast from a value to an unknown value,
10211021
// returning a non-empty optional with either a value or reference to the
10221022
// unknown value. Otherwise an empty optional is returned.
1023-
optional_ref<const UnknownValue> AsUnknown() & ABSL_ATTRIBUTE_LIFETIME_BOUND {
1024-
return std::as_const(*this).AsUnknown();
1025-
}
1023+
optional_ref<UnknownValue> AsUnknown() & ABSL_ATTRIBUTE_LIFETIME_BOUND;
10261024
optional_ref<const UnknownValue> AsUnknown()
10271025
const& ABSL_ATTRIBUTE_LIFETIME_BOUND;
10281026
absl::optional<UnknownValue> AsUnknown() &&;
@@ -1666,7 +1664,7 @@ class Value final : private common_internal::ValueMixin<Value> {
16661664
// `AsUnknown()`.
16671665
template <typename T>
16681666
std::enable_if_t<std::is_same_v<UnknownValue, T>,
1669-
optional_ref<const UnknownValue>>
1667+
optional_ref<UnknownValue>>
16701668
As() & ABSL_ATTRIBUTE_LIFETIME_BOUND {
16711669
return AsUnknown();
16721670
}
@@ -3193,6 +3191,13 @@ inline optional_ref<const UnknownValue> Value::AsUnknown() const& {
31933191
return std::nullopt;
31943192
}
31953193

3194+
inline optional_ref<UnknownValue> Value::AsUnknown() & {
3195+
if (auto* alternative = variant_.As<UnknownValue>(); alternative != nullptr) {
3196+
return *alternative;
3197+
}
3198+
return std::nullopt;
3199+
}
3200+
31963201
inline absl::optional<UnknownValue> Value::AsUnknown() && {
31973202
if (auto* alternative = variant_.As<UnknownValue>(); alternative != nullptr) {
31983203
return std::move(*alternative);

0 commit comments

Comments
 (0)