Integrate from Piper for C++, Java, and Python

CHANGES.txt

@@ -1,19 +1,18 @@
 2022-07-01 Unreleased version
-  * Add TransferAllMessages() util function
-  * Break ZeroCopyOutputByteSink into its own file.
-  * Explicitly instantiate InternalMetadata members at UnknownFieldSet.
-  * Support kdoc for Kotlin
-  * Delete unused function: IsProto3Field.
+  C++
+  * Reduced .pb.o object file size slightly by explicitly instantiating
+    InternalMetadata templates in the runtime.
   * Add C++20 keywords guarded by PROTOBUF_FUTURE_CPP20_KEYWORDS
-  * Escape Kotlin keywords in package names in proto generated code
-  * Fix StrAppend crashing on empty strings.
-
-2022-06-27 Unreleased version
-  * Handle reflection for message splitting.
-  * make metadata fields lazy.
-  * Extend visibility of plugin library to upb
-  * Modernize conformance_cpp.cc.
-  * Don't request 64-byte alignment unless the toolchain supports it.
+  * Fixed crash in ThreadLocalStorage for pre-C++17 compilers on 32-bit ARM.
+  * Clarified that JSON API non-OK statuses are not a stable API.
+
+  Kotlin
+  * Kotlin generated code comments now use kdoc format instead of javadoc.
+  * Escape keywords in package names in proto generated code
+
+  Java
+  * Performance improvement for repeated use of FieldMaskUtil#merge by caching
+    constructed FieldMaskTrees.
 
 2022-06-27 version 21.2 (C++/Java/Python/PHP/Objective-C/C#/Ruby)
   C++

benchmarks/java/src/main/java/com/google/protobuf/ProtoCaliperBenchmark.java

@@ -12,9 +12,8 @@
 import java.util.ArrayList;
 import java.util.List;
 
-/**
- * Basic benchmarks for Java protobuf parsing.
- */
+/** Basic benchmarks for Java protobuf parsing. */
+@SuppressWarnings("CheckReturnValue")
 public class ProtoCaliperBenchmark {
   public enum BenchmarkMessageType {
     GOOGLE_MESSAGE1_PROTO3 {

build_files_updated_unittest.sh

@@ -0,0 +1,62 @@
+#!/bin/bash
+
+# This script verifies that BUILD files and cmake files are in sync with src/Makefile.am
+
+set -eo pipefail
+
+if [ "$(uname)" != "Linux" ]; then
+  echo "build_files_updated_unittest only supported on Linux. Skipping..."
+  exit 0
+fi
+
+# Keep in sync with files needed by update_file_lists.sh
+generated_files=(
+  "BUILD"
+  "cmake/extract_includes.bat.in"
+  "cmake/libprotobuf-lite.cmake"
+  "cmake/libprotobuf.cmake"
+  "cmake/libprotoc.cmake"
+  "cmake/tests.cmake"
+  "src/Makefile.am"
+)
+
+# If we're running in Bazel, use the Bazel-provided temp-dir.
+if [ -n "${TEST_TMPDIR}" ]; then
+  # Env-var TEST_TMPDIR is set, assume that this is Bazel.
+  # Bazel may have opinions whether we are allowed to delete TEST_TMPDIR.
+  test_root="${TEST_TMPDIR}/build_files_updated_unittest"
+  mkdir "${test_root}"
+else
+  # Seems like we're not executed by Bazel.
+  test_root=$(mktemp -d)
+fi
+
+# From now on, fail if there are any unbound variables.
+set -u
+
+# Remove artifacts after test is finished.
+function cleanup {
+  rm -rf "${test_root}"
+}
+trap cleanup EXIT
+
+# Create golden dir and add snapshot of current state.
+golden_dir="${test_root}/golden"
+mkdir -p "${golden_dir}/cmake" "${golden_dir}/src"
+for file in ${generated_files[@]}; do
+  cp "${file}" "${golden_dir}/${file}"
+done
+
+# Create test dir, copy current state into it, and execute update script.
+test_dir="${test_root}/test"
+cp -R "${golden_dir}" "${test_dir}"
+
+cp "update_file_lists.sh" "${test_dir}/update_file_lists.sh"
+chmod +x "${test_dir}/update_file_lists.sh"
+cd "${test_root}/test"
+bash "${test_dir}/update_file_lists.sh"
+
+# Test whether there are any differences
+for file in ${generated_files[@]}; do
+  diff -du "${golden_dir}/${file}" "${test_dir}/${file}"
+done

java/core/src/test/java/com/google/protobuf/LiteEqualsAndHashTest.java

@@ -30,14 +30,13 @@
 
 package com.google.protobuf;
 
+import junit.framework.TestCase;
 import protobuf_unittest.lite_equals_and_hash.LiteEqualsAndHash.Bar;
 import protobuf_unittest.lite_equals_and_hash.LiteEqualsAndHash.BarPrime;
 import protobuf_unittest.lite_equals_and_hash.LiteEqualsAndHash.Foo;
 import protobuf_unittest.lite_equals_and_hash.LiteEqualsAndHash.TestOneofEquals;
 import protobuf_unittest.lite_equals_and_hash.LiteEqualsAndHash.TestRecursiveOneof;
 
-import junit.framework.TestCase;
-
 /**
  * Test generate equal and hash methods for the lite runtime.
  *
@@ -120,6 +119,6 @@ private void assertEqualsAndHashCodeAreFalse(Object o1, Object o2) {
 
   public void testRecursiveHashcode() {
     // This tests that we don't infinite loop.
-    TestRecursiveOneof.getDefaultInstance().hashCode();
+    int unused = TestRecursiveOneof.getDefaultInstance().hashCode();
   }
 }

python/google/protobuf/internal/descriptor_test.py

@@ -118,6 +118,22 @@ def setUp(self):
   def GetDescriptorPool(self):
     return symbol_database.Default().pool
 
+  def testMissingPackage(self):
+    file_proto = descriptor_pb2.FileDescriptorProto(
+        name='some/filename/some.proto')
+    serialized = file_proto.SerializeToString()
+    pool = descriptor_pool.DescriptorPool()
+    file_descriptor = pool.AddSerializedFile(serialized)
+    self.assertEqual('', file_descriptor.package)
+
+  def testEmptyPackage(self):
+    file_proto = descriptor_pb2.FileDescriptorProto(
+        name='some/filename/some.proto', package='')
+    serialized = file_proto.SerializeToString()
+    pool = descriptor_pool.DescriptorPool()
+    file_descriptor = pool.AddSerializedFile(serialized)
+    self.assertEqual('', file_descriptor.package)
+
   def testFindMethodByName(self):
     service_descriptor = (unittest_custom_options_pb2.
                           TestServiceWithCustomOptions.DESCRIPTOR)

python/google/protobuf/internal/message_test.py

@@ -2455,6 +2455,26 @@ def testSurrogatesInPython3(self):
     with self.assertRaises(ValueError):
       unittest_proto3_arena_pb2.TestAllTypes(optional_string=u'\ud801\ud801')
 
+  def testCrashNullAA(self):
+    self.assertEqual(
+        unittest_proto3_arena_pb2.TestAllTypes.NestedMessage(),
+        unittest_proto3_arena_pb2.TestAllTypes.NestedMessage())
+
+  def testCrashNullAB(self):
+    self.assertEqual(
+        unittest_proto3_arena_pb2.TestAllTypes.NestedMessage(),
+        unittest_proto3_arena_pb2.TestAllTypes().optional_nested_message)
+
+  def testCrashNullBA(self):
+    self.assertEqual(
+        unittest_proto3_arena_pb2.TestAllTypes().optional_nested_message,
+        unittest_proto3_arena_pb2.TestAllTypes.NestedMessage())
+
+  def testCrashNullBB(self):
+    self.assertEqual(
+        unittest_proto3_arena_pb2.TestAllTypes().optional_nested_message,
+        unittest_proto3_arena_pb2.TestAllTypes().optional_nested_message)
+
 
 
 

src/google/protobuf/arena.cc

@@ -382,7 +382,7 @@ void ThreadSafeArena::Init() {
 void ThreadSafeArena::SetInitialBlock(void* mem, size_t size) {
   SerialArena* serial = SerialArena::New({mem, size}, &thread_cache(),
                                          arena_stats_.MutableStats());
-  serial->set_next(NULL);
+  serial->set_next(nullptr);
   threads_.store(serial, std::memory_order_relaxed);
   CacheSerialArena(serial);
 }

src/google/protobuf/arena.h

@@ -120,7 +120,7 @@ struct ArenaOptions {
   // here.
   size_t max_block_size;
 
-  // An initial block of memory for the arena to use, or NULL for none. If
+  // An initial block of memory for the arena to use, or nullptr for none. If
   // provided, the block must live at least as long as the arena itself. The
   // creator of the Arena retains ownership of the block after the Arena is
   // destroyed.
@@ -143,7 +143,7 @@ struct ArenaOptions {
   ArenaOptions()
       : start_block_size(internal::AllocationPolicy::kDefaultStartBlockSize),
         max_block_size(internal::AllocationPolicy::kDefaultMaxBlockSize),
-        initial_block(NULL),
+        initial_block(nullptr),
         initial_block_size(0),
         block_alloc(nullptr),
         block_dealloc(nullptr),
@@ -180,7 +180,7 @@ struct ArenaOptions {
 #if PROTOBUF_RTTI
 #define RTTI_TYPE_ID(type) (&typeid(type))
 #else
-#define RTTI_TYPE_ID(type) (NULL)
+#define RTTI_TYPE_ID(type) (nullptr)
 #endif
 
 // Arena allocator. Arena allocation replaces ordinary (heap-based) allocation
@@ -210,7 +210,7 @@ struct ArenaOptions {
 //   with `args` (without `arena`), called when a T is allocated on the heap;
 //   and a constructor callable with `Arena* arena, Args&&... args`, called when
 //   a T is allocated on an arena. If the second constructor is called with a
-//   NULL arena pointer, it must be equivalent to invoking the first
+//   null arena pointer, it must be equivalent to invoking the first
 //   (`args`-only) constructor.
 //
 // - The type T must have a particular type trait: a nested type
@@ -220,7 +220,7 @@ struct ArenaOptions {
 //
 // - The type T *may* have the type trait |DestructorSkippable_|. If this type
 //   trait is present in the type, then its destructor will not be called if and
-//   only if it was passed a non-NULL arena pointer. If this type trait is not
+//   only if it was passed a non-null arena pointer. If this type trait is not
 //   present on the type, then its destructor is always called when the
 //   containing arena is destroyed.
 //
@@ -263,9 +263,9 @@ class PROTOBUF_EXPORT PROTOBUF_ALIGNAS(8) Arena final {
   void Init(const ArenaOptions&) {}
 
   // API to create proto2 message objects on the arena. If the arena passed in
-  // is NULL, then a heap allocated object is returned. Type T must be a message
-  // defined in a .proto file with cc_enable_arenas set to true, otherwise a
-  // compilation error will occur.
+  // is nullptr, then a heap allocated object is returned. Type T must be a
+  // message defined in a .proto file with cc_enable_arenas set to true,
+  // otherwise a compilation error will occur.
   //
   // RepeatedField and RepeatedPtrField may also be instantiated directly on an
   // arena with this method.
@@ -342,7 +342,7 @@ class PROTOBUF_EXPORT PROTOBUF_ALIGNAS(8) Arena final {
                   "CreateArray requires a trivially destructible type");
     GOOGLE_CHECK_LE(num_elements, std::numeric_limits<size_t>::max() / sizeof(T))
         << "Requested size is too large to fit into size_t.";
-    if (arena == NULL) {
+    if (arena == nullptr) {
       return static_cast<T*>(::operator new[](num_elements * sizeof(T)));
     } else {
       return arena->CreateInternalRawArray<T>(num_elements);
@@ -386,7 +386,7 @@ class PROTOBUF_EXPORT PROTOBUF_ALIGNAS(8) Arena final {
   // arena-allocated memory.
   template <typename T>
   PROTOBUF_ALWAYS_INLINE void OwnDestructor(T* object) {
-    if (object != NULL) {
+    if (object != nullptr) {
       impl_.AddCleanup(object, &internal::cleanup::arena_destruct_object<T>);
     }
   }
@@ -401,9 +401,9 @@ class PROTOBUF_EXPORT PROTOBUF_ALIGNAS(8) Arena final {
   }
 
   // Retrieves the arena associated with |value| if |value| is an arena-capable
-  // message, or NULL otherwise. If possible, the call resolves at compile time.
-  // Note that we can often devirtualize calls to `value->GetArena()` so usually
-  // calling this method is unnecessary.
+  // message, or nullptr otherwise. If possible, the call resolves at compile
+  // time. Note that we can often devirtualize calls to `value->GetArena()` so
+  // usually calling this method is unnecessary.
   template <typename T>
   PROTOBUF_ALWAYS_INLINE static Arena* GetArena(const T* value) {
     return GetArenaInternal(value);
@@ -568,7 +568,7 @@ class PROTOBUF_EXPORT PROTOBUF_ALIGNAS(8) Arena final {
     static_assert(
         InternalHelper<T>::is_arena_constructable::value,
         "CreateMessage can only construct types that are ArenaConstructable");
-    if (arena == NULL) {
+    if (arena == nullptr) {
       return new T(nullptr, static_cast<Args&&>(args)...);
     } else {
       return arena->DoCreateMessage<T>(static_cast<Args&&>(args)...);
@@ -583,7 +583,7 @@ class PROTOBUF_EXPORT PROTOBUF_ALIGNAS(8) Arena final {
     static_assert(
         InternalHelper<T>::is_arena_constructable::value,
         "CreateMessage can only construct types that are ArenaConstructable");
-    if (arena == NULL) {
+    if (arena == nullptr) {
       // Generated arena constructor T(Arena*) is protected. Call via
       // InternalHelper.
       return InternalHelper<T>::New();
@@ -730,13 +730,13 @@ class PROTOBUF_EXPORT PROTOBUF_ALIGNAS(8) Arena final {
   // using the virtual destructor instead.
   template <typename T>
   PROTOBUF_ALWAYS_INLINE void OwnInternal(T* object, std::true_type) {
-    if (object != NULL) {
+    if (object != nullptr) {
       impl_.AddCleanup(object, &internal::arena_delete_object<MessageLite>);
     }
   }
   template <typename T>
   PROTOBUF_ALWAYS_INLINE void OwnInternal(T* object, std::false_type) {
-    if (object != NULL) {
+    if (object != nullptr) {
       impl_.AddCleanup(object, &internal::arena_delete_object<T>);
     }
   }