Skip to content

Commit 7d3b4de

Browse files
jnthntatumcopybara-github
authored andcommitted
Update bit math constants for tagged messages for interop.
Updated names should help readability since the tagging scheme is overloaded for both lite proto support and for handle migration. PiperOrigin-RevId: 534145645
1 parent bb67412 commit 7d3b4de

6 files changed

Lines changed: 75 additions & 17 deletions

File tree

base/internal/message_wrapper.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,11 @@
1919

2020
namespace cel::base_internal {
2121

22-
inline constexpr uintptr_t kMessageWrapperTagMask = 1 << 0;
22+
inline constexpr uintptr_t kMessageWrapperTagMask = 0b1;
2323
inline constexpr uintptr_t kMessageWrapperPtrMask = ~kMessageWrapperTagMask;
24+
inline constexpr uintptr_t kMessageWrapperTagSize = 1;
25+
inline constexpr uintptr_t kMessageWrapperTagTypeInfoValue = 0b0;
26+
inline constexpr uintptr_t kMessageWrapperTagMessageValue = 0b1;
2427

2528
} // namespace cel::base_internal
2629

base/values/struct_value.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,11 +179,13 @@ class LegacyStructValueFieldIterator final : public StructValue::FieldIterator {
179179
};
180180

181181
Handle<StructType> LegacyStructValue::type() const {
182-
if ((msg_ & kMessageWrapperTagMask) == kMessageWrapperTagMask) {
182+
uintptr_t tag = msg_ & kMessageWrapperTagMask;
183+
if (tag == kMessageWrapperTagMessageValue) {
183184
// google::protobuf::Message
184185
return HandleFactory<StructType>::Make<LegacyStructType>(msg_);
185186
}
186187
// LegacyTypeInfoApis
188+
ABSL_ASSERT(tag == kMessageWrapperTagTypeInfoValue);
187189
return HandleFactory<StructType>::Make<LegacyStructType>(type_info_);
188190
}
189191

eval/internal/BUILD

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,7 @@ cc_library(
2222
hdrs = ["interop.h"],
2323
deps = [
2424
":errors",
25-
"//base:type",
26-
"//base:value",
25+
"//base:data",
2726
"//base/internal:message_wrapper",
2827
"//eval/public:cel_options",
2928
"//eval/public:cel_value",
@@ -51,14 +50,15 @@ cc_test(
5150
deps = [
5251
":errors",
5352
":interop",
53+
"//base:data",
5454
"//base:memory",
55-
"//base:type",
56-
"//base:value",
5755
"//eval/public:cel_value",
56+
"//eval/public:message_wrapper",
5857
"//eval/public:unknown_set",
5958
"//eval/public/containers:container_backed_list_impl",
6059
"//eval/public/containers:container_backed_map_impl",
6160
"//eval/public/structs:cel_proto_wrapper",
61+
"//eval/public/structs:trivial_legacy_type_info",
6262
"//extensions/protobuf:memory_manager",
6363
"//extensions/protobuf:type",
6464
"//extensions/protobuf:value",

eval/internal/interop.cc

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
#include "eval/internal/interop.h"
1616

17+
#include <cstdint>
1718
#include <string>
1819
#include <utility>
1920
#include <vector>
@@ -43,6 +44,7 @@
4344
#include "eval/public/unknown_set.h"
4445
#include "extensions/protobuf/memory_manager.h"
4546
#include "internal/status_macros.h"
47+
#include "google/protobuf/message.h"
4648

4749
namespace cel::interop_internal {
4850

@@ -667,12 +669,17 @@ using ::google::api::expr::runtime::MessageWrapper;
667669
} // namespace
668670

669671
absl::string_view MessageTypeName(uintptr_t msg) {
670-
if ((msg & kMessageWrapperTagMask) != kMessageWrapperTagMask) {
672+
uintptr_t tag = (msg & kMessageWrapperTagMask);
673+
uintptr_t ptr = (msg & kMessageWrapperPtrMask);
674+
675+
if (tag == kMessageWrapperTagTypeInfoValue) {
671676
// For google::protobuf::MessageLite, this is actually LegacyTypeInfoApis.
672-
return reinterpret_cast<const LegacyTypeInfoApis*>(msg)->GetTypename(
677+
return reinterpret_cast<const LegacyTypeInfoApis*>(ptr)->GetTypename(
673678
MessageWrapper());
674679
}
675-
return reinterpret_cast<const google::protobuf::Message*>(msg & kMessageWrapperPtrMask)
680+
ABSL_ASSERT(tag == kMessageWrapperTagMessageValue);
681+
682+
return reinterpret_cast<const google::protobuf::Message*>(ptr)
676683
->GetDescriptor()
677684
->full_name();
678685
}

eval/internal/interop_test.cc

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include <vector>
2222

2323
#include "google/protobuf/api.pb.h"
24+
#include "google/protobuf/empty.pb.h"
2425
#include "google/protobuf/arena.h"
2526
#include "absl/status/status.h"
2627
#include "absl/strings/escaping.h"
@@ -36,7 +37,9 @@
3637
#include "eval/public/cel_value.h"
3738
#include "eval/public/containers/container_backed_list_impl.h"
3839
#include "eval/public/containers/container_backed_map_impl.h"
40+
#include "eval/public/message_wrapper.h"
3941
#include "eval/public/structs/cel_proto_wrapper.h"
42+
#include "eval/public/structs/trivial_legacy_type_info.h"
4043
#include "eval/public/unknown_set.h"
4144
#include "extensions/protobuf/memory_manager.h"
4245
#include "extensions/protobuf/type_provider.h"
@@ -49,6 +52,7 @@ namespace {
4952
using ::google::api::expr::runtime::CelProtoWrapper;
5053
using ::google::api::expr::runtime::CelValue;
5154
using ::google::api::expr::runtime::ContainerBackedListImpl;
55+
using ::google::api::expr::runtime::MessageWrapper;
5256
using ::google::api::expr::runtime::UnknownSet;
5357
using testing::Eq;
5458
using testing::HasSubstr;
@@ -763,6 +767,38 @@ TEST(ValueInterop, StructFromLegacy) {
763767
value_wrapper.legacy_type_info());
764768
}
765769

770+
TEST(ValueInterop, StructFromLegacyMessageLite) {
771+
google::protobuf::Arena arena;
772+
extensions::ProtoMemoryManager memory_manager(&arena);
773+
TypeFactory type_factory(memory_manager);
774+
TypeManager type_manager(type_factory, TypeProvider::Builtin());
775+
ValueFactory value_factory(type_manager);
776+
google::protobuf::Empty opaque;
777+
MessageWrapper wrapper(
778+
static_cast<const google::protobuf::MessageLite*>(&opaque),
779+
google::api::expr::runtime::TrivialTypeInfo::GetInstance());
780+
CelValue legacy_value = CelValue::CreateMessageWrapper(wrapper);
781+
ASSERT_OK_AND_ASSIGN(auto value, FromLegacyValue(&arena, legacy_value));
782+
EXPECT_EQ(value->kind(), Kind::kStruct);
783+
EXPECT_EQ(value->type()->kind(), Kind::kStruct);
784+
EXPECT_EQ(value->type()->name(), "opaque type");
785+
EXPECT_THAT(
786+
value.As<StructValue>()->HasFieldByName(
787+
StructValue::HasFieldContext(type_manager), "name"),
788+
StatusIs(absl::StatusCode::kNotFound, HasSubstr("no_such_field")));
789+
EXPECT_THAT(value.As<StructValue>()->HasFieldByNumber(
790+
StructValue::HasFieldContext(type_manager), 1),
791+
StatusIs(absl::StatusCode::kUnimplemented));
792+
EXPECT_EQ(value.As<StructValue>()->DebugString(), "opaque type");
793+
auto value_wrapper = LegacyStructValueAccess::ToMessageWrapper(
794+
*value.As<base_internal::LegacyStructValue>());
795+
auto legacy_value_wrapper = legacy_value.MessageWrapperOrDie();
796+
EXPECT_EQ(legacy_value_wrapper.HasFullProto(), value_wrapper.HasFullProto());
797+
EXPECT_EQ(legacy_value_wrapper.message_ptr(), value_wrapper.message_ptr());
798+
EXPECT_EQ(legacy_value_wrapper.legacy_type_info(),
799+
value_wrapper.legacy_type_info());
800+
}
801+
766802
TEST(ValueInterop, LegacyStructRoundtrip) {
767803
google::protobuf::Arena arena;
768804
extensions::ProtoMemoryManager memory_manager(&arena);

eval/public/message_wrapper.h

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,18 +45,22 @@ class MessageWrapper {
4545
public:
4646
explicit Builder(google::protobuf::MessageLite* message)
4747
: message_ptr_(reinterpret_cast<uintptr_t>(message)) {
48-
ABSL_ASSERT(absl::countr_zero(reinterpret_cast<uintptr_t>(message)) >= 1);
48+
ABSL_ASSERT(absl::countr_zero(reinterpret_cast<uintptr_t>(message)) >=
49+
kTagSize);
4950
}
5051
explicit Builder(google::protobuf::Message* message)
51-
: message_ptr_(reinterpret_cast<uintptr_t>(message) | kTagMask) {
52-
ABSL_ASSERT(absl::countr_zero(reinterpret_cast<uintptr_t>(message)) >= 1);
52+
: message_ptr_(reinterpret_cast<uintptr_t>(message) | kMessageTag) {
53+
ABSL_ASSERT(absl::countr_zero(reinterpret_cast<uintptr_t>(message)) >=
54+
kTagSize);
5355
}
5456

5557
google::protobuf::MessageLite* message_ptr() const {
5658
return reinterpret_cast<google::protobuf::MessageLite*>(message_ptr_ & kPtrMask);
5759
}
5860

59-
bool HasFullProto() const { return (message_ptr_ & kTagMask) == kTagMask; }
61+
bool HasFullProto() const {
62+
return (message_ptr_ & kTagMask) == kMessageTag;
63+
}
6064

6165
MessageWrapper Build(const LegacyTypeInfoApis* type_info) {
6266
return MessageWrapper(message_ptr_, type_info);
@@ -78,19 +82,21 @@ class MessageWrapper {
7882
const LegacyTypeInfoApis* legacy_type_info)
7983
: message_ptr_(reinterpret_cast<uintptr_t>(message)),
8084
legacy_type_info_(legacy_type_info) {
81-
ABSL_ASSERT(absl::countr_zero(reinterpret_cast<uintptr_t>(message)) >= 1);
85+
ABSL_ASSERT(absl::countr_zero(reinterpret_cast<uintptr_t>(message)) >=
86+
kTagSize);
8287
}
8388

8489
MessageWrapper(const google::protobuf::Message* message,
8590
const LegacyTypeInfoApis* legacy_type_info)
86-
: message_ptr_(reinterpret_cast<uintptr_t>(message) | kTagMask),
91+
: message_ptr_(reinterpret_cast<uintptr_t>(message) | kMessageTag),
8792
legacy_type_info_(legacy_type_info) {
88-
ABSL_ASSERT(absl::countr_zero(reinterpret_cast<uintptr_t>(message)) >= 1);
93+
ABSL_ASSERT(absl::countr_zero(reinterpret_cast<uintptr_t>(message)) >=
94+
kTagSize);
8995
}
9096

9197
// If true, the message is using the full proto runtime and downcasting to
9298
// message should be safe.
93-
bool HasFullProto() const { return (message_ptr_ & kTagMask) == kTagMask; }
99+
bool HasFullProto() const { return (message_ptr_ & kTagMask) == kMessageTag; }
94100

95101
// Returns the underlying message.
96102
//
@@ -114,10 +120,14 @@ class MessageWrapper {
114120

115121
Builder ToBuilder() { return Builder(message_ptr_); }
116122

123+
static constexpr uintptr_t kTagSize =
124+
::cel::base_internal::kMessageWrapperTagSize;
117125
static constexpr uintptr_t kTagMask =
118126
::cel::base_internal::kMessageWrapperTagMask;
119127
static constexpr uintptr_t kPtrMask =
120128
::cel::base_internal::kMessageWrapperPtrMask;
129+
static constexpr uintptr_t kMessageTag =
130+
::cel::base_internal::kMessageWrapperTagMessageValue;
121131
uintptr_t message_ptr_;
122132
const LegacyTypeInfoApis* legacy_type_info_;
123133
};

0 commit comments

Comments
 (0)