Merge pull request #9576 from acozzette/sync-stage

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

CHANGES.txt

@@ -51,6 +51,7 @@ Unreleased Changes (C++/Java/Python/PHP/Objective-C/C#/Ruby/JavaScript)
   * Add "ensure_ascii" parameter to json_format.MessageToJson. This allows smaller
     JSON serializations with UTF-8 or other non-ASCII encodings.
   * Added experimental support for directly assigning numpy scalars and array.
+  * Improve the calculation of public_dependencies in DescriptorPool.
 
   Compiler
   * Migrate IsDefault(const std::string*) and UnsafeSetDefault(const std::string*)

Makefile.am

@@ -1037,7 +1037,10 @@ python_EXTRA_DIST=                                                           \
   python/google/protobuf/internal/factory_test2.proto                        \
   python/google/protobuf/internal/file_options_test.proto                    \
   python/google/protobuf/internal/generator_test.py                          \
+  python/google/protobuf/internal/import_test.py                             \
   python/google/protobuf/internal/import_test_package/__init__.py            \
+  python/google/protobuf/internal/import_test_package/import_public.proto    \
+  python/google/protobuf/internal/import_test_package/import_public_nested.proto \
   python/google/protobuf/internal/import_test_package/inner.proto            \
   python/google/protobuf/internal/import_test_package/outer.proto            \
   python/google/protobuf/internal/json_format_test.py                        \

java/core/src/main/java/com/google/protobuf/TextFormat.java

@@ -1554,10 +1554,6 @@ public final class TextFormat {
   public static class Parser {
     private int debugStringSilentMarker;
 
-    int getSilentMarkerCount() {
-      return debugStringSilentMarker;
-    }
-
     /**
      * A valid silent marker appears between a field name and its value. If there is a ":" in
      * between, the silent marker will only appear after the colon. This is called after a field

java/util/BUILD

@@ -68,6 +68,7 @@ junit_tests(
         ":util",
         "//java/core",
         "//java/core:generic_test_protos_java_proto",
+        "@maven//:com_google_code_gson_gson",
         "@maven//:com_google_guava_guava",
         "@maven//:com_google_truth_truth",
         "@maven//:junit_junit",

java/util/src/main/java/com/google/protobuf/util/JsonFormat.java

@@ -1319,11 +1319,11 @@ public class JsonFormat {
         if (e.getCause() instanceof IOException) {
           throw (IOException) e.getCause();
         } else {
-          throw new InvalidProtocolBufferException(e.getMessage());
+          throw new InvalidProtocolBufferException(e.getMessage(), e);
         }
       } catch (RuntimeException e) {
         // We convert all exceptions from JSON parsing to our own exceptions.
-        throw new InvalidProtocolBufferException(e.getMessage());
+        throw new InvalidProtocolBufferException(e.getMessage(), e);
       }
     }
 

java/util/src/test/java/com/google/protobuf/util/JsonFormatTest.java

@@ -34,6 +34,7 @@ import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth.assertWithMessage;
 
 import com.google.common.collect.ImmutableSet;
+import com.google.gson.JsonSyntaxException;
 import com.google.protobuf.Any;
 import com.google.protobuf.BoolValue;
 import com.google.protobuf.ByteString;
@@ -1861,7 +1862,7 @@ public class JsonFormatTest {
 
   // Test that we are not leaking out JSON exceptions.
   @Test
-  public void testJsonException() throws Exception {
+  public void testJsonException_forwardsIOException() throws Exception {
     InputStream throwingInputStream =
         new InputStream() {
           @Override
@@ -1879,7 +1880,10 @@ public class JsonFormatTest {
     } catch (IOException e) {
       assertThat(e).hasMessageThat().isEqualTo("12345");
     }
+  }
 
+  @Test
+  public void testJsonException_forwardsJsonException() throws Exception {
     Reader invalidJsonReader = new StringReader("{ xxx - yyy }");
     // When the JSON parser throws parser exceptions, JsonFormat should turn
     // that into InvalidProtocolBufferException.
@@ -1888,7 +1892,7 @@ public class JsonFormatTest {
       JsonFormat.parser().merge(invalidJsonReader, builder);
       assertWithMessage("Exception is expected.").fail();
     } catch (InvalidProtocolBufferException e) {
-      // Expected.
+      assertThat(e.getCause()).isInstanceOf(JsonSyntaxException.class);
     }
   }
 

python/google/protobuf/descriptor_pool.py

@@ -1235,21 +1235,25 @@ class DescriptorPool(object):
       for enum in desc.enum_types:
         yield (_PrefixWithDot(enum.full_name), enum)
 
-  def _GetDeps(self, dependencies):
+  def _GetDeps(self, dependencies, visited=None):
     """Recursively finds dependencies for file protos.
 
     Args:
       dependencies: The names of the files being depended on.
+      visited: The names of files already found.
 
     Yields:
       Each direct and indirect dependency.
     """
 
+    visited = visited or set()
     for dependency in dependencies:
-      dep_desc = self.FindFileByName(dependency)
-      yield dep_desc
-      for parent_dep in dep_desc.dependencies:
-        yield parent_dep
+      if dependency not in visited:
+        visited.add(dependency)
+        dep_desc = self.FindFileByName(dependency)
+        yield dep_desc
+        public_files = [d.name for d in dep_desc.public_dependencies]
+        yield from self._GetDeps(public_files, visited)
 
   def _GetTypeFromScope(self, package, type_name, scope):
     """Finds a given type name in the current scope.

python/google/protobuf/internal/import_test.py

@@ -0,0 +1,49 @@
+# -*- coding: utf-8 -*-
+# Protocol Buffers - Google's data interchange format
+# Copyright 2008 Google Inc.  All rights reserved.
+# https://developers.google.com/protocol-buffers/
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+#     * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above
+# copyright notice, this list of conditions and the following disclaimer
+# in the documentation and/or other materials provided with the
+# distribution.
+#     * Neither the name of Google Inc. nor the names of its
+# contributors may be used to endorse or promote products derived from
+# this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+"""Unittest for nested public imports."""
+
+import unittest
+
+from google.protobuf.internal.import_test_package import outer_pb2
+
+
+class ImportTest(unittest.TestCase):
+
+  def testPackageInitializationImport(self):
+    """Test that we can import nested import public messages."""
+
+    msg = outer_pb2.Outer()
+    self.assertEqual(58, msg.import_public_nested.value)
+
+
+if __name__ == '__main__':
+  unittest.main()

python/google/protobuf/internal/import_test_package/import_public.proto

@@ -0,0 +1,40 @@
+// Protocol Buffers - Google's data interchange format
+// Copyright 2008 Google Inc.  All rights reserved.
+// https://developers.google.com/protocol-buffers/
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//     * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// A proto file which is imported by inner.proto to test public importing.
+
+syntax = "proto2";
+
+package google.protobuf.python.internal.import_test_package;
+
+option optimize_for = SPEED;
+
+// Test nested public import
+import public "google/protobuf/internal/import_test_package/import_public_nested.proto";

python/google/protobuf/internal/import_test_package/import_public_nested.proto

@@ -0,0 +1,40 @@
+// Protocol Buffers - Google's data interchange format
+// Copyright 2008 Google Inc.  All rights reserved.
+// https://developers.google.com/protocol-buffers/
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//     * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// A proto file which is imported by import_public.proto to test nested public
+// importing.
+
+syntax = "proto2";
+
+package google.protobuf.python.internal.import_test_package;
+
+message ImportPublicNestedMessage {
+  optional int32 value = 1 [default = 58];
+}

python/google/protobuf/internal/import_test_package/inner.proto

@@ -32,6 +32,9 @@ syntax = "proto2";
 
 package google.protobuf.python.internal.import_test_package;
 
+// Test public import
+import public "google/protobuf/internal/import_test_package/import_public.proto";
+
 message Inner {
   optional int32 value = 1 [default = 57];
 }

python/google/protobuf/internal/import_test_package/outer.proto

@@ -36,4 +36,5 @@ import "google/protobuf/internal/import_test_package/inner.proto";
 
 message Outer {
   optional Inner inner = 1;
+  optional ImportPublicNestedMessage import_public_nested = 2;
 }

python/google/protobuf/pyext/message.cc

@@ -593,6 +593,7 @@ bool CheckAndGetInteger(PyObject* arg, T* value) {
   if (std::numeric_limits<T>::min() == 0) {
     // Unsigned case.
     unsigned PY_LONG_LONG ulong_result = PyLong_AsUnsignedLongLong(arg_py_int);
+    Py_DECREF(arg_py_int);
     if (VerifyIntegerCastAndRange<T, unsigned PY_LONG_LONG>(arg,
                                                             ulong_result)) {
       *value = static_cast<T>(ulong_result);
@@ -601,6 +602,7 @@ bool CheckAndGetInteger(PyObject* arg, T* value) {
     }
   } else {
     // Signed case.
+    Py_DECREF(arg_py_int);
     PY_LONG_LONG long_result = PyLong_AsLongLong(arg);
     if (VerifyIntegerCastAndRange<T, PY_LONG_LONG>(arg, long_result)) {
       *value = static_cast<T>(long_result);

python/setup.py

@@ -115,6 +115,8 @@ def GenerateUnittestProtos():
   GenProto('google/protobuf/internal/factory_test1.proto', False)
   GenProto('google/protobuf/internal/factory_test2.proto', False)
   GenProto('google/protobuf/internal/file_options_test.proto', False)
+  GenProto('google/protobuf/internal/import_test_package/import_public.proto', False)
+  GenProto('google/protobuf/internal/import_test_package/import_public_nested.proto', False)
   GenProto('google/protobuf/internal/import_test_package/inner.proto', False)
   GenProto('google/protobuf/internal/import_test_package/outer.proto', False)
   GenProto('google/protobuf/internal/missing_enum_values.proto', False)

src/google/protobuf/compiler/cpp/cpp_helpers.h

@@ -522,6 +522,10 @@ inline std::string MakeVarintCachedSizeFieldName(const FieldDescriptor* field) {
   return StrCat("_", FieldName(field), "_cached_byte_size_");
 }
 
+// Note: A lot of libraries detect Any protos based on Descriptor::full_name()
+// while the two functions below use FileDescriptor::name(). In a sane world the
+// two approaches should be equivalent. But if you are dealing with descriptors
+// from untrusted sources, you might need to match semantics across libraries.
 bool IsAnyMessage(const FileDescriptor* descriptor, const Options& options);
 bool IsAnyMessage(const Descriptor* descriptor, const Options& options);
 

src/google/protobuf/compiler/cpp/cpp_message.h

@@ -200,7 +200,8 @@ class MessageGenerator {
   int max_has_bit_index_;
 
   // A map from field index to inlined_string index. For non-inlined-string
-  // fields, the element is -1.
+  // fields, the element is -1. If there is no inlined string in the message,
+  // this is empty.
   std::vector<int> inlined_string_indices_;
   // The count of inlined_string fields in the message.
   int max_inlined_string_index_;

src/google/protobuf/compiler/cpp/cpp_parse_function_generator.cc

@@ -295,15 +295,9 @@ TailCallTableInfo::TailCallTableInfo(
         "_fl::Offset{offsetof(", ClassName(descriptor), ", _oneof_case_)}"));
   }
 
-  int inlined_string_count = 0;
-  for (const FieldDescriptor* field : ordered_fields) {
-    if (IsString(field, options) && IsStringInlined(field, options)) {
-      ++inlined_string_count;
-    }
-  }
   // If this message has any inlined string fields, store the donation state
   // offset in the second auxiliary entry.
-  if (inlined_string_count > 0) {
+  if (!inlined_string_indices.empty()) {
     aux_entries.resize(2);  // pad if necessary
     aux_entries[1] =
         StrCat("_fl::Offset{offsetof(", ClassName(descriptor),

src/google/protobuf/compiler/importer.h

@@ -41,6 +41,7 @@
 #include <string>
 #include <utility>
 #include <vector>
+
 #include <google/protobuf/compiler/parser.h>
 #include <google/protobuf/descriptor.h>
 #include <google/protobuf/descriptor_database.h>

src/google/protobuf/compiler/parser.cc

@@ -46,11 +46,11 @@
 #include <google/protobuf/stubs/casts.h>
 #include <google/protobuf/stubs/logging.h>
 #include <google/protobuf/stubs/common.h>
-#include <google/protobuf/io/tokenizer.h>
-#include <google/protobuf/descriptor.h>
-#include <google/protobuf/wire_format.h>
 #include <google/protobuf/stubs/strutil.h>
+#include <google/protobuf/descriptor.h>
 #include <google/protobuf/descriptor.pb.h>
+#include <google/protobuf/io/tokenizer.h>
+#include <google/protobuf/wire_format.h>
 #include <google/protobuf/stubs/map_util.h>
 #include <google/protobuf/stubs/hash.h>
 

src/google/protobuf/compiler/parser.h

@@ -42,10 +42,10 @@
 #include <string>
 #include <utility>
 
-#include <google/protobuf/io/tokenizer.h>
 #include <google/protobuf/descriptor.h>
-#include <google/protobuf/repeated_field.h>
 #include <google/protobuf/descriptor.pb.h>
+#include <google/protobuf/io/tokenizer.h>
+#include <google/protobuf/repeated_field.h>
 
 // Must be included last.
 #include <google/protobuf/port_def.inc>

src/google/protobuf/extension_set.h

@@ -190,6 +190,7 @@ class PROTOBUF_EXPORT ExtensionSet {
  public:
   constexpr ExtensionSet();
   explicit ExtensionSet(Arena* arena);
+  ExtensionSet(ArenaInitialized, Arena* arena) : ExtensionSet(arena) {}
   ~ExtensionSet();
 
   // These are called at startup by protocol-compiler-generated code to

src/google/protobuf/generated_message_tctable_lite.cc

@@ -34,6 +34,7 @@
 #include <google/protobuf/extension_set.h>
 #include <google/protobuf/generated_message_tctable_decl.h>
 #include <google/protobuf/generated_message_tctable_impl.h>
+#include <google/protobuf/inlined_string_field.h>
 #include <google/protobuf/message_lite.h>
 #include <google/protobuf/parse_context.h>
 #include <google/protobuf/wire_format_lite.h>
@@ -1698,8 +1699,9 @@ const char* TcParser::MpString(PROTOBUF_TC_PARAM_DECL) {
       break;
     }
 
-    case field_layout::kRepIString:
-      break;  // note: skipped above
+    case field_layout::kRepIString: {
+      break;
+    }
   }
 
   if (ptr == nullptr || !is_valid) {

src/google/protobuf/map_field.h

@@ -502,6 +502,8 @@ class TypeDefinedMapFieldBase : public MapFieldBase {
   constexpr TypeDefinedMapFieldBase(ConstantInitialized tag)
       : MapFieldBase(tag) {}
   explicit TypeDefinedMapFieldBase(Arena* arena) : MapFieldBase(arena) {}
+  TypeDefinedMapFieldBase(ArenaInitialized, Arena* arena)
+      : TypeDefinedMapFieldBase(arena) {}
 
  protected:
   ~TypeDefinedMapFieldBase() {}
@@ -574,6 +576,7 @@ class MapField : public TypeDefinedMapFieldBase<Key, T> {
       : TypeDefinedMapFieldBase<Key, T>(tag), impl_() {}
   explicit MapField(Arena* arena)
       : TypeDefinedMapFieldBase<Key, T>(arena), impl_(arena) {}
+  MapField(ArenaInitialized, Arena* arena) : MapField(arena) {}
 
   // Implement MapFieldBase
   bool ContainsMapKey(const MapKey& map_key) const override;

src/google/protobuf/map_field_lite.h

@@ -70,6 +70,7 @@ class MapFieldLite {
 
   constexpr MapFieldLite() : map_() {}
   explicit MapFieldLite(Arena* arena) : map_(arena) {}
+  MapFieldLite(ArenaInitialized, Arena* arena) : MapFieldLite(arena) {}
 
 #ifdef NDEBUG
   void Destruct() { map_.~Map(); }

src/google/protobuf/port.h

@@ -58,12 +58,21 @@ inline void SizedArrayDelete(void* p, size_t size) {
 #endif
 }
 
-// Tag type used to invoke the constinit constructor overload of some classes.
-// Such constructors are internal implementation details of the library.
+// Tag type used to invoke the constinit constructor overload of classes
+// such as ArenaStringPtr and MapFieldBase. Such constructors are internal
+// implementation details of the library.
 struct ConstantInitialized {
   explicit ConstantInitialized() = default;
 };
 
+// Tag type used to invoke the arena constructor overload of classes such
+// as ExtensionSet and MapFieldLite in aggregate initialization. These
+// classes typically don't have move/copy constructors, which rules out
+// explicit initialization in pre-C++17.
+struct ArenaInitialized {
+  explicit ArenaInitialized() = default;
+};
+
 }  // namespace internal
 }  // namespace protobuf
 }  // namespace google