From 152f97d8c17be9e41171a9cba9b8aaf048585e7b Mon Sep 17 00:00:00 2001 From: theodorerose Date: Thu, 30 Jun 2022 22:14:45 +0000 Subject: [PATCH] merged from upstream --- python/google/protobuf/descriptor_pool.py | 8 +- src/google/protobuf/arena.cc | 140 ++++--- src/google/protobuf/arena_unittest.cc | 98 +---- src/google/protobuf/arenaz_sampler.cc | 21 +- src/google/protobuf/arenaz_sampler.h | 14 +- src/google/protobuf/arenaz_sampler_test.cc | 34 +- src/google/protobuf/compiler/cpp/map_field.cc | 6 - .../compiler/cpp/parse_function_generator.cc | 47 ++- .../compiler/cpp/parse_function_generator.h | 2 + .../protobuf/compiler/java/doc_comment.cc | 173 ++++++--- .../protobuf/compiler/java/doc_comment.h | 18 +- .../protobuf/compiler/java/enum_field.cc | 21 +- .../protobuf/compiler/java/enum_field_lite.cc | 21 +- .../protobuf/compiler/java/map_field.cc | 12 +- .../protobuf/compiler/java/map_field_lite.cc | 12 +- src/google/protobuf/compiler/java/message.cc | 1 + .../protobuf/compiler/java/message_field.cc | 21 +- .../compiler/java/message_field_lite.cc | 21 +- .../protobuf/compiler/java/message_lite.cc | 1 + .../protobuf/compiler/java/primitive_field.cc | 21 +- .../compiler/java/primitive_field_lite.cc | 17 +- .../protobuf/compiler/java/string_field.cc | 22 +- .../compiler/java/string_field_lite.cc | 22 +- .../protobuf/generated_message_tctable_impl.h | 16 + .../generated_message_tctable_lite.cc | 150 ++++++-- src/google/protobuf/message.cc | 9 + src/google/protobuf/message.h | 1 + src/google/protobuf/message_unittest.inc | 117 ++++++ src/google/protobuf/metadata_lite.h | 16 + src/google/protobuf/parse_context.h | 1 - src/google/protobuf/port_def.inc | 17 +- src/google/protobuf/port_undef.inc | 4 + .../protobuf/repeated_field_unittest.cc | 4 +- src/google/protobuf/repeated_ptr_field.h | 8 +- src/google/protobuf/unittest.proto | 105 ++++++ src/google/protobuf/util/field_comparator.h | 9 +- src/google/protobuf/util/json_util_test.cc | 344 +++++++++++------- .../protobuf/util/message_differencer.cc | 4 +- .../protobuf/util/message_differencer.h | 4 +- 39 files changed, 1033 insertions(+), 529 deletions(-) diff --git a/python/google/protobuf/descriptor_pool.py b/python/google/protobuf/descriptor_pool.py index 7f84a63fea..dccac5e714 100644 --- a/python/google/protobuf/descriptor_pool.py +++ b/python/google/protobuf/descriptor_pool.py @@ -217,7 +217,7 @@ class DescriptorPool(object): file_desc.serialized_pb = serialized_file_desc_proto return file_desc - # Add Descriptor to descriptor pool is dreprecated. Please use Add() + # Add Descriptor to descriptor pool is deprecated. Please use Add() # or AddSerializedFile() to add a FileDescriptorProto instead. @_Deprecated def AddDescriptor(self, desc): @@ -242,7 +242,7 @@ class DescriptorPool(object): self._descriptors[desc.full_name] = desc self._AddFileDescriptor(desc.file) - # Add EnumDescriptor to descriptor pool is dreprecated. Please use Add() + # Add EnumDescriptor to descriptor pool is deprecated. Please use Add() # or AddSerializedFile() to add a FileDescriptorProto instead. @_Deprecated def AddEnumDescriptor(self, enum_desc): @@ -283,7 +283,7 @@ class DescriptorPool(object): self._top_enum_values[full_name] = enum_value self._AddFileDescriptor(enum_desc.file) - # Add ServiceDescriptor to descriptor pool is dreprecated. Please use Add() + # Add ServiceDescriptor to descriptor pool is deprecated. Please use Add() # or AddSerializedFile() to add a FileDescriptorProto instead. @_Deprecated def AddServiceDescriptor(self, service_desc): @@ -304,7 +304,7 @@ class DescriptorPool(object): service_desc.file.name) self._service_descriptors[service_desc.full_name] = service_desc - # Add ExtensionDescriptor to descriptor pool is dreprecated. Please use Add() + # Add ExtensionDescriptor to descriptor pool is deprecated. Please use Add() # or AddSerializedFile() to add a FileDescriptorProto instead. @_Deprecated def AddExtensionDescriptor(self, extension): diff --git a/src/google/protobuf/arena.cc b/src/google/protobuf/arena.cc index 300714068a..99be4a3c5e 100644 --- a/src/google/protobuf/arena.cc +++ b/src/google/protobuf/arena.cc @@ -105,11 +105,11 @@ class GetDeallocator { }; SerialArena::SerialArena(Block* b, void* owner, ThreadSafeArenaStats* stats) - : space_allocated_(b->size) { + : space_allocated_(b->size()) { owner_ = owner; - head_ = b; - ptr_ = b->Pointer(kBlockHeaderSize + ThreadSafeArena::kSerialArenaSize); - limit_ = b->Pointer(b->size & static_cast(-8)); + set_head(b); + set_ptr(b->Pointer(kBlockHeaderSize + ThreadSafeArena::kSerialArenaSize)); + limit_ = b->Pointer(b->size() & static_cast(-8)); arena_stats_ = stats; } @@ -124,12 +124,12 @@ SerialArena* SerialArena::New(Memory mem, void* owner, template SerialArena::Memory SerialArena::Free(Deallocator deallocator) { - Block* b = head_; - Memory mem = {b, b->size}; + Block* b = head(); + Memory mem = {b, b->size()}; while (b->next) { b = b->next; // We must first advance before deleting this block deallocator(mem); - mem = {b, b->size}; + mem = {b, b->size()}; } return mem; } @@ -149,31 +149,49 @@ void* SerialArena::AllocateAlignedFallback(size_t n, return AllocateFromExisting(n); } +PROTOBUF_NOINLINE +void* SerialArena::AllocateAlignedWithCleanupFallback( + size_t n, size_t align, void (*destructor)(void*), + const AllocationPolicy* policy) { + size_t required = AlignUpTo(n, align) + cleanup::Size(destructor); + AllocateNewBlock(required, policy); + return AllocateFromExistingWithCleanupFallback(n, align, destructor); +} + +PROTOBUF_NOINLINE +void SerialArena::AddCleanupFallback(void* elem, void (*destructor)(void*), + const AllocationPolicy* policy) { + AllocateNewBlock(0, policy); + AddCleanupFromExisting(elem, destructor); +} + void SerialArena::AllocateNewBlock(size_t n, const AllocationPolicy* policy) { // Sync limit to block - head_->start = reinterpret_cast(limit_); + head()->cleanup_nodes = limit_; // Record how much used in this block. - size_t used = ptr_ - head_->Pointer(kBlockHeaderSize); - size_t wasted = head_->size - used; - space_used_ += used; + size_t used = static_cast(ptr() - head()->Pointer(kBlockHeaderSize)); + size_t wasted = head()->size() - used; + space_used_.store(space_used_.load(std::memory_order_relaxed) + used, + std::memory_order_relaxed); // TODO(sbenza): Evaluate if pushing unused space into the cached blocks is a // win. In preliminary testing showed increased memory savings as expected, // but with a CPU regression. The regression might have been an artifact of // the microbenchmark. - auto mem = AllocateMemory(policy, head_->size, n); + auto mem = AllocateMemory(policy, head()->size(), n); // We don't want to emit an expensive RMW instruction that requires // exclusive access to a cacheline. Hence we write it in terms of a // regular add. - auto relaxed = std::memory_order_relaxed; - space_allocated_.store(space_allocated_.load(relaxed) + mem.size, relaxed); + space_allocated_.store( + space_allocated_.load(std::memory_order_relaxed) + mem.size, + std::memory_order_relaxed); ThreadSafeArenaStats::RecordAllocateStats(arena_stats_, /*used=*/used, /*allocated=*/mem.size, wasted); - head_ = new (mem.ptr) Block{head_, mem.size}; - ptr_ = head_->Pointer(kBlockHeaderSize); - limit_ = head_->Pointer(head_->size); + set_head(new (mem.ptr) Block{head(), mem.size}); + ptr_.store(head()->Pointer(kBlockHeaderSize), std::memory_order_relaxed); + limit_ = head()->Pointer(head()->size()); #ifdef ADDRESS_SANITIZER ASAN_POISON_MEMORY_REGION(ptr_, limit_ - ptr_); @@ -181,24 +199,63 @@ void SerialArena::AllocateNewBlock(size_t n, const AllocationPolicy* policy) { } uint64_t SerialArena::SpaceUsed() const { - uint64_t space_used = ptr_ - head_->Pointer(kBlockHeaderSize); - space_used += space_used_; + // Note: the calculation below technically causes a race with + // AllocateNewBlock when called from another thread (which happens in + // ThreadSafeArena::SpaceUsed). However, worst-case space_used_ will have + // stale data and the calculation will incorrectly assume 100% + // usage of the *current* block. + // TODO(mkruskal) Consider eliminating this race in exchange for a possible + // performance hit on ARM (see cl/455186837). + const uint64_t current_block_size = head()->size(); + uint64_t space_used = std::min( + static_cast( + ptr() - const_cast(head())->Pointer(kBlockHeaderSize)), + current_block_size); + space_used += space_used_.load(std::memory_order_relaxed); // Remove the overhead of the SerialArena itself. space_used -= ThreadSafeArena::kSerialArenaSize; return space_used; } void SerialArena::CleanupList() { - Block* b = head_; - b->start = reinterpret_cast(limit_); + Block* b = head(); + b->cleanup_nodes = limit_; do { - auto* limit = reinterpret_cast( - b->Pointer(b->size & static_cast(-8))); - auto it = b->start; - auto num = limit - it; - if (num > 0) { - for (; it < limit; it++) { - it->cleanup(it->elem); + char* limit = reinterpret_cast( + b->Pointer(b->size() & static_cast(-8))); + char* it = reinterpret_cast(b->cleanup_nodes); + if (it < limit) { + // A prefetch distance of 8 here was chosen arbitrarily. It makes the + // pending nodes fill a cacheline which seemed nice. + constexpr int kPrefetchDist = 8; + cleanup::Tag pending_type[kPrefetchDist]; + char* pending_node[kPrefetchDist]; + + int pos = 0; + for (; pos < kPrefetchDist && it < limit; ++pos) { + pending_type[pos] = cleanup::Type(it); + pending_node[pos] = it; + it += cleanup::Size(pending_type[pos]); + } + + if (pos < kPrefetchDist) { + for (int i = 0; i < pos; ++i) { + cleanup::DestroyNode(pending_type[i], pending_node[i]); + } + } else { + pos = 0; + while (it < limit) { + cleanup::PrefetchNode(it); + cleanup::DestroyNode(pending_type[pos], pending_node[pos]); + pending_type[pos] = cleanup::Type(it); + pending_node[pos] = it; + it += cleanup::Size(pending_type[pos]); + pos = (pos + 1) % kPrefetchDist; + } + for (int i = pos; i < pos + kPrefetchDist; ++i) { + cleanup::DestroyNode(pending_type[i % kPrefetchDist], + pending_node[i % kPrefetchDist]); + } } } b = b->next; @@ -298,12 +355,9 @@ void ThreadSafeArena::InitializeWithPolicy(void* mem, size_t size, #undef GOOGLE_DCHECK_POLICY_FLAGS_ } -void ThreadSafeArena::Init() { -#ifndef NDEBUG - const bool was_message_owned = IsMessageOwned(); -#endif // NDEBUG +uint64_t ThreadSafeArena::GetNextLifeCycleId() { ThreadCache& tc = thread_cache(); - auto id = tc.next_lifecycle_id; + uint64_t id = tc.next_lifecycle_id; // We increment lifecycle_id's by multiples of two so we can use bit 0 as // a tag. constexpr uint64_t kDelta = 2; @@ -316,14 +370,19 @@ void ThreadSafeArena::Init() { id = lifecycle_id_generator_.id.fetch_add(1, relaxed) * kInc; } tc.next_lifecycle_id = id + kDelta; - // Message ownership is stored in tag_and_id_, and is set in the constructor. - // This flag bit must be preserved, even across calls to Reset(). - tag_and_id_ = id | (tag_and_id_ & kMessageOwnedArena); - hint_.store(nullptr, std::memory_order_relaxed); + return id; +} + +void ThreadSafeArena::Init() { + const bool message_owned = IsMessageOwned(); + if (!message_owned) { + // Message-owned arenas bypass thread cache and do not need life cycle ID. + tag_and_id_ = GetNextLifeCycleId(); + } else { + GOOGLE_DCHECK_EQ(tag_and_id_, kMessageOwnedArena); + } threads_.store(nullptr, std::memory_order_relaxed); -#ifndef NDEBUG - GOOGLE_CHECK_EQ(was_message_owned, IsMessageOwned()); -#endif // NDEBUG + GOOGLE_DCHECK_EQ(message_owned, IsMessageOwned()); arena_stats_ = Sample(); } @@ -378,7 +437,6 @@ uint64_t ThreadSafeArena::Reset() { // Discard all blocks except the special block (if present). size_t space_allocated = 0; auto mem = Free(&space_allocated); - arena_stats_.RecordReset(); AllocationPolicy* policy = alloc_policy_.get(); if (policy) { diff --git a/src/google/protobuf/arena_unittest.cc b/src/google/protobuf/arena_unittest.cc index 017b5f8097..6c88505d8d 100644 --- a/src/google/protobuf/arena_unittest.cc +++ b/src/google/protobuf/arena_unittest.cc @@ -791,7 +791,7 @@ TEST(ArenaTest, AddAllocatedWithReflection) { } TEST(ArenaTest, RepeatedPtrFieldAddClearedTest) { -#ifndef PROTOBUF_FUTURE_BREAKING_CHANGES +#ifndef PROTOBUF_FUTURE_REMOVE_CLEARED_API { RepeatedPtrField repeated_field; EXPECT_TRUE(repeated_field.empty()); @@ -802,7 +802,7 @@ TEST(ArenaTest, RepeatedPtrFieldAddClearedTest) { EXPECT_TRUE(repeated_field.empty()); EXPECT_EQ(0, repeated_field.size()); } -#endif // !PROTOBUF_FUTURE_BREAKING_CHANGES +#endif // !PROTOBUF_FUTURE_REMOVE_CLEARED_API { RepeatedPtrField repeated_field; EXPECT_TRUE(repeated_field.empty()); @@ -1530,14 +1530,19 @@ TEST(ArenaTest, SpaceReuseForArraysSizeChecks) { TEST(ArenaTest, SpaceReusePoisonsAndUnpoisonsMemory) { #ifdef ADDRESS_SANITIZER char buf[1024]{}; + constexpr int kSize = 32; { Arena arena(buf, sizeof(buf)); std::vector pointers; for (int i = 0; i < 100; ++i) { - pointers.push_back(Arena::CreateArray(&arena, 16)); + void* p = Arena::CreateArray(&arena, kSize); + // Simulate other ASan client managing shadow memory. + ASAN_POISON_MEMORY_REGION(p, kSize); + ASAN_UNPOISON_MEMORY_REGION(p, kSize - 4); + pointers.push_back(p); } for (void* p : pointers) { - internal::ArenaTestPeer::ReturnArrayMemory(&arena, p, 16); + internal::ArenaTestPeer::ReturnArrayMemory(&arena, p, kSize); // The first one is not poisoned because it becomes the freelist. if (p != pointers[0]) EXPECT_TRUE(__asan_address_is_poisoned(p)); } @@ -1562,91 +1567,6 @@ TEST(ArenaTest, SpaceReusePoisonsAndUnpoisonsMemory) { #endif // ADDRESS_SANITIZER } -namespace { -uint32_t hooks_num_init = 0; -uint32_t hooks_num_allocations = 0; -uint32_t hooks_num_reset = 0; -uint32_t hooks_num_destruct = 0; - -void ClearHookCounts() { - hooks_num_init = 0; - hooks_num_allocations = 0; - hooks_num_reset = 0; - hooks_num_destruct = 0; -} -} // namespace - -// A helper utility class that handles arena callbacks. -class ArenaOptionsTestFriend final : public internal::ArenaMetricsCollector { - public: - static internal::ArenaMetricsCollector* NewWithAllocs() { - return new ArenaOptionsTestFriend(true); - } - - static internal::ArenaMetricsCollector* NewWithoutAllocs() { - return new ArenaOptionsTestFriend(false); - } - - static void Enable(ArenaOptions* options) { - ClearHookCounts(); - options->make_metrics_collector = &ArenaOptionsTestFriend::NewWithAllocs; - } - - static void EnableWithoutAllocs(ArenaOptions* options) { - ClearHookCounts(); - options->make_metrics_collector = &ArenaOptionsTestFriend::NewWithoutAllocs; - } - - explicit ArenaOptionsTestFriend(bool record_allocs) - : ArenaMetricsCollector(record_allocs) { - ++hooks_num_init; - } - void OnDestroy(uint64_t space_allocated) override { - ++hooks_num_destruct; - delete this; - } - void OnReset(uint64_t space_allocated) override { ++hooks_num_reset; } - void OnAlloc(const std::type_info* allocated_type, - uint64_t alloc_size) override { - ++hooks_num_allocations; - } -}; - -// Test the hooks are correctly called. -TEST(ArenaTest, ArenaHooksSanity) { - ArenaOptions options; - ArenaOptionsTestFriend::Enable(&options); - - // Scope for defining the arena - { - Arena arena(options); - EXPECT_EQ(1, hooks_num_init); - EXPECT_EQ(0, hooks_num_allocations); - Arena::Create(&arena); - if (std::is_trivially_destructible::value) { - EXPECT_EQ(1, hooks_num_allocations); - } else { - EXPECT_EQ(2, hooks_num_allocations); - } - arena.Reset(); - arena.Reset(); - EXPECT_EQ(2, hooks_num_reset); - } - EXPECT_EQ(2, hooks_num_reset); - EXPECT_EQ(1, hooks_num_destruct); -} - -// Test that allocation hooks are not called when we don't need them. -TEST(ArenaTest, ArenaHooksWhenAllocationsNotNeeded) { - ArenaOptions options; - ArenaOptionsTestFriend::EnableWithoutAllocs(&options); - - Arena arena(options); - EXPECT_EQ(0, hooks_num_allocations); - Arena::Create(&arena); - EXPECT_EQ(0, hooks_num_allocations); -} - } // namespace protobuf } // namespace google diff --git a/src/google/protobuf/arenaz_sampler.cc b/src/google/protobuf/arenaz_sampler.cc index 965a2de69e..6ad3a36504 100644 --- a/src/google/protobuf/arenaz_sampler.cc +++ b/src/google/protobuf/arenaz_sampler.cc @@ -69,7 +69,6 @@ ThreadSafeArenaStats::~ThreadSafeArenaStats() = default; void ThreadSafeArenaStats::PrepareForSampling(int64_t stride) { num_allocations.store(0, std::memory_order_relaxed); - num_resets.store(0, std::memory_order_relaxed); bytes_used.store(0, std::memory_order_relaxed); bytes_allocated.store(0, std::memory_order_relaxed); bytes_wasted.store(0, std::memory_order_relaxed); @@ -82,21 +81,6 @@ void ThreadSafeArenaStats::PrepareForSampling(int64_t stride) { depth = absl::GetStackTrace(stack, kMaxStackDepth, /* skip_count= */ 0); } -void RecordResetSlow(ThreadSafeArenaStats* info) { - const size_t max_bytes = - info->max_bytes_allocated.load(std::memory_order_relaxed); - const size_t allocated_bytes = - info->bytes_allocated.load(std::memory_order_relaxed); - if (max_bytes < allocated_bytes) { - info->max_bytes_allocated.store(allocated_bytes); - } - info->bytes_used.store(0, std::memory_order_relaxed); - info->bytes_allocated.store(0, std::memory_order_relaxed); - info->bytes_wasted.store(0, std::memory_order_relaxed); - info->num_allocations.store(0, std::memory_order_relaxed); - info->num_resets.fetch_add(1, std::memory_order_relaxed); -} - void RecordAllocateSlow(ThreadSafeArenaStats* info, size_t used, size_t allocated, size_t wasted) { info->num_allocations.fetch_add(1, std::memory_order_relaxed); @@ -144,6 +128,10 @@ void SetThreadSafeArenazSampleParameter(int32_t rate) { } } +int32_t ThreadSafeArenazSampleParameter() { + return g_arenaz_sample_parameter.load(std::memory_order_relaxed); +} + void SetThreadSafeArenazMaxSamples(int32_t max) { if (max > 0) { GlobalThreadSafeArenazSampler().SetMaxSamples(max); @@ -171,6 +159,7 @@ ThreadSafeArenaStats* SampleSlow(int64_t* next_sample) { void SetThreadSafeArenazEnabled(bool enabled) {} void SetThreadSafeArenazSampleParameter(int32_t rate) {} +int32_t ThreadSafeArenazSampleParameter() { return 0; } void SetThreadSafeArenazMaxSamples(int32_t max) {} void SetThreadSafeArenazGlobalNextSample(int64_t next_sample) {} #endif // defined(PROTOBUF_ARENAZ_SAMPLE) diff --git a/src/google/protobuf/arenaz_sampler.h b/src/google/protobuf/arenaz_sampler.h index 31097ec169..4a04c6a953 100644 --- a/src/google/protobuf/arenaz_sampler.h +++ b/src/google/protobuf/arenaz_sampler.h @@ -45,7 +45,6 @@ namespace internal { #if defined(PROTOBUF_ARENAZ_SAMPLE) struct ThreadSafeArenaStats; -void RecordResetSlow(ThreadSafeArenaStats* info); void RecordAllocateSlow(ThreadSafeArenaStats* info, size_t used, size_t allocated, size_t wasted); // Stores information about a sampled thread safe arena. All mutations to this @@ -67,16 +66,15 @@ struct ThreadSafeArenaStats // These fields are mutated by the various Record* APIs and need to be // thread-safe. std::atomic num_allocations; - std::atomic num_resets; std::atomic bytes_used; std::atomic bytes_allocated; std::atomic bytes_wasted; - // Records the largest size an arena ever had. Maintained across resets. + // Records the largest size an arena ever had. std::atomic max_bytes_allocated; // Bit `i` is set to 1 indicates that a thread with `tid % 63 = i` accessed // the underlying arena. We use `% 63` as a rudimentary hash to ensure some // bit mixing for thread-ids; `% 64` would only grab the low bits and might - // create sampling artifacts. Maintained across resets. + // create sampling artifacts. std::atomic thread_ids; // All of the fields below are set by `PrepareForSampling`, they must not @@ -131,11 +129,6 @@ class ThreadSafeArenaStatsHandle { return *this; } - void RecordReset() { - if (PROTOBUF_PREDICT_TRUE(info_ == nullptr)) return; - RecordResetSlow(info_); - } - ThreadSafeArenaStats* MutableStats() { return info_; } friend void swap(ThreadSafeArenaStatsHandle& lhs, @@ -212,6 +205,9 @@ void SetThreadSafeArenazEnabled(bool enabled); // Sets the rate at which thread safe arena will be sampled. void SetThreadSafeArenazSampleParameter(int32_t rate); +// Returns the rate at which thread safe arena will be sampled. +int32_t ThreadSafeArenazSampleParameter(); + // Sets a soft max for the number of samples that will be kept. void SetThreadSafeArenazMaxSamples(int32_t max); diff --git a/src/google/protobuf/arenaz_sampler_test.cc b/src/google/protobuf/arenaz_sampler_test.cc index 6eae73c1dc..7bac100ee1 100644 --- a/src/google/protobuf/arenaz_sampler_test.cc +++ b/src/google/protobuf/arenaz_sampler_test.cc @@ -86,7 +86,6 @@ TEST(ThreadSafeArenaStatsTest, PrepareForSampling) { info.PrepareForSampling(kTestStride); EXPECT_EQ(info.num_allocations.load(), 0); - EXPECT_EQ(info.num_resets.load(), 0); EXPECT_EQ(info.bytes_used.load(), 0); EXPECT_EQ(info.bytes_allocated.load(), 0); EXPECT_EQ(info.bytes_wasted.load(), 0); @@ -94,7 +93,6 @@ TEST(ThreadSafeArenaStatsTest, PrepareForSampling) { EXPECT_EQ(info.weight, kTestStride); info.num_allocations.store(1, std::memory_order_relaxed); - info.num_resets.store(1, std::memory_order_relaxed); info.bytes_used.store(1, std::memory_order_relaxed); info.bytes_allocated.store(1, std::memory_order_relaxed); info.bytes_wasted.store(1, std::memory_order_relaxed); @@ -102,7 +100,6 @@ TEST(ThreadSafeArenaStatsTest, PrepareForSampling) { info.PrepareForSampling(2 * kTestStride); EXPECT_EQ(info.num_allocations.load(), 0); - EXPECT_EQ(info.num_resets.load(), 0); EXPECT_EQ(info.bytes_used.load(), 0); EXPECT_EQ(info.bytes_allocated.load(), 0); EXPECT_EQ(info.bytes_wasted.load(), 0); @@ -117,7 +114,6 @@ TEST(ThreadSafeArenaStatsTest, RecordAllocateSlow) { info.PrepareForSampling(kTestStride); RecordAllocateSlow(&info, /*requested=*/100, /*allocated=*/128, /*wasted=*/0); EXPECT_EQ(info.num_allocations.load(), 1); - EXPECT_EQ(info.num_resets.load(), 0); EXPECT_EQ(info.bytes_used.load(), 100); EXPECT_EQ(info.bytes_allocated.load(), 128); EXPECT_EQ(info.bytes_wasted.load(), 0); @@ -125,26 +121,28 @@ TEST(ThreadSafeArenaStatsTest, RecordAllocateSlow) { RecordAllocateSlow(&info, /*requested=*/100, /*allocated=*/256, /*wasted=*/28); EXPECT_EQ(info.num_allocations.load(), 2); - EXPECT_EQ(info.num_resets.load(), 0); EXPECT_EQ(info.bytes_used.load(), 200); EXPECT_EQ(info.bytes_allocated.load(), 384); EXPECT_EQ(info.bytes_wasted.load(), 28); EXPECT_EQ(info.max_bytes_allocated.load(), 0); } -TEST(ThreadSafeArenaStatsTest, RecordResetSlow) { - ThreadSafeArenaStats info; - constexpr int64_t kTestStride = 584; - MutexLock l(&info.init_mu); - info.PrepareForSampling(kTestStride); - EXPECT_EQ(info.num_resets.load(), 0); - EXPECT_EQ(info.bytes_allocated.load(), 0); - RecordAllocateSlow(&info, /*requested=*/100, /*allocated=*/128, /*wasted=*/0); - EXPECT_EQ(info.num_resets.load(), 0); - EXPECT_EQ(info.bytes_allocated.load(), 128); - RecordResetSlow(&info); - EXPECT_EQ(info.num_resets.load(), 1); - EXPECT_EQ(info.bytes_allocated.load(), 0); +TEST(ThreadSafeArenazSamplerTest, SamplingCorrectness) { + SetThreadSafeArenazEnabled(true); + for (int p = 0; p <= 15; ++p) { + SetThreadSafeArenazSampleParameter(1 << p); + SetThreadSafeArenazGlobalNextSample(1 << p); + const int kTrials = 1000 << p; + std::vector hv; + for (int i = 0; i < kTrials; ++i) { + ThreadSafeArenaStatsHandle h = Sample(); + if (h.MutableStats() != nullptr) hv.push_back(std::move(h)); + } + // Ideally samples << p should be very close to kTrials. But we keep a + // factor of two guard band. + EXPECT_GE(hv.size() << p, kTrials / 2); + EXPECT_LE(hv.size() << p, 2 * kTrials); + } } TEST(ThreadSafeArenazSamplerTest, SmallSampleParameter) { diff --git a/src/google/protobuf/compiler/cpp/map_field.cc b/src/google/protobuf/compiler/cpp/map_field.cc index 3a55ef5352..1810c6496b 100644 --- a/src/google/protobuf/compiler/cpp/map_field.cc +++ b/src/google/protobuf/compiler/cpp/map_field.cc @@ -40,12 +40,6 @@ namespace google { namespace protobuf { namespace compiler { namespace cpp { - -bool IsProto3Field(const FieldDescriptor* field_descriptor) { - const FileDescriptor* file_descriptor = field_descriptor->file(); - return file_descriptor->syntax() == FileDescriptor::SYNTAX_PROTO3; -} - void SetMessageVariables(const FieldDescriptor* descriptor, std::map* variables, const Options& options) { diff --git a/src/google/protobuf/compiler/cpp/parse_function_generator.cc b/src/google/protobuf/compiler/cpp/parse_function_generator.cc index db4d209777..68c7d162b4 100644 --- a/src/google/protobuf/compiler/cpp/parse_function_generator.cc +++ b/src/google/protobuf/compiler/cpp/parse_function_generator.cc @@ -82,8 +82,9 @@ int TagSize(uint32_t field_number) { return 2; } -std::string FieldParseFunctionName( - const TailCallTableInfo::FieldEntryInfo& entry, const Options& options); +void PopulateFastFieldEntry(const TailCallTableInfo::FieldEntryInfo& entry, + const Options& options, + TailCallTableInfo::FastFieldInfo& info); bool IsFieldEligibleForFastParsing( const TailCallTableInfo::FieldEntryInfo& entry, const Options& options, @@ -196,18 +197,12 @@ std::vector SplitFastFieldsForSize( // Fill in this field's entry: GOOGLE_CHECK(info.func_name.empty()) << info.func_name; - info.func_name = FieldParseFunctionName(entry, options); info.field = field; info.coded_tag = tag; + PopulateFastFieldEntry(entry, options, info); // If this field does not have presence, then it can set an out-of-bounds // bit (tailcall parsing uses a uint64_t for hasbits, but only stores 32). info.hasbit_idx = HasHasbit(field) ? entry.hasbit_idx : 63; - if (IsStringInlined(field, options)) { - GOOGLE_CHECK(!field->is_repeated()); - info.aux_idx = static_cast(entry.inlined_string_idx); - } else { - info.aux_idx = static_cast(entry.aux_idx); - } } return result; } @@ -375,6 +370,8 @@ TailCallTableInfo::TailCallTableInfo( enum_values[0] <= std::numeric_limits::max() && enum_values.size() <= std::numeric_limits::max()) { entry.is_enum_range = true; + entry.enum_range_min = enum_values.front(); + entry.enum_range_max = enum_values.back(); aux_entries.push_back( StrCat(enum_values[0], ", ", enum_values.size())); } else { @@ -1658,10 +1655,12 @@ void ParseFunctionGenerator::GenerateFieldSwitch( namespace { -std::string FieldParseFunctionName( - const TailCallTableInfo::FieldEntryInfo& entry, const Options& options) { +void PopulateFastFieldEntry(const TailCallTableInfo::FieldEntryInfo& entry, + const Options& options, + TailCallTableInfo::FastFieldInfo& info) { const FieldDescriptor* field = entry.field; std::string name = "::_pbi::TcParser::Fast"; + uint8_t aux_idx = static_cast(entry.aux_idx); switch (field->type()) { case FieldDescriptor::TYPE_FIXED32: @@ -1695,9 +1694,22 @@ std::string FieldParseFunctionName( } if (field->is_repeated() && field->is_packed()) { GOOGLE_LOG(DFATAL) << "Enum validation not handled: " << field->DebugString(); - return ""; + return; + } + if (entry.is_enum_range) { + name.append("Er"); + if (entry.enum_range_max <= 127) { + if (entry.enum_range_min == 0) { + name.append("0"); + aux_idx = entry.enum_range_max; + } else if (entry.enum_range_min == 1) { + name.append("1"); + aux_idx = entry.enum_range_max; + } + } + } else { + name.append("Ev"); } - name.append(entry.is_enum_range ? "Er" : "Ev"); break; case FieldDescriptor::TYPE_SINT32: @@ -1727,10 +1739,12 @@ std::string FieldParseFunctionName( default: GOOGLE_LOG(DFATAL) << "Mode not handled: " << static_cast(GetUtf8CheckMode(field, options)); - return ""; + return; } if (IsStringInlined(field, options)) { name.append("i"); + GOOGLE_CHECK(!field->is_repeated()); + aux_idx = static_cast(entry.inlined_string_idx); } break; @@ -1743,7 +1757,7 @@ std::string FieldParseFunctionName( default: GOOGLE_LOG(DFATAL) << "Type not handled: " << field->DebugString(); - return ""; + return; } // The field implementation functions are prefixed by cardinality: @@ -1758,7 +1772,8 @@ std::string FieldParseFunctionName( // Append the tag length. Fast parsing only handles 1- or 2-byte tags. name.append(TagSize(field->number()) == 1 ? "1" : "2"); - return name; + info.func_name = std::move(name); + info.aux_idx = aux_idx; } } // namespace diff --git a/src/google/protobuf/compiler/cpp/parse_function_generator.h b/src/google/protobuf/compiler/cpp/parse_function_generator.h index 542a15a06a..973d9130a2 100644 --- a/src/google/protobuf/compiler/cpp/parse_function_generator.h +++ b/src/google/protobuf/compiler/cpp/parse_function_generator.h @@ -72,6 +72,8 @@ struct TailCallTableInfo { uint16_t aux_idx; // True for enums entirely covered by the start/length fields of FieldAux: bool is_enum_range; + int32_t enum_range_min; + int32_t enum_range_max; }; std::vector field_entries; std::vector aux_entries; diff --git a/src/google/protobuf/compiler/java/doc_comment.cc b/src/google/protobuf/compiler/java/doc_comment.cc index 066bff6432..d5fadb63cf 100644 --- a/src/google/protobuf/compiler/java/doc_comment.cc +++ b/src/google/protobuf/compiler/java/doc_comment.cc @@ -37,8 +37,10 @@ #include #include +#include #include #include +#include namespace google { namespace protobuf { @@ -103,26 +105,63 @@ std::string EscapeJavadoc(const std::string& input) { return result; } +static std::string EscapeKdoc(const std::string& input) { + std::string result; + result.reserve(input.size() * 2); + + char prev = 'a'; + + for (char c : input) { + switch (c) { + case '*': + // Avoid "/*". + if (prev == '/') { + result.append("*"); + } else { + result.push_back(c); + } + break; + case '/': + // Avoid "*/". + if (prev == '*') { + result.append("/"); + } else { + result.push_back(c); + } + break; + default: + result.push_back(c); + break; + } + + prev = c; + } + + return result; +} + static void WriteDocCommentBodyForLocation(io::Printer* printer, - const SourceLocation& location) { + const SourceLocation& location, + const bool kdoc) { std::string comments = location.leading_comments.empty() ? location.trailing_comments : location.leading_comments; if (!comments.empty()) { - // TODO(kenton): Ideally we should parse the comment text as Markdown and - // write it back as HTML, but this requires a Markdown parser. For now - // we just use
 to get fixed-width text formatting.
