Skip to content

Make cel::Value trivial again #2357

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
copybara-service merged 1 commit into from
Oct 12, 2026
Merged

Make cel::Value trivial again #2357

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions common/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -756,6 +756,7 @@ cc_library(
":unknown",
":value_kind",
"//base:attributes",
"//base:function_result_set",
"//common/internal:byte_string",
"//eval/internal:cel_value_equal",
"//eval/public:cel_value",
Expand Down
7 changes: 4 additions & 3 deletions common/legacy_value.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1071,8 +1071,8 @@ absl::Status ModernValue(google::protobuf::Arena* arena,
MapValue(common_internal::LegacyMapValue(legacy_value.MapOrDie()));
return absl::OkStatus();
case CelValue::Type::kUnknownSet:
result =
common_internal::MakeUnknownValue(*legacy_value.UnknownSetOrDie());
result = common_internal::MakeUnknownValue(
*legacy_value.UnknownSetOrDie(), arena);
return absl::OkStatus();
case CelValue::Type::kCelType: {
auto type_name = legacy_value.CelTypeOrDie().value();
Expand Down Expand Up @@ -1183,7 +1183,8 @@ 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 common_internal::MakeUnknownValue(*legacy_value.UnknownSetOrDie());
return common_internal::MakeUnknownValue(*legacy_value.UnknownSetOrDie(),
arena);
case CelValue::Type::kCelType:
return CreateTypeValueFromView(arena,
legacy_value.CelTypeOrDie().value());
Expand Down
49 changes: 0 additions & 49 deletions common/value.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,103 +298,54 @@ class Value final : private common_internal::ValueMixin<Value> {
Value() = default;
Value(const Value&) = default;
Value& operator=(const Value&) = default;
Value(Value&& other) = default;
Value& operator=(Value&&) = default;

// NOLINTNEXTLINE(google-explicit-constructor)
Value(const ListValue& value) : variant_(value.ToValueVariant()) {}

// NOLINTNEXTLINE(google-explicit-constructor)
Value(ListValue&& value) : variant_(std::move(value).ToValueVariant()) {}

// NOLINTNEXTLINE(google-explicit-constructor)
Value& operator=(const ListValue& value) {
variant_ = value.ToValueVariant();
return *this;
}

// NOLINTNEXTLINE(google-explicit-constructor)
Value& operator=(ListValue&& value) {
variant_ = std::move(value).ToValueVariant();
return *this;
}

// NOLINTNEXTLINE(google-explicit-constructor)
Value(const MapValue& value) : variant_(value.ToValueVariant()) {}

// NOLINTNEXTLINE(google-explicit-constructor)
Value(MapValue&& value) : variant_(std::move(value).ToValueVariant()) {}

// NOLINTNEXTLINE(google-explicit-constructor)
Value& operator=(const MapValue& value) {
variant_ = value.ToValueVariant();
return *this;
}

// NOLINTNEXTLINE(google-explicit-constructor)
Value& operator=(MapValue&& value) {
variant_ = std::move(value).ToValueVariant();
return *this;
}

// NOLINTNEXTLINE(google-explicit-constructor)
Value(const StructValue& value) : variant_(value.ToValueVariant()) {}

// NOLINTNEXTLINE(google-explicit-constructor)
Value(StructValue&& value) : variant_(std::move(value).ToValueVariant()) {}

// NOLINTNEXTLINE(google-explicit-constructor)
Value& operator=(const StructValue& value) {
variant_ = value.ToValueVariant();
return *this;
}

// NOLINTNEXTLINE(google-explicit-constructor)
Value& operator=(StructValue&& value) {
variant_ = std::move(value).ToValueVariant();
return *this;
}

// NOLINTNEXTLINE(google-explicit-constructor)
Value(const MessageValue& value) : variant_(value.ToValueVariant()) {}

// NOLINTNEXTLINE(google-explicit-constructor)
Value(MessageValue&& value) : variant_(std::move(value).ToValueVariant()) {}

// NOLINTNEXTLINE(google-explicit-constructor)
Value& operator=(const MessageValue& value) {
variant_ = value.ToValueVariant();
return *this;
}

// NOLINTNEXTLINE(google-explicit-constructor)
Value& operator=(MessageValue&& value) {
variant_ = std::move(value).ToValueVariant();
return *this;
}

// NOLINTNEXTLINE(google-explicit-constructor)
Value(const OptionalValue& value)
: variant_(absl::in_place_type<OpaqueValue>,
static_cast<const OpaqueValue&>(value)) {}

// NOLINTNEXTLINE(google-explicit-constructor)
Value(OptionalValue&& value)
: variant_(absl::in_place_type<OpaqueValue>,
static_cast<OpaqueValue&&>(value)) {}

// NOLINTNEXTLINE(google-explicit-constructor)
Value& operator=(const OptionalValue& value) {
variant_.Assign(static_cast<const OpaqueValue&>(value));
return *this;
}

// NOLINTNEXTLINE(google-explicit-constructor)
Value& operator=(OptionalValue&& value) {
variant_.Assign(static_cast<OpaqueValue&&>(value));
return *this;
}

template <typename T,
typename = std::enable_if_t<
common_internal::IsValueAlternativeV<absl::remove_cvref_t<T>>>>
Expand Down
11 changes: 11 additions & 0 deletions common/values/unknown_value.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@
// See the License for the specific language governing permissions and
// limitations under the License.

#include "absl/base/no_destructor.h"
#include "absl/base/nullability.h"
#include "absl/log/absl_check.h"
#include "absl/status/status.h"
#include "absl/strings/str_cat.h"
#include "base/function_result_set.h"
#include "common/value.h"
#include "google/protobuf/arena.h"
#include "google/protobuf/descriptor.h"
Expand Down Expand Up @@ -63,4 +65,13 @@ absl::Status UnknownValue::Equal(
return absl::OkStatus();
}

namespace common_internal {

const FunctionResultSet& GetEmptyFunctionResults() {
static const absl::NoDestructor<FunctionResultSet> instance;
return *instance;
}

} // namespace common_internal

} // namespace cel
25 changes: 16 additions & 9 deletions common/values/unknown_value.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "absl/base/nullability.h"
#include "absl/status/status.h"
#include "absl/strings/string_view.h"
#include "base/function_result_set.h"
#include "common/type.h"
#include "common/unknown.h"
#include "common/value_kind.h"
Expand All @@ -42,7 +43,7 @@ class UnknownValue;

namespace common_internal {
[[nodiscard]]
UnknownValue MakeUnknownValue(Unknown value);
UnknownValue MakeUnknownValue(Unknown value, google::protobuf::Arena* absl_nonnull arena);
[[nodiscard]]
Unknown GetUnknown(const UnknownValue& value);
[[nodiscard]]
Expand Down Expand Up @@ -90,7 +91,8 @@ class UnknownValue final : private common_internal::ValueMixin<UnknownValue> {

[[nodiscard]]
AttributeSet ToAttributeSet() const {
return unknown_.unknown_attributes();
return unknown_ != nullptr ? unknown_->unknown_attributes()
: AttributeSet();
}

void swap(UnknownValue& other) noexcept {
Expand All @@ -99,15 +101,16 @@ class UnknownValue final : private common_internal::ValueMixin<UnknownValue> {
}

private:
friend UnknownValue common_internal::MakeUnknownValue(Unknown value);
friend UnknownValue common_internal::MakeUnknownValue(
Unknown value, google::protobuf::Arena* absl_nonnull arena);
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)) {}
explicit UnknownValue(const Unknown* unknown) : unknown_(unknown) {}

Unknown unknown_;
const Unknown* unknown_ = nullptr;
};

inline void swap(UnknownValue& lhs, UnknownValue& rhs) noexcept {
Expand All @@ -120,20 +123,24 @@ inline std::ostream& operator<<(std::ostream& out, const UnknownValue& value) {

namespace common_internal {

const FunctionResultSet& GetEmptyFunctionResults();

[[nodiscard]]
inline UnknownValue MakeUnknownValue(Unknown value) {
return UnknownValue(std::move(value));
inline UnknownValue MakeUnknownValue(Unknown value,
google::protobuf::Arena* absl_nonnull arena) {
return UnknownValue(google::protobuf::Arena::Create<Unknown>(arena, std::move(value)));
}

[[nodiscard]]
inline Unknown GetUnknown(const UnknownValue& value) {
return value.unknown_;
return value.unknown_ != nullptr ? *value.unknown_ : Unknown();
}

[[nodiscard]]
inline const FunctionResultSet& GetUnknownFunctionResultSet(
const UnknownValue& value ABSL_ATTRIBUTE_LIFETIME_BOUND) {
return value.unknown_.unknown_function_results();
return value.unknown_ != nullptr ? value.unknown_->unknown_function_results()
: GetEmptyFunctionResults();
}

} // namespace common_internal
Expand Down
Loading
Loading