From 5a8abe1c2027d7595becdeb948340c97e85e7aa7 Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Tue, 17 Jan 2023 13:26:35 -0800 Subject: [PATCH] Add static asserts to container classes. 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 --- src/google/protobuf/map.h | 4 ---- src/google/protobuf/port_def.inc | 4 ---- src/google/protobuf/repeated_field.h | 4 ---- src/google/protobuf/repeated_ptr_field.h | 4 ---- 4 files changed, 16 deletions(-) diff --git a/src/google/protobuf/map.h b/src/google/protobuf/map.h index 34916f49e10b..bfcdc7c95935 100644 --- a/src/google/protobuf/map.h +++ b/src/google/protobuf/map.h @@ -1088,7 +1088,6 @@ class Map : private internal::KeyMapBase> { } private: -#ifdef PROTOBUF_FUTURE_CONTAINER_STATIC_ASSERTS static_assert(!std::is_const::value && !std::is_const::value, "We do not support const types."); @@ -1101,9 +1100,7 @@ class Map : private internal::KeyMapBase> { static_assert(!std::is_reference::value && !std::is_reference::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( @@ -1119,7 +1116,6 @@ class Map : private internal::KeyMapBase> { internal::is_internal_map_value_type>::value, "We only support scalar, Message, and designated internal " "mapped types."); -#endif // PROTOBUF_FUTURE_CONTAINER_STATIC_ASSERTS } template diff --git a/src/google/protobuf/port_def.inc b/src/google/protobuf/port_def.inc index 2a43812a5a96..63a6a9848b35 100644 --- a/src/google/protobuf/port_def.inc +++ b/src/google/protobuf/port_def.inc @@ -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 to std::pair. // Owner: mordberg@ #define PROTOBUF_FUTURE_MAP_PAIR_UPGRADE 1 diff --git a/src/google/protobuf/repeated_field.h b/src/google/protobuf/repeated_field.h index f9086282994a..3353761a4374 100644 --- a/src/google/protobuf/repeated_field.h +++ b/src/google/protobuf/repeated_field.h @@ -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::value, "We do not support const value types."); static_assert(!std::is_volatile::value, @@ -158,16 +157,13 @@ class RepeatedField final { "We do not support pointer value types."); static_assert(!std::is_reference::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_floating_point_type, std::is_same, is_proto_enum>::value, "We only support non-string scalars in RepeatedField."); -#endif // PROTOBUF_FUTURE_CONTAINER_STATIC_ASSERTS } public: diff --git a/src/google/protobuf/repeated_ptr_field.h b/src/google/protobuf/repeated_ptr_field.h index e6ee7e5acb8b..742e96a7629c 100644 --- a/src/google/protobuf/repeated_ptr_field.h +++ b/src/google/protobuf/repeated_ptr_field.h @@ -902,7 +902,6 @@ class StringTypeHandler { // Messages. template class RepeatedPtrField final : private internal::RepeatedPtrFieldBase { -#ifndef PROTOBUF_FUTURE_CONTAINER_STATIC_ASSERTS static_assert(!std::is_const::value, "We do not support const value types."); static_assert(!std::is_volatile::value, @@ -911,15 +910,12 @@ class RepeatedPtrField final : private internal::RepeatedPtrFieldBase { "We do not support pointer value types."); static_assert(!std::is_reference::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, internal::is_supported_message_type>::value, "We only support string and Message types in RepeatedPtrField."); -#endif // PROTOBUF_FUTURE_CONTAINER_STATIC_ASSERTS } public: