Skip to content

Commit eab6664

Browse files
Mizuchimeta-codesync[bot]
authored andcommitted
Use for_each_field_id_ascending in Json5ProtocolWriter
Summary: Update StructEncode to use for_each_field_id_ascending when protocol specifies IdAscending field order. Add fieldOrder() method to Json5ProtocolWriter. Reviewed By: hchokshi Differential Revision: D94950111 fbshipit-source-id: 30c3a3adc7e06b39169fcb7d0545565624cfd8af
1 parent 56510ab commit eab6664

3 files changed

Lines changed: 39 additions & 10 deletions

File tree

third-party/thrift/src/thrift/lib/cpp2/op/Get.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -497,10 +497,14 @@ constexpr void for_each_field_id_ascending_impl(
497497
std::array<int16_t, N> fieldIds{};
498498
std::copy_n(pa::field_ids<T>() + 1, N, fieldIds.begin());
499499
std::sort(fieldIds.begin(), fieldIds.end());
500-
return fieldIds;
500+
return std::move(fieldIds);
501501
});
502502

503-
(f(field_id<sortedFieldIds[I]>{}), ...);
503+
// Use array-based expansion instead of fold expression to avoid exceeding
504+
// compiler's expression nesting limit for structs with >256 fields.
505+
std::array<int, sizeof...(I) + 1> unused{
506+
{0, (f(field_id<sortedFieldIds[I]>{}), 0)...}};
507+
static_cast<void>(unused);
504508
}
505509

506510
template <size_t... I, typename F>

third-party/thrift/src/thrift/lib/cpp2/op/detail/Encode.h

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,14 @@ class SimpleJSONProtocolWriter;
4444

4545
namespace op::detail {
4646

47+
template <class Protocol>
48+
FieldOrder getFieldOrder(const Protocol& prot) {
49+
if constexpr (requires { prot.fieldOrder(); }) {
50+
return prot.fieldOrder();
51+
}
52+
return FieldOrder::Serialization;
53+
}
54+
4755
template <typename T, typename Tag>
4856
inline constexpr bool kIsStrongType =
4957
std::is_enum_v<folly::remove_cvref_t<T>> &&
@@ -549,13 +557,8 @@ struct StructEncode {
549557
uint32_t operator()(Protocol& prot, const T& t) const {
550558
uint32_t s = 0;
551559
s += prot.writeStructBegin(op::get_class_name_v<T>.data());
552-
op::for_each_ordinal<T>([&](auto id) {
553-
// To respect the SerializeInFieldIdOrder annotation, we perform a custom
554-
// mapping to FieldId that may switch the current ordinal being visited.
555-
// This new field id is the source of truth, and the old ordinal value
556-
// should not be used in this function.
557-
using Id = type::field_id<detail::pa::field_ids_in_serialization_order<
558-
T>()[static_cast<size_t>(decltype(id)::value)]>;
560+
auto writeField = [&]<class Id>(Id) {
561+
static_assert(type::is_field_id_v<Id>);
559562
using TypeTag = op::get_type_tag<T, Id>;
560563
using FieldTag = op::get_field_tag<T, Id>;
561564
auto&& field = op::get<Id>(t);
@@ -569,7 +572,20 @@ struct StructEncode {
569572
folly::to_underlying(Id::value));
570573
s += Encode<TypeTag>{}(prot, *field);
571574
s += prot.writeFieldEnd();
572-
});
575+
};
576+
if (getFieldOrder(prot) == FieldOrder::Serialization) {
577+
op::for_each_ordinal<T>([&](auto id) {
578+
// To respect the SerializeInFieldIdOrder annotation, we perform a
579+
// custom mapping to FieldId that may switch the current ordinal being
580+
// visited. This new field id is the source of truth, and the old
581+
// ordinal value should not be used in this function.
582+
using Id = type::field_id<detail::pa::field_ids_in_serialization_order<
583+
T>()[static_cast<size_t>(decltype(id)::value)]>;
584+
writeField(Id{});
585+
});
586+
} else {
587+
op::for_each_field_id_ascending<T>(writeField);
588+
}
573589
s += prot.writeFieldStop();
574590
s += prot.writeStructEnd();
575591
return s;

third-party/thrift/src/thrift/lib/cpp2/protocol/Protocol.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,15 @@ constexpr bool usesFieldNames() {
459459
}
460460
}
461461

462+
/// Specifies the order in which struct fields are serialized.
463+
enum class FieldOrder {
464+
/// Field declaration order in IDL, unless SerializeInFieldIdOrder annotation
465+
/// is present, in which case field id ascending order.
466+
Serialization,
467+
/// Field id ascending order (1, 2, 3, ...).
468+
IdAscending,
469+
};
470+
462471
} // namespace apache::thrift
463472

464473
#endif

0 commit comments

Comments
 (0)