-
-    // If the comment itself contains block comment start or end markers,
-    // HTML-escape them so that they don't accidentally close the doc comment.
-    comments = EscapeJavadoc(comments);
+    if (!kdoc) {
+      comments = EscapeJavadoc(comments);
+    }
 
     std::vector lines = Split(comments, "\n");
     while (!lines.empty() && lines.back().empty()) {
       lines.pop_back();
     }
 
-    printer->Print(" * 
\n");
+    if (kdoc) {
+      printer->Print(" * ```\n");
+    } else {
+      printer->Print(" * 
\n");
+    }
+
     for (int i = 0; i < lines.size(); i++) {
       // Most lines should start with a space.  Watch out for lines that start
       // with a /, since putting that right after the leading asterisk will
@@ -133,18 +172,23 @@ static void WriteDocCommentBodyForLocation(io::Printer* printer,
         printer->Print(" *$line$\n", "line", lines[i]);
       }
     }
-    printer->Print(
-        " * 
\n" - " *\n"); + + if (kdoc) { + printer->Print(" * ```\n"); + } else { + printer->Print(" *
\n"); + } + printer->Print(" *\n"); } } template static void WriteDocCommentBody(io::Printer* printer, - const DescriptorType* descriptor) { + const DescriptorType* descriptor, + const bool kdoc) { SourceLocation location; if (descriptor->GetSourceLocation(&location)) { - WriteDocCommentBodyForLocation(printer, location); + WriteDocCommentBodyForLocation(printer, location, kdoc); } } @@ -164,16 +208,36 @@ static std::string FirstLineOf(const std::string& value) { return result; } -void WriteMessageDocComment(io::Printer* printer, const Descriptor* message) { +static void WriteDebugString(io::Printer* printer, const FieldDescriptor* field, + const bool kdoc) { + if (kdoc) { + printer->Print(" * `$def$`\n", "def", + EscapeKdoc(FirstLineOf(field->DebugString()))); + } else { + printer->Print(" * $def$\n", "def", + EscapeJavadoc(FirstLineOf(field->DebugString()))); + } +} + +void WriteMessageDocComment(io::Printer* printer, const Descriptor* message, + const bool kdoc) { printer->Print("/**\n"); - WriteDocCommentBody(printer, message); - printer->Print( - " * Protobuf type {@code $fullname$}\n" - " */\n", - "fullname", EscapeJavadoc(message->full_name())); + WriteDocCommentBody(printer, message, kdoc); + if (kdoc) { + printer->Print( + " * Protobuf type `$fullname$`\n" + " */\n", + "fullname", EscapeKdoc(message->full_name())); + } else { + printer->Print( + " * Protobuf type {@code $fullname$}\n" + " */\n", + "fullname", EscapeJavadoc(message->full_name())); + } } -void WriteFieldDocComment(io::Printer* printer, const FieldDescriptor* field) { +void WriteFieldDocComment(io::Printer* printer, const FieldDescriptor* field, + const bool kdoc) { // We start the comment with the main body based on the comments from the // .proto file (if present). We then continue with the field declaration, // e.g.: @@ -181,9 +245,14 @@ void WriteFieldDocComment(io::Printer* printer, const FieldDescriptor* field) { // And then we end with the javadoc tags if applicable. // If the field is a group, the debug string might end with {. printer->Print("/**\n"); - WriteDocCommentBody(printer, field); - printer->Print(" * $def$\n", "def", - EscapeJavadoc(FirstLineOf(field->DebugString()))); + WriteDocCommentBody(printer, field, kdoc); + if (kdoc) { + printer->Print(" * `$def$`\n", "def", + EscapeKdoc(FirstLineOf(field->DebugString()))); + } else { + printer->Print(" * $def$\n", "def", + EscapeJavadoc(FirstLineOf(field->DebugString()))); + } printer->Print(" */\n"); } @@ -214,12 +283,11 @@ void WriteDeprecatedJavadoc(io::Printer* printer, const FieldDescriptor* field, void WriteFieldAccessorDocComment(io::Printer* printer, const FieldDescriptor* field, const FieldAccessorType type, - const bool builder) { + const bool builder, const bool kdoc) { printer->Print("/**\n"); - WriteDocCommentBody(printer, field); - printer->Print(" * $def$\n", "def", - EscapeJavadoc(FirstLineOf(field->DebugString()))); - WriteDeprecatedJavadoc(printer, field, type); + WriteDocCommentBody(printer, field, kdoc); + WriteDebugString(printer, field, kdoc); + if (!kdoc) WriteDeprecatedJavadoc(printer, field, type); switch (type) { case HAZZER: printer->Print(" * @return Whether the $name$ field is set.\n", "name", @@ -273,12 +341,12 @@ void WriteFieldAccessorDocComment(io::Printer* printer, void WriteFieldEnumValueAccessorDocComment(io::Printer* printer, const FieldDescriptor* field, const FieldAccessorType type, - const bool builder) { + const bool builder, + const bool kdoc) { printer->Print("/**\n"); - WriteDocCommentBody(printer, field); - printer->Print(" * $def$\n", "def", - EscapeJavadoc(FirstLineOf(field->DebugString()))); - WriteDeprecatedJavadoc(printer, field, type); + WriteDocCommentBody(printer, field, kdoc); + WriteDebugString(printer, field, kdoc); + if (!kdoc) WriteDeprecatedJavadoc(printer, field, type); switch (type) { case HAZZER: // Should never happen @@ -343,12 +411,12 @@ void WriteFieldEnumValueAccessorDocComment(io::Printer* printer, void WriteFieldStringBytesAccessorDocComment(io::Printer* printer, const FieldDescriptor* field, const FieldAccessorType type, - const bool builder) { + const bool builder, + const bool kdoc) { printer->Print("/**\n"); - WriteDocCommentBody(printer, field); - printer->Print(" * $def$\n", "def", - EscapeJavadoc(FirstLineOf(field->DebugString()))); - WriteDeprecatedJavadoc(printer, field, type); + WriteDocCommentBody(printer, field, kdoc); + WriteDebugString(printer, field, kdoc); + if (!kdoc) WriteDeprecatedJavadoc(printer, field, type); switch (type) { case HAZZER: // Should never happen @@ -399,19 +467,28 @@ void WriteFieldStringBytesAccessorDocComment(io::Printer* printer, // Enum -void WriteEnumDocComment(io::Printer* printer, const EnumDescriptor* enum_) { +void WriteEnumDocComment(io::Printer* printer, const EnumDescriptor* enum_, + const bool kdoc) { printer->Print("/**\n"); - WriteDocCommentBody(printer, enum_); - printer->Print( - " * Protobuf enum {@code $fullname$}\n" - " */\n", - "fullname", EscapeJavadoc(enum_->full_name())); + WriteDocCommentBody(printer, enum_, kdoc); + if (kdoc) { + printer->Print( + " * Protobuf enum `$fullname$`\n" + " */\n", + "fullname", EscapeKdoc(enum_->full_name())); + } else { + printer->Print( + " * Protobuf enum {@code $fullname$}\n" + " */\n", + "fullname", EscapeJavadoc(enum_->full_name())); + } } void WriteEnumValueDocComment(io::Printer* printer, const EnumValueDescriptor* value) { printer->Print("/**\n"); - WriteDocCommentBody(printer, value); + WriteDocCommentBody(printer, value, /* kdoc */ false); + printer->Print( " * $def$\n" " */\n", @@ -421,7 +498,7 @@ void WriteEnumValueDocComment(io::Printer* printer, void WriteServiceDocComment(io::Printer* printer, const ServiceDescriptor* service) { printer->Print("/**\n"); - WriteDocCommentBody(printer, service); + WriteDocCommentBody(printer, service, /* kdoc */ false); printer->Print( " * Protobuf service {@code $fullname$}\n" " */\n", @@ -431,7 +508,7 @@ void WriteServiceDocComment(io::Printer* printer, void WriteMethodDocComment(io::Printer* printer, const MethodDescriptor* method) { printer->Print("/**\n"); - WriteDocCommentBody(printer, method); + WriteDocCommentBody(printer, method, /* kdoc */ false); printer->Print( " * $def$\n" " */\n", diff --git a/src/google/protobuf/compiler/java/doc_comment.h b/src/google/protobuf/compiler/java/doc_comment.h index 7f687781fb..03672bf52d 100644 --- a/src/google/protobuf/compiler/java/doc_comment.h +++ b/src/google/protobuf/compiler/java/doc_comment.h @@ -67,21 +67,27 @@ enum FieldAccessorType { LIST_MULTI_ADDER }; -void WriteMessageDocComment(io::Printer* printer, const Descriptor* message); -void WriteFieldDocComment(io::Printer* printer, const FieldDescriptor* field); +void WriteMessageDocComment(io::Printer* printer, const Descriptor* message, + const bool kdoc = false); +void WriteFieldDocComment(io::Printer* printer, const FieldDescriptor* field, + const bool kdoc = false); void WriteFieldAccessorDocComment(io::Printer* printer, const FieldDescriptor* field, const FieldAccessorType type, - const bool builder = false); + const bool builder = false, + const bool kdoc = false); void WriteFieldEnumValueAccessorDocComment(io::Printer* printer, const FieldDescriptor* field, const FieldAccessorType type, - const bool builder = false); + const bool builder = false, + const bool kdoc = false); void WriteFieldStringBytesAccessorDocComment(io::Printer* printer, const FieldDescriptor* field, const FieldAccessorType type, - const bool builder = false); -void WriteEnumDocComment(io::Printer* printer, const EnumDescriptor* enum_); + const bool builder = false, + const bool kdoc = false); +void WriteEnumDocComment(io::Printer* printer, const EnumDescriptor* enum_, + const bool kdoc = false); void WriteEnumValueDocComment(io::Printer* printer, const EnumValueDescriptor* value); void WriteServiceDocComment(io::Printer* printer, diff --git a/src/google/protobuf/compiler/java/enum_field.cc b/src/google/protobuf/compiler/java/enum_field.cc index 4a93addbe1..c1cb11e51e 100644 --- a/src/google/protobuf/compiler/java/enum_field.cc +++ b/src/google/protobuf/compiler/java/enum_field.cc @@ -271,7 +271,7 @@ void ImmutableEnumFieldGenerator::GenerateBuilderMembers( void ImmutableEnumFieldGenerator::GenerateKotlinDslMembers( io::Printer* printer) const { - WriteFieldDocComment(printer, descriptor_); + WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print(variables_, "$kt_deprecation$ var $kt_name$: $kt_type$\n" " @JvmName(\"${$get$kt_capitalized_name$$}$\")\n" @@ -282,14 +282,15 @@ void ImmutableEnumFieldGenerator::GenerateKotlinDslMembers( " }\n"); WriteFieldAccessorDocComment(printer, descriptor_, CLEARER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "fun ${$clear$kt_capitalized_name$$}$() {\n" " $kt_dsl_builder$.${$clear$capitalized_name$$}$()\n" "}\n"); if (HasHazzer(descriptor_)) { - WriteFieldAccessorDocComment(printer, descriptor_, HAZZER); + WriteFieldAccessorDocComment(printer, descriptor_, HAZZER, + /* builder */ false, /* kdoc */ true); printer->Print( variables_, "fun ${$has$kt_capitalized_name$$}$(): kotlin.Boolean {\n" @@ -1072,7 +1073,7 @@ void RepeatedImmutableEnumFieldGenerator::GenerateKotlinDslMembers( "class ${$$kt_capitalized_name$Proxy$}$ private constructor()" " : com.google.protobuf.kotlin.DslProxy()\n"); - WriteFieldDocComment(printer, descriptor_); + WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print(variables_, "$kt_deprecation$ val $kt_name$: " "com.google.protobuf.kotlin.DslList" @@ -1083,7 +1084,7 @@ void RepeatedImmutableEnumFieldGenerator::GenerateKotlinDslMembers( " )\n"); WriteFieldAccessorDocComment(printer, descriptor_, LIST_ADDER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"add$kt_capitalized_name$\")\n" @@ -1094,7 +1095,7 @@ void RepeatedImmutableEnumFieldGenerator::GenerateKotlinDslMembers( "}"); WriteFieldAccessorDocComment(printer, descriptor_, LIST_ADDER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"plusAssign$kt_capitalized_name$\")\n" @@ -1106,7 +1107,7 @@ void RepeatedImmutableEnumFieldGenerator::GenerateKotlinDslMembers( "}"); WriteFieldAccessorDocComment(printer, descriptor_, LIST_MULTI_ADDER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"addAll$kt_capitalized_name$\")\n" @@ -1117,7 +1118,7 @@ void RepeatedImmutableEnumFieldGenerator::GenerateKotlinDslMembers( "}"); WriteFieldAccessorDocComment(printer, descriptor_, LIST_MULTI_ADDER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print( variables_, "@kotlin.jvm.JvmSynthetic\n" @@ -1130,7 +1131,7 @@ void RepeatedImmutableEnumFieldGenerator::GenerateKotlinDslMembers( "}"); WriteFieldAccessorDocComment(printer, descriptor_, LIST_INDEXED_SETTER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print( variables_, "@kotlin.jvm.JvmSynthetic\n" @@ -1142,7 +1143,7 @@ void RepeatedImmutableEnumFieldGenerator::GenerateKotlinDslMembers( "}"); WriteFieldAccessorDocComment(printer, descriptor_, CLEARER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"clear$kt_capitalized_name$\")\n" diff --git a/src/google/protobuf/compiler/java/enum_field_lite.cc b/src/google/protobuf/compiler/java/enum_field_lite.cc index 6fe683fe0b..bd69f844dd 100644 --- a/src/google/protobuf/compiler/java/enum_field_lite.cc +++ b/src/google/protobuf/compiler/java/enum_field_lite.cc @@ -286,7 +286,7 @@ void ImmutableEnumFieldLiteGenerator::GenerateBuilderMembers( void ImmutableEnumFieldLiteGenerator::GenerateKotlinDslMembers( io::Printer* printer) const { - WriteFieldDocComment(printer, descriptor_); + WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print(variables_, "$kt_deprecation$var $kt_name$: $kt_type$\n" " @JvmName(\"${$get$kt_capitalized_name$$}$\")\n" @@ -297,14 +297,15 @@ void ImmutableEnumFieldLiteGenerator::GenerateKotlinDslMembers( " }\n"); WriteFieldAccessorDocComment(printer, descriptor_, CLEARER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "fun ${$clear$kt_capitalized_name$$}$() {\n" " $kt_dsl_builder$.${$clear$capitalized_name$$}$()\n" "}\n"); if (HasHazzer(descriptor_)) { - WriteFieldAccessorDocComment(printer, descriptor_, HAZZER); + WriteFieldAccessorDocComment(printer, descriptor_, HAZZER, + /* builder */ false, /* kdoc */ true); printer->Print( variables_, "fun ${$has$kt_capitalized_name$$}$(): kotlin.Boolean {\n" @@ -830,7 +831,7 @@ void RepeatedImmutableEnumFieldLiteGenerator::GenerateKotlinDslMembers( "class ${$$kt_capitalized_name$Proxy$}$ private constructor()" " : com.google.protobuf.kotlin.DslProxy()\n"); - WriteFieldDocComment(printer, descriptor_); + WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print(variables_, "$kt_deprecation$ val $kt_name$: " "com.google.protobuf.kotlin.DslList" @@ -841,7 +842,7 @@ void RepeatedImmutableEnumFieldLiteGenerator::GenerateKotlinDslMembers( " )\n"); WriteFieldAccessorDocComment(printer, descriptor_, LIST_ADDER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"add$kt_capitalized_name$\")\n" @@ -852,7 +853,7 @@ void RepeatedImmutableEnumFieldLiteGenerator::GenerateKotlinDslMembers( "}"); WriteFieldAccessorDocComment(printer, descriptor_, LIST_ADDER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"plusAssign$kt_capitalized_name$\")\n" @@ -864,7 +865,7 @@ void RepeatedImmutableEnumFieldLiteGenerator::GenerateKotlinDslMembers( "}"); WriteFieldAccessorDocComment(printer, descriptor_, LIST_MULTI_ADDER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"addAll$kt_capitalized_name$\")\n" @@ -875,7 +876,7 @@ void RepeatedImmutableEnumFieldLiteGenerator::GenerateKotlinDslMembers( "}"); WriteFieldAccessorDocComment(printer, descriptor_, LIST_MULTI_ADDER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print( variables_, "@kotlin.jvm.JvmSynthetic\n" @@ -888,7 +889,7 @@ void RepeatedImmutableEnumFieldLiteGenerator::GenerateKotlinDslMembers( "}"); WriteFieldAccessorDocComment(printer, descriptor_, LIST_INDEXED_SETTER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print( variables_, "@kotlin.jvm.JvmSynthetic\n" @@ -900,7 +901,7 @@ void RepeatedImmutableEnumFieldLiteGenerator::GenerateKotlinDslMembers( "}"); WriteFieldAccessorDocComment(printer, descriptor_, CLEARER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"clear$kt_capitalized_name$\")\n" diff --git a/src/google/protobuf/compiler/java/map_field.cc b/src/google/protobuf/compiler/java/map_field.cc index ce0766129c..c430b69939 100644 --- a/src/google/protobuf/compiler/java/map_field.cc +++ b/src/google/protobuf/compiler/java/map_field.cc @@ -706,7 +706,7 @@ void ImmutableMapFieldGenerator::GenerateKotlinDslMembers( "class ${$$kt_capitalized_name$Proxy$}$ private constructor()" " : com.google.protobuf.kotlin.DslProxy()\n"); - WriteFieldDocComment(printer, descriptor_); + WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print( variables_, "$kt_deprecation$ val $kt_name$: " @@ -718,7 +718,7 @@ void ImmutableMapFieldGenerator::GenerateKotlinDslMembers( " $kt_dsl_builder$.${$get$capitalized_name$Map$}$()\n" " )\n"); - WriteFieldDocComment(printer, descriptor_); + WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print( variables_, "@JvmName(\"put$kt_capitalized_name$\")\n" @@ -728,7 +728,7 @@ void ImmutableMapFieldGenerator::GenerateKotlinDslMembers( " $kt_dsl_builder$.${$put$capitalized_name$$}$(key, value)\n" " }\n"); - WriteFieldDocComment(printer, descriptor_); + WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print( variables_, "@kotlin.jvm.JvmSynthetic\n" @@ -740,7 +740,7 @@ void ImmutableMapFieldGenerator::GenerateKotlinDslMembers( " put(key, value)\n" " }\n"); - WriteFieldDocComment(printer, descriptor_); + WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print( variables_, "@kotlin.jvm.JvmSynthetic\n" @@ -751,7 +751,7 @@ void ImmutableMapFieldGenerator::GenerateKotlinDslMembers( " $kt_dsl_builder$.${$remove$capitalized_name$$}$(key)\n" " }\n"); - WriteFieldDocComment(printer, descriptor_); + WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print( variables_, "@kotlin.jvm.JvmSynthetic\n" @@ -763,7 +763,7 @@ void ImmutableMapFieldGenerator::GenerateKotlinDslMembers( " $kt_dsl_builder$.${$putAll$capitalized_name$$}$(map)\n" " }\n"); - WriteFieldDocComment(printer, descriptor_); + WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print( variables_, "@kotlin.jvm.JvmSynthetic\n" diff --git a/src/google/protobuf/compiler/java/map_field_lite.cc b/src/google/protobuf/compiler/java/map_field_lite.cc index 7e5f829ba4..7657829c86 100644 --- a/src/google/protobuf/compiler/java/map_field_lite.cc +++ b/src/google/protobuf/compiler/java/map_field_lite.cc @@ -849,7 +849,7 @@ void ImmutableMapFieldLiteGenerator::GenerateKotlinDslMembers( "class ${$$kt_capitalized_name$Proxy$}$ private constructor()" " : com.google.protobuf.kotlin.DslProxy()\n"); - WriteFieldDocComment(printer, descriptor_); + WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print( variables_, "$kt_deprecation$ val $kt_name$: " @@ -861,7 +861,7 @@ void ImmutableMapFieldLiteGenerator::GenerateKotlinDslMembers( " $kt_dsl_builder$.${$get$capitalized_name$Map$}$()\n" " )\n"); - WriteFieldDocComment(printer, descriptor_); + WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print( variables_, "@JvmName(\"put$kt_capitalized_name$\")\n" @@ -871,7 +871,7 @@ void ImmutableMapFieldLiteGenerator::GenerateKotlinDslMembers( " $kt_dsl_builder$.${$put$capitalized_name$$}$(key, value)\n" " }\n"); - WriteFieldDocComment(printer, descriptor_); + WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print( variables_, "@kotlin.jvm.JvmSynthetic\n" @@ -883,7 +883,7 @@ void ImmutableMapFieldLiteGenerator::GenerateKotlinDslMembers( " put(key, value)\n" " }\n"); - WriteFieldDocComment(printer, descriptor_); + WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print( variables_, "@kotlin.jvm.JvmSynthetic\n" @@ -894,7 +894,7 @@ void ImmutableMapFieldLiteGenerator::GenerateKotlinDslMembers( " $kt_dsl_builder$.${$remove$capitalized_name$$}$(key)\n" " }\n"); - WriteFieldDocComment(printer, descriptor_); + WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print( variables_, "@kotlin.jvm.JvmSynthetic\n" @@ -906,7 +906,7 @@ void ImmutableMapFieldLiteGenerator::GenerateKotlinDslMembers( " $kt_dsl_builder$.${$putAll$capitalized_name$$}$(map)\n" " }\n"); - WriteFieldDocComment(printer, descriptor_); + WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print( variables_, "@kotlin.jvm.JvmSynthetic\n" diff --git a/src/google/protobuf/compiler/java/message.cc b/src/google/protobuf/compiler/java/message.cc index 5adacef3ad..44c85723ac 100644 --- a/src/google/protobuf/compiler/java/message.cc +++ b/src/google/protobuf/compiler/java/message.cc @@ -1451,6 +1451,7 @@ void ImmutableMessageGenerator::GenerateKotlinMembers( "message_kt", name_resolver_->GetKotlinExtensionsClassName(descriptor_), "message", name_resolver_->GetClassName(descriptor_, true)); + WriteMessageDocComment(printer, descriptor_, /* kdoc */ true); printer->Print("object $name$Kt {\n", "name", descriptor_->name()); printer->Indent(); GenerateKotlinDsl(printer); diff --git a/src/google/protobuf/compiler/java/message_field.cc b/src/google/protobuf/compiler/java/message_field.cc index f3833e978c..85fb5d4a14 100644 --- a/src/google/protobuf/compiler/java/message_field.cc +++ b/src/google/protobuf/compiler/java/message_field.cc @@ -419,7 +419,7 @@ void ImmutableMessageFieldGenerator::GenerateBuilderMembers( void ImmutableMessageFieldGenerator::GenerateKotlinDslMembers( io::Printer* printer) const { - WriteFieldDocComment(printer, descriptor_); + WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print(variables_, "$kt_deprecation$var $kt_name$: $kt_type$\n" " @JvmName(\"${$get$kt_capitalized_name$$}$\")\n" @@ -430,13 +430,14 @@ void ImmutableMessageFieldGenerator::GenerateKotlinDslMembers( " }\n"); WriteFieldAccessorDocComment(printer, descriptor_, CLEARER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "fun ${$clear$kt_capitalized_name$$}$() {\n" " $kt_dsl_builder$.${$clear$capitalized_name$$}$()\n" "}\n"); - WriteFieldAccessorDocComment(printer, descriptor_, HAZZER); + WriteFieldAccessorDocComment(printer, descriptor_, HAZZER, + /* builder */ false, /* kdoc */ true); printer->Print( variables_, "fun ${$has$kt_capitalized_name$$}$(): kotlin.Boolean {\n" @@ -1422,7 +1423,7 @@ void RepeatedImmutableMessageFieldGenerator::GenerateKotlinDslMembers( "class ${$$kt_capitalized_name$Proxy$}$ private constructor()" " : com.google.protobuf.kotlin.DslProxy()\n"); - WriteFieldDocComment(printer, descriptor_); + WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print(variables_, "$kt_deprecation$ val $kt_name$: " "com.google.protobuf.kotlin.DslList" @@ -1433,7 +1434,7 @@ void RepeatedImmutableMessageFieldGenerator::GenerateKotlinDslMembers( " )\n"); WriteFieldAccessorDocComment(printer, descriptor_, LIST_ADDER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"add$kt_capitalized_name$\")\n" @@ -1444,7 +1445,7 @@ void RepeatedImmutableMessageFieldGenerator::GenerateKotlinDslMembers( "}\n"); WriteFieldAccessorDocComment(printer, descriptor_, LIST_ADDER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"plusAssign$kt_capitalized_name$\")\n" @@ -1456,7 +1457,7 @@ void RepeatedImmutableMessageFieldGenerator::GenerateKotlinDslMembers( "}\n"); WriteFieldAccessorDocComment(printer, descriptor_, LIST_MULTI_ADDER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"addAll$kt_capitalized_name$\")\n" @@ -1467,7 +1468,7 @@ void RepeatedImmutableMessageFieldGenerator::GenerateKotlinDslMembers( "}\n"); WriteFieldAccessorDocComment(printer, descriptor_, LIST_MULTI_ADDER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print( variables_, "@kotlin.jvm.JvmSynthetic\n" @@ -1480,7 +1481,7 @@ void RepeatedImmutableMessageFieldGenerator::GenerateKotlinDslMembers( "}\n"); WriteFieldAccessorDocComment(printer, descriptor_, LIST_INDEXED_SETTER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print( variables_, "@kotlin.jvm.JvmSynthetic\n" @@ -1492,7 +1493,7 @@ void RepeatedImmutableMessageFieldGenerator::GenerateKotlinDslMembers( "}\n"); WriteFieldAccessorDocComment(printer, descriptor_, CLEARER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"clear$kt_capitalized_name$\")\n" diff --git a/src/google/protobuf/compiler/java/message_field_lite.cc b/src/google/protobuf/compiler/java/message_field_lite.cc index eb37ca682c..627aaeef37 100644 --- a/src/google/protobuf/compiler/java/message_field_lite.cc +++ b/src/google/protobuf/compiler/java/message_field_lite.cc @@ -288,7 +288,7 @@ void ImmutableMessageFieldLiteGenerator::GenerateBuilderMembers( void ImmutableMessageFieldLiteGenerator::GenerateKotlinDslMembers( io::Printer* printer) const { - WriteFieldDocComment(printer, descriptor_); + WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print(variables_, "$kt_deprecation$var $kt_name$: $kt_type$\n" " @JvmName(\"${$get$kt_capitalized_name$$}$\")\n" @@ -299,13 +299,14 @@ void ImmutableMessageFieldLiteGenerator::GenerateKotlinDslMembers( " }\n"); WriteFieldAccessorDocComment(printer, descriptor_, CLEARER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "fun ${$clear$kt_capitalized_name$$}$() {\n" " $kt_dsl_builder$.${$clear$capitalized_name$$}$()\n" "}\n"); - WriteFieldAccessorDocComment(printer, descriptor_, HAZZER); + WriteFieldAccessorDocComment(printer, descriptor_, HAZZER, + /* builder */ false, /* kdoc */ true); printer->Print( variables_, "fun ${$has$kt_capitalized_name$$}$(): kotlin.Boolean {\n" @@ -809,7 +810,7 @@ void RepeatedImmutableMessageFieldLiteGenerator::GenerateKotlinDslMembers( "class ${$$kt_capitalized_name$Proxy$}$ private constructor()" " : com.google.protobuf.kotlin.DslProxy()\n"); - WriteFieldDocComment(printer, descriptor_); + WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print(variables_, "$kt_deprecation$ val $kt_name$: " "com.google.protobuf.kotlin.DslList" @@ -820,7 +821,7 @@ void RepeatedImmutableMessageFieldLiteGenerator::GenerateKotlinDslMembers( " )\n"); WriteFieldAccessorDocComment(printer, descriptor_, LIST_ADDER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"add$kt_capitalized_name$\")\n" @@ -831,7 +832,7 @@ void RepeatedImmutableMessageFieldLiteGenerator::GenerateKotlinDslMembers( "}\n"); WriteFieldAccessorDocComment(printer, descriptor_, LIST_ADDER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"plusAssign$kt_capitalized_name$\")\n" @@ -843,7 +844,7 @@ void RepeatedImmutableMessageFieldLiteGenerator::GenerateKotlinDslMembers( "}\n"); WriteFieldAccessorDocComment(printer, descriptor_, LIST_MULTI_ADDER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"addAll$kt_capitalized_name$\")\n" @@ -854,7 +855,7 @@ void RepeatedImmutableMessageFieldLiteGenerator::GenerateKotlinDslMembers( "}\n"); WriteFieldAccessorDocComment(printer, descriptor_, LIST_MULTI_ADDER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print( variables_, "@kotlin.jvm.JvmSynthetic\n" @@ -867,7 +868,7 @@ void RepeatedImmutableMessageFieldLiteGenerator::GenerateKotlinDslMembers( "}\n"); WriteFieldAccessorDocComment(printer, descriptor_, LIST_INDEXED_SETTER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print( variables_, "@kotlin.jvm.JvmSynthetic\n" @@ -879,7 +880,7 @@ void RepeatedImmutableMessageFieldLiteGenerator::GenerateKotlinDslMembers( "}\n"); WriteFieldAccessorDocComment(printer, descriptor_, CLEARER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"clear$kt_capitalized_name$\")\n" diff --git a/src/google/protobuf/compiler/java/message_lite.cc b/src/google/protobuf/compiler/java/message_lite.cc index a42ae3f301..5ad41a061a 100644 --- a/src/google/protobuf/compiler/java/message_lite.cc +++ b/src/google/protobuf/compiler/java/message_lite.cc @@ -792,6 +792,7 @@ void ImmutableMessageLiteGenerator::GenerateKotlinMembers( "message_kt", name_resolver_->GetKotlinExtensionsClassName(descriptor_), "message", name_resolver_->GetClassName(descriptor_, true)); + WriteMessageDocComment(printer, descriptor_, /* kdoc */ true); printer->Print("object $name$Kt {\n", "name", descriptor_->name()); printer->Indent(); GenerateKotlinDsl(printer); diff --git a/src/google/protobuf/compiler/java/primitive_field.cc b/src/google/protobuf/compiler/java/primitive_field.cc index 9ec99f953b..69de34bc10 100644 --- a/src/google/protobuf/compiler/java/primitive_field.cc +++ b/src/google/protobuf/compiler/java/primitive_field.cc @@ -318,7 +318,7 @@ void ImmutablePrimitiveFieldGenerator::GenerateBuilderMembers( void ImmutablePrimitiveFieldGenerator::GenerateKotlinDslMembers( io::Printer* printer) const { - WriteFieldDocComment(printer, descriptor_); + WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print(variables_, "$kt_deprecation$var $kt_name$: $kt_type$\n" " @JvmName(\"${$get$kt_capitalized_name$$}$\")\n" @@ -329,14 +329,15 @@ void ImmutablePrimitiveFieldGenerator::GenerateKotlinDslMembers( " }\n"); WriteFieldAccessorDocComment(printer, descriptor_, CLEARER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "fun ${$clear$kt_capitalized_name$$}$() {\n" " $kt_dsl_builder$.${$clear$capitalized_name$$}$()\n" "}\n"); if (HasHazzer(descriptor_)) { - WriteFieldAccessorDocComment(printer, descriptor_, HAZZER); + WriteFieldAccessorDocComment(printer, descriptor_, HAZZER, + /* builder */ false, /* kdoc */ true); printer->Print( variables_, "fun ${$has$kt_capitalized_name$$}$(): kotlin.Boolean {\n" @@ -852,7 +853,7 @@ void RepeatedImmutablePrimitiveFieldGenerator::GenerateKotlinDslMembers( "class ${$$kt_capitalized_name$Proxy$}$ private constructor()" " : com.google.protobuf.kotlin.DslProxy()\n"); - WriteFieldDocComment(printer, descriptor_); + WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print(variables_, "$kt_deprecation$ val $kt_name$: " "com.google.protobuf.kotlin.DslList" @@ -863,7 +864,7 @@ void RepeatedImmutablePrimitiveFieldGenerator::GenerateKotlinDslMembers( " )\n"); WriteFieldAccessorDocComment(printer, descriptor_, LIST_ADDER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"add$kt_capitalized_name$\")\n" @@ -874,7 +875,7 @@ void RepeatedImmutablePrimitiveFieldGenerator::GenerateKotlinDslMembers( "}"); WriteFieldAccessorDocComment(printer, descriptor_, LIST_ADDER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"plusAssign$kt_capitalized_name$\")\n" @@ -886,7 +887,7 @@ void RepeatedImmutablePrimitiveFieldGenerator::GenerateKotlinDslMembers( "}"); WriteFieldAccessorDocComment(printer, descriptor_, LIST_MULTI_ADDER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"addAll$kt_capitalized_name$\")\n" @@ -897,7 +898,7 @@ void RepeatedImmutablePrimitiveFieldGenerator::GenerateKotlinDslMembers( "}"); WriteFieldAccessorDocComment(printer, descriptor_, LIST_MULTI_ADDER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print( variables_, "@kotlin.jvm.JvmSynthetic\n" @@ -910,7 +911,7 @@ void RepeatedImmutablePrimitiveFieldGenerator::GenerateKotlinDslMembers( "}"); WriteFieldAccessorDocComment(printer, descriptor_, LIST_INDEXED_SETTER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print( variables_, "@kotlin.jvm.JvmSynthetic\n" @@ -922,7 +923,7 @@ void RepeatedImmutablePrimitiveFieldGenerator::GenerateKotlinDslMembers( "}"); WriteFieldAccessorDocComment(printer, descriptor_, CLEARER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"clear$kt_capitalized_name$\")\n" diff --git a/src/google/protobuf/compiler/java/primitive_field_lite.cc b/src/google/protobuf/compiler/java/primitive_field_lite.cc index e323feff2a..daf6566f62 100644 --- a/src/google/protobuf/compiler/java/primitive_field_lite.cc +++ b/src/google/protobuf/compiler/java/primitive_field_lite.cc @@ -334,14 +334,15 @@ void ImmutablePrimitiveFieldLiteGenerator::GenerateKotlinDslMembers( " }\n"); WriteFieldAccessorDocComment(printer, descriptor_, CLEARER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "fun ${$clear$kt_capitalized_name$$}$() {\n" " $kt_dsl_builder$.${$clear$capitalized_name$$}$()\n" "}\n"); if (HasHazzer(descriptor_)) { - WriteFieldAccessorDocComment(printer, descriptor_, HAZZER); + WriteFieldAccessorDocComment(printer, descriptor_, HAZZER, + /* builder */ false, /* kdoc */ true); printer->Print( variables_, "fun ${$has$kt_capitalized_name$$}$(): kotlin.Boolean {\n" @@ -685,7 +686,7 @@ void RepeatedImmutablePrimitiveFieldLiteGenerator::GenerateKotlinDslMembers( " )\n"); WriteFieldAccessorDocComment(printer, descriptor_, LIST_ADDER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"add$kt_capitalized_name$\")\n" @@ -696,7 +697,7 @@ void RepeatedImmutablePrimitiveFieldLiteGenerator::GenerateKotlinDslMembers( "}"); WriteFieldAccessorDocComment(printer, descriptor_, LIST_ADDER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"plusAssign$kt_capitalized_name$\")\n" @@ -708,7 +709,7 @@ void RepeatedImmutablePrimitiveFieldLiteGenerator::GenerateKotlinDslMembers( "}"); WriteFieldAccessorDocComment(printer, descriptor_, LIST_MULTI_ADDER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"addAll$kt_capitalized_name$\")\n" @@ -719,7 +720,7 @@ void RepeatedImmutablePrimitiveFieldLiteGenerator::GenerateKotlinDslMembers( "}"); WriteFieldAccessorDocComment(printer, descriptor_, LIST_MULTI_ADDER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print( variables_, "@kotlin.jvm.JvmSynthetic\n" @@ -732,7 +733,7 @@ void RepeatedImmutablePrimitiveFieldLiteGenerator::GenerateKotlinDslMembers( "}"); WriteFieldAccessorDocComment(printer, descriptor_, LIST_INDEXED_SETTER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print( variables_, "@kotlin.jvm.JvmSynthetic\n" @@ -744,7 +745,7 @@ void RepeatedImmutablePrimitiveFieldLiteGenerator::GenerateKotlinDslMembers( "}"); WriteFieldAccessorDocComment(printer, descriptor_, CLEARER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"clear$kt_capitalized_name$\")\n" diff --git a/src/google/protobuf/compiler/java/string_field.cc b/src/google/protobuf/compiler/java/string_field.cc index 3228da6d4a..d3162a4dc7 100644 --- a/src/google/protobuf/compiler/java/string_field.cc +++ b/src/google/protobuf/compiler/java/string_field.cc @@ -377,7 +377,7 @@ void ImmutableStringFieldGenerator::GenerateBuilderMembers( void ImmutableStringFieldGenerator::GenerateKotlinDslMembers( io::Printer* printer) const { - WriteFieldDocComment(printer, descriptor_); + WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print(variables_, "$kt_deprecation$var $kt_name$: kotlin.String\n" " @JvmName(\"${$get$kt_capitalized_name$$}$\")\n" @@ -388,14 +388,15 @@ void ImmutableStringFieldGenerator::GenerateKotlinDslMembers( " }\n"); WriteFieldAccessorDocComment(printer, descriptor_, CLEARER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "fun ${$clear$kt_capitalized_name$$}$() {\n" " $kt_dsl_builder$.${$clear$capitalized_name$$}$()\n" "}\n"); if (HasHazzer(descriptor_)) { - WriteFieldAccessorDocComment(printer, descriptor_, HAZZER); + WriteFieldAccessorDocComment(printer, descriptor_, HAZZER, + /* builder */ false, /* kdoc */ true); printer->Print( variables_, "fun ${$has$kt_capitalized_name$$}$(): kotlin.Boolean {\n" @@ -967,7 +968,8 @@ void RepeatedImmutableStringFieldGenerator::GenerateKotlinDslMembers( " : com.google.protobuf.kotlin.DslProxy()\n"); // property for List - WriteFieldAccessorDocComment(printer, descriptor_, LIST_GETTER); + WriteFieldAccessorDocComment(printer, descriptor_, LIST_GETTER, + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "$kt_deprecation$ val $kt_name$: " "com.google.protobuf.kotlin.DslList" @@ -979,7 +981,7 @@ void RepeatedImmutableStringFieldGenerator::GenerateKotlinDslMembers( // List.add(String) WriteFieldAccessorDocComment(printer, descriptor_, LIST_ADDER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"add$kt_capitalized_name$\")\n" @@ -991,7 +993,7 @@ void RepeatedImmutableStringFieldGenerator::GenerateKotlinDslMembers( // List += String WriteFieldAccessorDocComment(printer, descriptor_, LIST_ADDER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"plusAssign$kt_capitalized_name$\")\n" @@ -1004,7 +1006,7 @@ void RepeatedImmutableStringFieldGenerator::GenerateKotlinDslMembers( // List.addAll(Iterable) WriteFieldAccessorDocComment(printer, descriptor_, LIST_MULTI_ADDER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print( variables_, "@kotlin.jvm.JvmSynthetic\n" @@ -1017,7 +1019,7 @@ void RepeatedImmutableStringFieldGenerator::GenerateKotlinDslMembers( // List += Iterable WriteFieldAccessorDocComment(printer, descriptor_, LIST_MULTI_ADDER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print( variables_, "@kotlin.jvm.JvmSynthetic\n" @@ -1031,7 +1033,7 @@ void RepeatedImmutableStringFieldGenerator::GenerateKotlinDslMembers( // List[Int] = String WriteFieldAccessorDocComment(printer, descriptor_, LIST_INDEXED_SETTER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print( variables_, "@kotlin.jvm.JvmSynthetic\n" @@ -1043,7 +1045,7 @@ void RepeatedImmutableStringFieldGenerator::GenerateKotlinDslMembers( "}"); WriteFieldAccessorDocComment(printer, descriptor_, CLEARER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"clear$kt_capitalized_name$\")\n" diff --git a/src/google/protobuf/compiler/java/string_field_lite.cc b/src/google/protobuf/compiler/java/string_field_lite.cc index 8e5b230b8b..b8861977b1 100644 --- a/src/google/protobuf/compiler/java/string_field_lite.cc +++ b/src/google/protobuf/compiler/java/string_field_lite.cc @@ -311,7 +311,7 @@ void ImmutableStringFieldLiteGenerator::GenerateBuilderMembers( void ImmutableStringFieldLiteGenerator::GenerateKotlinDslMembers( io::Printer* printer) const { - WriteFieldDocComment(printer, descriptor_); + WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print(variables_, "$kt_deprecation$var $kt_name$: kotlin.String\n" " @JvmName(\"${$get$kt_capitalized_name$$}$\")\n" @@ -322,14 +322,15 @@ void ImmutableStringFieldLiteGenerator::GenerateKotlinDslMembers( " }\n"); WriteFieldAccessorDocComment(printer, descriptor_, CLEARER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "fun ${$clear$kt_capitalized_name$$}$() {\n" " $kt_dsl_builder$.${$clear$capitalized_name$$}$()\n" "}\n"); if (HasHazzer(descriptor_)) { - WriteFieldAccessorDocComment(printer, descriptor_, HAZZER); + WriteFieldAccessorDocComment(printer, descriptor_, HAZZER, + /* builder */ false, /* kdoc */ true); printer->Print( variables_, "fun ${$has$kt_capitalized_name$$}$(): kotlin.Boolean {\n" @@ -752,7 +753,8 @@ void RepeatedImmutableStringFieldLiteGenerator::GenerateKotlinDslMembers( " : com.google.protobuf.kotlin.DslProxy()\n"); // property for List - WriteFieldAccessorDocComment(printer, descriptor_, LIST_GETTER); + WriteFieldAccessorDocComment(printer, descriptor_, LIST_GETTER, + /* builder */ false, /* kdoc */ true); printer->Print( variables_, "$kt_deprecation$ val $kt_name$: " @@ -766,7 +768,7 @@ void RepeatedImmutableStringFieldLiteGenerator::GenerateKotlinDslMembers( // List.add(String) WriteFieldAccessorDocComment(printer, descriptor_, LIST_ADDER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"add$kt_capitalized_name$\")\n" @@ -778,7 +780,7 @@ void RepeatedImmutableStringFieldLiteGenerator::GenerateKotlinDslMembers( // List += String WriteFieldAccessorDocComment(printer, descriptor_, LIST_ADDER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"plusAssign$kt_capitalized_name$\")\n" @@ -791,7 +793,7 @@ void RepeatedImmutableStringFieldLiteGenerator::GenerateKotlinDslMembers( // List.addAll(Iterable) WriteFieldAccessorDocComment(printer, descriptor_, LIST_MULTI_ADDER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print( variables_, "@kotlin.jvm.JvmSynthetic\n" @@ -804,7 +806,7 @@ void RepeatedImmutableStringFieldLiteGenerator::GenerateKotlinDslMembers( // List += Iterable WriteFieldAccessorDocComment(printer, descriptor_, LIST_MULTI_ADDER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print( variables_, "@kotlin.jvm.JvmSynthetic\n" @@ -818,7 +820,7 @@ void RepeatedImmutableStringFieldLiteGenerator::GenerateKotlinDslMembers( // List[Int] = String WriteFieldAccessorDocComment(printer, descriptor_, LIST_INDEXED_SETTER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print( variables_, "@kotlin.jvm.JvmSynthetic\n" @@ -830,7 +832,7 @@ void RepeatedImmutableStringFieldLiteGenerator::GenerateKotlinDslMembers( "}"); WriteFieldAccessorDocComment(printer, descriptor_, CLEARER, - /* builder */ false); + /* builder */ false, /* kdoc */ true); printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"clear$kt_capitalized_name$\")\n" diff --git a/src/google/protobuf/generated_message_tctable_impl.h b/src/google/protobuf/generated_message_tctable_impl.h index 1ea1250f8b..e016e4d831 100644 --- a/src/google/protobuf/generated_message_tctable_impl.h +++ b/src/google/protobuf/generated_message_tctable_impl.h @@ -356,6 +356,15 @@ class PROTOBUF_EXPORT TcParser final { static const char* FastEvR1(PROTOBUF_TC_PARAM_DECL); static const char* FastEvR2(PROTOBUF_TC_PARAM_DECL); + static const char* FastEr0S1(PROTOBUF_TC_PARAM_DECL); + static const char* FastEr0S2(PROTOBUF_TC_PARAM_DECL); + static const char* FastEr0R1(PROTOBUF_TC_PARAM_DECL); + static const char* FastEr0R2(PROTOBUF_TC_PARAM_DECL); + static const char* FastEr1S1(PROTOBUF_TC_PARAM_DECL); + static const char* FastEr1S2(PROTOBUF_TC_PARAM_DECL); + static const char* FastEr1R1(PROTOBUF_TC_PARAM_DECL); + static const char* FastEr1R2(PROTOBUF_TC_PARAM_DECL); + // Functions referenced by generated fast tables (string types): // B: bytes S: string U: UTF-8 string // (empty): ArenaStringPtr i: InlinedString @@ -449,6 +458,9 @@ class PROTOBUF_EXPORT TcParser final { private: friend class GeneratedTcTableLiteTest; + static void* MaybeGetSplitBase(MessageLite* msg, const bool is_split, + const TcParseTableBase* table, + google::protobuf::internal::ParseContext* ctx); template static inline const char* SingularParseMessageAuxImpl(PROTOBUF_TC_PARAM_DECL); @@ -526,8 +538,12 @@ class PROTOBUF_EXPORT TcParser final { // Implementations for fast enum field parsing functions: template static inline const char* SingularEnum(PROTOBUF_TC_PARAM_DECL); + template + static inline const char* SingularEnumSmallRange(PROTOBUF_TC_PARAM_DECL); template static inline const char* RepeatedEnum(PROTOBUF_TC_PARAM_DECL); + template + static inline const char* RepeatedEnumSmallRange(PROTOBUF_TC_PARAM_DECL); // Implementations for fast string field parsing functions: enum Utf8Type { kNoUtf8 = 0, kUtf8 = 1, kUtf8ValidateOnly = 2 }; diff --git a/src/google/protobuf/generated_message_tctable_lite.cc b/src/google/protobuf/generated_message_tctable_lite.cc index 8e4036974e..77ce7311e9 100644 --- a/src/google/protobuf/generated_message_tctable_lite.cc +++ b/src/google/protobuf/generated_message_tctable_lite.cc @@ -66,26 +66,25 @@ const char* TcParser::GenericFallbackLite(PROTOBUF_TC_PARAM_DECL) { // Core fast parsing implementation: ////////////////////////////////////////////////////////////////////////////// -class TcParser::ScopedArenaSwap final { - public: - ScopedArenaSwap(MessageLite* msg, ParseContext* ctx) - : ctx_(ctx), saved_(ctx->data().arena) { - ctx_->data().arena = msg->GetArenaForAllocation(); - } - ScopedArenaSwap(const ScopedArenaSwap&) = delete; - ~ScopedArenaSwap() { ctx_->data().arena = saved_; } - - private: - ParseContext* const ctx_; - Arena* const saved_; -}; - PROTOBUF_NOINLINE const char* TcParser::ParseLoop( MessageLite* msg, const char* ptr, ParseContext* ctx, const TcParseTableBase* table) { - ScopedArenaSwap saved(msg, ctx); + // Note: TagDispatch uses a dispatch table at "&table->fast_entries". + // For fast dispatch, we'd like to have a pointer to that, but if we use + // that expression, there's no easy way to get back to "table", which we also + // need during dispatch. It turns out that "table + 1" points exactly to + // fast_entries, so we just increment table by 1 here, to get the register + // holding the value we want. + table += 1; while (!ctx->Done(&ptr)) { - ptr = TagDispatch(msg, ptr, ctx, table, 0, {}); +#if defined(__GNUC__) + // Note: this asm prevents the compiler (clang, specifically) from + // believing (thanks to CSE) that it needs to dedicate a registeer both + // to "table" and "&table->fast_entries". + // TODO(b/64614992): remove this asm + asm("" : "+r"(table)); +#endif + ptr = TagDispatch(msg, ptr, ctx, table - 1, 0, {}); if (ptr == nullptr) break; if (ctx->LastTag() != 1) break; // Ended on terminating tag } @@ -388,7 +387,7 @@ inline PROTOBUF_ALWAYS_INLINE const char* TcParser::SingularParseMessageAuxImpl( if (aux_is_table) { const auto* inner_table = table->field_aux(data.aux_idx())->table; if (field == nullptr) { - field = inner_table->default_instance->New(ctx->data().arena); + field = inner_table->default_instance->New(msg->GetArenaForAllocation()); } if (group_coding) { return ctx->ParseGroup(field, ptr, FastDecodeTag(saved_tag), @@ -399,7 +398,7 @@ inline PROTOBUF_ALWAYS_INLINE const char* TcParser::SingularParseMessageAuxImpl( if (field == nullptr) { const MessageLite* default_instance = table->field_aux(data.aux_idx())->message_default(); - field = default_instance->New(ctx->data().arena); + field = default_instance->New(msg->GetArenaForAllocation()); } if (group_coding) { return ctx->ParseGroup(field, ptr, FastDecodeTag(saved_tag)); @@ -850,7 +849,13 @@ PROTOBUF_NOINLINE const char* TcParser::SingularVarBigint( const ::google::protobuf::internal::TcParseTableBase* table; uint64_t hasbits; }; - volatile Spill spill = {data.data, msg, table, hasbits}; + Spill spill = {data.data, msg, table, hasbits}; +#if defined(__GNUC__) + // This empty asm block convinces the compiler that the contents of spill may + // have changed, and thus can't be cached in registers. It's similar to, but + // more optimal then, the effect of declaring it "volatile". + asm("" : "+m"(spill)); +#endif uint64_t tmp; PROTOBUF_ASSUME(static_cast(*ptr) < 0); @@ -1167,6 +1172,91 @@ const char* TcParser::FastEvR2(PROTOBUF_TC_PARAM_DECL) { PROTOBUF_TC_PARAM_PASS); } +template +PROTOBUF_ALWAYS_INLINE const char* TcParser::SingularEnumSmallRange( + PROTOBUF_TC_PARAM_DECL) { + if (PROTOBUF_PREDICT_FALSE(data.coded_tag() != 0)) { + PROTOBUF_MUSTTAIL return MiniParse(PROTOBUF_TC_PARAM_PASS); + } + + uint8_t v = ptr[sizeof(TagType)]; + if (PROTOBUF_PREDICT_FALSE(min > v || v > data.aux_idx())) { + PROTOBUF_MUSTTAIL return MiniParse(PROTOBUF_TC_PARAM_PASS); + } + + RefAt(msg, data.offset()) = v; + ptr += sizeof(TagType) + 1; + hasbits |= (uint64_t{1} << data.hasbit_idx()); + PROTOBUF_MUSTTAIL return ToTagDispatch(PROTOBUF_TC_PARAM_PASS); +} + +const char* TcParser::FastEr0S1(PROTOBUF_TC_PARAM_DECL) { + PROTOBUF_MUSTTAIL return SingularEnumSmallRange( + PROTOBUF_TC_PARAM_PASS); +} + +const char* TcParser::FastEr0S2(PROTOBUF_TC_PARAM_DECL) { + PROTOBUF_MUSTTAIL return SingularEnumSmallRange( + PROTOBUF_TC_PARAM_PASS); +} + +const char* TcParser::FastEr1S1(PROTOBUF_TC_PARAM_DECL) { + PROTOBUF_MUSTTAIL return SingularEnumSmallRange( + PROTOBUF_TC_PARAM_PASS); +} + +const char* TcParser::FastEr1S2(PROTOBUF_TC_PARAM_DECL) { + PROTOBUF_MUSTTAIL return SingularEnumSmallRange( + PROTOBUF_TC_PARAM_PASS); +} + + +template +PROTOBUF_ALWAYS_INLINE const char* TcParser::RepeatedEnumSmallRange( + PROTOBUF_TC_PARAM_DECL) { + if (PROTOBUF_PREDICT_FALSE(data.coded_tag() != 0)) { + InvertPacked(data); + if (data.coded_tag() == 0) { + // Packed parsing is handled by generated fallback. + PROTOBUF_MUSTTAIL return FastUnknownEnumFallback(PROTOBUF_TC_PARAM_PASS); + } else { + PROTOBUF_MUSTTAIL return MiniParse(PROTOBUF_TC_PARAM_PASS); + } + } + auto& field = RefAt>(msg, data.offset()); + auto expected_tag = UnalignedLoad(ptr); + const uint8_t max = data.aux_idx(); + do { + uint8_t v = ptr[sizeof(TagType)]; + if (PROTOBUF_PREDICT_FALSE(min > v || v > max)) { + PROTOBUF_MUSTTAIL return MiniParse(PROTOBUF_TC_PARAM_PASS); + } + field.Add(static_cast(v)); + ptr += sizeof(TagType) + 1; + if (PROTOBUF_PREDICT_FALSE(!ctx->DataAvailable(ptr))) break; + } while (UnalignedLoad(ptr) == expected_tag); + + PROTOBUF_MUSTTAIL return ToParseLoop(PROTOBUF_TC_PARAM_PASS); +} + +const char* TcParser::FastEr0R1(PROTOBUF_TC_PARAM_DECL) { + PROTOBUF_MUSTTAIL return RepeatedEnumSmallRange( + PROTOBUF_TC_PARAM_PASS); +} +const char* TcParser::FastEr0R2(PROTOBUF_TC_PARAM_DECL) { + PROTOBUF_MUSTTAIL return RepeatedEnumSmallRange( + PROTOBUF_TC_PARAM_PASS); +} + +const char* TcParser::FastEr1R1(PROTOBUF_TC_PARAM_DECL) { + PROTOBUF_MUSTTAIL return RepeatedEnumSmallRange( + PROTOBUF_TC_PARAM_PASS); +} +const char* TcParser::FastEr1R2(PROTOBUF_TC_PARAM_DECL) { + PROTOBUF_MUSTTAIL return RepeatedEnumSmallRange( + PROTOBUF_TC_PARAM_PASS); +} + ////////////////////////////////////////////////////////////////////////////// // String/bytes fields ////////////////////////////////////////////////////////////////////////////// @@ -1206,7 +1296,7 @@ PROTOBUF_ALWAYS_INLINE const char* TcParser::SingularString( ptr += sizeof(TagType); hasbits |= (uint64_t{1} << data.hasbit_idx()); auto& field = RefAt(msg, data.offset()); - auto arena = ctx->data().arena; + auto arena = msg->GetArenaForAllocation(); if (arena) { ptr = ctx->ReadArenaString(ptr, &field, arena); } else { @@ -1395,7 +1485,7 @@ bool TcParser::ChangeOneof(const TcParseTableBase* table, case field_layout::kRepGroup: case field_layout::kRepIWeak: { auto& field = RefAt(msg, current_entry->offset); - if (!ctx->data().arena) { + if (!msg->GetArenaForAllocation()) { delete field; } break; @@ -1421,10 +1511,11 @@ uint32_t GetSplitOffset(const TcParseTableBase* table) { uint32_t GetSizeofSplit(const TcParseTableBase* table) { return table->field_aux(kSplitSizeIdx)->offset; } +} // namespace -void* MaybeGetSplitBase(MessageLite* msg, const bool is_split, - const TcParseTableBase* table, - ::google::protobuf::internal::ParseContext* ctx) { +void* TcParser::MaybeGetSplitBase(MessageLite* msg, const bool is_split, + const TcParseTableBase* table, + ::google::protobuf::internal::ParseContext* ctx) { void* out = msg; if (is_split) { const uint32_t split_offset = GetSplitOffset(table); @@ -1434,7 +1525,7 @@ void* MaybeGetSplitBase(MessageLite* msg, const bool is_split, if (split == default_split) { // Allocate split instance when needed. uint32_t size = GetSizeofSplit(table); - Arena* arena = ctx->data().arena; + Arena* arena = msg->GetArenaForAllocation(); split = (arena == nullptr) ? ::operator new(size) : arena->AllocateAligned(size); memcpy(split, default_split, size); @@ -1443,7 +1534,6 @@ void* MaybeGetSplitBase(MessageLite* msg, const bool is_split, } return out; } -} // namespace template const char* TcParser::MpFixed(PROTOBUF_TC_PARAM_DECL) { @@ -1792,12 +1882,12 @@ const char* TcParser::MpString(PROTOBUF_TC_PARAM_DECL) { } bool is_valid = false; - Arena* arena = ctx->data().arena; void* const base = MaybeGetSplitBase(msg, is_split, table, ctx); switch (rep) { case field_layout::kRepAString: { auto& field = RefAt(base, entry.offset); if (need_init) field.InitDefault(); + Arena* arena = msg->GetArenaForAllocation(); if (arena) { ptr = ctx->ReadArenaString(ptr, &field, arena); } else { @@ -1912,7 +2002,7 @@ const char* TcParser::MpMessage(PROTOBUF_TC_PARAM_DECL) { if ((type_card & field_layout::kTvMask) == field_layout::kTvTable) { auto* inner_table = table->field_aux(&entry)->table; if (need_init || field == nullptr) { - field = inner_table->default_instance->New(ctx->data().arena); + field = inner_table->default_instance->New(msg->GetArenaForAllocation()); } if (is_group) { return ctx->ParseGroup(field, ptr, decoded_tag, inner_table); @@ -1920,8 +2010,8 @@ const char* TcParser::MpMessage(PROTOBUF_TC_PARAM_DECL) { return ctx->ParseMessage(field, ptr, inner_table); } else { if (need_init || field == nullptr) { - field = - table->field_aux(&entry)->message_default()->New(ctx->data().arena); + field = table->field_aux(&entry)->message_default()->New( + msg->GetArenaForAllocation()); } if (is_group) { return ctx->ParseGroup(field, ptr, decoded_tag); diff --git a/src/google/protobuf/message.cc b/src/google/protobuf/message.cc index 3e044f742f..724a6220f0 100644 --- a/src/google/protobuf/message.cc +++ b/src/google/protobuf/message.cc @@ -406,6 +406,15 @@ PROTOBUF_NOINLINE GenericTypeHandler::GetOwningArena(Message* value) { return value->GetOwningArena(); } + +template void InternalMetadata::DoClear(); +template void InternalMetadata::DoMergeFrom( + const UnknownFieldSet& other); +template void InternalMetadata::DoSwap(UnknownFieldSet* other); +template Arena* InternalMetadata::DeleteOutOfLineHelper(); +template UnknownFieldSet* +InternalMetadata::mutable_unknown_fields_slow(); + } // namespace internal } // namespace protobuf diff --git a/src/google/protobuf/message.h b/src/google/protobuf/message.h index 82873885bb..1e0c9cbff3 100644 --- a/src/google/protobuf/message.h +++ b/src/google/protobuf/message.h @@ -1036,6 +1036,7 @@ class PROTOBUF_EXPORT Reflection final { return schema_.IsSplit(field); } + friend class FastReflectionBase; friend class FastReflectionMessageMutator; friend bool internal::IsDescendant(Message& root, const Message& message); diff --git a/src/google/protobuf/message_unittest.inc b/src/google/protobuf/message_unittest.inc index 7526ead779..44e3b3151c 100644 --- a/src/google/protobuf/message_unittest.inc +++ b/src/google/protobuf/message_unittest.inc @@ -40,7 +40,9 @@ #include #include +#include #include +#include #include #ifndef _MSC_VER @@ -285,6 +287,32 @@ TEST(MESSAGE_TEST_NAME, MergeFromUninitialized) { EXPECT_TRUE(TestUtil::EqualsToSerialized(q, p.SerializePartialAsString())); } +TEST(MESSAGE_TEST_NAME, UninitializedAndTooDeep) { + UNITTEST::TestRequiredForeign original; + original.mutable_optional_message()->set_a(1); + original.mutable_optional_lazy_message() + ->mutable_child() + ->mutable_payload() + ->set_optional_int64(0); + + std::string data; + ASSERT_TRUE(original.SerializePartialToString(&data)); + + UNITTEST::TestRequiredForeign pass; + ASSERT_TRUE(pass.ParsePartialFromString(data)); + ASSERT_FALSE(pass.IsInitialized()); + + io::ArrayInputStream array_stream(data.data(), data.size()); + io::CodedInputStream input_stream(&array_stream); + input_stream.SetRecursionLimit(2); + + UNITTEST::TestRequiredForeign fail; + EXPECT_FALSE(fail.ParsePartialFromCodedStream(&input_stream)); + + UNITTEST::TestRequiredForeign fail_uninitialized; + EXPECT_FALSE(fail_uninitialized.ParseFromString(data)); +} + TEST(MESSAGE_TEST_NAME, ExplicitLazyExceedRecursionLimit) { UNITTEST::NestedTestAllTypes original, parsed; // Build proto with recursion depth of 3. @@ -1146,5 +1174,94 @@ TEST(MESSAGE_TEST_NAME, PreservesFloatingPointNegative0) { std::signbit(out_message.optional_double())); } +std::string EncodeEnumValue(int number, int value, int non_canonical_bytes) { + uint8_t buf[100]; + uint8_t* p = buf; + + p = internal::WireFormatLite::WriteEnumToArray(number, value, p); + // varint can have a max of 10 bytes. + while (non_canonical_bytes-- > 0 && p - buf < 10) { + // Add a dummy byte at the end. + p[-1] |= 0x80; + p[0] = 0; + ++p; + } + + return std::string(buf, p); +} + +std::string EncodeOtherField() { + UNITTEST::EnumParseTester obj; + obj.set_other_field(1); + return obj.SerializeAsString(); +} + +TEST(MESSAGE_TEST_NAME, TestEnumParsers) { + UNITTEST::EnumParseTester obj; + + const auto other_field = EncodeOtherField(); + + constexpr int kInvalidValue = 0x900913; + auto* ref = obj.GetReflection(); + auto* descriptor = obj.descriptor(); + for (bool use_tail_field : {false, true}) { + SCOPED_TRACE(use_tail_field); + for (int non_canonical_bytes = 0; non_canonical_bytes < 5; + ++non_canonical_bytes) { + SCOPED_TRACE(non_canonical_bytes); + for (int i = 0; i < descriptor->field_count(); ++i) { + const auto* field = descriptor->field(i); + if (field->name() == "other_field") continue; + SCOPED_TRACE(field->full_name()); + const auto* enum_desc = field->enum_type(); + for (int e = 0; e < enum_desc->value_count(); ++e) { + const auto* value_desc = enum_desc->value(e); + if (value_desc->number() < 0 && non_canonical_bytes > 0) { + // Negative numbers only have a canonical representation. + continue; + } + SCOPED_TRACE(value_desc->number()); + GOOGLE_CHECK_NE(value_desc->number(), kInvalidValue) + << "Invalid value is a real label."; + auto encoded = EncodeEnumValue(field->number(), value_desc->number(), + non_canonical_bytes); + if (use_tail_field) { + encoded += other_field; + } + + EXPECT_TRUE(obj.ParseFromString(encoded)); + if (field->is_repeated()) { + ASSERT_EQ(ref->FieldSize(obj, field), 1); + EXPECT_EQ(ref->GetRepeatedEnumValue(obj, field, 0), + value_desc->number()); + } else { + EXPECT_TRUE(ref->HasField(obj, field)); + EXPECT_EQ(ref->GetEnumValue(obj, field), value_desc->number()); + } + auto& unknown = ref->GetUnknownFields(obj); + ASSERT_EQ(unknown.field_count(), 0); + } + + SCOPED_TRACE("Invalid value"); + // Try an invalid value, which should go to the unknown fields. + EXPECT_TRUE(obj.ParseFromString(EncodeEnumValue( + field->number(), kInvalidValue, non_canonical_bytes))); + if (field->is_repeated()) { + ASSERT_EQ(ref->FieldSize(obj, field), 0); + } else { + EXPECT_FALSE(ref->HasField(obj, field)); + EXPECT_EQ(ref->GetEnumValue(obj, field), + enum_desc->value(0)->number()); + } + auto& unknown = ref->GetUnknownFields(obj); + ASSERT_EQ(unknown.field_count(), 1); + EXPECT_EQ(unknown.field(0).number(), field->number()); + EXPECT_EQ(unknown.field(0).type(), unknown.field(0).TYPE_VARINT); + EXPECT_EQ(unknown.field(0).varint(), kInvalidValue); + } + } + } +} + } // namespace protobuf } // namespace google diff --git a/src/google/protobuf/metadata_lite.h b/src/google/protobuf/metadata_lite.h index 11faba69fc..af840e5065 100644 --- a/src/google/protobuf/metadata_lite.h +++ b/src/google/protobuf/metadata_lite.h @@ -45,6 +45,9 @@ namespace google { namespace protobuf { + +class UnknownFieldSet; + namespace internal { // This is the representation for messages that support arena allocation. It @@ -273,6 +276,19 @@ PROTOBUF_EXPORT void InternalMetadata::DoMergeFrom( template <> PROTOBUF_EXPORT void InternalMetadata::DoSwap(std::string* other); +// Instantiated once in message.cc (where the definition of UnknownFieldSet is +// known) to prevent much duplication across translation units of a large build. +extern template PROTOBUF_EXPORT void +InternalMetadata::DoClear(); +extern template PROTOBUF_EXPORT void +InternalMetadata::DoMergeFrom(const UnknownFieldSet& other); +extern template PROTOBUF_EXPORT void +InternalMetadata::DoSwap(UnknownFieldSet* other); +extern template PROTOBUF_EXPORT Arena* +InternalMetadata::DeleteOutOfLineHelper(); +extern template PROTOBUF_EXPORT UnknownFieldSet* +InternalMetadata::mutable_unknown_fields_slow(); + // This helper RAII class is needed to efficiently parse unknown fields. We // should only call mutable_unknown_fields if there are actual unknown fields. // The obvious thing to just use a stack string and swap it at the end of diff --git a/src/google/protobuf/parse_context.h b/src/google/protobuf/parse_context.h index 96ee320951..a7baf4d333 100644 --- a/src/google/protobuf/parse_context.h +++ b/src/google/protobuf/parse_context.h @@ -387,7 +387,6 @@ class PROTOBUF_EXPORT ParseContext : public EpsCopyInputStream { struct Data { const DescriptorPool* pool = nullptr; MessageFactory* factory = nullptr; - Arena* arena = nullptr; }; template diff --git a/src/google/protobuf/port_def.inc b/src/google/protobuf/port_def.inc index 38bd26b870..6f7ad7ca3a 100644 --- a/src/google/protobuf/port_def.inc +++ b/src/google/protobuf/port_def.inc @@ -176,9 +176,22 @@ // user code can be updated before upgrading versions of protobuf. // PROTOBUF_FUTURE_FINAL is used on classes that are historically not marked as // final, but that may be marked final in future (breaking) releases. -// #define PROTOBUF_FUTURE_BREAKING_CHANGES 1 -// #define PROTOBUF_FUTURE_FINAL final + +#ifdef PROTOBUF_FUTURE_BREAKING_CHANGES +// Used on classes that are historically not marked as final. +// Owner: kfm@ +#define PROTOBUF_FUTURE_FINAL final + +// Used to remove the RTTI checks for `DefaultFieldComparator`. +// Owner: kfm@ +#define PROTOBUF_FUTURE_REMOVE_DEFAULT_FIELD_COMPARATOR 1 + +// Used to remove the manipulation of cleared elements in RepeatedPtrField. +// Owner: mkruskal@ +#define PROTOBUF_FUTURE_REMOVE_CLEARED_API 1 +#else #define PROTOBUF_FUTURE_FINAL +#endif #ifdef PROTOBUF_VERSION #error PROTOBUF_VERSION was previously defined diff --git a/src/google/protobuf/port_undef.inc b/src/google/protobuf/port_undef.inc index 184980a7e9..cc7ccd05fe 100644 --- a/src/google/protobuf/port_undef.inc +++ b/src/google/protobuf/port_undef.inc @@ -108,8 +108,12 @@ #ifdef PROTOBUF_FUTURE_BREAKING_CHANGES #undef PROTOBUF_FUTURE_BREAKING_CHANGES +#undef PROTOBUF_FUTURE_REMOVE_DEFAULT_FIELD_COMPARATOR +#undef PROTOBUF_FUTURE_REMOVE_CLEARED_API #endif +#undef PROTOBUF_FUTURE_FINAL + // Restore macro that may have been #undef'd in port_def.inc. #ifdef _WIN32 #pragma pop_macro("CREATE_NEW") diff --git a/src/google/protobuf/repeated_field_unittest.cc b/src/google/protobuf/repeated_field_unittest.cc index d8a82bf09e..c5211470bc 100644 --- a/src/google/protobuf/repeated_field_unittest.cc +++ b/src/google/protobuf/repeated_field_unittest.cc @@ -1258,7 +1258,7 @@ TEST(RepeatedPtrField, ClearedElements) { field.Clear(); EXPECT_EQ(field.ClearedCount(), 2); -#ifndef PROTOBUF_FUTURE_BREAKING_CHANGES +#ifndef PROTOBUF_FUTURE_REMOVE_CLEARED_API EXPECT_EQ(field.ReleaseCleared(), original); // Take ownership again. EXPECT_EQ(field.ClearedCount(), 1); EXPECT_NE(field.Add(), original); @@ -1270,7 +1270,7 @@ TEST(RepeatedPtrField, ClearedElements) { EXPECT_EQ(field.ClearedCount(), 1); EXPECT_EQ(field.Add(), original); EXPECT_EQ(field.ClearedCount(), 0); -#endif // !PROTOBUF_FUTURE_BREAKING_CHANGES +#endif // !PROTOBUF_FUTURE_REMOVE_CLEARED_API } // Test all code paths in AddAllocated(). diff --git a/src/google/protobuf/repeated_ptr_field.h b/src/google/protobuf/repeated_ptr_field.h index c0822e083b..2fca2d6206 100644 --- a/src/google/protobuf/repeated_ptr_field.h +++ b/src/google/protobuf/repeated_ptr_field.h @@ -1092,7 +1092,7 @@ class RepeatedPtrField final : private internal::RepeatedPtrFieldBase { // Gets the number of cleared objects that are currently being kept // around for reuse. int ClearedCount() const; -#ifndef PROTOBUF_FUTURE_BREAKING_CHANGES +#ifndef PROTOBUF_FUTURE_REMOVE_CLEARED_API // Adds an element to the pool of cleared objects, passing ownership to // the RepeatedPtrField. The element must be cleared prior to calling // this method. @@ -1107,7 +1107,7 @@ class RepeatedPtrField final : private internal::RepeatedPtrFieldBase { // This method cannot be called when the repeated field is on an arena; doing // so will trigger a GOOGLE_DCHECK-failure. PROTOBUF_NODISCARD Element* ReleaseCleared(); -#endif // !PROTOBUF_FUTURE_BREAKING_CHANGES +#endif // !PROTOBUF_FUTURE_REMOVE_CLEARED_API // Removes the element referenced by position. // @@ -1514,7 +1514,7 @@ inline int RepeatedPtrField::ClearedCount() const { return RepeatedPtrFieldBase::ClearedCount(); } -#ifndef PROTOBUF_FUTURE_BREAKING_CHANGES +#ifndef PROTOBUF_FUTURE_REMOVE_CLEARED_API template inline void RepeatedPtrField::AddCleared(Element* value) { return RepeatedPtrFieldBase::AddCleared(value); @@ -1524,7 +1524,7 @@ template inline Element* RepeatedPtrField::ReleaseCleared() { return RepeatedPtrFieldBase::ReleaseCleared(); } -#endif // !PROTOBUF_FUTURE_BREAKING_CHANGES +#endif // !PROTOBUF_FUTURE_REMOVE_CLEARED_API template inline void RepeatedPtrField::Reserve(int new_size) { diff --git a/src/google/protobuf/unittest.proto b/src/google/protobuf/unittest.proto index f1b0bd04ba..4d08b44432 100644 --- a/src/google/protobuf/unittest.proto +++ b/src/google/protobuf/unittest.proto @@ -459,6 +459,9 @@ message TestRequiredForeign { optional TestRequired optional_message = 1; repeated TestRequired repeated_message = 2; optional int32 dummy = 3; + + // Missing required fields must not affect verification of child messages. + optional NestedTestAllTypes optional_lazy_message = 4 [lazy = true]; } message TestRequiredMessage { @@ -1416,3 +1419,105 @@ message TestVerifyBigFieldNumberUint32 { } +// This message contains different kind of enums to exercise the different +// parsers in table-driven. +message EnumParseTester { + enum SeqSmall0 { + SEQ_SMALL_0_DEFAULT = 0; + SEQ_SMALL_0_1 = 1; + SEQ_SMALL_0_2 = 2; + }; + optional SeqSmall0 optional_seq_small_0_lowfield = 1; + optional SeqSmall0 optional_seq_small_0_midfield = 1001; + optional SeqSmall0 optional_seq_small_0_hifield = 1000001; + repeated SeqSmall0 repeated_seq_small_0_lowfield = 2; + repeated SeqSmall0 repeated_seq_small_0_midfield = 1002; + repeated SeqSmall0 repeated_seq_small_0_hifield = 1000002; + repeated SeqSmall0 packed_seq_small_0_lowfield = 3 [packed = true]; + repeated SeqSmall0 packed_seq_small_0_midfield = 1003 [packed = true]; + repeated SeqSmall0 packed_seq_small_0_hifield = 1000003 [packed = true]; + + enum SeqSmall1 { + SEQ_SMALL_1_DEFAULT = 1; + SEQ_SMALL_1_2 = 2; + SEQ_SMALL_1_3 = 3; + }; + optional SeqSmall1 optional_seq_small_1_lowfield = 4; + optional SeqSmall1 optional_seq_small_1_midfield = 1004; + optional SeqSmall1 optional_seq_small_1_hifield = 1000004; + repeated SeqSmall1 repeated_seq_small_1_lowfield = 5; + repeated SeqSmall1 repeated_seq_small_1_midfield = 1005; + repeated SeqSmall1 repeated_seq_small_1_hifield = 1000005; + repeated SeqSmall1 packed_seq_small_1_lowfield = 6 [packed = true]; + repeated SeqSmall1 packed_seq_small_1_midfield = 1006 [packed = true]; + repeated SeqSmall1 packed_seq_small_1_hifield = 1000006 [packed = true]; + + enum SeqLarge { + SEQ_LARGE_DEFAULT = -1; + SEQ_LARGE_0 = 0; + SEQ_LARGE_1 = 1; + SEQ_LARGE_2 = 2; + SEQ_LARGE_3 = 3; + SEQ_LARGE_4 = 4; + SEQ_LARGE_5 = 5; + SEQ_LARGE_6 = 6; + SEQ_LARGE_7 = 7; + SEQ_LARGE_8 = 8; + SEQ_LARGE_9 = 9; + SEQ_LARGE_10 = 10; + SEQ_LARGE_11 = 11; + SEQ_LARGE_12 = 12; + SEQ_LARGE_13 = 13; + SEQ_LARGE_14 = 14; + SEQ_LARGE_15 = 15; + SEQ_LARGE_16 = 16; + SEQ_LARGE_17 = 17; + SEQ_LARGE_18 = 18; + SEQ_LARGE_19 = 19; + SEQ_LARGE_20 = 20; + SEQ_LARGE_21 = 21; + SEQ_LARGE_22 = 22; + SEQ_LARGE_23 = 23; + SEQ_LARGE_24 = 24; + SEQ_LARGE_25 = 25; + SEQ_LARGE_26 = 26; + SEQ_LARGE_27 = 27; + SEQ_LARGE_28 = 28; + SEQ_LARGE_29 = 29; + SEQ_LARGE_30 = 30; + SEQ_LARGE_31 = 31; + SEQ_LARGE_32 = 32; + SEQ_LARGE_33 = 33; + }; + optional SeqLarge optional_seq_large_lowfield = 7; + optional SeqLarge optional_seq_large_midfield = 1007; + optional SeqLarge optional_seq_large_hifield = 1000007; + repeated SeqLarge repeated_seq_large_lowfield = 8; + repeated SeqLarge repeated_seq_large_midfield = 1008; + repeated SeqLarge repeated_seq_large_hifield = 1000008; + repeated SeqLarge packed_seq_large_lowfield = 9 [packed = true]; + repeated SeqLarge packed_seq_large_midfield = 1009 [packed = true]; + repeated SeqLarge packed_seq_large_hifield = 1000009 [packed = true]; + + enum Arbitrary { + ARBITRARY_DEFAULT = -123123; + ARBITRARY_1 = -123; + ARBITRARY_2 = 213; + ARBITRARY_3 = 213213; + ARBITRARY_MIN = -2147483648; + ARBITRARY_MAX = 2147483647; + }; + optional Arbitrary optional_arbitrary_lowfield = 10; + optional Arbitrary optional_arbitrary_midfield = 1010; + optional Arbitrary optional_arbitrary_hifield = 1000010; + repeated Arbitrary repeated_arbitrary_lowfield = 11; + repeated Arbitrary repeated_arbitrary_midfield = 1011; + repeated Arbitrary repeated_arbitrary_hifield = 1000011; + repeated Arbitrary packed_arbitrary_lowfield = 12 [packed = true]; + repeated Arbitrary packed_arbitrary_midfield = 1012 [packed = true]; + repeated Arbitrary packed_arbitrary_hifield = 1000012 [packed = true]; + + // An arbitrary field we can append to to break the runs of repeated fields. + optional int32 other_field = 99; +}; + diff --git a/src/google/protobuf/util/field_comparator.h b/src/google/protobuf/util/field_comparator.h index 5706da357a..4e8ab50ff5 100644 --- a/src/google/protobuf/util/field_comparator.h +++ b/src/google/protobuf/util/field_comparator.h @@ -262,13 +262,8 @@ class PROTOBUF_EXPORT SimpleFieldComparator : public FieldComparator { }; // Default field comparison: use the basic implementation of FieldComparator. -#ifdef PROTOBUF_FUTURE_BREAKING_CHANGES -class PROTOBUF_EXPORT DefaultFieldComparator final - : public SimpleFieldComparator -#else // PROTOBUF_FUTURE_BREAKING_CHANGES -class PROTOBUF_EXPORT DefaultFieldComparator : public SimpleFieldComparator -#endif // PROTOBUF_FUTURE_BREAKING_CHANGES -{ +class PROTOBUF_EXPORT DefaultFieldComparator PROTOBUF_FUTURE_FINAL + : public SimpleFieldComparator { public: ComparisonResult Compare(const Message& message_1, const Message& message_2, const FieldDescriptor* field, int index_1, diff --git a/src/google/protobuf/util/json_util_test.cc b/src/google/protobuf/util/json_util_test.cc index 29ffe865d9..d4e1b1eb9f 100644 --- a/src/google/protobuf/util/json_util_test.cc +++ b/src/google/protobuf/util/json_util_test.cc @@ -32,9 +32,12 @@ #include #include +#include +#include #include #include +#include #include #include #include @@ -43,7 +46,7 @@ #include #include #include -#include +#include #include #include #include @@ -59,12 +62,12 @@ namespace google { namespace protobuf { namespace util { namespace { - using ::proto3::TestAny; using ::proto3::TestEnumValue; using ::proto3::TestMap; using ::proto3::TestMessage; using ::proto3::TestOneof; +using ::proto3::TestWrapper; using ::proto_util_converter::testing::MapIn; // TODO(b/234474291): Use the gtest versions once that's available in OSS. @@ -288,7 +291,7 @@ TEST(JsonUtilTest, ParseMessage) { { "int32Value": 1234567891, "int64Value": 5302428716536692736, - "floatValue": 3.4028235e+38, + "floatValue": 3.40282002e+38, "repeatedInt32Value": [1, 2], "messageValue": { "value": 2048 @@ -303,7 +306,7 @@ TEST(JsonUtilTest, ParseMessage) { EXPECT_EQ(m.int32_value(), 1234567891); EXPECT_EQ(m.int64_value(), 5302428716536692736); - EXPECT_EQ(m.float_value(), 3.402823466e+38f); + EXPECT_EQ(m.float_value(), 3.40282002e+38f); ASSERT_EQ(m.repeated_int32_value_size(), 2); EXPECT_EQ(m.repeated_int32_value(0), 1); EXPECT_EQ(m.repeated_int32_value(1), 2); @@ -436,7 +439,8 @@ TEST(JsonUtilTest, TestParsingAny) { EXPECT_THAT( ToJson(m), IsOkAndHolds( - R"json({"value":{"@type":"type.googleapis.com/proto3.TestMessage","int32Value":5,"stringValue":"expected_value","messageValue":{"value":1}}})json")); + R"({"value":{"@type":"type.googleapis.com/proto3.TestMessage",)" + R"("int32Value":5,"stringValue":"expected_value","messageValue":{"value":1}}})")); } TEST(JsonUtilTest, TestParsingAnyMiddleAtType) { @@ -513,7 +517,32 @@ TEST(JsonUtilTest, TestParsingNestedAnys) { EXPECT_THAT( ToJson(m), IsOkAndHolds( - R"json({"value":{"@type":"type.googleapis.com/google.protobuf.Any","value":{"@type":"type.googleapis.com/proto3.TestMessage","int32Value":5,"stringValue":"expected_value","messageValue":{"value":1}}}})json")); + R"({"value":{"@type":"type.googleapis.com/google.protobuf.Any",)" + R"("value":{"@type":"type.googleapis.com/proto3.TestMessage",)" + R"("int32Value":5,"stringValue":"expected_value","messageValue":{"value":1}}}})")); +} + +TEST(JsonUtilTest, ParseWrappers) { + StringPiece input = R"json( + { + "boolValue": true, + "int32Value": 42, + "stringValue": "ieieo", + } + )json"; + + TestWrapper m; + auto proto_buffer = FromJson(input, &m); + ASSERT_OK(proto_buffer); + + EXPECT_TRUE(m.bool_value().value()); + EXPECT_EQ(m.int32_value().value(), 42); + EXPECT_EQ(m.string_value().value(), "ieieo"); + + EXPECT_THAT( + ToJson(m), + IsOkAndHolds( + R"({"boolValue":true,"int32Value":42,"stringValue":"ieieo"})")); } TEST(JsonUtilTest, TestParsingUnknownAnyFields) { @@ -632,157 +661,200 @@ TEST(JsonUtilTest, TestParsingEnumIgnoreCase) { ASSERT_EQ(m.enum_value(), proto3::BAR); } -// A ZeroCopyOutputStream that writes to multiple buffers. -class SegmentedZeroCopyOutputStream : public io::ZeroCopyOutputStream { - public: - explicit SegmentedZeroCopyOutputStream( - std::vector segments) - : segments_(segments) { - // absl::c_* functions are not cloned in OSS. - std::reverse(segments_.begin(), segments_.end()); - } +class TypeResolverTest : public testing::Test { + protected: + util::StatusOr Proto2Json(StringPiece proto, + StringPiece type, + const JsonPrintOptions& options = {}) { + io::ArrayInputStream in(proto.data(), proto.size()); - bool Next(void** buffer, int* length) override { - if (segments_.empty()) { - return false; - } - last_segment_ = segments_.back(); - segments_.pop_back(); - // TODO(b/234159981): This is only ever constructed in test code, and only - // from non-const bytes, so this is a valid cast. We need to do this since - // OSS proto does not yet have absl::Span; once we take a full Abseil - // dependency we should use that here instead. - *buffer = const_cast(last_segment_.data()); - *length = static_cast(last_segment_.size()); - byte_count_ += static_cast(last_segment_.size()); - return true; + std::string result; + io::StringOutputStream out(&result); + + RETURN_IF_ERROR(BinaryToJsonStream( + resolver_.get(), StrCat("type.googleapis.com/", type), &in, + &out)); + return result; } - void BackUp(int length) override { - GOOGLE_CHECK(length <= static_cast(last_segment_.size())); + util::StatusOr Json2Proto(StringPiece json, + StringPiece type, + const JsonParseOptions& options = {}) { + io::ArrayInputStream in(json.data(), json.size()); - size_t backup = last_segment_.size() - static_cast(length); - segments_.push_back(last_segment_.substr(backup)); - last_segment_ = last_segment_.substr(0, backup); - byte_count_ -= static_cast(length); - } + std::string result; + io::StringOutputStream out(&result); - int64_t ByteCount() const override { return byte_count_; } + RETURN_IF_ERROR(JsonToBinaryStream( + resolver_.get(), StrCat("type.googleapis.com/", type), &in, + &out)); - private: - std::vector segments_; - StringPiece last_segment_; - int64_t byte_count_ = 0; + return result; + } + + std::unique_ptr resolver_{NewTypeResolverForDescriptorPool( + "type.googleapis.com", DescriptorPool::generated_pool())}; }; -// This test splits the output buffer and also the input data into multiple -// segments and checks that the implementation of ZeroCopyStreamByteSink -// handles all possible cases correctly. -TEST(ZeroCopyStreamByteSinkTest, TestAllInputOutputPatterns) { - static constexpr int kOutputBufferLength = 10; - // An exhaustive test takes too long, skip some combinations to make the test - // run faster. - static constexpr int kSkippedPatternCount = 7; - - char buffer[kOutputBufferLength]; - for (int split_pattern = 0; split_pattern < (1 << (kOutputBufferLength - 1)); - split_pattern += kSkippedPatternCount) { - // Split the buffer into small segments according to the split_pattern. - std::vector segments; - int segment_start = 0; - for (int i = 0; i < kOutputBufferLength - 1; ++i) { - if (split_pattern & (1 << i)) { - segments.push_back( - StringPiece(buffer + segment_start, i - segment_start + 1)); - segment_start = i + 1; - } - } - segments.push_back(StringPiece(buffer + segment_start, - kOutputBufferLength - segment_start)); - - // Write exactly 10 bytes through the ByteSink. - std::string input_data = "0123456789"; - for (int input_pattern = 0; input_pattern < (1 << (input_data.size() - 1)); - input_pattern += kSkippedPatternCount) { - memset(buffer, 0, sizeof(buffer)); - { - SegmentedZeroCopyOutputStream output_stream(segments); - internal::ZeroCopyStreamByteSink byte_sink(&output_stream); - int start = 0; - for (int j = 0; j < input_data.length() - 1; ++j) { - if (input_pattern & (1 << j)) { - byte_sink.Append(&input_data[start], j - start + 1); - start = j + 1; - } - } - byte_sink.Append(&input_data[start], input_data.length() - start); - } - EXPECT_EQ(std::string(buffer, input_data.length()), input_data); - } +TEST_F(TypeResolverTest, ParseFromResolver) { + StringPiece json = R"json( + { + "boolValue": true, + "int32Value": 1234567891, + "int64Value": -5302428716536692736, + "uint32Value": 42, + "uint64Value": 530242871653669, + "floatValue": 3.4e+38, + "doubleValue": -55.5, + "stringValue": "foo bar baz", + "enumValue": "BAR", + "messageValue": { + "value": 2048 + }, - // Write only 9 bytes through the ByteSink. - input_data = "012345678"; - for (int input_pattern = 0; input_pattern < (1 << (input_data.size() - 1)); - input_pattern += kSkippedPatternCount) { - memset(buffer, 0, sizeof(buffer)); - { - SegmentedZeroCopyOutputStream output_stream(segments); - internal::ZeroCopyStreamByteSink byte_sink(&output_stream); - int start = 0; - for (int j = 0; j < input_data.length() - 1; ++j) { - if (input_pattern & (1 << j)) { - byte_sink.Append(&input_data[start], j - start + 1); - start = j + 1; - } - } - byte_sink.Append(&input_data[start], input_data.length() - start); - } - EXPECT_EQ(std::string(buffer, input_data.length()), input_data); - EXPECT_EQ(buffer[input_data.length()], 0); + "repeatedBoolValue": [true], + "repeatedInt32Value": [0, -42], + "repeatedUint64Value": [1, 2], + "repeatedDoubleValue": [1.5, -2.1], + "repeatedStringValue": ["foo", "bar ", ""], + "repeatedEnumValue": [1, "FOO"], + "repeatedMessageValue": [ + {"value": 40}, + {"value": 96} + ] } + )json"; + + auto proto_buffer = Json2Proto(json, "proto3.TestMessage"); + ASSERT_OK(proto_buffer); + + // Some random message but good enough to verify that the parsing wrapper + // functions are working properly. + TestMessage m; + ASSERT_TRUE(m.ParseFromString(*proto_buffer)); - // Write 11 bytes through the ByteSink. The extra byte will just - // be ignored. - input_data = "0123456789A"; - for (int input_pattern = 0; input_pattern < (1 << (input_data.size() - 1)); - input_pattern += kSkippedPatternCount) { - memset(buffer, 0, sizeof(buffer)); - { - SegmentedZeroCopyOutputStream output_stream(segments); - internal::ZeroCopyStreamByteSink byte_sink(&output_stream); - int start = 0; - for (int j = 0; j < input_data.length() - 1; ++j) { - if (input_pattern & (1 << j)) { - byte_sink.Append(&input_data[start], j - start + 1); - start = j + 1; - } - } - byte_sink.Append(&input_data[start], input_data.length() - start); + EXPECT_TRUE(m.bool_value()); + EXPECT_EQ(m.int32_value(), 1234567891); + EXPECT_EQ(m.int64_value(), -5302428716536692736); + EXPECT_EQ(m.uint32_value(), 42); + EXPECT_EQ(m.uint64_value(), 530242871653669); + EXPECT_EQ(m.float_value(), 3.4e+38f); + EXPECT_EQ(m.double_value(), -55.5); + EXPECT_EQ(m.string_value(), "foo bar baz"); + EXPECT_EQ(m.enum_value(), proto3::EnumType::BAR); + EXPECT_EQ(m.message_value().value(), 2048); + + ASSERT_EQ(m.repeated_bool_value_size(), 1); + EXPECT_TRUE(m.repeated_bool_value(0)); + + ASSERT_EQ(m.repeated_int32_value_size(), 2); + EXPECT_EQ(m.repeated_int32_value(0), 0); + EXPECT_EQ(m.repeated_int32_value(1), -42); + + ASSERT_EQ(m.repeated_uint64_value_size(), 2); + EXPECT_EQ(m.repeated_uint64_value(0), 1); + EXPECT_EQ(m.repeated_uint64_value(1), 2); + + ASSERT_EQ(m.repeated_double_value_size(), 2); + EXPECT_EQ(m.repeated_double_value(0), 1.5); + EXPECT_EQ(m.repeated_double_value(1), -2.1); + + ASSERT_EQ(m.repeated_string_value_size(), 3); + EXPECT_EQ(m.repeated_string_value(0), "foo"); + EXPECT_EQ(m.repeated_string_value(1), "bar "); + EXPECT_EQ(m.repeated_string_value(2), ""); + + ASSERT_EQ(m.repeated_enum_value_size(), 2); + EXPECT_EQ(m.repeated_enum_value(0), proto3::EnumType::BAR); + EXPECT_EQ(m.repeated_enum_value(1), proto3::EnumType::FOO); + + ASSERT_EQ(m.repeated_message_value_size(), 2); + EXPECT_EQ(m.repeated_message_value(0).value(), 40); + EXPECT_EQ(m.repeated_message_value(1).value(), 96); + + StringPiece compacted_json = + R"({"boolValue":true,"int32Value":1234567891,"int64Value":"-5302428716536692736",)" + R"("uint32Value":42,"uint64Value":"530242871653669","floatValue":3.4e+38,)" + R"("doubleValue":-55.5,"stringValue":"foo bar baz","enumValue":"BAR",)" + R"("messageValue":{"value":2048},"repeatedBoolValue":[true],"repeatedInt32Value":[0,-42])" + R"(,"repeatedUint64Value":["1","2"],"repeatedDoubleValue":[1.5,-2.1],)" + R"("repeatedStringValue":["foo","bar ",""],"repeatedEnumValue":["BAR","FOO"],)" + R"("repeatedMessageValue":[{"value":40},{"value":96}]})"; + + EXPECT_THAT(Proto2Json(*proto_buffer, "proto3.TestMessage"), + IsOkAndHolds(compacted_json)); + + // Proto3 messages always used packed, so this will make sure to exercise + // packed decoding. + std::string proto_buffer2; + m.SerializeToString(&proto_buffer2); + + EXPECT_THAT(Proto2Json(proto_buffer2, "proto3.TestMessage"), + IsOkAndHolds(compacted_json)); +} + +TEST_F(TypeResolverTest, ParseAnyFromResolver) { + StringPiece input = R"json( + { + "value": { + "@type": "type.googleapis.com/proto3.TestMessage", + "int32_value": 5, + "string_value": "expected_value", + "message_value": {"value": 1} } - EXPECT_EQ(input_data.substr(0, kOutputBufferLength), - std::string(buffer, kOutputBufferLength)); } - } + )json"; + + auto proto_buffer = Json2Proto(input, "proto3.TestAny"); + ASSERT_OK(proto_buffer); + + TestAny m; + ASSERT_TRUE(m.ParseFromString(*proto_buffer)); + + TestMessage t; + EXPECT_TRUE(m.value().UnpackTo(&t)); + EXPECT_EQ(t.int32_value(), 5); + EXPECT_EQ(t.string_value(), "expected_value"); + EXPECT_EQ(t.message_value().value(), 1); + + EXPECT_THAT( + Proto2Json(*proto_buffer, "proto3.TestAny"), + IsOkAndHolds( + R"({"value":{"@type":"type.googleapis.com/proto3.TestMessage",)" + R"("int32Value":5,"stringValue":"expected_value","messageValue":{"value":1}}})")); } -TEST(JsonUtilTest, TestWrongJsonInput) { - StringPiece json = "{\"unknown_field\":\"some_value\"}"; - io::ArrayInputStream input_stream(json.data(), json.size()); - char proto_buffer[10000]; +TEST_F(TypeResolverTest, ParseWrappers) { + StringPiece input = R"json( + { + "boolValue": true, + "int32Value": 42, + "stringValue": "ieieo", + } + )json"; - io::ArrayOutputStream output_stream(proto_buffer, sizeof(proto_buffer)); - std::string message_type = "type.googleapis.com/proto3.TestMessage"; + auto proto_buffer = Json2Proto(input, "proto3.TestWrapper"); + ASSERT_OK(proto_buffer); - auto* resolver = NewTypeResolverForDescriptorPool( - "type.googleapis.com", DescriptorPool::generated_pool()); + TestWrapper m; + ASSERT_TRUE(m.ParseFromString(*proto_buffer)); + EXPECT_TRUE(m.bool_value().value()); + EXPECT_EQ(m.int32_value().value(), 42); + EXPECT_EQ(m.string_value().value(), "ieieo"); EXPECT_THAT( - JsonToBinaryStream(resolver, message_type, &input_stream, &output_stream), - StatusIs(util::StatusCode::kInvalidArgument)); - delete resolver; + Proto2Json(*proto_buffer, "proto3.TestWrapper"), + IsOkAndHolds( + R"({"boolValue":true,"int32Value":42,"stringValue":"ieieo"})")); +} + +TEST_F(TypeResolverTest, TestWrongJsonInput) { + EXPECT_THAT(Json2Proto(R"json({"unknown_field": "some_value"})json", + "proto3.TestMessage"), + StatusIs(util::StatusCode::kInvalidArgument)); } -TEST(JsonUtilTest, HtmlEscape) { +TEST(JsonUtilTest, DISABLED_HtmlEscape) { TestMessage m; m.set_string_value(""); JsonPrintOptions options; diff --git a/src/google/protobuf/util/message_differencer.cc b/src/google/protobuf/util/message_differencer.cc index 30560ed5f4..35e4ed02bf 100644 --- a/src/google/protobuf/util/message_differencer.cc +++ b/src/google/protobuf/util/message_differencer.cc @@ -337,14 +337,14 @@ void MessageDifferencer::set_field_comparator(FieldComparator* comparator) { field_comparator_.base = comparator; } -#ifdef PROTOBUF_FUTURE_BREAKING_CHANGES +#ifdef PROTOBUF_FUTURE_REMOVE_DEFAULT_FIELD_COMPARATOR void MessageDifferencer::set_field_comparator( DefaultFieldComparator* comparator) { GOOGLE_CHECK(comparator) << "Field comparator can't be NULL."; field_comparator_kind_ = kFCDefault; field_comparator_.default_impl = comparator; } -#endif // PROTOBUF_FUTURE_BREAKING_CHANGES +#endif // PROTOBUF_FUTURE_REMOVE_DEFAULT_FIELD_COMPARATOR void MessageDifferencer::set_message_field_comparison( MessageFieldComparison comparison) { diff --git a/src/google/protobuf/util/message_differencer.h b/src/google/protobuf/util/message_differencer.h index f63cd54185..4df31524ba 100644 --- a/src/google/protobuf/util/message_differencer.h +++ b/src/google/protobuf/util/message_differencer.h @@ -524,9 +524,9 @@ class PROTOBUF_EXPORT MessageDifferencer { // Note that this method must be called before Compare for the comparator to // be used. void set_field_comparator(FieldComparator* comparator); -#ifdef PROTOBUF_FUTURE_BREAKING_CHANGES +#ifdef PROTOBUF_FUTURE_REMOVE_DEFAULT_FIELD_COMPARATOR void set_field_comparator(DefaultFieldComparator* comparator); -#endif // PROTOBUF_FUTURE_BREAKING_CHANGES +#endif // PROTOBUF_FUTURE_REMOVE_DEFAULT_FIELD_COMPARATOR // DEPRECATED. Pass a DefaultFieldComparator instance instead. // Sets the fraction and margin for the float comparison of a given field.