Update ValueView and friends to avoid binding to temporaries

PiperOrigin-RevId: 589187169
google · Dec 8, 2023 · 26f1a9c · 26f1a9c
1 parent bfb79d8
commit 26f1a9c
Show file tree

Hide file tree

Showing 8 changed files with 115 additions and 21 deletions.
diff --git a/common/memory.h b/common/memory.h
@@ -250,10 +250,6 @@ class ABSL_ATTRIBUTE_TRIVIAL_ABI SharedView final {
   SharedView(const Shared<U>& other ABSL_ATTRIBUTE_LIFETIME_BOUND) noexcept
       : value_(other.value_), refcount_(other.refcount_) {}
 
-  template <typename U,
-            typename = std::enable_if_t<std::is_convertible_v<U*, T*>>>
-  SharedView(Shared<U>&&) = delete;
-
   template <
       typename U,
       typename = std::enable_if_t<std::conjunction_v<
@@ -276,6 +272,21 @@ class ABSL_ATTRIBUTE_TRIVIAL_ABI SharedView final {
     return *this;
   }
 
+  template <typename U,
+            typename = std::enable_if_t<std::is_convertible_v<U*, T*>>>
+  // NOLINTNEXTLINE(google-explicit-constructor)
+  SharedView& operator=(
+      const Shared<U>& other ABSL_ATTRIBUTE_LIFETIME_BOUND) noexcept {
+    value_ = other.value_;
+    refcount_ = other.refcount_;
+    return *this;
+  }
+
+  template <typename U,
+            typename = std::enable_if_t<std::is_convertible_v<U*, T*>>>
+  // NOLINTNEXTLINE(google-explicit-constructor)
+  SharedView& operator=(Shared<U>&&) = delete;
+
   T& operator*() const noexcept {
     ABSL_DCHECK(!IsEmpty());
     return *value_;

diff --git a/common/value.h b/common/value.h
@@ -403,19 +403,14 @@ class ValueView final {
                      common_internal::BaseValueViewAlternativeForT<T>>,
                  alternative) {}
 
-  template <typename T,
-            typename = std::enable_if_t<
-                common_internal::IsValueAlternativeV<absl::remove_cvref_t<T>>>>
-  ValueView(T&&) = delete;
-
   template <typename T, typename = std::enable_if_t<
                             common_internal::IsValueViewAlternativeV<T>>>
   // NOLINTNEXTLINE(google-explicit-constructor)
   ValueView(T alternative) noexcept
       : variant_(
             absl::in_place_type<common_internal::BaseValueViewAlternativeForT<
                 absl::remove_cvref_t<T>>>,
-            std::forward<T>(alternative)) {}
+            alternative) {}
 
   template <typename T,
             typename = std::enable_if_t<common_internal::IsValueInterfaceV<T>>>
@@ -425,6 +420,38 @@ class ValueView final {
                      common_internal::BaseValueViewAlternativeForT<T>>,
                  interface) {}
 
+  ValueView& operator=(const Value& other) {
+    variant_ = (other.AssertIsValid(), other.ToViewVariant());
+    return *this;
+  }
+
+  ValueView& operator=(Value&&) = delete;
+
+  template <typename T>
+  std::enable_if_t<common_internal::IsValueAlternativeV<T>, ValueView&>
+  operator=(const T& alternative ABSL_ATTRIBUTE_LIFETIME_BOUND) {
+    variant_.emplace<common_internal::BaseValueViewAlternativeForT<T>>(
+        alternative);
+    return *this;
+  }
+
+  template <typename T>
+  std::enable_if_t<
+      std::conjunction_v<
+          std::negation<std::is_lvalue_reference<T>>,
+          common_internal::IsValueAlternative<absl::remove_cvref_t<T>>>,
+      ValueView&>
+  operator=(T&&) = delete;
+
+  template <typename T>
+  std::enable_if_t<common_internal::IsValueViewAlternativeV<T>, ValueView&>
+  operator=(T alternative) {
+    variant_.emplace<
+        common_internal::BaseValueViewAlternativeForT<absl::remove_cvref_t<T>>>(
+        alternative);
+    return *this;
+  }
+
   ValueKind kind() const {
     AssertIsValid();
     return absl::visit(

diff --git a/common/values/bytes_value.h b/common/values/bytes_value.h
@@ -128,6 +128,15 @@ class BytesValueView final {
   BytesValueView(const BytesValue& value ABSL_ATTRIBUTE_LIFETIME_BOUND) noexcept
       : value_(value.value_) {}
 
+  // NOLINTNEXTLINE(google-explicit-constructor)
+  BytesValueView& operator=(
+      const BytesValue& value ABSL_ATTRIBUTE_LIFETIME_BOUND) {
+    value_ = value.value_;
+    return *this;
+  }
+
+  BytesValueView& operator=(BytesValue&&) = delete;
+
   BytesValueView() = default;
   BytesValueView(const BytesValueView&) = default;
   BytesValueView(BytesValueView&&) = default;

diff --git a/common/values/list_value.h b/common/values/list_value.h
@@ -211,10 +211,23 @@ class ListValueView {
 
   static constexpr ValueKind kKind = ListValue::kKind;
 
+  // NOLINTNEXTLINE(google-explicit-constructor)
+  ListValueView(SharedView<const ListValueInterface> interface)
+      : interface_(interface) {}
+
   // NOLINTNEXTLINE(google-explicit-constructor)
   ListValueView(const ListValue& value ABSL_ATTRIBUTE_LIFETIME_BOUND) noexcept
       : interface_(value.interface_) {}
 
+  // NOLINTNEXTLINE(google-explicit-constructor)
+  ListValueView& operator=(
+      const ListValue& value ABSL_ATTRIBUTE_LIFETIME_BOUND) {
+    interface_ = value.interface_;
+    return *this;
+  }
+
+  ListValueView& operator=(ListValue&&) = delete;
+
   // By default, this creates an empty list whose type is `list(dyn)`. Unless
   // you can help it, you should use a more specific typed list value.
   ListValueView();

diff --git a/common/values/map_value.h b/common/values/map_value.h
@@ -268,10 +268,22 @@ class MapValueView {
 
   static absl::Status CheckKey(ValueView key);
 
+  // NOLINTNEXTLINE(google-explicit-constructor)
+  MapValueView(SharedView<const MapValueInterface> interface)
+      : interface_(interface) {}
+
   // NOLINTNEXTLINE(google-explicit-constructor)
   MapValueView(const MapValue& value ABSL_ATTRIBUTE_LIFETIME_BOUND) noexcept
       : interface_(value.interface_) {}
 
+  // NOLINTNEXTLINE(google-explicit-constructor)
+  MapValueView& operator=(const MapValue& value ABSL_ATTRIBUTE_LIFETIME_BOUND) {
+    interface_ = value.interface_;
+    return *this;
+  }
+
+  MapValueView& operator=(MapValue&&) = delete;
+
   // By default, this creates an empty map whose type is `map(dyn, dyn)`. Unless
   // you can help it, you should use a more specific typed map value.
   MapValueView();

diff --git a/common/values/opaque_value.h b/common/values/opaque_value.h
@@ -171,17 +171,23 @@ class OpaqueValueView {
 
   static constexpr ValueKind kKind = OpaqueValue::kKind;
 
+  // NOLINTNEXTLINE(google-explicit-constructor)
+  OpaqueValueView(SharedView<const OpaqueValueInterface> interface)
+      : interface_(interface) {}
+
   // NOLINTNEXTLINE(google-explicit-constructor)
   OpaqueValueView(
       const OpaqueValue& value ABSL_ATTRIBUTE_LIFETIME_BOUND) noexcept
       : interface_(value.interface_) {}
 
-  // Prevent binding to temporaries.
-  OpaqueValueView(OpaqueValue&&) = delete;
-
   // NOLINTNEXTLINE(google-explicit-constructor)
-  OpaqueValueView(SharedView<const OpaqueValueInterface> interface)
-      : interface_(interface) {}
+  OpaqueValueView& operator=(
+      const OpaqueValue& value ABSL_ATTRIBUTE_LIFETIME_BOUND) {
+    interface_ = value.interface_;
+    return *this;
+  }
+
+  OpaqueValueView& operator=(OpaqueValue&&) = delete;
 
   OpaqueValueView() = delete;
   OpaqueValueView(const OpaqueValueView&) = default;

diff --git a/common/values/optional_value.h b/common/values/optional_value.h
@@ -147,15 +147,22 @@ class OptionalValueView final : public OpaqueValueView {
   OptionalValueView() : OptionalValueView(None()) {}
 
   // NOLINTNEXTLINE(google-explicit-constructor)
-  OptionalValueView(const OptionalValue& value ABSL_ATTRIBUTE_LIFETIME_BOUND)
-      : OpaqueValueView(value) {}
+  OptionalValueView(SharedView<const OptionalValueInterface> interface)
+      : OpaqueValueView(interface) {}
 
-  // Prevent binding to temporaries.
-  OptionalValueView(OptionalValue&&) = delete;
+  // NOLINTNEXTLINE(google-explicit-constructor)
+  OptionalValueView(
+      const OptionalValue& value ABSL_ATTRIBUTE_LIFETIME_BOUND) noexcept
+      : OpaqueValueView(value) {}
 
   // NOLINTNEXTLINE(google-explicit-constructor)
-  OptionalValueView(SharedView<const OptionalValueInterface> interface)
-      : OpaqueValueView(interface) {}
+  OptionalValueView& operator=(
+      const OptionalValue& value ABSL_ATTRIBUTE_LIFETIME_BOUND) {
+    OpaqueValueView::operator=(value);
+    return *this;
+  }
+
+  OptionalValueView& operator=(OptionalValue&&) = delete;
 
   OptionalValueView(const OptionalValueView&) = default;
   OptionalValueView& operator=(const OptionalValueView&) = default;

diff --git a/common/values/string_value.h b/common/values/string_value.h
@@ -146,6 +146,15 @@ class StringValueView final {
       const StringValue& value ABSL_ATTRIBUTE_LIFETIME_BOUND) noexcept
       : value_(value.value_) {}
 
+  // NOLINTNEXTLINE(google-explicit-constructor)
+  StringValueView& operator=(
+      const StringValue& value ABSL_ATTRIBUTE_LIFETIME_BOUND) {
+    value_ = value.value_;
+    return *this;
+  }
+
+  StringValueView& operator=(StringValue&&) = delete;
+
   StringValueView() = default;
   StringValueView(const StringValueView&) = default;
   StringValueView(StringValueView&&) = default;