From 8d8527b77236a8df30ea99bd7061e32f78b85c86 Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Wed, 15 Feb 2023 15:18:06 -0800 Subject: [PATCH] Remove the exception for multiple extension ranges for TDP. It is unnecessary since the regular extension parser can handle unknown fields directly. PiperOrigin-RevId: 509946221 --- .../compiler/cpp/parse_function_generator.cc | 10 ++--- .../protobuf/generated_message_reflection.cc | 23 ++++++----- .../protobuf/generated_message_tctable_decl.h | 5 --- .../protobuf/generated_message_tctable_gen.cc | 3 +- .../protobuf/generated_message_tctable_impl.h | 17 ++++---- .../generated_message_tctable_lite_test.cc | 40 +++++++++---------- 6 files changed, 48 insertions(+), 50 deletions(-) diff --git a/src/google/protobuf/compiler/cpp/parse_function_generator.cc b/src/google/protobuf/compiler/cpp/parse_function_generator.cc index 7e93fcd6c2..791a7d5503 100644 --- a/src/google/protobuf/compiler/cpp/parse_function_generator.cc +++ b/src/google/protobuf/compiler/cpp/parse_function_generator.cc @@ -494,14 +494,10 @@ void ParseFunctionGenerator::GenerateTailCallTable(Formatter& format) { } else { format("0, // no _has_bits_\n"); } - if (descriptor_->extension_range_count() == 1) { - format( - "PROTOBUF_FIELD_OFFSET($classname$, $extensions$),\n" - "$1$, $2$, // extension_range_{low,high}\n", - descriptor_->extension_range(0)->start, - descriptor_->extension_range(0)->end); + if (descriptor_->extension_range_count() != 0) { + format("PROTOBUF_FIELD_OFFSET($classname$, $extensions$),\n"); } else { - format("0, 0, 0, // no _extensions_\n"); + format("0, // no _extensions_\n"); } format("$1$, $2$, // max_field_number, fast_idx_mask\n", (ordered_fields_.empty() ? 0 : ordered_fields_.back()->number()), diff --git a/src/google/protobuf/generated_message_reflection.cc b/src/google/protobuf/generated_message_reflection.cc index 80a3eb5e91..efe5d7aae2 100644 --- a/src/google/protobuf/generated_message_reflection.cc +++ b/src/google/protobuf/generated_message_reflection.cc @@ -3092,9 +3092,9 @@ const internal::TcParseTableBase* Reflection::CreateTcParseTableForMessageSet() // We use `operator new` here because the destruction will be done with // `operator delete` unconditionally. void* p = ::operator new(sizeof(Table)); - auto* full_table = ::new (p) Table{ - {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, schema_.default_instance_, nullptr}, - {{{&internal::TcParser::ReflectionParseLoop, {}}}}}; + auto* full_table = ::new (p) + Table{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, schema_.default_instance_, nullptr}, + {{{&internal::TcParser::ReflectionParseLoop, {}}}}}; ABSL_DCHECK_EQ(static_cast(&full_table->header), static_cast(full_table)); return &full_table->header; @@ -3292,14 +3292,19 @@ const internal::TcParseTableBase* Reflection::CreateTcParseTable() const { void* p = ::operator new(byte_size); auto* res = ::new (p) TcParseTableBase{ static_cast(schema_.HasHasbits() ? schema_.HasBitsOffset() : 0), - // extensions handled through reflection. - 0, 0, 0, + schema_.HasExtensionSet() + ? static_cast(schema_.GetExtensionSetOffset()) + : uint16_t{0}, static_cast(fields.empty() ? 0 : fields.back()->number()), - static_cast((fast_entries_count - 1) << 3), lookup_table_offset, - table_info.num_to_entry_table.skipmap32, field_entry_offset, + static_cast((fast_entries_count - 1) << 3), + lookup_table_offset, + table_info.num_to_entry_table.skipmap32, + field_entry_offset, static_cast(fields.size()), - static_cast(table_info.aux_entries.size()), aux_offset, - schema_.default_instance_, &internal::TcParser::ReflectionFallback}; + static_cast(table_info.aux_entries.size()), + aux_offset, + schema_.default_instance_, + &internal::TcParser::ReflectionFallback}; // Now copy the rest of the payloads PopulateTcParseFastEntries(table_info, res->fast_entry(0)); diff --git a/src/google/protobuf/generated_message_tctable_decl.h b/src/google/protobuf/generated_message_tctable_decl.h index 4ec6973d23..33b094efff 100644 --- a/src/google/protobuf/generated_message_tctable_decl.h +++ b/src/google/protobuf/generated_message_tctable_decl.h @@ -170,8 +170,6 @@ struct alignas(uint64_t) TcParseTableBase { // Common attributes for message layout: uint16_t has_bits_offset; uint16_t extension_offset; - uint32_t extension_range_low; - uint32_t extension_range_high; uint32_t max_field_number; uint8_t fast_idx_mask; uint16_t lookup_table_offset; @@ -194,7 +192,6 @@ struct alignas(uint64_t) TcParseTableBase { // compiled. constexpr TcParseTableBase( uint16_t has_bits_offset, uint16_t extension_offset, - uint32_t extension_range_low, uint32_t extension_range_high, uint32_t max_field_number, uint8_t fast_idx_mask, uint16_t lookup_table_offset, uint32_t skipmap32, uint32_t field_entries_offset, uint16_t num_field_entries, @@ -202,8 +199,6 @@ struct alignas(uint64_t) TcParseTableBase { const MessageLite* default_instance, TailCallParseFunc fallback) : has_bits_offset(has_bits_offset), extension_offset(extension_offset), - extension_range_low(extension_range_low), - extension_range_high(extension_range_high), max_field_number(max_field_number), fast_idx_mask(fast_idx_mask), lookup_table_offset(lookup_table_offset), diff --git a/src/google/protobuf/generated_message_tctable_gen.cc b/src/google/protobuf/generated_message_tctable_gen.cc index b829bb761e..a5791f9217 100644 --- a/src/google/protobuf/generated_message_tctable_gen.cc +++ b/src/google/protobuf/generated_message_tctable_gen.cc @@ -824,8 +824,7 @@ TailCallTableInfo::TailCallTableInfo( // If there are no fallback fields, and at most one extension range, the // parser can use a generic fallback function. Otherwise, a message-specific // fallback routine is needed. - use_generated_fallback = - !fallback_fields.empty() || descriptor->extension_range_count() > 1; + use_generated_fallback = !fallback_fields.empty(); } } // namespace internal diff --git a/src/google/protobuf/generated_message_tctable_impl.h b/src/google/protobuf/generated_message_tctable_impl.h index 84ee0e1284..1adea2d9f9 100644 --- a/src/google/protobuf/generated_message_tctable_impl.h +++ b/src/google/protobuf/generated_message_tctable_impl.h @@ -602,18 +602,21 @@ class PROTOBUF_EXPORT TcParser final { return ptr; } - uint32_t num = tag >> 3; - if (table->extension_range_low <= num && - num <= table->extension_range_high) { + if (table->extension_offset != 0) { + // We don't need to check the extension ranges. If it is not an extension + // it will be handled just like if it was an unknown extension: sent to + // the unknown field set. return RefAt(msg, table->extension_offset) .ParseField(tag, ptr, static_cast(table->default_instance), &msg->_internal_metadata_, ctx); + } else { + // Otherwise, we directly put it on the unknown field set. + return UnknownFieldParse( + tag, + msg->_internal_metadata_.mutable_unknown_fields(), + ptr, ctx); } - - return UnknownFieldParse( - tag, msg->_internal_metadata_.mutable_unknown_fields(), - ptr, ctx); } // Note: `inline` is needed on template function declarations below to avoid diff --git a/src/google/protobuf/generated_message_tctable_lite_test.cc b/src/google/protobuf/generated_message_tctable_lite_test.cc index 8aeacf57a2..12aeced544 100644 --- a/src/google/protobuf/generated_message_tctable_lite_test.cc +++ b/src/google/protobuf/generated_message_tctable_lite_test.cc @@ -94,7 +94,7 @@ TEST(FastVarints, NameHere) { const TcParseTable<0, 1, 0, 0, 2> parse_table = { { kHasBitsOffset, // - 0, 0, 0, // no _extensions_ + 0, // no _extensions_ 1, 0, // max_field_number, fast_idx_mask offsetof(decltype(parse_table), field_lookup_table), 0xFFFFFFFF - 1, // skipmap @@ -285,9 +285,9 @@ TEST(IsEntryForFieldNumTest, Matcher) { TcParseTable<0, 3, 0, 0, 2> table = { // header: { - 0, 0, 0, 0, // has_bits_offset, extensions - 0, // max_field_number - 0, // fast_idx_mask, + 0, 0, // has_bits_offset, extensions + 0, // max_field_number + 0, // fast_idx_mask, offsetof(decltype(table), field_lookup_table), 0xFFFFFFFF - 7, // 7 = fields 1, 2, and 3. offsetof(decltype(table), field_names), @@ -352,9 +352,9 @@ TEST_F(FindFieldEntryTest, SequentialFieldRange) { TcParseTable<0, 5, 0, 0, 8> table = { // header: { - 0, 0, 0, 0, // has_bits_offset, extensions - 111, // max_field_number - 0, // fast_idx_mask, + 0, 0, // has_bits_offset, extensions + 111, // max_field_number + 0, // fast_idx_mask, offsetof(decltype(table), field_lookup_table), 0xFFFFFFFF - (1 << 1) - (1 << 2) // fields 2, 3 - (1 << 3) - (1 << 4), // fields 4, 5 @@ -393,9 +393,9 @@ TEST_F(FindFieldEntryTest, SmallScanRange) { TcParseTable<0, 6, 0, 0, 8> table = { // header: { - 0, 0, 0, 0, // has_bits_offset, extensions - 111, // max_field_number - 0, // fast_idx_mask, + 0, 0, // has_bits_offset, extensions + 111, // max_field_number + 0, // fast_idx_mask, offsetof(decltype(table), field_lookup_table), 0xFFFFFFFF - (1<<0) - (1<<2) - (1<<3) - (1<<4) - (1<<6), // 1,3-5,7 offsetof(decltype(table), field_entries), @@ -439,9 +439,9 @@ TEST_F(FindFieldEntryTest, BinarySearchRange) { TcParseTable<0, 10, 0, 0, 8> table = { // header: { - 0, 0, 0, 0, // has_bits_offset, extensions - 70, // max_field_number - 0, // fast_idx_mask, + 0, 0, // has_bits_offset, extensions + 70, // max_field_number + 0, // fast_idx_mask, offsetof(decltype(table), field_lookup_table), 0xFFFFFFFF - (1<<0) - (1<<2) - (1<<3) - (1<<4) // 1, 3, 4, 5, 6 - (1<<5) - (1<<7) - (1<<8) - (1<<10) // 8, 9, 11, 12 @@ -485,9 +485,9 @@ TEST_F(FindFieldEntryTest, OutOfRange) { TcParseTable<0, 3, 0, 15, 2> table = { // header: { - 0, 0, 0, 0, // has_bits_offset, extensions - 3, // max_field_number - 0, // fast_idx_mask, + 0, 0, // has_bits_offset, extensions + 3, // max_field_number + 0, // fast_idx_mask, offsetof(decltype(table), field_lookup_table), 0xFFFFFFFF - (1<<0) - (1<<1) - (1<<2), // fields 1, 2, 3 offsetof(decltype(table), field_entries), @@ -535,9 +535,9 @@ TEST_F(FindFieldEntryTest, EmptyMessage) { TableType table = { // header: { - 0, 0, 0, 0, // has_bits_offset, extensions - 0, // max_field_number - 0, // fast_idx_mask, + 0, 0, // has_bits_offset, extensions + 0, // max_field_number + 0, // fast_idx_mask, offsetof(decltype(table), field_lookup_table), 0xFFFFFFFF, // no fields offsetof(decltype(table), field_names), // no field_entries @@ -586,7 +586,7 @@ int32_t test_all_types_table_field_numbers[] = { const TcParseTable<5, 134, 5, 2176, 55> test_all_types_table = { // header: { - 0, 0, 0, 0, // has_bits_offset, extensions + 0, 0, // has_bits_offset, extensions 418, 248, // max_field_number, fast_idx_mask offsetof(decltype(test_all_types_table), field_lookup_table), 977895424, // skipmap for fields 1-15,18-19,21-22,24-25,27,31-32