Skip to content

Commit

Permalink
Add static asserts to container classes.
Browse files Browse the repository at this point in the history
This will prevent unsupported uses of these classes downstream, which can behave surprisingly and increase the friction of changes to protobuf.  Specifically RepeatedField, RepeatedPtrField, and Map will only be allowed to hold the types listed in https://protobuf.dev/programming-guides/proto.  The old behavior allowed them to hold just about anything, including custom user types.

PiperOrigin-RevId: 502672732
  • Loading branch information
mkruskal-google authored and copybara-github committed Jan 17, 2023
1 parent 8ea499d commit 5a8abe1
Show file tree
Hide file tree
Showing 4 changed files with 0 additions and 16 deletions.
4 changes: 0 additions & 4 deletions src/google/protobuf/map.h
Original file line number Diff line number Diff line change
Expand Up @@ -1088,7 +1088,6 @@ class Map : private internal::KeyMapBase<internal::KeyForBase<Key>> {
}

private:
#ifdef PROTOBUF_FUTURE_CONTAINER_STATIC_ASSERTS
static_assert(!std::is_const<mapped_type>::value &&
!std::is_const<key_type>::value,
"We do not support const types.");
Expand All @@ -1101,9 +1100,7 @@ class Map : private internal::KeyMapBase<internal::KeyForBase<Key>> {
static_assert(!std::is_reference<mapped_type>::value &&
!std::is_reference<key_type>::value,
"We do not support reference types.");
#endif // PROTOBUF_FUTURE_CONTAINER_STATIC_ASSERTS
static constexpr PROTOBUF_ALWAYS_INLINE void StaticValidityCheck() {
#ifdef PROTOBUF_FUTURE_CONTAINER_STATIC_ASSERTS
static_assert(alignof(internal::NodeBase) >= alignof(mapped_type),
"Alignment of mapped type is too high.");
static_assert(
Expand All @@ -1119,7 +1116,6 @@ class Map : private internal::KeyMapBase<internal::KeyForBase<Key>> {
internal::is_internal_map_value_type<mapped_type>>::value,
"We only support scalar, Message, and designated internal "
"mapped types.");
#endif // PROTOBUF_FUTURE_CONTAINER_STATIC_ASSERTS
}

template <typename P>
Expand Down
4 changes: 0 additions & 4 deletions src/google/protobuf/port_def.inc
Original file line number Diff line number Diff line change
Expand Up @@ -204,10 +204,6 @@ static_assert(PROTOBUF_CPLUSPLUS_MIN(201402L), "Protobuf only supports C++14 and

#ifdef PROTOBUF_FUTURE_BREAKING_CHANGES

// Include static asserts to lock down assumptions about our containers.
// Owner: mkruskal@
#define PROTOBUF_FUTURE_CONTAINER_STATIC_ASSERTS 1

// Used to upgrade google::protobuf::MapPair<K, V> to std::pair<const K, V>.
// Owner: mordberg@
#define PROTOBUF_FUTURE_MAP_PAIR_UPGRADE 1
Expand Down
4 changes: 0 additions & 4 deletions src/google/protobuf/repeated_field.h
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,6 @@ class RepeatedField final {
static_assert(
alignof(Arena) >= alignof(Element),
"We only support types that have an alignment smaller than Arena");
#ifndef PROTOBUF_FUTURE_CONTAINER_STATIC_ASSERTS
static_assert(!std::is_const<Element>::value,
"We do not support const value types.");
static_assert(!std::is_volatile<Element>::value,
Expand All @@ -158,16 +157,13 @@ class RepeatedField final {
"We do not support pointer value types.");
static_assert(!std::is_reference<Element>::value,
"We do not support reference value types.");
#endif // !PROTOBUF_FUTURE_CONTAINER_STATIC_ASSERTS
static constexpr PROTOBUF_ALWAYS_INLINE void StaticValidityCheck() {
#ifdef PROTOBUF_FUTURE_CONTAINER_STATIC_ASSERTS
static_assert(
absl::disjunction<internal::is_supported_integral_type<Element>,
internal::is_supported_floating_point_type<Element>,
std::is_same<absl::Cord, Element>,
is_proto_enum<Element>>::value,
"We only support non-string scalars in RepeatedField.");
#endif // PROTOBUF_FUTURE_CONTAINER_STATIC_ASSERTS
}

public:
Expand Down
4 changes: 0 additions & 4 deletions src/google/protobuf/repeated_ptr_field.h
Original file line number Diff line number Diff line change
Expand Up @@ -902,7 +902,6 @@ class StringTypeHandler {
// Messages.
template <typename Element>
class RepeatedPtrField final : private internal::RepeatedPtrFieldBase {
#ifndef PROTOBUF_FUTURE_CONTAINER_STATIC_ASSERTS
static_assert(!std::is_const<Element>::value,
"We do not support const value types.");
static_assert(!std::is_volatile<Element>::value,
Expand All @@ -911,15 +910,12 @@ class RepeatedPtrField final : private internal::RepeatedPtrFieldBase {
"We do not support pointer value types.");
static_assert(!std::is_reference<Element>::value,
"We do not support reference value types.");
#endif // !PROTOBUF_FUTURE_CONTAINER_STATIC_ASSERTS
static constexpr PROTOBUF_ALWAYS_INLINE void StaticValidityCheck() {
#ifdef PROTOBUF_FUTURE_CONTAINER_STATIC_ASSERTS
static_assert(
absl::disjunction<
internal::is_supported_string_type<Element>,
internal::is_supported_message_type<Element>>::value,
"We only support string and Message types in RepeatedPtrField.");
#endif // PROTOBUF_FUTURE_CONTAINER_STATIC_ASSERTS
}

public:
Expand Down

0 comments on commit 5a8abe1

Please sign in to comment.