diff --git a/.github/workflows/codespell.yml b/.github/workflows/codespell.yml index 41dc715db2..2e5ee80cde 100644 --- a/.github/workflows/codespell.yml +++ b/.github/workflows/codespell.yml @@ -13,4 +13,4 @@ jobs: with: check_filenames: true skip: ./.git,./conformance/third_party,*.snk,*.pb,*.pb.cc,*.pb.h,./src/google/protobuf/testdata,./objectivec/Tests,./python/compatibility_tests/v2.5.0/tests/google/protobuf/internal,./.github/workflows/codespell.yml - ignore_words_list: "alow,alse,ba,cleare,copyable,cloneable,dedup,dur,errorprone,files',fo,fundementals,hel,importd,inout,leapyear,nd,nin,ois,ons,parseable,process',te,testof,ue,unparseable,wasn,wee,gae,keyserver,objext,od,optin,streem,sur" + ignore_words_list: "alow,alse,ba,cleare,copyable,cloneable,dedup,dur,errorprone,files',fo,fundementals,hel,importd,inout,leapyear,nd,nin,ois,ons,parseable,process',te,testof,ue,unparseable,wasn,wee,gae,keyserver,objext,od,optin,streem,sur,falsy" diff --git a/CHANGES.txt b/CHANGES.txt index 5f9d23912a..cd2fbcc42f 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,5 +1,4 @@ 2022-07-01 Unreleased version - C++ * cpp_generated_lib_linked support is removed in protoc * Reduced .pb.o object file size slightly by explicitly instantiating @@ -11,6 +10,8 @@ * proto2::MapPair is now an alias to std::pair. * Hide C++ RepeatedField::UnsafeArenaSwap * Use table-driven parser for reflection based objects. + * Update Map's InternalSwap() to take a pointer to the other Map. + * Add ARM-optimized Varint decoding functions. Kotlin * Suppress deprecation warnings in Kotlin generated code. @@ -25,6 +26,8 @@ Python * Changes ordering of printed fields in .pyi files from lexicographic to the same ordering found in the proto descriptor. + * Adds GeneratedCodeInfo annotations to python proto .pyi outputs as a base64 encoded docstring in the last line of the .pyi file for code analysis tools. + * Fix message factory's behavior in python cpp extension to return same message classes for same descriptor, even if the factories are different. Compiler * Print full path name of source .proto file on error diff --git a/cmake/conformance.cmake b/cmake/conformance.cmake index a1c1cae360..fa5c479bc6 100644 --- a/cmake/conformance.cmake +++ b/cmake/conformance.cmake @@ -48,9 +48,11 @@ target_include_directories( conformance_cpp PUBLIC ${protobuf_SOURCE_DIR}) +target_include_directories(conformance_test_runner PRIVATE ${ABSL_ROOT_DIR}) target_include_directories(conformance_cpp PRIVATE ${ABSL_ROOT_DIR}) target_link_libraries(conformance_test_runner ${protobuf_LIB_PROTOBUF}) +target_link_libraries(conformance_test_runner ${protobuf_ABSL_USED_TARGETS}) target_link_libraries(conformance_cpp ${protobuf_LIB_PROTOBUF}) target_link_libraries(conformance_cpp ${protobuf_ABSL_USED_TARGETS}) diff --git a/cmake/tests.cmake b/cmake/tests.cmake index 9dbab93092..8838b4c4de 100644 --- a/cmake/tests.cmake +++ b/cmake/tests.cmake @@ -143,6 +143,7 @@ set(test_plugin_files ) add_executable(test_plugin ${test_plugin_files}) +target_include_directories(test_plugin PRIVATE ${ABSL_ROOT_DIR}) target_link_libraries(test_plugin ${protobuf_LIB_PROTOC} ${protobuf_LIB_PROTOBUF} GTest::gmock) add_executable(lite-test ${protobuf_lite_test_files}) diff --git a/conformance/conformance_test.h b/conformance/conformance_test.h index 298e780f2b..f072888edf 100644 --- a/conformance/conformance_test.h +++ b/conformance/conformance_test.h @@ -124,7 +124,7 @@ class ForkPipeRunner : public ConformanceTestRunner { // class MyConformanceTestSuite : public ConformanceTestSuite { // public: // void RunSuiteImpl() { -// // INSERT ACTURAL TESTS. +// // INSERT ACTUAL TESTS. // } // }; // diff --git a/conformance/conformance_test_runner.cc b/conformance/conformance_test_runner.cc index d1ede2bd49..ee7f653f4d 100644 --- a/conformance/conformance_test_runner.cc +++ b/conformance/conformance_test_runner.cc @@ -73,10 +73,10 @@ using std::vector; #define STRINGIFY(x) #x #define TOSTRING(x) STRINGIFY(x) -#define GOOGLE_CHECK_SYSCALL(call) \ - if (call < 0) { \ +#define GOOGLE_CHECK_SYSCALL(call) \ + if (call < 0) { \ perror(#call " " __FILE__ ":" TOSTRING(__LINE__)); \ - exit(1); \ + exit(1); \ } namespace google { @@ -93,8 +93,7 @@ void ParseFailureList(const char *filename, for (string line; getline(infile, line);) { // Remove whitespace. - line.erase(std::remove_if(line.begin(), line.end(), ::isspace), - line.end()); + line.erase(std::remove_if(line.begin(), line.end(), ::isspace), line.end()); // Remove comments. line = line.substr(0, line.find("#")); @@ -106,8 +105,7 @@ void ParseFailureList(const char *filename, } void UsageError() { - fprintf(stderr, - "Usage: conformance-test-runner [options] \n"); + fprintf(stderr, "Usage: conformance-test-runner [options] \n"); fprintf(stderr, "\n"); fprintf(stderr, "Options:\n"); fprintf(stderr, @@ -122,8 +120,7 @@ void UsageError() { " --text_format_failure_list Use to specify list \n"); fprintf(stderr, " of tests that are expected to \n"); - fprintf(stderr, - " fail in the \n"); + fprintf(stderr, " fail in the \n"); fprintf(stderr, " text_format_conformance_suite. \n"); fprintf(stderr, @@ -139,18 +136,16 @@ void UsageError() { " this flag if you want to be\n"); fprintf(stderr, " strictly conforming to protobuf\n"); - fprintf(stderr, - " spec.\n"); + fprintf(stderr, " spec.\n"); fprintf(stderr, " --output_dir Directory to write\n" " output files.\n"); exit(1); } -void ForkPipeRunner::RunTest( - const std::string& test_name, - const std::string& request, - std::string* response) { +void ForkPipeRunner::RunTest(const std::string &test_name, + const std::string &request, + std::string *response) { if (child_pid_ < 0) { SpawnTestProgram(); } @@ -170,11 +165,9 @@ void ForkPipeRunner::RunTest( string error_msg; if (WIFEXITED(status)) { - StringAppendF(&error_msg, - "child exited, status=%d", WEXITSTATUS(status)); + StringAppendF(&error_msg, "child exited, status=%d", WEXITSTATUS(status)); } else if (WIFSIGNALED(status)) { - StringAppendF(&error_msg, - "child killed by signal %d", WTERMSIG(status)); + StringAppendF(&error_msg, "child killed by signal %d", WTERMSIG(status)); } GOOGLE_LOG(INFO) << error_msg; child_pid_ = -1; @@ -186,17 +179,17 @@ void ForkPipeRunner::RunTest( } response->resize(len); - CheckedRead(read_fd_, (void*)response->c_str(), len); + CheckedRead(read_fd_, (void *)response->c_str(), len); } -int ForkPipeRunner::Run( - int argc, char *argv[], const std::vector& suites) { +int ForkPipeRunner::Run(int argc, char *argv[], + const std::vector &suites) { if (suites.empty()) { fprintf(stderr, "No test suites found.\n"); return EXIT_FAILURE; } bool all_ok = true; - for (ConformanceTestSuite* suite : suites) { + for (ConformanceTestSuite *suite : suites) { string program; std::vector program_args; string failure_list_filename; @@ -216,7 +209,7 @@ int ForkPipeRunner::Run( suite->SetOutputDir(argv[arg]); } else if (argv[arg][0] == '-') { bool recognized_flag = false; - for (ConformanceTestSuite* suite : suites) { + for (ConformanceTestSuite *suite : suites) { if (strcmp(argv[arg], suite->GetFailureListFlagName().c_str()) == 0) { if (++arg == argc) UsageError(); recognized_flag = true; @@ -238,8 +231,8 @@ int ForkPipeRunner::Run( ForkPipeRunner runner(program, program_args); std::string output; - all_ok = all_ok && - suite->RunSuite(&runner, &output, failure_list_filename, &failure_list); + all_ok = all_ok && suite->RunSuite(&runner, &output, failure_list_filename, + &failure_list); fwrite(output.c_str(), 1, output.size(), stderr); } @@ -324,7 +317,7 @@ void ForkPipeRunner::CheckedWrite(int fd, const void *buf, size_t len) { bool ForkPipeRunner::TryRead(int fd, void *buf, size_t len) { size_t ofs = 0; while (len > 0) { - ssize_t bytes_read = read(fd, (char*)buf + ofs, len); + ssize_t bytes_read = read(fd, (char *)buf + ofs, len); if (bytes_read == 0) { GOOGLE_LOG(ERROR) << current_test_name_ << ": unexpected EOF from test program"; diff --git a/docs/cpp_build_systems.md b/docs/cpp_build_systems.md index e093f9a2d3..b7f827b872 100644 --- a/docs/cpp_build_systems.md +++ b/docs/cpp_build_systems.md @@ -111,7 +111,7 @@ cc_dist_library( ":a", ":b", ], - visbility = ["//visibility:public"], + visibility = ["//visibility:public"], ) # Note: the output below has been formatted for clarity: diff --git a/kokoro/windows/bazel/build.bat b/kokoro/windows/bazel/build.bat index b7b77a6a01..0b2ba9fc13 100644 --- a/kokoro/windows/bazel/build.bat +++ b/kokoro/windows/bazel/build.bat @@ -8,7 +8,7 @@ fsutil 8dot3name set 0 @rem TODO(b/241475022) Use docker to guarantee better stability. -@rem Reinstall Bazel due to corupt installation in kokoro. +@rem Reinstall Bazel due to corrupt installation in kokoro. bazel version choco install bazel -y -i bazel version diff --git a/objectivec/GPBCodedInputStream.h b/objectivec/GPBCodedInputStream.h index fbe5009c92..1886ccf61b 100644 --- a/objectivec/GPBCodedInputStream.h +++ b/objectivec/GPBCodedInputStream.h @@ -220,7 +220,7 @@ CF_EXTERN_C_END /** * Moves the limit to the given byte offset starting at the current location. * - * @exception GPBCodedInputStreamException If the requested bytes exceeed the + * @exception GPBCodedInputStreamException If the requested bytes exceed the * current limit. * * @param byteLimit The number of bytes to move the limit, offset to the current diff --git a/pkg/BUILD.bazel b/pkg/BUILD.bazel index f9111ce06c..6c04df7b9b 100644 --- a/pkg/BUILD.bazel +++ b/pkg/BUILD.bazel @@ -375,6 +375,7 @@ cc_dist_library( "//src/google/protobuf/io:io_win32", "//src/google/protobuf/io:printer", "//src/google/protobuf/io:tokenizer", + "//src/google/protobuf/io:zero_copy_sink", "//src/google/protobuf/stubs", "//src/google/protobuf/stubs:lite", "//src/google/protobuf/util:delimited_message_util", @@ -383,7 +384,6 @@ cc_dist_library( "//src/google/protobuf/util:json_util", "//src/google/protobuf/util:time_util", "//src/google/protobuf/util:type_resolver_util", - "//src/google/protobuf/util:zero_copy_sink", "//src/google/protobuf/util/internal:datapiece", "//src/google/protobuf/util/internal:default_value", "//src/google/protobuf/util/internal:field_mask_utility", diff --git a/protobuf.bzl b/protobuf.bzl index 2cb1cdd090..46a8493903 100644 --- a/protobuf.bzl +++ b/protobuf.bzl @@ -405,7 +405,7 @@ def internal_objc_proto_library( testonly = None, visibility = ["//visibility:public"], **kwargs): - """Bazel rule to create a Objective-C protobuf library from proto sourc + """Bazel rule to create a Objective-C protobuf library from proto source files NOTE: the rule is only an internal workaround to generate protos. The diff --git a/protobuf_deps.bzl b/protobuf_deps.bzl index 2fbe94bb2d..4a2d791c9f 100644 --- a/protobuf_deps.bzl +++ b/protobuf_deps.bzl @@ -115,6 +115,6 @@ def protobuf_deps(): _github_archive( name = "upb", repo = "https://github.com/protocolbuffers/upb", - commit = "63a4a7d74bcbc8c6b94e6b18e599ffc68c71b91c", - sha256 = "9c921e799c4a5446b7164368a6579e20121103646045cd93986ba5bb8b376b29", + commit = "470f06cccbf26f98dd2df7ddecf24a78f140fe11", + sha256 = "c3137f3da811142d33d2ad278d093152610d3a773b17839d272bca4b1a6e304b", ) diff --git a/python/google/protobuf/internal/message_factory_test.py b/python/google/protobuf/internal/message_factory_test.py index efba6194c0..3ab3b8ed37 100644 --- a/python/google/protobuf/internal/message_factory_test.py +++ b/python/google/protobuf/internal/message_factory_test.py @@ -118,6 +118,16 @@ class MessageFactoryTest(unittest.TestCase): 'google.protobuf.python.internal.Factory2Message')) self.assertTrue(hasattr(cls, 'additional_field')) + def testGetExistingPrototype(self): + factory = message_factory.MessageFactory() + # Get Existing Prototype should not create a new class. + cls = factory.GetPrototype( + descriptor=factory_test2_pb2.Factory2Message.DESCRIPTOR) + msg = factory_test2_pb2.Factory2Message() + self.assertIsInstance(msg, cls) + self.assertIsInstance(msg.factory_1_message, + factory_test1_pb2.Factory1Message) + def testGetMessages(self): # performed twice because multiple calls with the same input must be allowed for _ in range(2): diff --git a/python/google/protobuf/internal/message_test.py b/python/google/protobuf/internal/message_test.py index 0a6c0b1ab2..2b003a4799 100644 --- a/python/google/protobuf/internal/message_test.py +++ b/python/google/protobuf/internal/message_test.py @@ -2244,7 +2244,7 @@ class Proto3Test(unittest.TestCase): def testMapItems(self): # Map items used to have strange behaviors when use c extension. Because - # [] may reorder the map and invalidate any exsting iterators. + # [] may reorder the map and invalidate any existing iterators. # TODO(jieluo): Check if [] reordering the map is a bug or intended # behavior. msg = map_unittest_pb2.TestMap() diff --git a/python/google/protobuf/internal/text_format_test.py b/python/google/protobuf/internal/text_format_test.py index 8377076e99..99cf21f57c 100644 --- a/python/google/protobuf/internal/text_format_test.py +++ b/python/google/protobuf/internal/text_format_test.py @@ -38,7 +38,6 @@ import string import textwrap import unittest -import unittest.mock from google.protobuf import any_pb2 from google.protobuf import struct_pb2 @@ -2485,3 +2484,7 @@ class OptionalColonMessageToStringTest(unittest.TestCase): self.assertEqual('repeated_int32: [1]\n', output) + + +if __name__ == '__main__': + unittest.main() diff --git a/python/google/protobuf/message_factory.py b/python/google/protobuf/message_factory.py index 8d65204581..ce5b5a7f65 100644 --- a/python/google/protobuf/message_factory.py +++ b/python/google/protobuf/message_factory.py @@ -60,9 +60,6 @@ class MessageFactory(object): """Initializes a new factory.""" self.pool = pool or descriptor_pool.DescriptorPool() - # local cache of all classes built from protobuf descriptors - self._classes = {} - def GetPrototype(self, descriptor): """Obtains a proto2 message class based on the passed in descriptor. @@ -75,14 +72,11 @@ class MessageFactory(object): Returns: A class describing the passed in descriptor. """ - if descriptor not in self._classes: - result_class = self.CreatePrototype(descriptor) - # The assignment to _classes is redundant for the base implementation, but - # might avoid confusion in cases where CreatePrototype gets overridden and - # does not call the base implementation. - self._classes[descriptor] = result_class - return result_class - return self._classes[descriptor] + concrete_class = getattr(descriptor, '_concrete_class', None) + if concrete_class: + return concrete_class + result_class = self.CreatePrototype(descriptor) + return result_class def CreatePrototype(self, descriptor): """Builds a proto2 message class based on the passed in descriptor. @@ -107,16 +101,11 @@ class MessageFactory(object): '__module__': None, }) result_class._FACTORY = self # pylint: disable=protected-access - # Assign in _classes before doing recursive calls to avoid infinite - # recursion. - self._classes[descriptor] = result_class for field in descriptor.fields: if field.message_type: self.GetPrototype(field.message_type) for extension in result_class.DESCRIPTOR.extensions: - if extension.containing_type not in self._classes: - self.GetPrototype(extension.containing_type) - extended_class = self._classes[extension.containing_type] + extended_class = self.GetPrototype(extension.containing_type) extended_class.RegisterExtension(extension) if extension.message_type: self.GetPrototype(extension.message_type) @@ -152,9 +141,7 @@ class MessageFactory(object): # an error if they were different. for extension in file_desc.extensions_by_name.values(): - if extension.containing_type not in self._classes: - self.GetPrototype(extension.containing_type) - extended_class = self._classes[extension.containing_type] + extended_class = self.GetPrototype(extension.containing_type) extended_class.RegisterExtension(extension) if extension.message_type: self.GetPrototype(extension.message_type) diff --git a/python/google/protobuf/pyext/descriptor.cc b/python/google/protobuf/pyext/descriptor.cc index b434934ee1..ed094af924 100644 --- a/python/google/protobuf/pyext/descriptor.cc +++ b/python/google/protobuf/pyext/descriptor.cc @@ -490,6 +490,12 @@ static PyObject* GetConcreteClass(PyBaseDescriptor* self, void *closure) { GetDescriptorPool_FromPool( _GetDescriptor(self)->file()->pool())->py_message_factory, _GetDescriptor(self))); + + if (concrete_class == nullptr) { + PyErr_Clear(); + return nullptr; + } + Py_XINCREF(concrete_class); return concrete_class->AsPyObject(); } diff --git a/python/google/protobuf/symbol_database.py b/python/google/protobuf/symbol_database.py index fdcf8cf06c..ed5fce39e9 100644 --- a/python/google/protobuf/symbol_database.py +++ b/python/google/protobuf/symbol_database.py @@ -66,6 +66,9 @@ from google.protobuf import message_factory class SymbolDatabase(message_factory.MessageFactory): """A database of Python generated symbols.""" + # local cache of registered classes. + _classes = {} + def RegisterMessage(self, message): """Registers the given message type in the local database. diff --git a/python/google/protobuf/text_format.py b/python/google/protobuf/text_format.py index a812dc69cb..2b794287ae 100644 --- a/python/google/protobuf/text_format.py +++ b/python/google/protobuf/text_format.py @@ -862,6 +862,8 @@ class _Parser(object): except UnicodeDecodeError as e: raise self._StringParseError(e) tokenizer = Tokenizer(str_lines) + if message: + self.root_type = message.DESCRIPTOR.full_name while not tokenizer.AtEnd(): self._MergeField(tokenizer, message) @@ -881,7 +883,8 @@ class _Parser(object): type_url_prefix, packed_type_name = self._ConsumeAnyTypeUrl(tokenizer) tokenizer.Consume(']') tokenizer.TryConsume(':') - self._DetectSilentMarker(tokenizer) + self._DetectSilentMarker(tokenizer, + type_url_prefix + '/' + packed_type_name) if tokenizer.TryConsume('<'): expanded_any_end_token = '>' else: @@ -981,11 +984,11 @@ class _Parser(object): if field.cpp_type == descriptor.FieldDescriptor.CPPTYPE_MESSAGE: tokenizer.TryConsume(':') - self._DetectSilentMarker(tokenizer) + self._DetectSilentMarker(tokenizer, field.full_name) merger = self._MergeMessageField else: tokenizer.Consume(':') - self._DetectSilentMarker(tokenizer) + self._DetectSilentMarker(tokenizer, field.full_name) merger = self._MergeScalarField if (field.label == descriptor.FieldDescriptor.LABEL_REPEATED and @@ -1003,20 +1006,18 @@ class _Parser(object): else: # Proto field is unknown. assert (self.allow_unknown_extension or self.allow_unknown_field) - self._SkipFieldContents(tokenizer) + self._SkipFieldContents(tokenizer, name) # For historical reasons, fields may optionally be separated by commas or # semicolons. if not tokenizer.TryConsume(','): tokenizer.TryConsume(';') - - def _LogSilentMarker(self): pass - def _DetectSilentMarker(self, tokenizer): + def _DetectSilentMarker(self, tokenizer, field_name): if tokenizer.contains_silent_marker_before_current_token: - self._LogSilentMarker() + self._LogSilentMarker(field_name) def _ConsumeAnyTypeUrl(self, tokenizer): """Consumes a google.protobuf.Any type URL and returns the type name.""" @@ -1172,11 +1173,12 @@ class _Parser(object): else: setattr(message, field.name, value) - def _SkipFieldContents(self, tokenizer): + def _SkipFieldContents(self, tokenizer, field_name): """Skips over contents (value or message) of a field. Args: tokenizer: A tokenizer to parse the field name and values. + field_name: The field name currently being parsed. """ # Try to guess the type of this field. # If this field is not a message, there should be a ":" between the @@ -1186,13 +1188,13 @@ class _Parser(object): # to be a message or the input is ill-formed. if tokenizer.TryConsume( ':') and not tokenizer.LookingAt('{') and not tokenizer.LookingAt('<'): - self._DetectSilentMarker(tokenizer) + self._DetectSilentMarker(tokenizer, field_name) if tokenizer.LookingAt('['): self._SkipRepeatedFieldValue(tokenizer) else: self._SkipFieldValue(tokenizer) else: - self._DetectSilentMarker(tokenizer) + self._DetectSilentMarker(tokenizer, field_name) self._SkipFieldMessage(tokenizer) def _SkipField(self, tokenizer): @@ -1201,23 +1203,25 @@ class _Parser(object): Args: tokenizer: A tokenizer to parse the field name and values. """ + field_name = '' if tokenizer.TryConsume('['): # Consume extension or google.protobuf.Any type URL - tokenizer.ConsumeIdentifier() + field_name += '[' + tokenizer.ConsumeIdentifier() num_identifiers = 1 while tokenizer.TryConsume('.'): - tokenizer.ConsumeIdentifier() + field_name += '.' + tokenizer.ConsumeIdentifier() num_identifiers += 1 # This is possibly a type URL for an Any message. if num_identifiers == 3 and tokenizer.TryConsume('/'): - tokenizer.ConsumeIdentifier() + field_name += '/' + tokenizer.ConsumeIdentifier() while tokenizer.TryConsume('.'): - tokenizer.ConsumeIdentifier() + field_name += '.' + tokenizer.ConsumeIdentifier() tokenizer.Consume(']') + field_name += ']' else: - tokenizer.ConsumeIdentifierOrNumber() + field_name += tokenizer.ConsumeIdentifierOrNumber() - self._SkipFieldContents(tokenizer) + self._SkipFieldContents(tokenizer, field_name) # For historical reasons, fields may optionally be separated by commas or # semicolons. diff --git a/python/setup.py b/python/setup.py index b86886f03b..eee6c82ef4 100755 --- a/python/setup.py +++ b/python/setup.py @@ -297,6 +297,10 @@ if __name__ == '__main__': extra_objects = ['../bazel-bin/src/google/protobuf/libprotobuf.a'] else: extra_objects = ['../libprotobuf.a'] + extra_objects += ['../third_party/abseil-cpp/absl/synchronization/libabsl_synchronization.a'] + # Include all of these twice to eliminate order-dependence. + extra_objects += list(glob.iglob('../third_party/abseil-cpp/absl/**/*.a')) + extra_objects += list(glob.iglob('../third_party/abseil-cpp/absl/**/*.a')) else: libraries = ['protobuf'] if HasLibraryDirsOpt(): @@ -374,7 +378,7 @@ if __name__ == '__main__': Extension( 'google.protobuf.pyext._message', glob.glob('google/protobuf/pyext/*.cc'), - include_dirs=['.', '../src'], + include_dirs=['.', '../src', '../third_party/abseil-cpp'], libraries=libraries, extra_objects=extra_objects, extra_link_args=message_extra_link_args, diff --git a/src/file_lists.cmake b/src/file_lists.cmake index ee66fad4c6..f083729839 100644 --- a/src/file_lists.cmake +++ b/src/file_lists.cmake @@ -44,6 +44,7 @@ set(libprotobuf_srcs ${protobuf_SOURCE_DIR}/src/google/protobuf/io/printer.cc ${protobuf_SOURCE_DIR}/src/google/protobuf/io/strtod.cc ${protobuf_SOURCE_DIR}/src/google/protobuf/io/tokenizer.cc + ${protobuf_SOURCE_DIR}/src/google/protobuf/io/zero_copy_sink.cc ${protobuf_SOURCE_DIR}/src/google/protobuf/io/zero_copy_stream.cc ${protobuf_SOURCE_DIR}/src/google/protobuf/io/zero_copy_stream_impl.cc ${protobuf_SOURCE_DIR}/src/google/protobuf/io/zero_copy_stream_impl_lite.cc @@ -93,7 +94,6 @@ set(libprotobuf_srcs ${protobuf_SOURCE_DIR}/src/google/protobuf/util/message_differencer.cc ${protobuf_SOURCE_DIR}/src/google/protobuf/util/time_util.cc ${protobuf_SOURCE_DIR}/src/google/protobuf/util/type_resolver_util.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/util/zero_copy_sink.cc ${protobuf_SOURCE_DIR}/src/google/protobuf/wire_format.cc ${protobuf_SOURCE_DIR}/src/google/protobuf/wire_format_lite.cc ${protobuf_SOURCE_DIR}/src/google/protobuf/wrappers.pb.cc @@ -139,6 +139,7 @@ set(libprotobuf_hdrs ${protobuf_SOURCE_DIR}/src/google/protobuf/io/printer.h ${protobuf_SOURCE_DIR}/src/google/protobuf/io/strtod.h ${protobuf_SOURCE_DIR}/src/google/protobuf/io/tokenizer.h + ${protobuf_SOURCE_DIR}/src/google/protobuf/io/zero_copy_sink.h ${protobuf_SOURCE_DIR}/src/google/protobuf/io/zero_copy_stream.h ${protobuf_SOURCE_DIR}/src/google/protobuf/io/zero_copy_stream_impl.h ${protobuf_SOURCE_DIR}/src/google/protobuf/io/zero_copy_stream_impl_lite.h @@ -173,7 +174,6 @@ set(libprotobuf_hdrs ${protobuf_SOURCE_DIR}/src/google/protobuf/stubs/macros.h ${protobuf_SOURCE_DIR}/src/google/protobuf/stubs/map_util.h ${protobuf_SOURCE_DIR}/src/google/protobuf/stubs/mathutil.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/stubs/mutex.h ${protobuf_SOURCE_DIR}/src/google/protobuf/stubs/once.h ${protobuf_SOURCE_DIR}/src/google/protobuf/stubs/platform_macros.h ${protobuf_SOURCE_DIR}/src/google/protobuf/stubs/port.h @@ -216,7 +216,6 @@ set(libprotobuf_hdrs ${protobuf_SOURCE_DIR}/src/google/protobuf/util/time_util.h ${protobuf_SOURCE_DIR}/src/google/protobuf/util/type_resolver.h ${protobuf_SOURCE_DIR}/src/google/protobuf/util/type_resolver_util.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/util/zero_copy_sink.h ${protobuf_SOURCE_DIR}/src/google/protobuf/wire_format.h ${protobuf_SOURCE_DIR}/src/google/protobuf/wire_format_lite.h ${protobuf_SOURCE_DIR}/src/google/protobuf/wrappers.pb.h @@ -300,7 +299,6 @@ set(libprotobuf_lite_hdrs ${protobuf_SOURCE_DIR}/src/google/protobuf/stubs/macros.h ${protobuf_SOURCE_DIR}/src/google/protobuf/stubs/map_util.h ${protobuf_SOURCE_DIR}/src/google/protobuf/stubs/mathutil.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/stubs/mutex.h ${protobuf_SOURCE_DIR}/src/google/protobuf/stubs/once.h ${protobuf_SOURCE_DIR}/src/google/protobuf/stubs/platform_macros.h ${protobuf_SOURCE_DIR}/src/google/protobuf/stubs/port.h @@ -818,6 +816,7 @@ set(io_test_files ${protobuf_SOURCE_DIR}/src/google/protobuf/io/io_win32_unittest.cc ${protobuf_SOURCE_DIR}/src/google/protobuf/io/printer_unittest.cc ${protobuf_SOURCE_DIR}/src/google/protobuf/io/tokenizer_unittest.cc + ${protobuf_SOURCE_DIR}/src/google/protobuf/io/zero_copy_sink_test.cc ${protobuf_SOURCE_DIR}/src/google/protobuf/io/zero_copy_stream_unittest.cc ) @@ -848,7 +847,6 @@ set(util_test_files ${protobuf_SOURCE_DIR}/src/google/protobuf/util/message_differencer_unittest.cc ${protobuf_SOURCE_DIR}/src/google/protobuf/util/time_util_test.cc ${protobuf_SOURCE_DIR}/src/google/protobuf/util/type_resolver_util_test.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/util/zero_copy_sink_test.cc ) # //src/google/protobuf/util:test_proto_srcs diff --git a/src/google/protobuf/BUILD.bazel b/src/google/protobuf/BUILD.bazel index ffcf9e6b8e..7570a4528f 100644 --- a/src/google/protobuf/BUILD.bazel +++ b/src/google/protobuf/BUILD.bazel @@ -128,6 +128,7 @@ cc_library( ], deps = [ "//src/google/protobuf/stubs:lite", + "@com_google_absl//absl/synchronization", ], ) @@ -197,6 +198,7 @@ cc_library( ":arena", "//src/google/protobuf/io", "//src/google/protobuf/stubs:lite", + "@com_google_absl//absl/synchronization", ], ) @@ -279,6 +281,7 @@ cc_library( "//src/google/protobuf/io:printer", "//src/google/protobuf/io:tokenizer", "//src/google/protobuf/stubs", + "@com_google_absl//absl/synchronization", ], ) @@ -1033,6 +1036,7 @@ cc_test( "//src/google/protobuf/io", "//src/google/protobuf/stubs", "//src/google/protobuf/testing", + "@com_google_absl//absl/synchronization", "@com_google_googletest//:gtest", "@com_google_googletest//:gtest_main", ], diff --git a/src/google/protobuf/arena.cc b/src/google/protobuf/arena.cc index 30fdac5afc..8045c4b76e 100644 --- a/src/google/protobuf/arena.cc +++ b/src/google/protobuf/arena.cc @@ -37,11 +37,12 @@ #include #include +#include "absl/synchronization/mutex.h" #include #include #include -#include + #ifdef ADDRESS_SANITIZER #include #endif // ADDRESS_SANITIZER diff --git a/src/google/protobuf/arena_unittest.cc b/src/google/protobuf/arena_unittest.cc index 1466376ed7..1cbfeaad52 100644 --- a/src/google/protobuf/arena_unittest.cc +++ b/src/google/protobuf/arena_unittest.cc @@ -58,6 +58,7 @@ #include #include +#include "absl/synchronization/mutex.h" // Must be included last #include diff --git a/src/google/protobuf/arenastring.cc b/src/google/protobuf/arenastring.cc index 8d7e6ff487..2f454c226d 100644 --- a/src/google/protobuf/arenastring.cc +++ b/src/google/protobuf/arenastring.cc @@ -31,11 +31,12 @@ #include #include + #include #include #include -#include #include +#include "absl/synchronization/mutex.h" #include #include #include @@ -71,7 +72,7 @@ static_assert(alignof(ExplicitlyConstructedArenaString) >= 4, ""); } // namespace const std::string& LazyString::Init() const { - static WrappedMutex mu{GOOGLE_PROTOBUF_LINKER_INITIALIZED}; + static absl::Mutex mu{absl::kConstInit}; mu.Lock(); const std::string* res = inited_.load(std::memory_order_acquire); if (res == nullptr) { diff --git a/src/google/protobuf/arenaz_sampler_test.cc b/src/google/protobuf/arenaz_sampler_test.cc index b49c8659ff..31ef0cd673 100644 --- a/src/google/protobuf/arenaz_sampler_test.cc +++ b/src/google/protobuf/arenaz_sampler_test.cc @@ -91,7 +91,7 @@ namespace { TEST(ThreadSafeArenaStatsTest, PrepareForSampling) { ThreadSafeArenaStats info; constexpr int64_t kTestStride = 107; - MutexLock l(&info.init_mu); + absl::MutexLock l(&info.init_mu); info.PrepareForSampling(kTestStride); for (const auto& block_stats : info.block_histogram) { @@ -161,7 +161,7 @@ TEST(ThreadSafeArenaStatsTest, MinMaxBlockSizeForBin) { TEST(ThreadSafeArenaStatsTest, RecordAllocateSlow) { ThreadSafeArenaStats info; constexpr int64_t kTestStride = 458; - MutexLock l(&info.init_mu); + absl::MutexLock l(&info.init_mu); info.PrepareForSampling(kTestStride); RecordAllocateSlow(&info, /*requested=*/0, /*allocated=*/128, /*wasted=*/0); EXPECT_EQ( @@ -193,7 +193,7 @@ TEST(ThreadSafeArenaStatsTest, RecordAllocateSlow) { TEST(ThreadSafeArenaStatsTest, RecordAllocateSlowMaxBlockSizeTest) { ThreadSafeArenaStats info; constexpr int64_t kTestStride = 458; - MutexLock l(&info.init_mu); + absl::MutexLock l(&info.init_mu); info.PrepareForSampling(kTestStride); RecordAllocateSlow(&info, /*requested=*/100, /*allocated=*/128, /*wasted=*/0); EXPECT_EQ(info.max_block_size.load(std::memory_order_relaxed), 128); diff --git a/src/google/protobuf/compiler/cpp/BUILD.bazel b/src/google/protobuf/compiler/cpp/BUILD.bazel index 44f08fec90..d03317737a 100644 --- a/src/google/protobuf/compiler/cpp/BUILD.bazel +++ b/src/google/protobuf/compiler/cpp/BUILD.bazel @@ -55,7 +55,10 @@ cc_library( deps = [ "//:protobuf", "//src/google/protobuf/compiler:code_generator", + "@com_google_absl//absl/base:core_headers", + "@com_google_absl//absl/container:flat_hash_map", "@com_google_absl//absl/container:flat_hash_set", + "@com_google_absl//absl/synchronization", ], ) diff --git a/src/google/protobuf/compiler/cpp/helpers.cc b/src/google/protobuf/compiler/cpp/helpers.cc index 91351596da..26d313a900 100644 --- a/src/google/protobuf/compiler/cpp/helpers.cc +++ b/src/google/protobuf/compiler/cpp/helpers.cc @@ -43,10 +43,13 @@ #include #include -#include "absl/container/flat_hash_set.h" #include #include #include +#include "absl/container/flat_hash_map.h" +#include "absl/container/flat_hash_set.h" +#include +#include "absl/synchronization/mutex.h" #include #include #include @@ -56,7 +59,6 @@ #include #include #include -#include #include #include @@ -177,7 +179,7 @@ static const char* const kKeywordList[] = { }; const absl::flat_hash_set& Keywords() { - static const auto* keywords = []{ + static const auto* keywords = [] { auto* keywords = new absl::flat_hash_set(); for (const auto keyword : kKeywordList) { @@ -1068,7 +1070,7 @@ bool ShouldVerify(const FileDescriptor* file, const Options& options, bool IsUtf8String(const FieldDescriptor* field) { return IsProto3(field->file()) && - field->type() == FieldDescriptor::TYPE_STRING; + field->type() == FieldDescriptor::TYPE_STRING; } VerifySimpleType ShouldVerifySimple(const Descriptor* descriptor) { @@ -1347,16 +1349,19 @@ bool GetBootstrapBasename(const Options& options, const std::string& basename, return false; } - std::unordered_map bootstrap_mapping{ - {"net/proto2/proto/descriptor", - "third_party/protobuf/descriptor"}, - {"net/proto2/compiler/proto/plugin", - "net/proto2/compiler/proto/plugin"}, - {"net/proto2/compiler/proto/profile", - "net/proto2/compiler/proto/profile_bootstrap"}, - }; - auto iter = bootstrap_mapping.find(basename); - if (iter == bootstrap_mapping.end()) { + static const auto* bootstrap_mapping = + // TODO(b/242858704) Replace these with string_view once we remove + // StringPiece. + new absl::flat_hash_map{ + {"net/proto2/proto/descriptor", + "third_party/protobuf/descriptor"}, + {"net/proto2/compiler/proto/plugin", + "net/proto2/compiler/proto/plugin"}, + {"net/proto2/compiler/proto/profile", + "net/proto2/compiler/proto/profile_bootstrap"}, + }; + auto iter = bootstrap_mapping->find(basename); + if (iter == bootstrap_mapping->end()) { *bootstrap_basename = basename; return false; } else { @@ -1492,9 +1497,18 @@ static bool HasExtensionFromFile(const Message& msg, const FileDescriptor* file, static bool HasBootstrapProblem(const FileDescriptor* file, const Options& options, bool* has_opt_codesize_extension) { - static auto& cache = *new std::unordered_map; - auto it = cache.find(file); - if (it != cache.end()) return it->second; + struct BoostrapGlobals { + absl::Mutex mutex; + absl::flat_hash_set cached ABSL_GUARDED_BY(mutex); + absl::flat_hash_set non_cached + ABSL_GUARDED_BY(mutex); + }; + static auto& bootstrap_cache = *new BoostrapGlobals(); + + absl::MutexLock lock(&bootstrap_cache.mutex); + if (bootstrap_cache.cached.contains(file)) return true; + if (bootstrap_cache.non_cached.contains(file)) return false; + // In order to build the data structures for the reflective parse, it needs // to parse the serialized descriptor describing all the messages defined in // this file. Obviously this presents a bootstrap problem for descriptor @@ -1530,9 +1544,13 @@ static bool HasBootstrapProblem(const FileDescriptor* file, Message* fd_proto = factory.GetPrototype(fd_proto_descriptor)->New(); fd_proto->ParseFromString(linkedin_fd_proto.SerializeAsString()); - bool& res = cache[file]; - res = HasExtensionFromFile(*fd_proto, file, options, - has_opt_codesize_extension); + bool res = HasExtensionFromFile(*fd_proto, file, options, + has_opt_codesize_extension); + if (res) { + bootstrap_cache.cached.insert(file); + } else { + bootstrap_cache.non_cached.insert(file); + } delete fd_proto; return res; } diff --git a/src/google/protobuf/compiler/cpp/parse_function_generator.cc b/src/google/protobuf/compiler/cpp/parse_function_generator.cc index 73c91332cb..fb238f3d74 100644 --- a/src/google/protobuf/compiler/cpp/parse_function_generator.cc +++ b/src/google/protobuf/compiler/cpp/parse_function_generator.cc @@ -1105,7 +1105,10 @@ void ParseFunctionGenerator::GenerateFieldBody( "$uint32$ val = ::$proto_ns$::internal::ReadVarint32(&ptr);\n" "CHK_(ptr);\n"); if (!internal::cpp::HasPreservingUnknownEnumSemantics(field)) { - format("if (PROTOBUF_PREDICT_TRUE($enum_type$_IsValid(static_cast(val)))) {\n"); + format( + "if " + "(PROTOBUF_PREDICT_TRUE($enum_type$_IsValid(static_cast(val)" + "))) {\n"); format.Indent(); } format("$msg$_internal_$put_field$(static_cast<$enum_type$>(val));\n"); diff --git a/src/google/protobuf/compiler/java/message.cc b/src/google/protobuf/compiler/java/message.cc index 943c7b32cb..855090ef32 100644 --- a/src/google/protobuf/compiler/java/message.cc +++ b/src/google/protobuf/compiler/java/message.cc @@ -296,7 +296,7 @@ void ImmutableMessageGenerator::GenerateInterface(io::Printer* printer) { for (auto oneof : oneofs_) { printer->Print( "\n" - "public $classname$.$oneof_capitalized_name$Case " + "$classname$.$oneof_capitalized_name$Case " "get$oneof_capitalized_name$Case();\n", "oneof_capitalized_name", context_->GetOneofGeneratorInfo(oneof)->capitalized_name, "classname", diff --git a/src/google/protobuf/compiler/python/BUILD.bazel b/src/google/protobuf/compiler/python/BUILD.bazel index d2d0ce12f7..a647f30da1 100644 --- a/src/google/protobuf/compiler/python/BUILD.bazel +++ b/src/google/protobuf/compiler/python/BUILD.bazel @@ -27,6 +27,7 @@ cc_library( deps = [ "//:protobuf", "//src/google/protobuf/compiler:code_generator", + "@com_google_absl//absl/synchronization", ], ) diff --git a/src/google/protobuf/compiler/python/generator.cc b/src/google/protobuf/compiler/python/generator.cc index 068b4fd12d..40c546b8b5 100644 --- a/src/google/protobuf/compiler/python/generator.cc +++ b/src/google/protobuf/compiler/python/generator.cc @@ -190,29 +190,42 @@ uint64_t Generator::GetSupportedFeatures() const { return CodeGenerator::Feature::FEATURE_PROTO3_OPTIONAL; } -bool Generator::Generate(const FileDescriptor* file, - const std::string& parameter, - GeneratorContext* context, std::string* error) const { - // ----------------------------------------------------------------- - // parse generator options - bool bootstrap = false; +GeneratorOptions Generator::ParseParameter(const std::string& parameter, + std::string* error) const { + GeneratorOptions options; - std::vector > options; - ParseGeneratorParameter(parameter, &options); + std::vector > option_pairs; + ParseGeneratorParameter(parameter, &option_pairs); - for (int i = 0; i < options.size(); i++) { + for (const std::pair& option : option_pairs) { if (!opensource_runtime_ && - options[i].first == "no_enforce_api_compatibility") { + option.first == "no_enforce_api_compatibility") { // TODO(b/241584880): remove this legacy option, it has no effect. - } else if (!opensource_runtime_ && options[i].first == "bootstrap") { - bootstrap = true; - } else if (options[i].first == "pyi_out") { - python::PyiGenerator pyi_generator; - if (!pyi_generator.Generate(file, "", context, error)) { - return false; - } + } else if (!opensource_runtime_ && option.first == "bootstrap") { + options.bootstrap = true; + } else if (option.first == "pyi_out") { + options.generate_pyi = true; + } else if (option.first == "annotate_code") { + options.annotate_pyi = true; } else { - *error = "Unknown generator option: " + options[i].first; + *error = "Unknown generator option: " + option.first; + } + } + return options; +} + +bool Generator::Generate(const FileDescriptor* file, + const std::string& parameter, + GeneratorContext* context, std::string* error) const { + // ----------------------------------------------------------------- + GeneratorOptions options = ParseParameter(parameter, error); + if (!error->empty()) return false; + + // Generate pyi typing information + if (options.generate_pyi) { + python::PyiGenerator pyi_generator; + std::string pyi_options = options.annotate_pyi ? "annotate_code" : ""; + if (!pyi_generator.Generate(file, pyi_options, context, error)) { return false; } } @@ -224,7 +237,7 @@ bool Generator::Generate(const FileDescriptor* file, // TODO(kenton): The proper thing to do would be to allocate any state on // the stack and use that, so that the Generator class itself does not need // to have any mutable members. Then it is implicitly thread-safe. - MutexLock lock(&mutex_); + absl::MutexLock lock(&mutex_); file_ = file; std::string filename = GetFileName(file, ".py"); @@ -236,7 +249,7 @@ bool Generator::Generate(const FileDescriptor* file, if (!opensource_runtime_ && GeneratingDescriptorProto()) { std::string bootstrap_filename = "net/proto2/python/internal/descriptor_pb2.py"; - if (bootstrap) { + if (options.bootstrap) { filename = bootstrap_filename; } else { std::unique_ptr output(context->Open(filename)); diff --git a/src/google/protobuf/compiler/python/generator.h b/src/google/protobuf/compiler/python/generator.h index ed077eb651..99ff9d7b35 100644 --- a/src/google/protobuf/compiler/python/generator.h +++ b/src/google/protobuf/compiler/python/generator.h @@ -37,7 +37,7 @@ #include -#include +#include "absl/synchronization/mutex.h" #include // Must be included last. @@ -64,6 +64,13 @@ namespace python { // If you create your own protocol compiler binary and you want it to support // Python output, you can do so by registering an instance of this // CodeGenerator with the CommandLineInterface in your main() function. + +struct GeneratorOptions { + bool generate_pyi = false; + bool annotate_pyi = false; + bool bootstrap = false; +}; + class PROTOC_EXPORT Generator : public CodeGenerator { public: Generator(); @@ -81,6 +88,8 @@ class PROTOC_EXPORT Generator : public CodeGenerator { } private: + GeneratorOptions ParseParameter(const std::string& parameter, + std::string* error) const; void PrintImports() const; void PrintFileDescriptor() const; void PrintAllNestedEnumsInFile() const; @@ -173,7 +182,7 @@ class PROTOC_EXPORT Generator : public CodeGenerator { // Very coarse-grained lock to ensure that Generate() is reentrant. // Guards file_, printer_ and file_descriptor_serialized_. - mutable Mutex mutex_; + mutable absl::Mutex mutex_; mutable const FileDescriptor* file_; // Set in Generate(). Under mutex_. mutable std::string file_descriptor_serialized_; mutable io::Printer* printer_; // Set in Generate(). Under mutex_. diff --git a/src/google/protobuf/compiler/python/helpers.cc b/src/google/protobuf/compiler/python/helpers.cc index e4d3c1385f..303072d521 100644 --- a/src/google/protobuf/compiler/python/helpers.cc +++ b/src/google/protobuf/compiler/python/helpers.cc @@ -101,6 +101,12 @@ bool HasGenericServices(const FileDescriptor* file) { return file->service_count() > 0 && file->options().py_generic_services(); } +std::string GeneratedCodeToBase64(const GeneratedCodeInfo& annotations) { + std::string result; + Base64Escape(annotations.SerializeAsString(), &result); + return result; +} + template std::string NamePrefixedWithNestedTypes(const DescriptorT& descriptor, const std::string& separator) { diff --git a/src/google/protobuf/compiler/python/helpers.h b/src/google/protobuf/compiler/python/helpers.h index a68ceb1969..8120a6efa4 100644 --- a/src/google/protobuf/compiler/python/helpers.h +++ b/src/google/protobuf/compiler/python/helpers.h @@ -34,6 +34,7 @@ #include #include +#include namespace google { namespace protobuf { @@ -49,6 +50,7 @@ std::string ResolveKeyword(const std::string& name); std::string GetFileName(const FileDescriptor* file_des, const std::string& suffix); bool HasGenericServices(const FileDescriptor* file); +std::string GeneratedCodeToBase64(const GeneratedCodeInfo& annotations); template std::string NamePrefixedWithNestedTypes(const DescriptorT& descriptor, diff --git a/src/google/protobuf/compiler/python/pyi_generator.cc b/src/google/protobuf/compiler/python/pyi_generator.cc index c542b10def..ee6044dce9 100644 --- a/src/google/protobuf/compiler/python/pyi_generator.cc +++ b/src/google/protobuf/compiler/python/pyi_generator.cc @@ -31,6 +31,7 @@ #include #include +#include #include #include @@ -283,12 +284,21 @@ void PyiGenerator::PrintImports() const { printer_->Print("\n"); } +// Annotate wrapper for debugging purposes +// Print a message after Annotate to see what is annotated. +template +void PyiGenerator::Annotate(const std::string& label, + const DescriptorT* descriptor) const { +printer_->Annotate(label.c_str(), descriptor); +} + void PyiGenerator::PrintEnum(const EnumDescriptor& enum_descriptor) const { std::string enum_name = enum_descriptor.name(); printer_->Print( "class $enum_name$(int, metaclass=_enum_type_wrapper.EnumTypeWrapper):\n" " __slots__ = []\n", "enum_name", enum_name); + Annotate("enum_name", &enum_descriptor); } void PyiGenerator::PrintEnumValues( @@ -300,6 +310,7 @@ void PyiGenerator::PrintEnumValues( printer_->Print("$name$: $module_enum_name$\n", "name", value_descriptor->name(), "module_enum_name", module_enum_name); + Annotate("name", value_descriptor); } } @@ -320,6 +331,7 @@ void PyiGenerator::PrintExtensions(const DescriptorT& descriptor) const { "constant_name", constant_name); printer_->Print("$name$: _descriptor.FieldDescriptor\n", "name", extension_field->name()); + Annotate("name", extension_field); } } @@ -379,6 +391,7 @@ void PyiGenerator::PrintMessage( } printer_->Print("class $class_name$(_message.Message$extra_base$):\n", "class_name", class_name, "extra_base", extra_base); + Annotate("class_name", &message_descriptor); printer_->Indent(); printer_->Indent(); @@ -454,6 +467,7 @@ void PyiGenerator::PrintMessage( } printer_->Print("$name$: $type$\n", "name", field_des.name(), "type", field_type); + Annotate("name", &field_des); } // Prints __init__ @@ -546,18 +560,39 @@ bool PyiGenerator::Generate(const FileDescriptor* file, const std::string& parameter, GeneratorContext* context, std::string* error) const { - MutexLock lock(&mutex_); + absl::MutexLock lock(&mutex_); import_map_.clear(); // Calculate file name. file_ = file; // In google3, devtools/python/blaze/pytype/pytype_impl.bzl uses --pyi_out to // directly set the output file name. - std::string filename = - parameter.empty() ? GetFileName(file, ".pyi") : parameter; + std::vector > options; + ParseGeneratorParameter(parameter, &options); + + std::string filename; + bool annotate_code = false; + for (const std::pair& option : options) { + if (option.first == "annotate_code") { + annotate_code = true; + } else if (HasSuffixString(option.first, ".pyi")) { + filename = option.first; + } else { + *error = "Unknown generator option: " + option.first; + return false; + } + } + + if (filename.empty()) { + filename = GetFileName(file, ".pyi"); + } std::unique_ptr output(context->Open(filename)); GOOGLE_CHECK(output.get()); - io::Printer printer(output.get(), '$'); + GeneratedCodeInfo annotations; + io::AnnotationProtoCollector annotation_collector( + &annotations); + io::Printer printer(output.get(), '$', + annotate_code ? &annotation_collector : nullptr); printer_ = &printer; PrintImports(); diff --git a/src/google/protobuf/compiler/python/pyi_generator.h b/src/google/protobuf/compiler/python/pyi_generator.h index be9d8a69cd..872e412c21 100644 --- a/src/google/protobuf/compiler/python/pyi_generator.h +++ b/src/google/protobuf/compiler/python/pyi_generator.h @@ -39,7 +39,7 @@ #include #include -#include +#include "absl/synchronization/mutex.h" #include // Must be included last. @@ -77,6 +77,8 @@ class PROTOC_EXPORT PyiGenerator : public google::protobuf::compiler::CodeGenera private: void PrintImportForDescriptor(const FileDescriptor& desc, std::set* seen_aliases) const; + template + void Annotate(const std::string& label, const DescriptorT* descriptor) const; void PrintImports() const; void PrintTopLevelEnums() const; void PrintEnum(const EnumDescriptor& enum_descriptor) const; @@ -97,7 +99,7 @@ class PROTOC_EXPORT PyiGenerator : public google::protobuf::compiler::CodeGenera // Very coarse-grained lock to ensure that Generate() is reentrant. // Guards file_, printer_, and import_map_. - mutable Mutex mutex_; + mutable absl::Mutex mutex_; mutable const FileDescriptor* file_; // Set in Generate(). Under mutex_. mutable io::Printer* printer_; // Set in Generate(). Under mutex_. // import_map will be a mapping from filename to module alias, e.g. diff --git a/src/google/protobuf/descriptor.cc b/src/google/protobuf/descriptor.cc index cbc9660cef..fb6185349c 100644 --- a/src/google/protobuf/descriptor.cc +++ b/src/google/protobuf/descriptor.cc @@ -60,6 +60,7 @@ #include #include #include +#include "absl/synchronization/mutex.h" #include #include #include @@ -1221,7 +1222,7 @@ class FileDescriptorTables { // Mutex to protect the unknown-enum-value map due to dynamic // EnumValueDescriptor creation on unknown values. - mutable internal::WrappedMutex unknown_enum_values_mu_; + mutable absl::Mutex unknown_enum_values_mu_; }; namespace internal { @@ -1523,13 +1524,13 @@ Symbol DescriptorPool::Tables::FindByNameHelper(const DescriptorPool* pool, StringPiece name) { if (pool->mutex_ != nullptr) { // Fast path: the Symbol is already cached. This is just a hash lookup. - ReaderMutexLock lock(pool->mutex_); + absl::ReaderMutexLock lock(pool->mutex_); if (known_bad_symbols_.empty() && known_bad_files_.empty()) { Symbol result = FindSymbol(name); if (!result.IsNull()) return result; } } - MutexLockMaybe lock(pool->mutex_); + absl::MutexLockMaybe lock(pool->mutex_); if (pool->fallback_database_ != nullptr) { known_bad_symbols_.clear(); known_bad_files_.clear(); @@ -1674,7 +1675,7 @@ FileDescriptorTables::FindEnumValueByNumberCreatingIfUnknown( // Second try, with reader lock held on unknown enum values: common case. { - ReaderMutexLock l(&unknown_enum_values_mu_); + absl::ReaderMutexLock l(&unknown_enum_values_mu_); auto it = unknown_enum_values_by_number_.find(query); if (it != unknown_enum_values_by_number_.end() && it->enum_value_descriptor() != nullptr) { @@ -1684,7 +1685,7 @@ FileDescriptorTables::FindEnumValueByNumberCreatingIfUnknown( // If not found, try again with writer lock held, and create new descriptor if // necessary. { - WriterMutexLock l(&unknown_enum_values_mu_); + absl::WriterMutexLock l(&unknown_enum_values_mu_); auto it = unknown_enum_values_by_number_.find(query); if (it != unknown_enum_values_by_number_.end() && it->enum_value_descriptor() != nullptr) { @@ -1705,7 +1706,7 @@ FileDescriptorTables::FindEnumValueByNumberCreatingIfUnknown( { // Must lock the pool because we will do allocations in the shared arena. - MutexLockMaybe l2(pool->mutex_); + absl::MutexLockMaybe l2(pool->mutex_); alloc.FinalizePlanning(tables); } EnumValueDescriptor* result = alloc.AllocateArray(1); @@ -1874,7 +1875,7 @@ DescriptorPool::DescriptorPool() DescriptorPool::DescriptorPool(DescriptorDatabase* fallback_database, ErrorCollector* error_collector) - : mutex_(new internal::WrappedMutex), + : mutex_(new absl::Mutex), fallback_database_(fallback_database), default_error_collector_(error_collector), underlay_(nullptr), @@ -1918,7 +1919,7 @@ void DescriptorPool::ClearUnusedImportTrackFiles() { } bool DescriptorPool::InternalIsFileLoaded(ConstStringParam filename) const { - MutexLockMaybe lock(mutex_); + absl::MutexLockMaybe lock(mutex_); return tables_->FindFile(filename) != nullptr; } @@ -1995,7 +1996,7 @@ void DescriptorPool::InternalAddGeneratedFile( const FileDescriptor* DescriptorPool::FindFileByName( ConstStringParam name) const { - MutexLockMaybe lock(mutex_); + absl::MutexLockMaybe lock(mutex_); if (fallback_database_ != nullptr) { tables_->known_bad_symbols_.clear(); tables_->known_bad_files_.clear(); @@ -2015,7 +2016,7 @@ const FileDescriptor* DescriptorPool::FindFileByName( const FileDescriptor* DescriptorPool::FindFileContainingSymbol( ConstStringParam symbol_name) const { - MutexLockMaybe lock(mutex_); + absl::MutexLockMaybe lock(mutex_); if (fallback_database_ != nullptr) { tables_->known_bad_symbols_.clear(); tables_->known_bad_files_.clear(); @@ -2092,13 +2093,13 @@ const FieldDescriptor* DescriptorPool::FindExtensionByNumber( // A faster path to reduce lock contention in finding extensions, assuming // most extensions will be cache hit. if (mutex_ != nullptr) { - ReaderMutexLock lock(mutex_); + absl::ReaderMutexLock lock(mutex_); const FieldDescriptor* result = tables_->FindExtension(extendee, number); if (result != nullptr) { return result; } } - MutexLockMaybe lock(mutex_); + absl::MutexLockMaybe lock(mutex_); if (fallback_database_ != nullptr) { tables_->known_bad_symbols_.clear(); tables_->known_bad_files_.clear(); @@ -2167,7 +2168,7 @@ const FieldDescriptor* DescriptorPool::FindExtensionByPrintableName( void DescriptorPool::FindAllExtensions( const Descriptor* extendee, std::vector* out) const { - MutexLockMaybe lock(mutex_); + absl::MutexLockMaybe lock(mutex_); if (fallback_database_ != nullptr) { tables_->known_bad_symbols_.clear(); tables_->known_bad_files_.clear(); @@ -4277,7 +4278,7 @@ Symbol DescriptorBuilder::FindSymbolNotEnforcingDepsHelper( const DescriptorPool* pool, const std::string& name, bool build_it) { // If we are looking at an underlay, we must lock its mutex_, since we are // accessing the underlay's tables_ directly. - MutexLockMaybe lock((pool == pool_) ? nullptr : pool->mutex_); + absl::MutexLockMaybe lock((pool == pool_) ? nullptr : pool->mutex_); Symbol result = pool->tables_->FindSymbol(name); if (result.IsNull() && pool->underlay_ != nullptr) { @@ -4463,7 +4464,7 @@ static bool ValidateQualifiedName(StringPiece name) { Symbol DescriptorPool::NewPlaceholder(StringPiece name, PlaceholderType placeholder_type) const { - MutexLockMaybe lock(mutex_); + absl::MutexLockMaybe lock(mutex_); return NewPlaceholderWithMutexHeld(name, placeholder_type); } @@ -4584,7 +4585,7 @@ Symbol DescriptorPool::NewPlaceholderWithMutexHeld( FileDescriptor* DescriptorPool::NewPlaceholderFile( StringPiece name) const { - MutexLockMaybe lock(mutex_); + absl::MutexLockMaybe lock(mutex_); internal::FlatAllocator alloc; alloc.PlanArray(1); alloc.PlanArray(1); diff --git a/src/google/protobuf/descriptor.h b/src/google/protobuf/descriptor.h index 653f734438..1f79e293c8 100644 --- a/src/google/protobuf/descriptor.h +++ b/src/google/protobuf/descriptor.h @@ -54,11 +54,8 @@ #ifndef GOOGLE_PROTOBUF_DESCRIPTOR_H__ #define GOOGLE_PROTOBUF_DESCRIPTOR_H__ -#include - -#include - #include +#include #include #include #include @@ -68,18 +65,15 @@ #include #include -#include #include #include +#include +#include "absl/synchronization/mutex.h" + // Must be included last. #include -// TYPE_BOOL is defined in the MacOS's ConditionalMacros.h. -#ifdef TYPE_BOOL -#undef TYPE_BOOL -#endif // TYPE_BOOL - #ifdef SWIG #define PROTOBUF_EXPORT #endif @@ -900,7 +894,7 @@ class PROTOBUF_EXPORT FieldDescriptor : private internal::SymbolBase { // formats the default value appropriately and returns it as a string. // Must have a default value to call this. If quote_string_type is true, then - // types of CPPTYPE_STRING whill be surrounded by quotes and CEscaped. + // types of CPPTYPE_STRING will be surrounded by quotes and CEscaped. std::string DefaultValueAsString(bool quote_string_type) const; // Helper function that returns the field type name for DebugString. @@ -2058,7 +2052,7 @@ class PROTOBUF_EXPORT DescriptorPool { // If fallback_database_ is nullptr, this is nullptr. Otherwise, this is a // mutex which must be locked while accessing tables_. - internal::WrappedMutex* mutex_; + absl::Mutex* mutex_; // See constructor. DescriptorDatabase* fallback_database_; diff --git a/src/google/protobuf/dynamic_message.cc b/src/google/protobuf/dynamic_message.cc index 9214de6f64..f839a491ba 100644 --- a/src/google/protobuf/dynamic_message.cc +++ b/src/google/protobuf/dynamic_message.cc @@ -637,7 +637,7 @@ DynamicMessageFactory::~DynamicMessageFactory() { } const Message* DynamicMessageFactory::GetPrototype(const Descriptor* type) { - MutexLock lock(&prototypes_mutex_); + absl::MutexLock lock(&prototypes_mutex_); return GetPrototypeNoLock(type); } diff --git a/src/google/protobuf/dynamic_message.h b/src/google/protobuf/dynamic_message.h index e318259e01..2e65ca9fbd 100644 --- a/src/google/protobuf/dynamic_message.h +++ b/src/google/protobuf/dynamic_message.h @@ -45,7 +45,7 @@ #include #include -#include +#include "absl/synchronization/mutex.h" #include #include #include @@ -136,7 +136,7 @@ class PROTOBUF_EXPORT DynamicMessageFactory : public MessageFactory { struct TypeInfo; std::unordered_map prototypes_; - mutable internal::WrappedMutex prototypes_mutex_; + mutable absl::Mutex prototypes_mutex_; friend class DynamicMessage; const Message* GetPrototypeNoLock(const Descriptor* type); diff --git a/src/google/protobuf/generated_message_reflection.cc b/src/google/protobuf/generated_message_reflection.cc index fab8f2eb59..aec6e1a00c 100644 --- a/src/google/protobuf/generated_message_reflection.cc +++ b/src/google/protobuf/generated_message_reflection.cc @@ -44,9 +44,9 @@ #include #include -#include #include #include +#include "absl/synchronization/mutex.h" #include #include #include @@ -80,7 +80,6 @@ using google::protobuf::internal::OnShutdownDelete; using google::protobuf::internal::ReflectionSchema; using google::protobuf::internal::RepeatedPtrFieldBase; using google::protobuf::internal::StringSpaceUsedExcludingSelfLong; -using google::protobuf::internal::WrappedMutex; namespace google { namespace protobuf { @@ -3431,7 +3430,7 @@ struct MetadataOwner { private: MetadataOwner() = default; // private because singleton - WrappedMutex mu_; + absl::Mutex mu_; std::vector > metadata_arrays_; }; @@ -3442,7 +3441,7 @@ void AssignDescriptorsImpl(const DescriptorTable* table, bool eager) { { // This only happens once per proto file. So a global mutex to serialize // calls to AddDescriptors. - static WrappedMutex mu{GOOGLE_PROTOBUF_LINKER_INITIALIZED}; + static absl::Mutex mu{absl::kConstInit}; mu.Lock(); AddDescriptors(table); mu.Unlock(); diff --git a/src/google/protobuf/generated_message_tctable_decl.h b/src/google/protobuf/generated_message_tctable_decl.h index b64767f09e..8959acb821 100644 --- a/src/google/protobuf/generated_message_tctable_decl.h +++ b/src/google/protobuf/generated_message_tctable_decl.h @@ -36,6 +36,7 @@ #define GOOGLE_PROTOBUF_GENERATED_MESSAGE_TCTABLE_DECL_H__ #include +#include #include #include #include @@ -175,9 +176,35 @@ struct alignas(uint64_t) TcParseTableBase { // Table entry for fast-path tailcall dispatch handling. struct FastFieldEntry { // Target function for dispatch: - TailCallParseFunc target; + mutable std::atomic target_atomic; + // Field data used during parse: TcFieldData bits; + + // Default initializes this instance with undefined values. + FastFieldEntry() = default; + + // Constant initializes this instance + constexpr FastFieldEntry(TailCallParseFunc func, TcFieldData bits) + : target_atomic(func), bits(bits) {} + + // FastFieldEntry is copy-able and assignable, which is intended + // mainly for testing and debugging purposes. + FastFieldEntry(const FastFieldEntry& rhs) noexcept + : FastFieldEntry(rhs.target(), rhs.bits) {} + FastFieldEntry& operator=(const FastFieldEntry& rhs) noexcept { + SetTarget(rhs.target()); + bits = rhs.bits; + return *this; + } + + // Protocol buffer code should use these relaxed accessors. + TailCallParseFunc target() const { + return target_atomic.load(std::memory_order_relaxed); + } + void SetTarget(TailCallParseFunc func) const { + return target_atomic.store(func, std::memory_order_relaxed); + } }; // There is always at least one table entry. const FastFieldEntry* fast_entry(size_t idx) const { diff --git a/src/google/protobuf/generated_message_tctable_impl.h b/src/google/protobuf/generated_message_tctable_impl.h index d1fbe108d5..965f1ed0c3 100644 --- a/src/google/protobuf/generated_message_tctable_impl.h +++ b/src/google/protobuf/generated_message_tctable_impl.h @@ -708,7 +708,7 @@ inline PROTOBUF_ALWAYS_INLINE const char* TcParser::TagDispatch( auto* fast_entry = table->fast_entry(idx >> 3); data = fast_entry->bits; data.data ^= coded_tag; - PROTOBUF_MUSTTAIL return fast_entry->target(PROTOBUF_TC_PARAM_PASS); + PROTOBUF_MUSTTAIL return fast_entry->target()(PROTOBUF_TC_PARAM_PASS); } // We can only safely call from field to next field if the call is optimized diff --git a/src/google/protobuf/io/BUILD.bazel b/src/google/protobuf/io/BUILD.bazel index bf0d87cbdf..9364ed3e95 100644 --- a/src/google/protobuf/io/BUILD.bazel +++ b/src/google/protobuf/io/BUILD.bazel @@ -31,6 +31,31 @@ cc_library( ], ) +cc_library( + name = "zero_copy_sink", + srcs = ["zero_copy_sink.cc"], + hdrs = ["zero_copy_sink.h"], + copts = COPTS, + strip_include_prefix = "/src", + deps = [ + ":io", + "//src/google/protobuf", + "//src/google/protobuf/stubs", + ], +) + +cc_test( + name = "zero_copy_sink_test", + srcs = ["zero_copy_sink_test.cc"], + copts = COPTS, + deps = [ + ":zero_copy_sink", + "//src/google/protobuf/stubs", + "@com_google_googletest//:gtest", + "@com_google_googletest//:gtest_main", + ], +) + cc_library( name = "printer", srcs = ["printer.cc"], diff --git a/src/google/protobuf/util/zero_copy_sink.cc b/src/google/protobuf/io/zero_copy_sink.cc similarity index 96% rename from src/google/protobuf/util/zero_copy_sink.cc rename to src/google/protobuf/io/zero_copy_sink.cc index 2b24f11238..e7809bd4ff 100644 --- a/src/google/protobuf/util/zero_copy_sink.cc +++ b/src/google/protobuf/io/zero_copy_sink.cc @@ -28,11 +28,11 @@ // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -#include +#include namespace google { namespace protobuf { -namespace util { +namespace io { namespace zc_sink_internal { void ZeroCopyStreamByteSink::Append(const char* bytes, size_t len) { while (true) { @@ -55,6 +55,6 @@ void ZeroCopyStreamByteSink::Append(const char* bytes, size_t len) { } } } // namespace zc_sink_internal -} // namespace util +} // namespace io } // namespace protobuf } // namespace google diff --git a/src/google/protobuf/util/zero_copy_sink.h b/src/google/protobuf/io/zero_copy_sink.h similarity index 95% rename from src/google/protobuf/util/zero_copy_sink.h rename to src/google/protobuf/io/zero_copy_sink.h index f2fd9d7867..07fb8fc6c1 100644 --- a/src/google/protobuf/util/zero_copy_sink.h +++ b/src/google/protobuf/io/zero_copy_sink.h @@ -42,9 +42,9 @@ namespace google { namespace protobuf { -namespace util { +namespace io { namespace zc_sink_internal { -// Internal helper class. Put in the header so we can write unit-tests for it. +// Internal helper class, for turning a ZeroCopyOutputStream into a sink. class PROTOBUF_EXPORT ZeroCopyStreamByteSink : public strings::ByteSink { public: explicit ZeroCopyStreamByteSink(io::ZeroCopyOutputStream* stream) @@ -66,7 +66,7 @@ class PROTOBUF_EXPORT ZeroCopyStreamByteSink : public strings::ByteSink { GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(ZeroCopyStreamByteSink); }; } // namespace zc_sink_internal -} // namespace util +} // namespace io } // namespace protobuf } // namespace google diff --git a/src/google/protobuf/util/zero_copy_sink_test.cc b/src/google/protobuf/io/zero_copy_sink_test.cc similarity index 98% rename from src/google/protobuf/util/zero_copy_sink_test.cc rename to src/google/protobuf/io/zero_copy_sink_test.cc index 9eed0fe160..da8fc05f23 100644 --- a/src/google/protobuf/util/zero_copy_sink_test.cc +++ b/src/google/protobuf/io/zero_copy_sink_test.cc @@ -28,7 +28,7 @@ // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -#include +#include #include #include @@ -43,7 +43,7 @@ namespace google { namespace protobuf { -namespace util { +namespace io { namespace zc_sink_internal { namespace { @@ -221,6 +221,6 @@ TEST_F(ZeroCopyStreamByteSinkTest, WriteLong) { } } // namespace } // namespace zc_sink_internal -} // namespace util +} // namespace io } // namespace protobuf } // namespace google diff --git a/src/google/protobuf/map.h b/src/google/protobuf/map.h index a3f79f1545..d19addacce 100644 --- a/src/google/protobuf/map.h +++ b/src/google/protobuf/map.h @@ -1435,7 +1435,7 @@ class Map { void swap(Map& other) { if (arena() == other.arena()) { - InternalSwap(other); + InternalSwap(&other); } else { // TODO(zuguang): optimize this. The temporary copy can be allocated // in the same arena as the other message, and the "other = copy" can @@ -1446,7 +1446,7 @@ class Map { } } - void InternalSwap(Map& other) { elements_.Swap(&other.elements_); } + void InternalSwap(Map* other) { elements_.Swap(&other->elements_); } // Access to hasher. Currently this returns a copy, but it may // be modified to return a const reference in the future. diff --git a/src/google/protobuf/map_field.h b/src/google/protobuf/map_field.h index 287d58f312..73681bbc99 100644 --- a/src/google/protobuf/map_field.h +++ b/src/google/protobuf/map_field.h @@ -35,8 +35,8 @@ #include #include -#include #include +#include "absl/synchronization/mutex.h" #include #include #include @@ -342,7 +342,7 @@ class PROTOBUF_EXPORT MapFieldBase { constexpr MapFieldBase(ConstantInitialized) : arena_(nullptr), repeated_field_(nullptr), - mutex_(GOOGLE_PROTOBUF_LINKER_INITIALIZED), + mutex_(absl::kConstInit), state_(STATE_MODIFIED_MAP) {} explicit MapFieldBase(Arena* arena) : arena_(arena), repeated_field_(nullptr), state_(STATE_MODIFIED_MAP) {} @@ -449,9 +449,8 @@ class PROTOBUF_EXPORT MapFieldBase { Arena* arena_; mutable RepeatedPtrField* repeated_field_; - mutable internal::WrappedMutex - mutex_; // The thread to synchronize map and repeated field - // needs to get lock first; + mutable absl::Mutex mutex_; // The thread to synchronize map and repeated + // field needs to get lock first; mutable std::atomic state_; private: diff --git a/src/google/protobuf/map_field_lite.h b/src/google/protobuf/map_field_lite.h index 53bf7a0811..ee028efdd5 100644 --- a/src/google/protobuf/map_field_lite.h +++ b/src/google/protobuf/map_field_lite.h @@ -83,7 +83,7 @@ class MapFieldLite { // data in it, as would happen if a vector was resize'd to zero. // Map::Swap with an empty map accomplishes that. decltype(map_) swapped_map(map_.arena()); - map_.InternalSwap(swapped_map); + map_.InternalSwap(&swapped_map); } ~MapFieldLite() { if (map_.arena() == nullptr && !map_.empty()) { @@ -105,7 +105,7 @@ class MapFieldLite { } } void Swap(MapFieldLite* other) { map_.swap(other->map_); } - void InternalSwap(MapFieldLite* other) { map_.InternalSwap(other->map_); } + void InternalSwap(MapFieldLite* other) { map_.InternalSwap(&other->map_); } // Used in the implementation of parsing. Caller should take the ownership iff // arena_ is nullptr. diff --git a/src/google/protobuf/message.cc b/src/google/protobuf/message.cc index c1cde4c5db..edd466624e 100644 --- a/src/google/protobuf/message.cc +++ b/src/google/protobuf/message.cc @@ -262,7 +262,7 @@ class GeneratedMessageFactory final : public MessageFactory { STR_HASH_FXN> file_map_; - internal::WrappedMutex mutex_; + absl::Mutex mutex_; // Initialized lazily, so requires locking. std::unordered_map type_map_; }; @@ -298,7 +298,7 @@ void GeneratedMessageFactory::RegisterType(const Descriptor* descriptor, const Message* GeneratedMessageFactory::GetPrototype(const Descriptor* type) { { - ReaderMutexLock lock(&mutex_); + absl::ReaderMutexLock lock(&mutex_); const Message* result = FindPtrOrNull(type_map_, type); if (result != nullptr) return result; } @@ -317,7 +317,7 @@ const Message* GeneratedMessageFactory::GetPrototype(const Descriptor* type) { return nullptr; } - WriterMutexLock lock(&mutex_); + absl::WriterMutexLock lock(&mutex_); // Check if another thread preempted us. const Message* result = FindPtrOrNull(type_map_, type); diff --git a/src/google/protobuf/message_lite.cc b/src/google/protobuf/message_lite.cc index 2674b2698e..b7e980bb3d 100644 --- a/src/google/protobuf/message_lite.cc +++ b/src/google/protobuf/message_lite.cc @@ -42,17 +42,19 @@ #include #include -#include #include #include #include #include #include +#include "absl/base/dynamic_annotations.h" #include +#include "absl/synchronization/mutex.h" #include +#include #include #include -#include + // Must be included last. #include @@ -567,7 +569,7 @@ struct ShutdownData { } std::vector> functions; - Mutex mutex; + absl::Mutex mutex; }; static void RunZeroArgFunc(const void* arg) { @@ -581,7 +583,7 @@ void OnShutdown(void (*func)()) { void OnShutdownRun(void (*f)(const void*), const void* arg) { auto shutdown_data = ShutdownData::get(); - MutexLock lock(&shutdown_data->mutex); + absl::MutexLock lock(&shutdown_data->mutex); shutdown_data->functions.push_back(std::make_pair(f, arg)); } diff --git a/src/google/protobuf/parse_context.h b/src/google/protobuf/parse_context.h index 51f2e326c7..c72e19fe0d 100644 --- a/src/google/protobuf/parse_context.h +++ b/src/google/protobuf/parse_context.h @@ -636,7 +636,7 @@ RotRight7AndReplaceLowByte(uint64_t res, const char& byte) { res |= 0xFF & byte; #endif return res; -} +}; inline PROTOBUF_ALWAYS_INLINE const char* ReadTagInlined(const char* ptr, uint32_t* out) { diff --git a/src/google/protobuf/port_def.inc b/src/google/protobuf/port_def.inc index 268aa74cde..5e034bae0f 100644 --- a/src/google/protobuf/port_def.inc +++ b/src/google/protobuf/port_def.inc @@ -945,6 +945,9 @@ #undef UID_MAX #pragma push_macro("GID_MAX") #undef GID_MAX +// TYPE_BOOL is defined in the MacOS's ConditionalMacros.h. +#pragma push_macro("TYPE_BOOL") +#undef TYPE_BOOL #endif // __APPLE__ #if defined(__clang__) || PROTOBUF_GNUC_MIN(3, 0) || defined(_MSC_VER) diff --git a/src/google/protobuf/port_undef.inc b/src/google/protobuf/port_undef.inc index b52ee72e91..788743a341 100644 --- a/src/google/protobuf/port_undef.inc +++ b/src/google/protobuf/port_undef.inc @@ -159,6 +159,7 @@ #pragma pop_macro("FALSE") #pragma pop_macro("UID_MAX") #pragma pop_macro("GID_MAX") +#pragma pop_macro("TYPE_BOOL") #endif // __APPLE__ #if defined(__clang__) || defined(__GNUC__) || defined(_MSC_VER) diff --git a/src/google/protobuf/stubs/BUILD.bazel b/src/google/protobuf/stubs/BUILD.bazel index 73def31f51..fbc19c282e 100644 --- a/src/google/protobuf/stubs/BUILD.bazel +++ b/src/google/protobuf/stubs/BUILD.bazel @@ -34,7 +34,6 @@ cc_library( "macros.h", "map_util.h", "mathutil.h", - "mutex.h", "once.h", "platform_macros.h", "port.h", @@ -75,7 +74,6 @@ cc_library( "macros.h", "map_util.h", "mathutil.h", - "mutex.h", "once.h", "platform_macros.h", "port.h", diff --git a/src/google/protobuf/stubs/mutex.h b/src/google/protobuf/stubs/mutex.h deleted file mode 100644 index c4599913be..0000000000 --- a/src/google/protobuf/stubs/mutex.h +++ /dev/null @@ -1,218 +0,0 @@ -// Copyright (c) 2006, Google Inc. -// All rights reserved. -// -// 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. - -#ifndef GOOGLE_PROTOBUF_STUBS_MUTEX_H_ -#define GOOGLE_PROTOBUF_STUBS_MUTEX_H_ - -#include - -#ifdef GOOGLE_PROTOBUF_SUPPORT_WINDOWS_XP - -#include - -// GetMessage conflicts with GeneratedMessageReflection::GetMessage(). -#ifdef GetMessage -#undef GetMessage -#endif - -#endif - -#include - -// Define thread-safety annotations for use below, if we are building with -// Clang. -#if defined(__clang__) && !defined(SWIG) -#define GOOGLE_PROTOBUF_ACQUIRE(...) \ - __attribute__((acquire_capability(__VA_ARGS__))) -#define GOOGLE_PROTOBUF_RELEASE(...) \ - __attribute__((release_capability(__VA_ARGS__))) -#define GOOGLE_PROTOBUF_SCOPED_CAPABILITY __attribute__((scoped_lockable)) -#define GOOGLE_PROTOBUF_CAPABILITY(x) __attribute__((capability(x))) -#else -#define GOOGLE_PROTOBUF_ACQUIRE(...) -#define GOOGLE_PROTOBUF_RELEASE(...) -#define GOOGLE_PROTOBUF_SCOPED_CAPABILITY -#define GOOGLE_PROTOBUF_CAPABILITY(x) -#endif - -#include - -// =================================================================== -// emulates google3/base/mutex.h -namespace google { -namespace protobuf { -namespace internal { - -#define GOOGLE_PROTOBUF_LINKER_INITIALIZED - -#ifdef GOOGLE_PROTOBUF_SUPPORT_WINDOWS_XP - -// This class is a lightweight replacement for std::mutex on Windows platforms. -// std::mutex does not work on Windows XP SP2 with the latest VC++ libraries, -// because it utilizes the Concurrency Runtime that is only supported on Windows -// XP SP3 and above. -class PROTOBUF_EXPORT CriticalSectionLock { - public: - CriticalSectionLock() { InitializeCriticalSection(&critical_section_); } - ~CriticalSectionLock() { DeleteCriticalSection(&critical_section_); } - void lock() { EnterCriticalSection(&critical_section_); } - void unlock() { LeaveCriticalSection(&critical_section_); } - - private: - CRITICAL_SECTION critical_section_; - - GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(CriticalSectionLock); -}; - -#endif - -// In MSVC std::mutex does not have a constexpr constructor. -// This wrapper makes the constructor constexpr. -template -class CallOnceInitializedMutex { - public: - constexpr CallOnceInitializedMutex() : flag_{}, buf_{} {} - ~CallOnceInitializedMutex() { get().~T(); } - - void lock() { get().lock(); } - void unlock() { get().unlock(); } - - private: - T& get() { - std::call_once(flag_, [&] { ::new (static_cast(&buf_)) T(); }); - return reinterpret_cast(buf_); - } - - std::once_flag flag_; - alignas(T) char buf_[sizeof(T)]; -}; - -// Mutex is a natural type to wrap. As both google and other organization have -// specialized mutexes. gRPC also provides an injection mechanism for custom -// mutexes. -class GOOGLE_PROTOBUF_CAPABILITY("mutex") PROTOBUF_EXPORT WrappedMutex { - public: -#if defined(__QNX__) - constexpr WrappedMutex() = default; -#else - constexpr WrappedMutex() {} -#endif - void Lock() GOOGLE_PROTOBUF_ACQUIRE() { mu_.lock(); } - void Unlock() GOOGLE_PROTOBUF_RELEASE() { mu_.unlock(); } - // Crash if this Mutex is not held exclusively by this thread. - // May fail to crash when it should; will never crash when it should not. - void AssertHeld() const {} - - private: -#if defined(GOOGLE_PROTOBUF_SUPPORT_WINDOWS_XP) - CallOnceInitializedMutex mu_{}; -#elif defined(_WIN32) - CallOnceInitializedMutex mu_{}; -#else - std::mutex mu_{}; -#endif -}; - -using Mutex = WrappedMutex; - -// MutexLock(mu) acquires mu when constructed and releases it when destroyed. -class GOOGLE_PROTOBUF_SCOPED_CAPABILITY PROTOBUF_EXPORT MutexLock { - public: - explicit MutexLock(Mutex* mu) GOOGLE_PROTOBUF_ACQUIRE(mu) : mu_(mu) { - this->mu_->Lock(); - } - ~MutexLock() GOOGLE_PROTOBUF_RELEASE() { this->mu_->Unlock(); } - - private: - Mutex *const mu_; - GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(MutexLock); -}; - -// TODO(kenton): Implement these? Hard to implement portably. -typedef MutexLock ReaderMutexLock; -typedef MutexLock WriterMutexLock; - -// MutexLockMaybe is like MutexLock, but is a no-op when mu is nullptr. -class PROTOBUF_EXPORT MutexLockMaybe { - public: - explicit MutexLockMaybe(Mutex *mu) : - mu_(mu) { if (this->mu_ != nullptr) { this->mu_->Lock(); } } - ~MutexLockMaybe() { if (this->mu_ != nullptr) { this->mu_->Unlock(); } } - private: - Mutex *const mu_; - GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(MutexLockMaybe); -}; - -#if defined(GOOGLE_PROTOBUF_NO_THREADLOCAL) -template -class ThreadLocalStorage { - public: - ThreadLocalStorage() { - pthread_key_create(&key_, &ThreadLocalStorage::Delete); - } - ~ThreadLocalStorage() { - pthread_key_delete(key_); - } - T* Get() { - T* result = static_cast(pthread_getspecific(key_)); - if (result == nullptr) { - result = new T(); - pthread_setspecific(key_, result); - } - return result; - } - private: - static void Delete(void* value) { - delete static_cast(value); - } - pthread_key_t key_; - - GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(ThreadLocalStorage); -}; -#endif - -} // namespace internal - -// We made these internal so that they would show up as such in the docs, -// but we don't want to stick "internal::" in front of them everywhere. -using internal::Mutex; -using internal::MutexLock; -using internal::ReaderMutexLock; -using internal::WriterMutexLock; -using internal::MutexLockMaybe; - -} // namespace protobuf -} // namespace google - -#undef GOOGLE_PROTOBUF_ACQUIRE -#undef GOOGLE_PROTOBUF_RELEASE - -#include - -#endif // GOOGLE_PROTOBUF_STUBS_MUTEX_H_ diff --git a/src/google/protobuf/text_format.cc b/src/google/protobuf/text_format.cc index 1f80ece691..8efa9cf4f2 100644 --- a/src/google/protobuf/text_format.cc +++ b/src/google/protobuf/text_format.cc @@ -984,7 +984,7 @@ class TextFormat::Parser::ParserImpl { } // Consumes an identifier and saves its value in the identifier parameter. - // Returns false if the token is not of type IDENTFIER. + // Returns false if the token is not of type IDENTIFIER. bool ConsumeIdentifier(std::string* identifier) { if (LookingAtType(io::Tokenizer::TYPE_IDENTIFIER)) { *identifier = tokenizer_.current().text; @@ -2293,7 +2293,7 @@ class MapEntryMessageComparator { namespace internal { class MapFieldPrinterHelper { public: - // DynamicMapSorter::Sort cannot be used because it enfores syncing with + // DynamicMapSorter::Sort cannot be used because it enforces syncing with // repeated field. static bool SortMap(const Message& message, const Reflection* reflection, const FieldDescriptor* field, diff --git a/src/google/protobuf/unknown_field_set_unittest.cc b/src/google/protobuf/unknown_field_set_unittest.cc index 7f9a59896d..6996d39221 100644 --- a/src/google/protobuf/unknown_field_set_unittest.cc +++ b/src/google/protobuf/unknown_field_set_unittest.cc @@ -49,16 +49,17 @@ #include #include #include -#include #include #include #include #include #include +#include "absl/synchronization/mutex.h" #include #include #include + namespace google { namespace protobuf { diff --git a/src/google/protobuf/util/BUILD.bazel b/src/google/protobuf/util/BUILD.bazel index f9de867170..833cebdea5 100644 --- a/src/google/protobuf/util/BUILD.bazel +++ b/src/google/protobuf/util/BUILD.bazel @@ -115,20 +115,6 @@ cc_test( ], ) -cc_library( - name = "zero_copy_sink", - srcs = ["zero_copy_sink.cc"], - hdrs = ["zero_copy_sink.h"], - copts = COPTS, - strip_include_prefix = "/src", - visibility = ["//pkg:__pkg__"], - deps = [ - "//src/google/protobuf", - "//src/google/protobuf/io", - "//src/google/protobuf/stubs", - ], -) - cc_library( name = "json_util", srcs = ["json_util.cc"], @@ -138,9 +124,9 @@ cc_library( visibility = ["//:__subpackages__"], deps = [ ":type_resolver_util", - ":zero_copy_sink", "//src/google/protobuf", "//src/google/protobuf/io", + "//src/google/protobuf/io:zero_copy_sink", "//src/google/protobuf/stubs", "//src/google/protobuf/util/internal:default_value", "//src/google/protobuf/util/internal:json", @@ -149,18 +135,6 @@ cc_library( ], ) -cc_test( - name = "zero_copy_sink_test", - srcs = ["zero_copy_sink_test.cc"], - copts = COPTS, - deps = [ - ":zero_copy_sink", - "//src/google/protobuf/stubs", - "@com_google_googletest//:gtest", - "@com_google_googletest//:gtest_main", - ], -) - cc_test( name = "json_util_test", srcs = ["json_util_test.cc"], diff --git a/src/google/protobuf/util/internal/datapiece.cc b/src/google/protobuf/util/internal/datapiece.cc index 3e7aa84da7..63c1ac2c7d 100644 --- a/src/google/protobuf/util/internal/datapiece.cc +++ b/src/google/protobuf/util/internal/datapiece.cc @@ -221,9 +221,10 @@ util::StatusOr DataPiece::ToBool() const { case TYPE_STRING: return StringToNumber(safe_strtob); default: - return util::InvalidArgumentError( - ValueAsStringOrDefault("Wrong type. Cannot convert to Bool.")); + break; } + return util::InvalidArgumentError( + ValueAsStringOrDefault("Wrong type. Cannot convert to Bool.")); } util::StatusOr DataPiece::ToString() const { diff --git a/src/google/protobuf/util/json_util.cc b/src/google/protobuf/util/json_util.cc index 04cab9cf06..23f2900b6c 100644 --- a/src/google/protobuf/util/json_util.cc +++ b/src/google/protobuf/util/json_util.cc @@ -36,6 +36,7 @@ #include #include #include +#include #include #include #include @@ -45,7 +46,6 @@ #include #include #include -#include #include @@ -57,7 +57,7 @@ namespace google { namespace protobuf { namespace util { -using ::google::protobuf::util::zc_sink_internal::ZeroCopyStreamByteSink; +using ::google::protobuf::io::zc_sink_internal::ZeroCopyStreamByteSink; util::Status BinaryToJsonStream(TypeResolver* resolver, const std::string& type_url, diff --git a/src/google/protobuf/util/json_util_test.cc b/src/google/protobuf/util/json_util_test.cc index 99272d71ac..8df9880a5c 100644 --- a/src/google/protobuf/util/json_util_test.cc +++ b/src/google/protobuf/util/json_util_test.cc @@ -398,6 +398,14 @@ TEST_P(JsonTest, TestPrintProto2EnumAsIntWithDefaultValue) { EXPECT_EQ(parsed->enum_value(), protobuf_unittest::DEFAULT); } +TEST_P(JsonTest, QuotedEnumValue) { + auto m = ToProto(R"json( + {"enumValue1": "1"} + )json"); + ASSERT_OK(m); + EXPECT_THAT(m->enum_value1(), proto3::BAR); +} + TEST_P(JsonTest, WebSafeBytes) { auto m = ToProto(R"json({ "bytesValue": "-_" @@ -1176,6 +1184,17 @@ TEST_P(JsonTest, TestFieldMask) { EXPECT_THAT(m2->value().paths(), ElementsAre("yep.really")); } +TEST_P(JsonTest, TestFieldMaskSnakeCase) { + auto m = ToProto(R"json( + { + "value": "foo_bar" + } + )json"); + ASSERT_OK(m); + + EXPECT_THAT(m->value().paths(), ElementsAre("foo_bar")); +} + TEST_P(JsonTest, TestLegalNullsInArray) { auto m = ToProto(R"json({ "repeatedNullValue": [null] @@ -1194,13 +1213,26 @@ TEST_P(JsonTest, TestLegalNullsInArray) { EXPECT_TRUE(m2->repeated_value(0).has_null_value()); m2->Clear(); - m2->mutable_repeated_value(); // Materialize an empty singular Value. + m2->mutable_value(); // Materialize an empty singular Value. m2->add_repeated_value(); m2->add_repeated_value()->set_string_value("solitude"); m2->add_repeated_value(); EXPECT_THAT(ToJson(*m2), IsOkAndHolds(R"({"repeatedValue":["solitude"]})")); } +TEST_P(JsonTest, EmptyValue) { + EXPECT_THAT(ToJson(google::protobuf::Value()), IsOkAndHolds("")); + + google::protobuf::Struct s; + s.mutable_fields()->emplace("empty", google::protobuf::Value()); + EXPECT_THAT(ToJson(s), IsOkAndHolds("{}")); +} + +TEST_P(JsonTest, TrailingGarbage) { + EXPECT_THAT(ToProto("{}garbage"), + StatusIs(util::StatusCode::kInvalidArgument)); +} + TEST_P(JsonTest, ListList) { auto m = ToProto(R"json({ "repeated_value": [["ayy", "lmao"]]