Skip to content

Commit

Permalink
Make Type always valid
Browse files Browse the repository at this point in the history 
PiperOrigin-RevId: 663866779
  • Loading branch information
@jcking @copybara-github
jcking authored and copybara-github committed Aug 16, 2024
1 parent 25bc83d commit ba08c55
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 102 deletions.
55 changes: 12 additions & 43 deletions common/type.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
#include "absl/base/attributes.h"
#include "absl/base/nullability.h"
#include "absl/log/absl_check.h"
#include "absl/meta/type_traits.h"
#include "absl/strings/string_view.h"
#include "absl/types/optional.h"
#include "absl/types/span.h"
Expand Down Expand Up @@ -79,17 +78,17 @@ Type Type::Enum(absl::Nonnull<const google::protobuf::EnumDescriptor*> descripto

namespace {

static constexpr std::array<TypeKind, 29> kTypeToKindArray = {
TypeKind::kError, TypeKind::kAny, TypeKind::kBool,
static constexpr std::array<TypeKind, 28> kTypeToKindArray = {
TypeKind::kDyn, TypeKind::kAny, TypeKind::kBool,
TypeKind::kBoolWrapper, TypeKind::kBytes, TypeKind::kBytesWrapper,
TypeKind::kDouble, TypeKind::kDoubleWrapper, TypeKind::kDuration,
TypeKind::kDyn, TypeKind::kEnum, TypeKind::kError,
TypeKind::kFunction, TypeKind::kInt, TypeKind::kIntWrapper,
TypeKind::kList, TypeKind::kMap, TypeKind::kNull,
TypeKind::kOpaque, TypeKind::kString, TypeKind::kStringWrapper,
TypeKind::kStruct, TypeKind::kStruct, TypeKind::kTimestamp,
TypeKind::kTypeParam, TypeKind::kType, TypeKind::kUint,
TypeKind::kUintWrapper, TypeKind::kUnknown};
TypeKind::kEnum, TypeKind::kError, TypeKind::kFunction,
TypeKind::kInt, TypeKind::kIntWrapper, TypeKind::kList,
TypeKind::kMap, TypeKind::kNull, TypeKind::kOpaque,
TypeKind::kString, TypeKind::kStringWrapper, TypeKind::kStruct,
TypeKind::kStruct, TypeKind::kTimestamp, TypeKind::kTypeParam,
TypeKind::kType, TypeKind::kUint, TypeKind::kUintWrapper,
TypeKind::kUnknown};

static_assert(kTypeToKindArray.size() ==
absl::variant_size<common_internal::TypeVariant>(),
Expand All @@ -98,64 +97,34 @@ static_assert(kTypeToKindArray.size() ==
} // namespace

TypeKind Type::kind() const {
ABSL_DCHECK(*this);
return kTypeToKindArray[variant_.index()];
}

absl::string_view Type::name() const ABSL_ATTRIBUTE_LIFETIME_BOUND {
ABSL_DCHECK(*this);
return absl::visit(
[](const auto& alternative) -> absl::string_view {
if constexpr (std::is_same_v<
absl::remove_cvref_t<decltype(alternative)>,
absl::monostate>) {
// In optimized builds, we just return an empty string. In debug
// builds we cannot reach here.
return absl::string_view();
} else {
return alternative.name();
}
return alternative.name();
},
variant_);
}

std::string Type::DebugString() const {
ABSL_DCHECK(*this);
return absl::visit(
[](const auto& alternative) -> std::string {
if constexpr (std::is_same_v<
absl::remove_cvref_t<decltype(alternative)>,
absl::monostate>) {
// In optimized builds, we just return an empty string. In debug
// builds we cannot reach here.
return std::string();
} else {
return alternative.DebugString();
}
return alternative.DebugString();
},
variant_);
}

absl::Span<const Type> Type::parameters() const {
ABSL_DCHECK(*this);
return absl::visit(
[](const auto& alternative) -> absl::Span<const Type> {
if constexpr (std::is_same_v<
absl::remove_cvref_t<decltype(alternative)>,
absl::monostate>) {
// In optimized builds, we just return an empty string. In debug
// builds we cannot reach here.
return {};
} else {
return alternative.parameters();
}
return alternative.parameters();
},
variant_);
}

bool operator==(const Type& lhs, const Type& rhs) {
lhs.AssertIsValid();
rhs.AssertIsValid();
if (lhs.IsStruct() && rhs.IsStruct()) {
return static_cast<StructType>(lhs) == static_cast<StructType>(rhs);
} else if (lhs.IsStruct() || rhs.IsStruct()) {
Expand Down
35 changes: 4 additions & 31 deletions common/type.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
#include "absl/algorithm/container.h"
#include "absl/base/attributes.h"
#include "absl/base/nullability.h"
#include "absl/log/absl_check.h"
#include "absl/meta/type_traits.h"
#include "absl/strings/string_view.h"
#include "absl/types/optional.h"
Expand Down Expand Up @@ -93,6 +92,7 @@ class Type final {
static Type Enum(absl::Nonnull<const google::protobuf::EnumDescriptor*> descriptor
ABSL_ATTRIBUTE_LIFETIME_BOUND);

// The default constructor results in Type being DynType.
Type() = default;
Type(const Type&) = default;
Type(Type&&) = default;
Expand Down Expand Up @@ -147,20 +147,11 @@ class Type final {

template <typename H>
friend H AbslHashValue(H state, const Type& type) {
type.AssertIsValid();
return H::combine(
absl::visit(
[state = std::move(state)](const auto& alternative) mutable -> H {
if constexpr (std::is_same_v<
absl::remove_cvref_t<decltype(alternative)>,
absl::monostate>) {
// In optimized builds, we just hash `absl::monostate`. In debug
// builds we cannot reach here.
return H::combine(std::move(state), alternative);
} else {
return H::combine(std::move(state), alternative.kind(),
alternative);
}
return H::combine(std::move(state), alternative.kind(),
alternative);
},
type.variant_),
type.variant_.index());
Expand All @@ -171,15 +162,7 @@ class Type final {
friend std::ostream& operator<<(std::ostream& out, const Type& type) {
return absl::visit(
[&out](const auto& alternative) -> std::ostream& {
if constexpr (std::is_same_v<
absl::remove_cvref_t<decltype(alternative)>,
absl::monostate>) {
// In optimized builds, we do nothing. In debug builds we cannot
// reach here.
return out;
} else {
return out << alternative;
}
return out << alternative;
},
type.variant_);
}
Expand Down Expand Up @@ -677,10 +660,6 @@ class Type final {
return AsUnknown();
}

explicit operator bool() const {
return !absl::holds_alternative<absl::monostate>(variant_);
}

explicit operator AnyType() const;

explicit operator BoolType() const;
Expand Down Expand Up @@ -744,12 +723,6 @@ class Type final {
friend class MessageType;
friend class common_internal::BasicStructType;

bool IsValid() const { return static_cast<bool>(*this); }

void AssertIsValid() const {
ABSL_DCHECK(IsValid()) << "use of invalid Type";
}

common_internal::StructTypeVariant ToStructTypeVariant() const;

common_internal::TypeVariant variant_;
Expand Down
90 changes: 69 additions & 21 deletions common/type_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

#include "common/type.h"

#include "absl/hash/hash.h"
#include "absl/hash/hash_testing.h"
#include "absl/log/die_if_null.h"
#include "internal/testing.h"
Expand All @@ -25,10 +24,14 @@ namespace cel {
namespace {

using ::cel::internal::GetTestingDescriptorPool;
using testing::_;
using testing::An;
using testing::Optional;

TEST(Type, Default) {
EXPECT_EQ(Type(), DynType());
EXPECT_TRUE(Type().IsDyn());
}

TEST(Type, Enum) {
EXPECT_EQ(
Type::Enum(
Expand All @@ -42,28 +45,73 @@ TEST(Type, Enum) {
NullType());
}

TEST(Type, KindDebugDeath) {
Type type;
static_cast<void>(type);
EXPECT_DEBUG_DEATH(static_cast<void>(type.kind()), _);
}
TEST(Type, Kind) {
google::protobuf::Arena arena;

TEST(Type, NameDebugDeath) {
Type type;
static_cast<void>(type);
EXPECT_DEBUG_DEATH(static_cast<void>(type.name()), _);
}
EXPECT_EQ(Type(AnyType()).kind(), AnyType::kKind);

TEST(Type, HashDebugDeath) {
Type type;
static_cast<void>(type);
EXPECT_DEBUG_DEATH(static_cast<void>(absl::HashOf(type)), _);
}
EXPECT_EQ(Type(BoolType()).kind(), BoolType::kKind);

EXPECT_EQ(Type(BoolWrapperType()).kind(), BoolWrapperType::kKind);

EXPECT_EQ(Type(BytesType()).kind(), BytesType::kKind);

EXPECT_EQ(Type(BytesWrapperType()).kind(), BytesWrapperType::kKind);

EXPECT_EQ(Type(DoubleType()).kind(), DoubleType::kKind);

EXPECT_EQ(Type(DoubleWrapperType()).kind(), DoubleWrapperType::kKind);

EXPECT_EQ(Type(DurationType()).kind(), DurationType::kKind);

EXPECT_EQ(Type(DynType()).kind(), DynType::kKind);

EXPECT_EQ(
Type(EnumType(
ABSL_DIE_IF_NULL(GetTestingDescriptorPool()->FindEnumTypeByName(
"google.api.expr.test.v1.proto3.TestAllTypes.NestedEnum"))))
.kind(),
EnumType::kKind);

EXPECT_EQ(Type(ErrorType()).kind(), ErrorType::kKind);

EXPECT_EQ(Type(FunctionType(&arena, DynType(), {})).kind(),
FunctionType::kKind);

EXPECT_EQ(Type(IntType()).kind(), IntType::kKind);

EXPECT_EQ(Type(IntWrapperType()).kind(), IntWrapperType::kKind);

EXPECT_EQ(Type(ListType()).kind(), ListType::kKind);

EXPECT_EQ(Type(MapType()).kind(), MapType::kKind);

EXPECT_EQ(Type(MessageType(ABSL_DIE_IF_NULL(
GetTestingDescriptorPool()->FindMessageTypeByName(
"google.api.expr.test.v1.proto3.TestAllTypes"))))
.kind(),
MessageType::kKind);
EXPECT_EQ(Type(MessageType(ABSL_DIE_IF_NULL(
GetTestingDescriptorPool()->FindMessageTypeByName(
"google.api.expr.test.v1.proto3.TestAllTypes"))))
.kind(),
MessageType::kKind);

EXPECT_EQ(Type(NullType()).kind(), NullType::kKind);

EXPECT_EQ(Type(OptionalType()).kind(), OpaqueType::kKind);

EXPECT_EQ(Type(StringType()).kind(), StringType::kKind);

EXPECT_EQ(Type(StringWrapperType()).kind(), StringWrapperType::kKind);

EXPECT_EQ(Type(TimestampType()).kind(), TimestampType::kKind);

EXPECT_EQ(Type(UintType()).kind(), UintType::kKind);

EXPECT_EQ(Type(UintWrapperType()).kind(), UintWrapperType::kKind);

TEST(Type, EqualDebugDeath) {
Type type;
static_cast<void>(type);
EXPECT_DEBUG_DEATH(static_cast<void>(type == type), _);
EXPECT_EQ(Type(UnknownType()).kind(), UnknownType::kKind);
}

TEST(Type, Is) {
Expand Down
14 changes: 7 additions & 7 deletions common/types/types.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,13 @@ template <typename T>
inline constexpr bool IsTypeAlternativeV = IsTypeAlternative<T>::value;

using TypeVariant =
absl::variant<absl::monostate, AnyType, BoolType, BoolWrapperType,
BytesType, BytesWrapperType, DoubleType, DoubleWrapperType,
DurationType, DynType, EnumType, ErrorType, FunctionType,
IntType, IntWrapperType, ListType, MapType, NullType,
OpaqueType, StringType, StringWrapperType, MessageType,
BasicStructType, TimestampType, TypeParamType, TypeType,
UintType, UintWrapperType, UnknownType>;
absl::variant<DynType, AnyType, BoolType, BoolWrapperType, BytesType,
BytesWrapperType, DoubleType, DoubleWrapperType, DurationType,
EnumType, ErrorType, FunctionType, IntType, IntWrapperType,
ListType, MapType, NullType, OpaqueType, StringType,
StringWrapperType, MessageType, BasicStructType,
TimestampType, TypeParamType, TypeType, UintType,
UintWrapperType, UnknownType>;

using StructTypeVariant =
absl::variant<absl::monostate, BasicStructType, MessageType>;
Expand Down

0 comments on commit ba08c55

Please sign in to comment.