Make cel::AttributeSet copy-on-write

PiperOrigin-RevId: 985421843

Author: jcking

base/BUILD

@@ -35,6 +35,7 @@ cc_library(
         ":kind",
         "//internal:status_macros",
         "@com_google_absl//absl/base:core_headers",
+        "@com_google_absl//absl/base:no_destructor",
         "@com_google_absl//absl/base:nullability",
         "@com_google_absl//absl/container:btree",
         "@com_google_absl//absl/status",

base/attribute_set.h

@@ -15,6 +15,9 @@
 #ifndef THIRD_PARTY_CEL_CPP_BASE_ATTRIBUTE_SET_H_
 #define THIRD_PARTY_CEL_CPP_BASE_ATTRIBUTE_SET_H_
 
+#include <memory>
+
+#include "absl/base/no_destructor.h"
 #include "absl/container/btree_set.h"
 #include "absl/types/span.h"
 #include "base/attribute.h"
@@ -54,27 +57,43 @@ class AttributeSet final {
     }
   }
 
-  AttributeSet(const AttributeSet& set1, const AttributeSet& set2)
-      : attributes_(set1.attributes_) {
-    for (const auto& attr : set2.attributes_) {
-      Add(attr);
-    }
+  AttributeSet(const AttributeSet& set1, const AttributeSet& set2) {
+    Add(set1);
+    Add(set2);
   }
 
-  iterator begin() const { return attributes_.begin(); }
+  iterator begin() const {
+    return attributes_ != nullptr ? attributes_->begin()
+                                  : EmptyContainer().begin();
+  }
 
-  const_iterator cbegin() const { return attributes_.cbegin(); }
+  const_iterator cbegin() const {
+    return attributes_ != nullptr ? attributes_->cbegin()
+                                  : EmptyContainer().cbegin();
+  }
 
-  iterator end() const { return attributes_.end(); }
+  iterator end() const {
+    return attributes_ != nullptr ? attributes_->end() : EmptyContainer().end();
+  }
 
-  const_iterator cend() const { return attributes_.cend(); }
+  const_iterator cend() const {
+    return attributes_ != nullptr ? attributes_->cend()
+                                  : EmptyContainer().cend();
+  }
 
-  size_type size() const { return attributes_.size(); }
+  size_type size() const {
+    return attributes_ != nullptr ? attributes_->size() : 0;
+  }
 
-  bool empty() const { return attributes_.empty(); }
+  bool empty() const { return attributes_ == nullptr || attributes_->empty(); }
 
   bool operator==(const AttributeSet& other) const {
-    return this == &other || attributes_ == other.attributes_;
+    if (attributes_ == other.attributes_) {
+      return true;
+    }
+    return (attributes_ != nullptr ? *attributes_ : EmptyContainer()) ==
+           (other.attributes_ != nullptr ? *other.attributes_
+                                         : EmptyContainer());
   }
 
   bool operator!=(const AttributeSet& other) const {
@@ -91,16 +110,31 @@ class AttributeSet final {
   friend class UnknownValue;
   friend class base_internal::UnknownSet;
 
-  void Add(const Attribute& attribute) { attributes_.insert(attribute); }
+  static const Container& EmptyContainer() {
+    static const absl::NoDestructor<Container> container;
+    return *container;
+  }
+
+  void Add(const Attribute& attribute) {
+    if (attributes_ == nullptr) {
+      attributes_ = std::make_shared<Container>();
+    }
+    attributes_->insert(attribute);
+  }
 
   void Add(const AttributeSet& other) {
-    for (const auto& attribute : other) {
-      Add(attribute);
+    if (!other.empty()) {
+      if (attributes_ == nullptr) {
+        attributes_ = std::make_shared<Container>();
+      }
+      for (const auto& attribute : other) {
+        attributes_->insert(attribute);
+      }
     }
   }
 
   // Attribute container.
-  Container attributes_;
+  std::shared_ptr<Container> attributes_;
 };
 
 }  // namespace cel

common/legacy_value.cc

@@ -1073,7 +1073,8 @@ absl::Status ModernValue(google::protobuf::Arena* arena,
           MapValue(common_internal::LegacyMapValue(legacy_value.MapOrDie()));
       return absl::OkStatus();
     case CelValue::Type::kUnknownSet:
-      result = UnknownValue{*legacy_value.UnknownSetOrDie()};
+      result =
+          common_internal::MakeUnknownValue(*legacy_value.UnknownSetOrDie());
       return absl::OkStatus();
     case CelValue::Type::kCelType: {
       auto type_name = legacy_value.CelTypeOrDie().value();
@@ -1132,7 +1133,8 @@ absl::StatusOr<google::api::expr::runtime::CelValue> LegacyValue(
       return common_internal::LegacyTrivialMapValue(arena, modern_value);
     case ValueKind::kUnknown:
       return CelValue::CreateUnknownSet(google::protobuf::Arena::Create<Unknown>(
-          arena, Cast<UnknownValue>(modern_value).NativeValue()));
+          arena,
+          common_internal::GetUnknown(Cast<UnknownValue>(modern_value))));
     case ValueKind::kType:
       return CelValue::CreateCelType(
           CelValue::CelTypeHolder(google::protobuf::Arena::Create<std::string>(
@@ -1185,7 +1187,7 @@ absl::StatusOr<Value> FromLegacyValue(google::protobuf::Arena* arena,
     case CelValue::Type::kMap:
       return MapValue(common_internal::LegacyMapValue(legacy_value.MapOrDie()));
     case CelValue::Type::kUnknownSet:
-      return UnknownValue{*legacy_value.UnknownSetOrDie()};
+      return common_internal::MakeUnknownValue(*legacy_value.UnknownSetOrDie());
     case CelValue::Type::kCelType:
       return CreateTypeValueFromView(arena,
                                      legacy_value.CelTypeOrDie().value());
@@ -1236,7 +1238,7 @@ absl::StatusOr<google::api::expr::runtime::CelValue> ToLegacyValue(
       return common_internal::LegacyTrivialMapValue(arena, value);
     case ValueKind::kUnknown:
       return CelValue::CreateUnknownSet(google::protobuf::Arena::Create<Unknown>(
-          arena, Cast<UnknownValue>(value).NativeValue()));
+          arena, common_internal::GetUnknown(Cast<UnknownValue>(value))));
     case ValueKind::kType:
       return CelValue::CreateCelType(
           CelValue::CelTypeHolder(google::protobuf::Arena::Create<std::string>(

common/values/unknown_value.h

@@ -40,13 +40,21 @@ namespace cel {
 class Value;
 class UnknownValue;
 
+namespace common_internal {
+[[nodiscard]]
+UnknownValue MakeUnknownValue(Unknown value);
+[[nodiscard]]
+Unknown GetUnknown(const UnknownValue& value);
+[[nodiscard]]
+const FunctionResultSet& GetUnknownFunctionResultSet(
+    const UnknownValue& value ABSL_ATTRIBUTE_LIFETIME_BOUND);
+}  // namespace common_internal
+
 // `UnknownValue` represents values of the primitive `duration` type.
 class UnknownValue final : private common_internal::ValueMixin<UnknownValue> {
  public:
   static constexpr ValueKind kKind = ValueKind::kUnknown;
 
-  explicit UnknownValue(Unknown unknown) : unknown_(std::move(unknown)) {}
-
   UnknownValue() = default;
   UnknownValue(const UnknownValue&) = default;
   UnknownValue(UnknownValue&&) = default;
@@ -80,31 +88,25 @@ class UnknownValue final : private common_internal::ValueMixin<UnknownValue> {
 
   bool IsZeroValue() const { return false; }
 
-  void swap(UnknownValue& other) noexcept {
-    using std::swap;
-    swap(unknown_, other.unknown_);
-  }
-
-  const Unknown& NativeValue() const& ABSL_ATTRIBUTE_LIFETIME_BOUND {
-    return unknown_;
-  }
-
-  Unknown NativeValue() && {
-    Unknown unknown = std::move(unknown_);
-    return unknown;
-  }
-
-  const AttributeSet& attribute_set() const {
+  [[nodiscard]]
+  AttributeSet ToAttributeSet() const {
     return unknown_.unknown_attributes();
   }
 
-  const FunctionResultSet& function_result_set() const {
-    return unknown_.unknown_function_results();
+  void swap(UnknownValue& other) noexcept {
+    using std::swap;
+    swap(unknown_, other.unknown_);
   }
 
  private:
+  friend UnknownValue common_internal::MakeUnknownValue(Unknown value);
+  friend Unknown common_internal::GetUnknown(const UnknownValue&);
+  friend const FunctionResultSet& common_internal::GetUnknownFunctionResultSet(
+      const UnknownValue& value);
   friend class common_internal::ValueMixin<UnknownValue>;
 
+  explicit UnknownValue(Unknown unknown) : unknown_(std::move(unknown)) {}
+
   Unknown unknown_;
 };
 
@@ -116,6 +118,26 @@ inline std::ostream& operator<<(std::ostream& out, const UnknownValue& value) {
   return out << value.DebugString();
 }
 
+namespace common_internal {
+
+[[nodiscard]]
+inline UnknownValue MakeUnknownValue(Unknown value) {
+  return UnknownValue(std::move(value));
+}
+
+[[nodiscard]]
+inline Unknown GetUnknown(const UnknownValue& value) {
+  return value.unknown_;
+}
+
+[[nodiscard]]
+inline const FunctionResultSet& GetUnknownFunctionResultSet(
+    const UnknownValue& value ABSL_ATTRIBUTE_LIFETIME_BOUND) {
+  return value.unknown_.unknown_function_results();
+}
+
+}  // namespace common_internal
+
 }  // namespace cel
 
 #endif  // THIRD_PARTY_CEL_CPP_COMMON_VALUES_UNKNOWN_VALUE_H_

eval/eval/attribute_utility.cc

@@ -118,16 +118,19 @@ absl::optional<UnknownValue> AttributeUtility::MergeUnknowns(
     const auto& current_set = value.GetUnknown();
 
     cel::base_internal::UnknownSetAccess::Add(
-        *result_set, UnknownSet(current_set.attribute_set(),
-                                current_set.function_result_set()));
+        *result_set,
+        UnknownSet(
+            current_set.ToAttributeSet(),
+            cel::common_internal::GetUnknownFunctionResultSet(current_set)));
   }
 
   if (!result_set.has_value()) {
     return std::nullopt;
   }
 
-  return UnknownValue(cel::Unknown(result_set->unknown_attributes(),
-                                   result_set->unknown_function_results()));
+  return cel::common_internal::MakeUnknownValue(
+      cel::Unknown(result_set->unknown_attributes(),
+                   result_set->unknown_function_results()));
 }
 
 UnknownValue AttributeUtility::MergeUnknownValues(
@@ -136,12 +139,13 @@ UnknownValue AttributeUtility::MergeUnknownValues(
   // distinguish unset (nullopt) and empty(engaged empty value).
   AttributeSet attributes;
   FunctionResultSet function_results;
-  attributes.Add(left.attribute_set());
-  function_results.Add(left.function_result_set());
-  attributes.Add(right.attribute_set());
-  function_results.Add(right.function_result_set());
+  attributes.Add(left.ToAttributeSet());
+  function_results.Add(cel::common_internal::GetUnknownFunctionResultSet(left));
+  attributes.Add(right.ToAttributeSet());
+  function_results.Add(
+      cel::common_internal::GetUnknownFunctionResultSet(right));
 
-  return UnknownValue(
+  return cel::common_internal::MakeUnknownValue(
       cel::Unknown(std::move(attributes), std::move(function_results)));
 }
 
@@ -191,16 +195,17 @@ absl::optional<UnknownValue> AttributeUtility::IdentifyAndMergeUnknowns(
 
   if (arg_unknowns.has_value()) {
     cel::base_internal::UnknownSetAccess::Add(
-        *result_set, UnknownSet((*arg_unknowns).attribute_set(),
-                                (*arg_unknowns).function_result_set()));
+        *result_set, cel::common_internal::GetUnknown(*arg_unknowns));
   }
 
-  return UnknownValue(cel::Unknown(result_set->unknown_attributes(),
-                                   result_set->unknown_function_results()));
+  return cel::common_internal::MakeUnknownValue(
+      cel::Unknown(result_set->unknown_attributes(),
+                   result_set->unknown_function_results()));
 }
 
 UnknownValue AttributeUtility::CreateUnknownSet(cel::Attribute attr) const {
-  return UnknownValue(cel::Unknown(AttributeSet({std::move(attr)})));
+  return cel::common_internal::MakeUnknownValue(
+      cel::Unknown(AttributeSet({std::move(attr)})));
 }
 
 absl::StatusOr<ErrorValue> AttributeUtility::CreateMissingAttributeError(
@@ -213,13 +218,14 @@ absl::StatusOr<ErrorValue> AttributeUtility::CreateMissingAttributeError(
 UnknownValue AttributeUtility::CreateUnknownSet(
     const cel::FunctionDescriptor& fn_descriptor, int64_t expr_id,
     absl::Span<const cel::Value> args) const {
-  return UnknownValue(
+  return cel::common_internal::MakeUnknownValue(
       cel::Unknown(FunctionResultSet(FunctionResult(fn_descriptor, expr_id))));
 }
 
 void AttributeUtility::Add(Accumulator& a, const cel::UnknownValue& v) const {
-  a.attribute_set_.Add(v.attribute_set());
-  a.function_result_set_.Add(v.function_result_set());
+  a.attribute_set_.Add(v.ToAttributeSet());
+  a.function_result_set_.Add(
+      cel::common_internal::GetUnknownFunctionResultSet(v));
 }
 
 void AttributeUtility::Add(Accumulator& a, const AttributeTrail& attr) const {
@@ -253,7 +259,7 @@ bool Accumulator::IsEmpty() const {
 }
 
 cel::UnknownValue Accumulator::Build() && {
-  return cel::UnknownValue(
+  return cel::common_internal::MakeUnknownValue(
       cel::Unknown(std::move(attribute_set_), std::move(function_result_set_)));
 }
 

eval/eval/attribute_utility_test.cc

@@ -85,10 +85,10 @@ TEST_F(AttributeUtilityTest, UnknownsUtilityMergeUnknownsFromValues) {
 
   AttributeUtility utility(unknown_patterns, missing_attribute_patterns);
 
-  UnknownValue unknown_set0 =
-      cel::UnknownValue(cel::Unknown(AttributeSet({attribute0})));
-  UnknownValue unknown_set1 =
-      cel::UnknownValue(cel::Unknown(AttributeSet({attribute1})));
+  UnknownValue unknown_set0 = cel::common_internal::MakeUnknownValue(
+      cel::Unknown(AttributeSet({attribute0})));
+  UnknownValue unknown_set1 = cel::common_internal::MakeUnknownValue(
+      cel::Unknown(AttributeSet({attribute1})));
 
   std::vector<cel::Value> values = {
       unknown_set0,
@@ -99,7 +99,7 @@ TEST_F(AttributeUtilityTest, UnknownsUtilityMergeUnknownsFromValues) {
 
   absl::optional<UnknownValue> unknown_set = utility.MergeUnknowns(values);
   ASSERT_TRUE(unknown_set.has_value());
-  EXPECT_THAT((*unknown_set).attribute_set(),
+  EXPECT_THAT((*unknown_set).ToAttributeSet(),
               UnorderedPointwise(
                   Eq(), std::vector<CelAttribute>{attribute0, attribute1}));
 }
@@ -162,8 +162,8 @@ TEST_F(AttributeUtilityTest, CreateUnknownSet) {
   AttributeUtility utility(empty_patterns, empty_patterns);
 
   UnknownValue set = utility.CreateUnknownSet(trail.attribute());
-  ASSERT_THAT(set.attribute_set(), SizeIs(1));
-  ASSERT_OK_AND_ASSIGN(auto elem, set.attribute_set().begin()->AsString());
+  ASSERT_THAT(set.ToAttributeSet(), SizeIs(1));
+  ASSERT_OK_AND_ASSIGN(auto elem, set.ToAttributeSet().begin()->AsString());
   EXPECT_EQ(elem, "destination.ip");
 }