diff --git a/BUILD.bazel b/BUILD.bazel index b63b7bf984..fd9ee70aeb 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -324,6 +324,12 @@ alias( # Test protos ################################################################################ +alias( + name = "lite_test_proto_srcs", + actual = "//src/google/protobuf:lite_test_proto_srcs", # proto_library + visibility = ["//:__subpackages__"], +) + alias( name = "lite_test_protos", actual = "//src/google/protobuf:lite_test_protos", # proto_library diff --git a/CHANGES.txt b/CHANGES.txt index 13d7b7332e..fa223a22d1 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -39,6 +39,9 @@ constructed FieldMaskTrees. * Optimized Java proto serialization gencode for protos having many extension ranges with few fields in between. * More thoroughly annotate public generated code in Java lite protocol buffers. + * Fixed Bug in proto3 java lite repeated enum fields. Failed to call copyOnWrite before modifying previously built message. Causes modification to already "built" messages that should be immutable. + * Refactoring java full runtime to reuse sub-message builders and prepare to migrate parsing logic from parse constructor to builder. + * Fix Java reflection serialization of empty packed fields. Python * Changes ordering of printed fields in .pyi files from lexicographic to the same ordering found in the proto descriptor. diff --git a/Protobuf-C++.podspec b/Protobuf-C++.podspec index 2043adbdec..8ab70d04ac 100644 --- a/Protobuf-C++.podspec +++ b/Protobuf-C++.podspec @@ -1,6 +1,6 @@ Pod::Spec.new do |s| s.name = 'Protobuf-C++' - s.version = '3.21.5' + s.version = '3.21.6' s.summary = 'Protocol Buffers v3 runtime library for C++.' s.homepage = 'https://github.com/google/protobuf' s.license = 'BSD-3-Clause' diff --git a/cmake/conformance.cmake b/cmake/conformance.cmake index fa5c479bc6..056250ebe9 100644 --- a/cmake/conformance.cmake +++ b/cmake/conformance.cmake @@ -1,6 +1,8 @@ add_custom_command( - OUTPUT ${protobuf_SOURCE_DIR}/conformance/conformance.pb.cc + OUTPUT + ${protobuf_SOURCE_DIR}/conformance/conformance.pb.h + ${protobuf_SOURCE_DIR}/conformance/conformance.pb.cc DEPENDS ${protobuf_PROTOC_EXE} ${protobuf_SOURCE_DIR}/conformance/conformance.proto COMMAND ${protobuf_PROTOC_EXE} ${protobuf_SOURCE_DIR}/conformance/conformance.proto --proto_path=${protobuf_SOURCE_DIR}/conformance @@ -8,8 +10,11 @@ add_custom_command( ) add_custom_command( - OUTPUT ${protobuf_SOURCE_DIR}/src/google/protobuf/test_messages_proto3.pb.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/test_messages_proto2.pb.cc + OUTPUT + ${protobuf_SOURCE_DIR}/src/google/protobuf/test_messages_proto3.pb.h + ${protobuf_SOURCE_DIR}/src/google/protobuf/test_messages_proto3.pb.cc + ${protobuf_SOURCE_DIR}/src/google/protobuf/test_messages_proto2.pb.h + ${protobuf_SOURCE_DIR}/src/google/protobuf/test_messages_proto2.pb.cc DEPENDS ${protobuf_PROTOC_EXE} ${protobuf_SOURCE_DIR}/src/google/protobuf/test_messages_proto3.proto ${protobuf_PROTOC_EXE} ${protobuf_SOURCE_DIR}/src/google/protobuf/test_messages_proto2.proto COMMAND ${protobuf_PROTOC_EXE} ${protobuf_SOURCE_DIR}/src/google/protobuf/test_messages_proto3.proto @@ -21,6 +26,7 @@ add_custom_command( add_executable(conformance_test_runner ${protobuf_SOURCE_DIR}/conformance/binary_json_conformance_suite.cc ${protobuf_SOURCE_DIR}/conformance/binary_json_conformance_suite.h + ${protobuf_SOURCE_DIR}/conformance/conformance.pb.h ${protobuf_SOURCE_DIR}/conformance/conformance.pb.cc ${protobuf_SOURCE_DIR}/conformance/conformance_test.cc ${protobuf_SOURCE_DIR}/conformance/conformance_test_runner.cc @@ -29,14 +35,19 @@ add_executable(conformance_test_runner ${protobuf_SOURCE_DIR}/conformance/text_format_conformance_suite.h ${protobuf_SOURCE_DIR}/conformance/third_party/jsoncpp/json.h ${protobuf_SOURCE_DIR}/conformance/third_party/jsoncpp/jsoncpp.cpp + ${protobuf_SOURCE_DIR}/src/google/protobuf/test_messages_proto2.pb.h ${protobuf_SOURCE_DIR}/src/google/protobuf/test_messages_proto2.pb.cc + ${protobuf_SOURCE_DIR}/src/google/protobuf/test_messages_proto3.pb.h ${protobuf_SOURCE_DIR}/src/google/protobuf/test_messages_proto3.pb.cc ) add_executable(conformance_cpp + ${protobuf_SOURCE_DIR}/conformance/conformance.pb.h ${protobuf_SOURCE_DIR}/conformance/conformance.pb.cc ${protobuf_SOURCE_DIR}/conformance/conformance_cpp.cc + ${protobuf_SOURCE_DIR}/src/google/protobuf/test_messages_proto2.pb.h ${protobuf_SOURCE_DIR}/src/google/protobuf/test_messages_proto2.pb.cc + ${protobuf_SOURCE_DIR}/src/google/protobuf/test_messages_proto3.pb.h ${protobuf_SOURCE_DIR}/src/google/protobuf/test_messages_proto3.pb.cc ) diff --git a/cmake/install.cmake b/cmake/install.cmake index 26a55be8b6..90230c537e 100644 --- a/cmake/install.cmake +++ b/cmake/install.cmake @@ -47,7 +47,7 @@ include(${protobuf_SOURCE_DIR}/src/file_lists.cmake) set(protobuf_HEADERS ${libprotobuf_hdrs} ${libprotoc_hdrs} - ${wkt_protos_proto_srcs} + ${wkt_protos_files} ${descriptor_proto_proto_srcs} ${plugin_proto_proto_srcs} ) diff --git a/cmake/tests.cmake b/cmake/tests.cmake index a9fdd524ca..530a66ba8b 100644 --- a/cmake/tests.cmake +++ b/cmake/tests.cmake @@ -43,19 +43,20 @@ endif() include(${protobuf_SOURCE_DIR}/src/file_lists.cmake) set(lite_test_protos - ${protobuf_lite_test_protos_proto_srcs} + ${protobuf_lite_test_protos_files} ) set(tests_protos - ${protobuf_test_protos_proto_srcs} + ${protobuf_test_protos_files} ${compiler_test_protos_files} ${util_test_protos_files} ) macro(compile_proto_file filename) - string(REPLACE .proto .pb.cc pb_file ${filename}) + string(REPLACE .proto .pb.h pb_hdr ${filename}) + string(REPLACE .proto .pb.cc pb_src ${filename}) add_custom_command( - OUTPUT ${pb_file} + OUTPUT ${pb_hdr} ${pb_src} DEPENDS ${protobuf_PROTOC_EXE} ${filename} COMMAND ${protobuf_PROTOC_EXE} ${filename} --proto_path=${protobuf_SOURCE_DIR}/src @@ -67,13 +68,13 @@ endmacro(compile_proto_file) set(lite_test_proto_files) foreach(proto_file ${lite_test_protos}) compile_proto_file(${proto_file}) - set(lite_test_proto_files ${lite_test_proto_files} ${pb_file}) + set(lite_test_proto_files ${lite_test_proto_files} ${pb_src} ${pb_hdr}) endforeach(proto_file) set(tests_proto_files) foreach(proto_file ${tests_protos}) compile_proto_file(${proto_file}) - set(tests_proto_files ${tests_proto_files} ${pb_file}) + set(tests_proto_files ${tests_proto_files} ${pb_src} ${pb_hdr}) endforeach(proto_file) add_library(protobuf-lite-test-common STATIC @@ -85,8 +86,8 @@ target_link_libraries(protobuf-lite-test-common set(common_test_files ${test_util_hdrs} ${test_util_srcs} - ${mock_code_generator_srcs} - ${testing_srcs} + ${common_test_hdrs} + ${common_test_srcs} ) add_library(protobuf-test-common STATIC @@ -138,8 +139,8 @@ target_link_libraries(tests protobuf-lite-test-common protobuf-test-common ${pro set(test_plugin_files ${test_plugin_files} - ${mock_code_generator_srcs} - ${testing_srcs} + ${common_test_hdrs} + ${common_test_srcs} ) add_executable(test_plugin ${test_plugin_files}) @@ -172,13 +173,23 @@ add_custom_target(save-installed-headers) add_custom_target(remove-installed-headers) add_custom_target(restore-installed-headers) -# Explicitly skip the bootstrapping headers as it's directly used in tests -set(_installed_hdrs ${libprotobuf_hdrs} ${libprotoc_hdrs}) -list(REMOVE_ITEM _installed_hdrs +file(GLOB_RECURSE _local_hdrs + "${PROJECT_SOURCE_DIR}/src/*.h" + "${PROJECT_SOURCE_DIR}/src/*.inc") + +# Exclude the bootstrapping that are directly used by tests. +set(_exclude_hdrs "${protobuf_SOURCE_DIR}/src/google/protobuf/descriptor.pb.h" "${protobuf_SOURCE_DIR}/src/google/protobuf/compiler/plugin.pb.h") -foreach(_hdr ${_installed_hdrs}) +# Exclude test library headers. +list(APPEND _exclude_hdrs ${test_util_hdrs} ${lite_test_util_hdrs} ${common_test_hdrs} + ${compiler_test_utils_hdrs}) +foreach(_hdr ${_exclude_hdrs}) + list(REMOVE_ITEM _local_hdrs ${_hdr}) +endforeach() + +foreach(_hdr ${_local_hdrs}) string(REPLACE "${protobuf_SOURCE_DIR}/src" "" _file ${_hdr}) set(_tmp_file "${CMAKE_BINARY_DIR}/tmp-install-test/${_file}") add_custom_command(TARGET remove-installed-headers PRE_BUILD @@ -193,6 +204,6 @@ endforeach() add_dependencies(remove-installed-headers save-installed-headers) if(protobuf_REMOVE_INSTALLED_HEADERS) - add_dependencies(protobuf-lite-test-common remove-installed-headers) - add_dependencies(protobuf-test-common remove-installed-headers) + # Make sure we remove all the headers *before* any codegen occurs. + add_dependencies(${protobuf_PROTOC_EXE} remove-installed-headers) endif() diff --git a/java/core/src/main/java/com/google/protobuf/AbstractMessage.java b/java/core/src/main/java/com/google/protobuf/AbstractMessage.java index 0a87859d88..bbfb965024 100644 --- a/java/core/src/main/java/com/google/protobuf/AbstractMessage.java +++ b/java/core/src/main/java/com/google/protobuf/AbstractMessage.java @@ -424,27 +424,22 @@ public abstract class AbstractMessage throws IOException { boolean discardUnknown = input.shouldDiscardUnknownFields(); final UnknownFieldSet.Builder unknownFields = - discardUnknown ? null : UnknownFieldSet.newBuilder(getUnknownFields()); - while (true) { - final int tag = input.readTag(); - if (tag == 0) { - break; - } - - MessageReflection.BuilderAdapter builderAdapter = - new MessageReflection.BuilderAdapter(this); - if (!MessageReflection.mergeFieldFrom( - input, unknownFields, extensionRegistry, getDescriptorForType(), builderAdapter, tag)) { - // end group tag - break; - } - } + discardUnknown ? null : getUnknownFieldSetBuilder(); + MessageReflection.mergeMessageFrom(this, unknownFields, input, extensionRegistry); if (unknownFields != null) { - setUnknownFields(unknownFields.build()); + setUnknownFieldSetBuilder(unknownFields); } return (BuilderType) this; } + protected UnknownFieldSet.Builder getUnknownFieldSetBuilder() { + return UnknownFieldSet.newBuilder(getUnknownFields()); + } + + protected void setUnknownFieldSetBuilder(final UnknownFieldSet.Builder builder) { + setUnknownFields(builder.build()); + } + @Override public BuilderType mergeUnknownFields(final UnknownFieldSet unknownFields) { setUnknownFields( diff --git a/java/core/src/main/java/com/google/protobuf/DescriptorMessageInfoFactory.java b/java/core/src/main/java/com/google/protobuf/DescriptorMessageInfoFactory.java index cc26855217..272c5a1890 100644 --- a/java/core/src/main/java/com/google/protobuf/DescriptorMessageInfoFactory.java +++ b/java/core/src/main/java/com/google/protobuf/DescriptorMessageInfoFactory.java @@ -427,8 +427,8 @@ final class DescriptorMessageInfoFactory implements MessageInfoFactory { boolean enforceUtf8 = true; for (int i = 0; i < fieldDescriptors.size(); ++i) { FieldDescriptor fd = fieldDescriptors.get(i); - if (fd.getContainingOneof() != null) { - // Build a oneof member field. + if (fd.getContainingOneof() != null && !fd.getContainingOneof().isSynthetic()) { + // Build a oneof member field. But only if it is a real oneof, not a proto3 optional builder.withField(buildOneofMember(messageType, fd, oneofState, enforceUtf8, null)); continue; } diff --git a/java/core/src/main/java/com/google/protobuf/DynamicMessage.java b/java/core/src/main/java/com/google/protobuf/DynamicMessage.java index fee643f807..87d85a5e4b 100644 --- a/java/core/src/main/java/com/google/protobuf/DynamicMessage.java +++ b/java/core/src/main/java/com/google/protobuf/DynamicMessage.java @@ -413,7 +413,10 @@ public final class DynamicMessage extends AbstractMessage { DynamicMessage result = new DynamicMessage( - type, fields.build(), Arrays.copyOf(oneofCases, oneofCases.length), unknownFields); + type, + fields.buildPartial(), + Arrays.copyOf(oneofCases, oneofCases.length), + unknownFields); return result; } diff --git a/java/core/src/main/java/com/google/protobuf/FieldSet.java b/java/core/src/main/java/com/google/protobuf/FieldSet.java index 0597ef758b..21dd8d229f 100644 --- a/java/core/src/main/java/com/google/protobuf/FieldSet.java +++ b/java/core/src/main/java/com/google/protobuf/FieldSet.java @@ -926,8 +926,27 @@ final class FieldSet> { this.isMutable = true; } - /** Creates the FieldSet */ + /** + * Creates the FieldSet + * + * @throws UninitializedMessageException if a message field is missing required fields. + */ public FieldSet build() { + return buildImpl(false); + } + + /** Creates the FieldSet but does not validate that all required fields are present. */ + public FieldSet buildPartial() { + return buildImpl(true); + } + + /** + * Creates the FieldSet. + * + * @param partial controls whether to do a build() or buildPartial() when converting submessage + * builders to messages. + */ + private FieldSet buildImpl(boolean partial) { if (fields.isEmpty()) { return FieldSet.emptySet(); } @@ -936,7 +955,7 @@ final class FieldSet> { if (hasNestedBuilders) { // Make a copy of the fields map with all Builders replaced by Message. fieldsForBuild = cloneAllFieldsMap(fields, /* copyList */ false); - replaceBuilders(fieldsForBuild); + replaceBuilders(fieldsForBuild, partial); } FieldSet fieldSet = new FieldSet<>(fieldsForBuild); fieldSet.hasLazyField = hasLazyField; @@ -944,22 +963,22 @@ final class FieldSet> { } private static > void replaceBuilders( - SmallSortedMap fieldMap) { + SmallSortedMap fieldMap, boolean partial) { for (int i = 0; i < fieldMap.getNumArrayEntries(); i++) { - replaceBuilders(fieldMap.getArrayEntryAt(i)); + replaceBuilders(fieldMap.getArrayEntryAt(i), partial); } for (Map.Entry entry : fieldMap.getOverflowEntries()) { - replaceBuilders(entry); + replaceBuilders(entry, partial); } } private static > void replaceBuilders( - Map.Entry entry) { - entry.setValue(replaceBuilders(entry.getKey(), entry.getValue())); + Map.Entry entry, boolean partial) { + entry.setValue(replaceBuilders(entry.getKey(), entry.getValue(), partial)); } private static > Object replaceBuilders( - T descriptor, Object value) { + T descriptor, Object value, boolean partial) { if (value == null) { return value; } @@ -974,7 +993,7 @@ final class FieldSet> { List list = (List) value; for (int i = 0; i < list.size(); i++) { Object oldElement = list.get(i); - Object newElement = replaceBuilder(oldElement); + Object newElement = replaceBuilder(oldElement, partial); if (newElement != oldElement) { // If the list contains a Message.Builder, then make a copy of that list and then // modify the Message.Builder into a Message and return the new list. This way, the @@ -988,14 +1007,21 @@ final class FieldSet> { } return list; } else { - return replaceBuilder(value); + return replaceBuilder(value, partial); } } return value; } - private static Object replaceBuilder(Object value) { - return (value instanceof MessageLite.Builder) ? ((MessageLite.Builder) value).build() : value; + private static Object replaceBuilder(Object value, boolean partial) { + if (!(value instanceof MessageLite.Builder)) { + return value; + } + MessageLite.Builder builder = (MessageLite.Builder) value; + if (partial) { + return builder.buildPartial(); + } + return builder.build(); } /** Returns a new Builder using the fields from {@code fieldSet}. */ @@ -1014,7 +1040,7 @@ final class FieldSet> { if (fields.isImmutable()) { result.makeImmutable(); } else { - replaceBuilders(result); + replaceBuilders(result, true); } return result; } @@ -1037,7 +1063,7 @@ final class FieldSet> { */ public Object getField(final T descriptor) { Object value = getFieldAllowBuilders(descriptor); - return replaceBuilders(descriptor, value); + return replaceBuilders(descriptor, value, true); } /** Same as {@link #getField(F)}, but allow a {@link MessageLite.Builder} to be returned. */ @@ -1123,7 +1149,7 @@ final class FieldSet> { ensureIsMutable(); } Object value = getRepeatedFieldAllowBuilders(descriptor, index); - return replaceBuilder(value); + return replaceBuilder(value, true); } /** diff --git a/java/core/src/main/java/com/google/protobuf/GeneratedMessageV3.java b/java/core/src/main/java/com/google/protobuf/GeneratedMessageV3.java index e2b1a741b4..41ea0a11ba 100644 --- a/java/core/src/main/java/com/google/protobuf/GeneratedMessageV3.java +++ b/java/core/src/main/java/com/google/protobuf/GeneratedMessageV3.java @@ -129,7 +129,10 @@ public abstract class GeneratedMessageV3 extends AbstractMessage implements Seri return internalGetFieldAccessorTable().descriptor; } - // TODO(gberg): remove this. Have to leave it for now to support old gencode. + // TODO(b/248143958): This method should be removed. It enables parsing directly into an + // "immutable" message. Have to leave it for now to support old gencode. + // @deprecated use newBuilder().mergeFrom() instead + @Deprecated protected void mergeFromAndMakeImmutableInternal( CodedInputStream input, ExtensionRegistryLite extensionRegistry) throws InvalidProtocolBufferException { @@ -287,12 +290,14 @@ public abstract class GeneratedMessageV3 extends AbstractMessage implements Seri @Override public UnknownFieldSet getUnknownFields() { - throw new UnsupportedOperationException("This is supposed to be overridden by subclasses."); + return unknownFields; } /** * Called by subclasses to parse an unknown field. * + *

TODO(b/248153893) remove this method + * * @return {@code true} unless the tag is an end-group tag. */ protected boolean parseUnknownField( @@ -310,6 +315,8 @@ public abstract class GeneratedMessageV3 extends AbstractMessage implements Seri /** * Delegates to parseUnknownField. This method is obsolete, but we must retain it for * compatibility with older generated code. + * + *

TODO(b/248153893) remove this method */ protected boolean parseUnknownFieldProto3( CodedInputStream input, @@ -525,7 +532,18 @@ public abstract class GeneratedMessageV3 extends AbstractMessage implements Seri // to dispatch dirty invalidations. See GeneratedMessageV3.BuilderListener. private boolean isClean; - private UnknownFieldSet unknownFields = UnknownFieldSet.getDefaultInstance(); + /** + * This field holds either an {@link UnknownFieldSet} or {@link UnknownFieldSet.Builder}. + * + *

We use an object because it should only be one or the other of those things at a time and + * Object is the only common base. This also saves space. + * + *

Conversions are lazy: if {@link #setUnknownFields} is called, this will contain {@link + * UnknownFieldSet}. If unknown fields are merged into this builder, the current {@link + * UnknownFieldSet} will be converted to a {@link UnknownFieldSet.Builder} and left that way + * until either {@link #setUnknownFields} or {@link #buildPartial} or {@link #build} is called. + */ + private Object unknownFieldsOrBuilder = UnknownFieldSet.getDefaultInstance(); protected Builder() { this(null); @@ -578,7 +596,7 @@ public abstract class GeneratedMessageV3 extends AbstractMessage implements Seri */ @Override public BuilderType clear() { - unknownFields = UnknownFieldSet.getDefaultInstance(); + unknownFieldsOrBuilder = UnknownFieldSet.getDefaultInstance(); onChanged(); return (BuilderType) this; } @@ -725,7 +743,7 @@ public abstract class GeneratedMessageV3 extends AbstractMessage implements Seri } private BuilderType setUnknownFieldsInternal(final UnknownFieldSet unknownFields) { - this.unknownFields = unknownFields; + unknownFieldsOrBuilder = unknownFields; onChanged(); return (BuilderType) this; } @@ -744,8 +762,19 @@ public abstract class GeneratedMessageV3 extends AbstractMessage implements Seri @Override public BuilderType mergeUnknownFields(final UnknownFieldSet unknownFields) { - return setUnknownFields( - UnknownFieldSet.newBuilder(this.unknownFields).mergeFrom(unknownFields).build()); + if (UnknownFieldSet.getDefaultInstance().equals(unknownFields)) { + return (BuilderType) this; + } + + if (UnknownFieldSet.getDefaultInstance().equals(unknownFieldsOrBuilder)) { + unknownFieldsOrBuilder = unknownFields; + onChanged(); + return (BuilderType) this; + } + + getUnknownFieldSetBuilder().mergeFrom(unknownFields); + onChanged(); + return (BuilderType) this; } @Override @@ -779,7 +808,50 @@ public abstract class GeneratedMessageV3 extends AbstractMessage implements Seri @Override public final UnknownFieldSet getUnknownFields() { - return unknownFields; + if (unknownFieldsOrBuilder instanceof UnknownFieldSet) { + return (UnknownFieldSet) unknownFieldsOrBuilder; + } else { + return ((UnknownFieldSet.Builder) unknownFieldsOrBuilder).buildPartial(); + } + } + + /** + * Called by generated subclasses to parse an unknown field. + * + * @return {@code true} unless the tag is an end-group tag. + */ + protected boolean parseUnknownField( + CodedInputStream input, ExtensionRegistryLite extensionRegistry, int tag) + throws IOException { + if (input.shouldDiscardUnknownFields()) { + return input.skipField(tag); + } + return getUnknownFieldSetBuilder().mergeFieldFrom(tag, input); + } + + /** Called by generated subclasses to add to the unknown field set. */ + protected final void mergeUnknownLengthDelimitedField(int number, ByteString bytes) { + getUnknownFieldSetBuilder().mergeLengthDelimitedField(number, bytes); + } + + /** Called by generated subclasses to add to the unknown field set. */ + protected final void mergeUnknownVarintField(int number, int value) { + getUnknownFieldSetBuilder().mergeVarintField(number, value); + } + + @Override + protected UnknownFieldSet.Builder getUnknownFieldSetBuilder() { + if (unknownFieldsOrBuilder instanceof UnknownFieldSet) { + unknownFieldsOrBuilder = ((UnknownFieldSet) unknownFieldsOrBuilder).toBuilder(); + } + onChanged(); + return (UnknownFieldSet.Builder) unknownFieldsOrBuilder; + } + + @Override + protected void setUnknownFieldSetBuilder(UnknownFieldSet.Builder builder) { + unknownFieldsOrBuilder = builder; + onChanged(); } /** @@ -1541,7 +1613,7 @@ public abstract class GeneratedMessageV3 extends AbstractMessage implements Seri private FieldSet buildExtensions() { return extensions == null ? (FieldSet) FieldSet.emptySet() - : extensions.build(); + : extensions.buildPartial(); } @Override @@ -1744,6 +1816,20 @@ public abstract class GeneratedMessageV3 extends AbstractMessage implements Seri } } + @Override + protected boolean parseUnknownField( + CodedInputStream input, ExtensionRegistryLite extensionRegistry, int tag) + throws IOException { + ensureExtensionsIsMutable(); + return MessageReflection.mergeFieldFrom( + input, + input.shouldDiscardUnknownFields() ? null : getUnknownFieldSetBuilder(), + extensionRegistry, + getDescriptorForType(), + new MessageReflection.ExtensionBuilderAdapter(extensions), + tag); + } + private void verifyContainingType(final FieldDescriptor field) { if (field.getContainingType() != getDescriptorForType()) { throw new IllegalArgumentException("FieldDescriptor does not match message type."); diff --git a/java/core/src/main/java/com/google/protobuf/MessageReflection.java b/java/core/src/main/java/com/google/protobuf/MessageReflection.java index b7f5d52d4a..e9bc9f1330 100644 --- a/java/core/src/main/java/com/google/protobuf/MessageReflection.java +++ b/java/core/src/main/java/com/google/protobuf/MessageReflection.java @@ -30,6 +30,7 @@ package com.google.protobuf; +import com.google.protobuf.Descriptors.Descriptor; import com.google.protobuf.Descriptors.FieldDescriptor; import java.io.IOException; import java.util.ArrayList; @@ -323,6 +324,34 @@ class MessageReflection { Message defaultInstance) throws IOException; + /** + * Read the given group field from the wire, merging with the existing field if it is already + * present. + * + *

For extensions, defaultInstance must be specified. For regular fields, defaultInstance can + * be null. + */ + void mergeGroup( + CodedInputStream input, + ExtensionRegistryLite extensionRegistry, + FieldDescriptor field, + Message defaultInstance) + throws IOException; + + /** + * Read the given message field from the wire, merging with the existing field if it is already + * present. + * + *

For extensions, defaultInstance must be specified. For regular fields, defaultInstance can + * be null. + */ + void mergeMessage( + CodedInputStream input, + ExtensionRegistryLite extensionRegistry, + FieldDescriptor field, + Message defaultInstance) + throws IOException; + /** Returns the UTF8 validation level for the field. */ WireFormat.Utf8Validation getUtf8Validation(Descriptors.FieldDescriptor descriptor); @@ -499,6 +528,60 @@ class MessageReflection { return subBuilder.buildPartial(); } + @Override + public void mergeGroup( + CodedInputStream input, + ExtensionRegistryLite extensionRegistry, + FieldDescriptor field, + Message defaultInstance) + throws IOException { + if (!field.isRepeated()) { + if (hasField(field)) { + input.readGroup(field.getNumber(), builder.getFieldBuilder(field), extensionRegistry); + return; + } + Message.Builder subBuilder = newMessageFieldInstance(field, defaultInstance); + input.readGroup(field.getNumber(), subBuilder, extensionRegistry); + Object unused = setField(field, subBuilder.buildPartial()); + } else { + Message.Builder subBuilder = newMessageFieldInstance(field, defaultInstance); + input.readGroup(field.getNumber(), subBuilder, extensionRegistry); + Object unused = addRepeatedField(field, subBuilder.buildPartial()); + } + } + + @Override + public void mergeMessage( + CodedInputStream input, + ExtensionRegistryLite extensionRegistry, + Descriptors.FieldDescriptor field, + Message defaultInstance) + throws IOException { + if (!field.isRepeated()) { + if (hasField(field)) { + input.readMessage(builder.getFieldBuilder(field), extensionRegistry); + return; + } + Message.Builder subBuilder = newMessageFieldInstance(field, defaultInstance); + input.readMessage(subBuilder, extensionRegistry); + Object unused = setField(field, subBuilder.buildPartial()); + } else { + Message.Builder subBuilder = newMessageFieldInstance(field, defaultInstance); + input.readMessage(subBuilder, extensionRegistry); + Object unused = addRepeatedField(field, subBuilder.buildPartial()); + } + } + + private Message.Builder newMessageFieldInstance( + FieldDescriptor field, Message defaultInstance) { + // When default instance is not null. The field is an extension field. + if (defaultInstance != null) { + return defaultInstance.newBuilderForType(); + } else { + return builder.newBuilderForField(field); + } + } + @Override public MergeTarget newMergeTargetForField( Descriptors.FieldDescriptor field, Message defaultInstance) { @@ -665,6 +748,54 @@ class MessageReflection { return subBuilder.buildPartial(); } + @Override + public void mergeGroup( + CodedInputStream input, + ExtensionRegistryLite extensionRegistry, + FieldDescriptor field, + Message defaultInstance) + throws IOException { + if (!field.isRepeated()) { + if (hasField(field)) { + MessageLite.Builder current = ((MessageLite) getField(field)).toBuilder(); + input.readGroup(field.getNumber(), current, extensionRegistry); + Object unused = setField(field, current.buildPartial()); + return; + } + Message.Builder subBuilder = defaultInstance.newBuilderForType(); + input.readGroup(field.getNumber(), subBuilder, extensionRegistry); + Object unused = setField(field, subBuilder.buildPartial()); + } else { + Message.Builder subBuilder = defaultInstance.newBuilderForType(); + input.readGroup(field.getNumber(), subBuilder, extensionRegistry); + Object unused = addRepeatedField(field, subBuilder.buildPartial()); + } + } + + @Override + public void mergeMessage( + CodedInputStream input, + ExtensionRegistryLite extensionRegistry, + FieldDescriptor field, + Message defaultInstance) + throws IOException { + if (!field.isRepeated()) { + if (hasField(field)) { + MessageLite.Builder current = ((MessageLite) getField(field)).toBuilder(); + input.readMessage(current, extensionRegistry); + Object unused = setField(field, current.buildPartial()); + return; + } + Message.Builder subBuilder = defaultInstance.newBuilderForType(); + input.readMessage(subBuilder, extensionRegistry); + Object unused = setField(field, subBuilder.buildPartial()); + } else { + Message.Builder subBuilder = defaultInstance.newBuilderForType(); + input.readMessage(subBuilder, extensionRegistry); + Object unused = addRepeatedField(field, subBuilder.buildPartial()); + } + } + @Override public Object parseMessageFromBytes( ByteString bytes, @@ -700,7 +831,234 @@ class MessageReflection { if (descriptor.needsUtf8Check()) { return WireFormat.Utf8Validation.STRICT; } - // TODO(liujisi): support lazy strings for ExtesnsionSet. + // TODO(b/248145492): support lazy strings for ExtesnsionSet. + return WireFormat.Utf8Validation.LOOSE; + } + + @Override + public Object finish() { + throw new UnsupportedOperationException("finish() called on FieldSet object"); + } + } + + static class ExtensionBuilderAdapter implements MergeTarget { + + private final FieldSet.Builder extensions; + + ExtensionBuilderAdapter(FieldSet.Builder extensions) { + this.extensions = extensions; + } + + @Override + public Descriptors.Descriptor getDescriptorForType() { + throw new UnsupportedOperationException("getDescriptorForType() called on FieldSet object"); + } + + @Override + public Object getField(Descriptors.FieldDescriptor field) { + return extensions.getField(field); + } + + @Override + public boolean hasField(Descriptors.FieldDescriptor field) { + return extensions.hasField(field); + } + + @Override + @CanIgnoreReturnValue + public MergeTarget setField(Descriptors.FieldDescriptor field, Object value) { + extensions.setField(field, value); + return this; + } + + @Override + @CanIgnoreReturnValue + public MergeTarget clearField(Descriptors.FieldDescriptor field) { + extensions.clearField(field); + return this; + } + + @Override + @CanIgnoreReturnValue + public MergeTarget setRepeatedField( + Descriptors.FieldDescriptor field, int index, Object value) { + extensions.setRepeatedField(field, index, value); + return this; + } + + @Override + @CanIgnoreReturnValue + public MergeTarget addRepeatedField(Descriptors.FieldDescriptor field, Object value) { + extensions.addRepeatedField(field, value); + return this; + } + + @Override + public boolean hasOneof(Descriptors.OneofDescriptor oneof) { + return false; + } + + @Override + @CanIgnoreReturnValue + public MergeTarget clearOneof(Descriptors.OneofDescriptor oneof) { + // Nothing to clear. + return this; + } + + @Override + public Descriptors.FieldDescriptor getOneofFieldDescriptor(Descriptors.OneofDescriptor oneof) { + return null; + } + + @Override + public ContainerType getContainerType() { + return ContainerType.EXTENSION_SET; + } + + @Override + public ExtensionRegistry.ExtensionInfo findExtensionByName( + ExtensionRegistry registry, String name) { + return registry.findImmutableExtensionByName(name); + } + + @Override + public ExtensionRegistry.ExtensionInfo findExtensionByNumber( + ExtensionRegistry registry, Descriptors.Descriptor containingType, int fieldNumber) { + return registry.findImmutableExtensionByNumber(containingType, fieldNumber); + } + + @Override + public Object parseGroup( + CodedInputStream input, + ExtensionRegistryLite registry, + Descriptors.FieldDescriptor field, + Message defaultInstance) + throws IOException { + Message.Builder subBuilder = defaultInstance.newBuilderForType(); + if (!field.isRepeated()) { + Message originalMessage = (Message) getField(field); + if (originalMessage != null) { + subBuilder.mergeFrom(originalMessage); + } + } + input.readGroup(field.getNumber(), subBuilder, registry); + return subBuilder.buildPartial(); + } + + @Override + public Object parseMessage( + CodedInputStream input, + ExtensionRegistryLite registry, + Descriptors.FieldDescriptor field, + Message defaultInstance) + throws IOException { + Message.Builder subBuilder = defaultInstance.newBuilderForType(); + if (!field.isRepeated()) { + Message originalMessage = (Message) getField(field); + if (originalMessage != null) { + subBuilder.mergeFrom(originalMessage); + } + } + input.readMessage(subBuilder, registry); + return subBuilder.buildPartial(); + } + + @Override + public void mergeGroup( + CodedInputStream input, + ExtensionRegistryLite extensionRegistry, + FieldDescriptor field, + Message defaultInstance) + throws IOException { + if (!field.isRepeated()) { + if (hasField(field)) { + Object fieldOrBuilder = extensions.getFieldAllowBuilders(field); + MessageLite.Builder subBuilder; + if (fieldOrBuilder instanceof MessageLite.Builder) { + subBuilder = (MessageLite.Builder) fieldOrBuilder; + } else { + subBuilder = ((MessageLite) fieldOrBuilder).toBuilder(); + extensions.setField(field, subBuilder); + } + input.readGroup(field.getNumber(), subBuilder, extensionRegistry); + return; + } + Message.Builder subBuilder = defaultInstance.newBuilderForType(); + input.readGroup(field.getNumber(), subBuilder, extensionRegistry); + Object unused = setField(field, subBuilder); + } else { + Message.Builder subBuilder = defaultInstance.newBuilderForType(); + input.readGroup(field.getNumber(), subBuilder, extensionRegistry); + Object unused = addRepeatedField(field, subBuilder.buildPartial()); + } + } + + @Override + public void mergeMessage( + CodedInputStream input, + ExtensionRegistryLite extensionRegistry, + FieldDescriptor field, + Message defaultInstance) + throws IOException { + if (!field.isRepeated()) { + if (hasField(field)) { + Object fieldOrBuilder = extensions.getFieldAllowBuilders(field); + MessageLite.Builder subBuilder; + if (fieldOrBuilder instanceof MessageLite.Builder) { + subBuilder = (MessageLite.Builder) fieldOrBuilder; + } else { + subBuilder = ((MessageLite) fieldOrBuilder).toBuilder(); + extensions.setField(field, subBuilder); + } + input.readMessage(subBuilder, extensionRegistry); + return; + } + Message.Builder subBuilder = defaultInstance.newBuilderForType(); + input.readMessage(subBuilder, extensionRegistry); + Object unused = setField(field, subBuilder); + } else { + Message.Builder subBuilder = defaultInstance.newBuilderForType(); + input.readMessage(subBuilder, extensionRegistry); + Object unused = addRepeatedField(field, subBuilder.buildPartial()); + } + } + + @Override + public Object parseMessageFromBytes( + ByteString bytes, + ExtensionRegistryLite registry, + Descriptors.FieldDescriptor field, + Message defaultInstance) + throws IOException { + Message.Builder subBuilder = defaultInstance.newBuilderForType(); + if (!field.isRepeated()) { + Message originalMessage = (Message) getField(field); + if (originalMessage != null) { + subBuilder.mergeFrom(originalMessage); + } + } + subBuilder.mergeFrom(bytes, registry); + return subBuilder.buildPartial(); + } + + @Override + public MergeTarget newMergeTargetForField( + Descriptors.FieldDescriptor descriptor, Message defaultInstance) { + throw new UnsupportedOperationException("newMergeTargetForField() called on FieldSet object"); + } + + @Override + public MergeTarget newEmptyTargetForField( + Descriptors.FieldDescriptor descriptor, Message defaultInstance) { + throw new UnsupportedOperationException("newEmptyTargetForField() called on FieldSet object"); + } + + @Override + public WireFormat.Utf8Validation getUtf8Validation(Descriptors.FieldDescriptor descriptor) { + if (descriptor.needsUtf8Check()) { + return WireFormat.Utf8Validation.STRICT; + } + // TODO(b/248145492): support lazy strings for ExtesnsionSet. return WireFormat.Utf8Validation.LOOSE; } @@ -829,13 +1187,13 @@ class MessageReflection { switch (field.getType()) { case GROUP: { - value = target.parseGroup(input, extensionRegistry, field, defaultInstance); - break; + target.mergeGroup(input, extensionRegistry, field, defaultInstance); + return true; } case MESSAGE: { - value = target.parseMessage(input, extensionRegistry, field, defaultInstance); - break; + target.mergeMessage(input, extensionRegistry, field, defaultInstance); + return true; } case ENUM: final int rawValue = input.readEnum(); @@ -870,6 +1228,29 @@ class MessageReflection { return true; } + /** Read a message from the given input stream into the provided target and UnknownFieldSet. */ + static void mergeMessageFrom( + Message.Builder target, + UnknownFieldSet.Builder unknownFields, + CodedInputStream input, + ExtensionRegistryLite extensionRegistry) + throws IOException { + BuilderAdapter builderAdapter = new BuilderAdapter(target); + Descriptor descriptorForType = target.getDescriptorForType(); + while (true) { + final int tag = input.readTag(); + if (tag == 0) { + break; + } + + if (!mergeFieldFrom( + input, unknownFields, extensionRegistry, descriptorForType, builderAdapter, tag)) { + // end group tag + break; + } + } + } + /** Called by {@code #mergeFieldFrom()} to parse a MessageSet extension into MergeTarget. */ private static void mergeMessageSetExtensionFromCodedStream( CodedInputStream input, diff --git a/java/core/src/main/java/com/google/protobuf/TextFormat.java b/java/core/src/main/java/com/google/protobuf/TextFormat.java index aa1cc34a6a..9b3c3569b6 100644 --- a/java/core/src/main/java/com/google/protobuf/TextFormat.java +++ b/java/core/src/main/java/com/google/protobuf/TextFormat.java @@ -604,7 +604,7 @@ public final class TextFormat { case MESSAGE: case GROUP: - print((Message) value, generator); + print((MessageOrBuilder) value, generator); break; } } diff --git a/java/util/src/main/java/com/google/protobuf/util/FieldMaskTree.java b/java/util/src/main/java/com/google/protobuf/util/FieldMaskTree.java index 096acee299..093740d51d 100644 --- a/java/util/src/main/java/com/google/protobuf/util/FieldMaskTree.java +++ b/java/util/src/main/java/com/google/protobuf/util/FieldMaskTree.java @@ -35,6 +35,7 @@ import com.google.errorprone.annotations.CanIgnoreReturnValue; import com.google.protobuf.Descriptors.Descriptor; import com.google.protobuf.Descriptors.FieldDescriptor; import com.google.protobuf.FieldMask; +import com.google.protobuf.GeneratedMessage; import com.google.protobuf.Message; import java.util.ArrayList; import java.util.List; @@ -292,7 +293,11 @@ final class FieldMaskTree { // so we don't create unnecessary empty messages. continue; } - Message.Builder childBuilder = ((Message) destination.getField(field)).toBuilder(); + // This is a mess because of java proto API 1 still hanging around. + Message.Builder childBuilder = + destination instanceof GeneratedMessage.Builder + ? destination.getFieldBuilder(field) + : ((Message) destination.getField(field)).toBuilder(); merge(entry.getValue(), (Message) source.getField(field), childBuilder, options); destination.setField(field, childBuilder.buildPartial()); continue; diff --git a/pkg/BUILD.bazel b/pkg/BUILD.bazel index 65dc37c0c6..3ad6a27c7c 100644 --- a/pkg/BUILD.bazel +++ b/pkg/BUILD.bazel @@ -314,20 +314,18 @@ gen_file_lists( "//src/google/protobuf:descriptor_proto": "descriptor_proto", "//src/google/protobuf/compiler:plugin_proto": "plugin_proto", # Test libraries: + ":common_test": "common_test", ":lite_test_util": "lite_test_util", ":test_util": "test_util", # Tests and test-only protos: "//src/google/protobuf:full_test_srcs": "protobuf_test", - "//src/google/protobuf:test_protos": "protobuf_test_protos", + "//src/google/protobuf:test_proto_srcs": "protobuf_test_protos", "//src/google/protobuf:lite_test_srcs": "protobuf_lite_test", - "//src/google/protobuf:lite_test_protos": "protobuf_lite_test_protos", + "//src/google/protobuf:lite_test_proto_srcs": "protobuf_lite_test_protos", "//src/google/protobuf/compiler:test_srcs": "compiler_test", - ":compiler_annotation_test_util": "annotation_test_util", - ":compiler_mock_code_generator": "mock_code_generator", "//src/google/protobuf/compiler:test_proto_srcs": "compiler_test_protos", "//src/google/protobuf/compiler:test_plugin_srcs": "test_plugin", "//src/google/protobuf/io:test_srcs": "io_test", - ":testinglib": "testing", "//src/google/protobuf/util:test_srcs": "util_test", "//src/google/protobuf/util:test_proto_srcs": "util_test_protos", "//src/google/protobuf/stubs:test_srcs": "stubs_test", @@ -365,10 +363,11 @@ cc_dist_library( }), tags = ["manual"], deps = [ - "//src/google/protobuf:wkt_cc_proto", - "//src/google/protobuf:protobuf_nowkt", "//src/google/protobuf:arena", + "//src/google/protobuf:port_def", "//src/google/protobuf:protobuf_lite", + "//src/google/protobuf:protobuf_nowkt", + "//src/google/protobuf:wkt_cc_proto", "//src/google/protobuf/compiler:importer", "//src/google/protobuf/io", "//src/google/protobuf/io:gzip_stream", @@ -384,6 +383,7 @@ 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/internal:constants", "//src/google/protobuf/util/internal:datapiece", "//src/google/protobuf/util/internal:default_value", "//src/google/protobuf/util/internal:field_mask_utility", @@ -422,28 +422,25 @@ cc_dist_library( name = "test_util", testonly = 1, tags = ["manual"], - deps = ["//src/google/protobuf:test_util"], -) - -cc_dist_library( - name = "compiler_annotation_test_util", - testonly = 1, - tags = ["manual"], - deps = ["//src/google/protobuf/compiler:annotation_test_util"], -) - -cc_dist_library( - name = "compiler_mock_code_generator", - testonly = 1, - tags = ["manual"], - deps = ["//src/google/protobuf/compiler:mock_code_generator"], + deps = [ + "//src/google/protobuf:test_util", + "//src/google/protobuf:test_util2", + "//src/google/protobuf/compiler:annotation_test_util", + "//src/google/protobuf/compiler/cpp:unittest_lib", + "//src/google/protobuf/util/internal:expecting_objectwriter", + "//src/google/protobuf/util/internal:mock_error_listener", + "//src/google/protobuf/util/internal:type_info_test_helper", + ], ) cc_dist_library( - name = "testinglib", + name = "common_test", testonly = 1, tags = ["manual"], - deps = ["//src/google/protobuf/testing"], + deps = [ + "//src/google/protobuf/compiler:mock_code_generator", + "//src/google/protobuf/testing", + ], ) ################################################################################ diff --git a/python/google/protobuf/__init__.py b/python/google/protobuf/__init__.py index 4d288e20d0..9ba2b78c85 100644 --- a/python/google/protobuf/__init__.py +++ b/python/google/protobuf/__init__.py @@ -30,4 +30,4 @@ # Copyright 2007 Google Inc. All Rights Reserved. -__version__ = '4.21.5' +__version__ = '4.21.6' diff --git a/src/file_lists.cmake b/src/file_lists.cmake index 3e88c55dd8..a61e3f1373 100644 --- a/src/file_lists.cmake +++ b/src/file_lists.cmake @@ -156,6 +156,8 @@ set(libprotobuf_hdrs ${protobuf_SOURCE_DIR}/src/google/protobuf/metadata_lite.h ${protobuf_SOURCE_DIR}/src/google/protobuf/parse_context.h ${protobuf_SOURCE_DIR}/src/google/protobuf/port.h + ${protobuf_SOURCE_DIR}/src/google/protobuf/port_def.inc + ${protobuf_SOURCE_DIR}/src/google/protobuf/port_undef.inc ${protobuf_SOURCE_DIR}/src/google/protobuf/reflection.h ${protobuf_SOURCE_DIR}/src/google/protobuf/reflection_internal.h ${protobuf_SOURCE_DIR}/src/google/protobuf/reflection_ops.h @@ -177,6 +179,7 @@ set(libprotobuf_hdrs ${protobuf_SOURCE_DIR}/src/google/protobuf/util/delimited_message_util.h ${protobuf_SOURCE_DIR}/src/google/protobuf/util/field_comparator.h ${protobuf_SOURCE_DIR}/src/google/protobuf/util/field_mask_util.h + ${protobuf_SOURCE_DIR}/src/google/protobuf/util/internal/constants.h ${protobuf_SOURCE_DIR}/src/google/protobuf/util/internal/datapiece.h ${protobuf_SOURCE_DIR}/src/google/protobuf/util/internal/default_value_objectwriter.h ${protobuf_SOURCE_DIR}/src/google/protobuf/util/internal/error_listener.h @@ -520,6 +523,20 @@ set(plugin_proto_files ${protobuf_SOURCE_DIR}/src/google/protobuf/compiler/plugin_proto-descriptor-set.proto.bin ) +# //pkg:common_test +set(common_test_srcs + ${protobuf_SOURCE_DIR}/src/google/protobuf/compiler/mock_code_generator.cc + ${protobuf_SOURCE_DIR}/src/google/protobuf/testing/file.cc + ${protobuf_SOURCE_DIR}/src/google/protobuf/testing/googletest.cc +) + +# //pkg:common_test +set(common_test_hdrs + ${protobuf_SOURCE_DIR}/src/google/protobuf/compiler/mock_code_generator.h + ${protobuf_SOURCE_DIR}/src/google/protobuf/testing/file.h + ${protobuf_SOURCE_DIR}/src/google/protobuf/testing/googletest.h +) + # //pkg:lite_test_util set(lite_test_util_srcs ${protobuf_SOURCE_DIR}/src/google/protobuf/arena_test_util.cc @@ -538,12 +555,17 @@ set(lite_test_util_hdrs # //pkg:test_util set(test_util_srcs + ${protobuf_SOURCE_DIR}/src/google/protobuf/compiler/annotation_test_util.cc ${protobuf_SOURCE_DIR}/src/google/protobuf/reflection_tester.cc ${protobuf_SOURCE_DIR}/src/google/protobuf/test_util.cc + ${protobuf_SOURCE_DIR}/src/google/protobuf/util/internal/type_info_test_helper.cc ) # //pkg:test_util set(test_util_hdrs + ${protobuf_SOURCE_DIR}/src/google/protobuf/compiler/annotation_test_util.h + ${protobuf_SOURCE_DIR}/src/google/protobuf/compiler/cpp/unittest.h + ${protobuf_SOURCE_DIR}/src/google/protobuf/compiler/cpp/unittest.inc ${protobuf_SOURCE_DIR}/src/google/protobuf/map_test.inc ${protobuf_SOURCE_DIR}/src/google/protobuf/map_test_util.h ${protobuf_SOURCE_DIR}/src/google/protobuf/map_test_util.inc @@ -551,7 +573,11 @@ set(test_util_hdrs ${protobuf_SOURCE_DIR}/src/google/protobuf/reflection_tester.h ${protobuf_SOURCE_DIR}/src/google/protobuf/test_util.h ${protobuf_SOURCE_DIR}/src/google/protobuf/test_util.inc + ${protobuf_SOURCE_DIR}/src/google/protobuf/test_util2.h ${protobuf_SOURCE_DIR}/src/google/protobuf/test_util_lite.h + ${protobuf_SOURCE_DIR}/src/google/protobuf/util/internal/expecting_objectwriter.h + ${protobuf_SOURCE_DIR}/src/google/protobuf/util/internal/mock_error_listener.h + ${protobuf_SOURCE_DIR}/src/google/protobuf/util/internal/type_info_test_helper.h ${protobuf_SOURCE_DIR}/src/google/protobuf/wire_format_unittest.inc ) @@ -586,8 +612,8 @@ set(protobuf_test_files ${protobuf_SOURCE_DIR}/src/google/protobuf/wire_format_unittest.cc ) -# //src/google/protobuf:test_protos -set(protobuf_test_protos_proto_srcs +# //src/google/protobuf:test_proto_srcs +set(protobuf_test_protos_files ${protobuf_SOURCE_DIR}/src/google/protobuf/any_test.proto ${protobuf_SOURCE_DIR}/src/google/protobuf/map_proto2_unittest.proto ${protobuf_SOURCE_DIR}/src/google/protobuf/map_unittest.proto @@ -619,112 +645,20 @@ set(protobuf_test_protos_proto_srcs ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_well_known_types.proto ) -# //src/google/protobuf:test_protos -set(protobuf_test_protos_srcs - ${protobuf_SOURCE_DIR}/src/google/protobuf/any_test.proto.pb.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/map_proto2_unittest.proto.pb.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/map_unittest.proto.pb.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest.proto.pb.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_arena.proto.pb.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_custom_options.proto.pb.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_drop_unknown_fields.proto.pb.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_embed_optimize_for.proto.pb.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_empty.proto.pb.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_enormous_descriptor.proto.pb.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_import.proto.pb.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_import_public.proto.pb.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_lazy_dependencies.proto.pb.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_lazy_dependencies_custom_option.proto.pb.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_lazy_dependencies_enum.proto.pb.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_lite_imports_nonlite.proto.pb.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_mset.proto.pb.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_mset_wire_format.proto.pb.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_no_field_presence.proto.pb.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_no_generic_services.proto.pb.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_optimize_for.proto.pb.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_preserve_unknown_enum.proto.pb.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_preserve_unknown_enum2.proto.pb.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_proto3.proto.pb.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_proto3_arena.proto.pb.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_proto3_arena_lite.proto.pb.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_proto3_lite.proto.pb.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_proto3_optional.proto.pb.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_well_known_types.proto.pb.cc -) - -# //src/google/protobuf:test_protos -set(protobuf_test_protos_hdrs - ${protobuf_SOURCE_DIR}/src/google/protobuf/any_test.proto.pb.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/map_proto2_unittest.proto.pb.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/map_unittest.proto.pb.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest.proto.pb.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_arena.proto.pb.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_custom_options.proto.pb.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_drop_unknown_fields.proto.pb.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_embed_optimize_for.proto.pb.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_empty.proto.pb.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_enormous_descriptor.proto.pb.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_import.proto.pb.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_import_public.proto.pb.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_lazy_dependencies.proto.pb.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_lazy_dependencies_custom_option.proto.pb.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_lazy_dependencies_enum.proto.pb.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_lite_imports_nonlite.proto.pb.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_mset.proto.pb.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_mset_wire_format.proto.pb.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_no_field_presence.proto.pb.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_no_generic_services.proto.pb.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_optimize_for.proto.pb.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_preserve_unknown_enum.proto.pb.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_preserve_unknown_enum2.proto.pb.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_proto3.proto.pb.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_proto3_arena.proto.pb.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_proto3_arena_lite.proto.pb.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_proto3_lite.proto.pb.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_proto3_optional.proto.pb.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_well_known_types.proto.pb.h -) - -# //src/google/protobuf:test_protos -set(protobuf_test_protos_files - ${protobuf_SOURCE_DIR}/src/google/protobuf/test_protos-descriptor-set.proto.bin -) - # //src/google/protobuf:lite_test_srcs set(protobuf_lite_test_files ${protobuf_SOURCE_DIR}/src/google/protobuf/lite_arena_unittest.cc ${protobuf_SOURCE_DIR}/src/google/protobuf/lite_unittest.cc ) -# //src/google/protobuf:lite_test_protos -set(protobuf_lite_test_protos_proto_srcs +# //src/google/protobuf:lite_test_proto_srcs +set(protobuf_lite_test_protos_files ${protobuf_SOURCE_DIR}/src/google/protobuf/map_lite_unittest.proto ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_import_lite.proto ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_import_public_lite.proto ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_lite.proto ) -# //src/google/protobuf:lite_test_protos -set(protobuf_lite_test_protos_srcs - ${protobuf_SOURCE_DIR}/src/google/protobuf/map_lite_unittest.proto.pb.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_import_lite.proto.pb.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_import_public_lite.proto.pb.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_lite.proto.pb.cc -) - -# //src/google/protobuf:lite_test_protos -set(protobuf_lite_test_protos_hdrs - ${protobuf_SOURCE_DIR}/src/google/protobuf/map_lite_unittest.proto.pb.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_import_lite.proto.pb.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_import_public_lite.proto.pb.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/unittest_lite.proto.pb.h -) - -# //src/google/protobuf:lite_test_protos -set(protobuf_lite_test_protos_files - ${protobuf_SOURCE_DIR}/src/google/protobuf/lite_test_protos-descriptor-set.proto.bin -) - # //src/google/protobuf/compiler:test_srcs set(compiler_test_files ${protobuf_SOURCE_DIR}/src/google/protobuf/compiler/command_line_interface_unittest.cc @@ -746,26 +680,6 @@ set(compiler_test_files ${protobuf_SOURCE_DIR}/src/google/protobuf/compiler/ruby/ruby_generator_unittest.cc ) -# //pkg:compiler_annotation_test_util -set(annotation_test_util_srcs - ${protobuf_SOURCE_DIR}/src/google/protobuf/compiler/annotation_test_util.cc -) - -# //pkg:compiler_annotation_test_util -set(annotation_test_util_hdrs - ${protobuf_SOURCE_DIR}/src/google/protobuf/compiler/annotation_test_util.h -) - -# //pkg:compiler_mock_code_generator -set(mock_code_generator_srcs - ${protobuf_SOURCE_DIR}/src/google/protobuf/compiler/mock_code_generator.cc -) - -# //pkg:compiler_mock_code_generator -set(mock_code_generator_hdrs - ${protobuf_SOURCE_DIR}/src/google/protobuf/compiler/mock_code_generator.h -) - # //src/google/protobuf/compiler:test_proto_srcs set(compiler_test_protos_files ${protobuf_SOURCE_DIR}/src/google/protobuf/compiler/cpp/test_bad_identifiers.proto @@ -788,18 +702,6 @@ set(io_test_files ${protobuf_SOURCE_DIR}/src/google/protobuf/io/zero_copy_stream_unittest.cc ) -# //pkg:testinglib -set(testing_srcs - ${protobuf_SOURCE_DIR}/src/google/protobuf/testing/file.cc - ${protobuf_SOURCE_DIR}/src/google/protobuf/testing/googletest.cc -) - -# //pkg:testinglib -set(testing_hdrs - ${protobuf_SOURCE_DIR}/src/google/protobuf/testing/file.h - ${protobuf_SOURCE_DIR}/src/google/protobuf/testing/googletest.h -) - # //src/google/protobuf/util:test_srcs set(util_test_files ${protobuf_SOURCE_DIR}/src/google/protobuf/util/delimited_message_util_test.cc diff --git a/src/google/protobuf/BUILD.bazel b/src/google/protobuf/BUILD.bazel index cd80b1d4ab..0578e6681c 100644 --- a/src/google/protobuf/BUILD.bazel +++ b/src/google/protobuf/BUILD.bazel @@ -392,14 +392,20 @@ filegroup( ], ) -proto_library( - name = "lite_test_protos", +filegroup( + name = "lite_test_proto_srcs", srcs = [ "map_lite_unittest.proto", "unittest_import_lite.proto", "unittest_import_public_lite.proto", "unittest_lite.proto", ], + visibility = ["//:__subpackages__"], +) + +proto_library( + name = "lite_test_protos", + srcs = [":lite_test_proto_srcs"], strip_import_prefix = "/src", visibility = ["//:__subpackages__"], deps = [ diff --git a/src/google/protobuf/arena.cc b/src/google/protobuf/arena.cc index 49591abc83..b5dbb30f6a 100644 --- a/src/google/protobuf/arena.cc +++ b/src/google/protobuf/arena.cc @@ -122,7 +122,7 @@ SerialArena::SerialArena(ArenaBlock* b, ThreadSafeArena& parent) : ptr_{b->Pointer(kBlockHeaderSize + ThreadSafeArena::kSerialArenaSize)}, limit_{b->Limit()}, head_{b}, - space_allocated_{b->size()}, + space_allocated_{b->size}, parent_{parent} { GOOGLE_DCHECK(!b->IsSentry()); } @@ -135,7 +135,7 @@ SerialArena::SerialArena(ThreadSafeArena& parent) // provided or newly allocated to store AllocationPolicy. SerialArena::SerialArena(FirstSerialArena, ArenaBlock* b, ThreadSafeArena& parent) - : head_{b}, space_allocated_{b->size()}, parent_{parent} { + : head_{b}, space_allocated_{b->size}, parent_{parent} { if (b->IsSentry()) return; set_ptr(b->Pointer(kBlockHeaderSize)); @@ -145,9 +145,9 @@ SerialArena::SerialArena(FirstSerialArena, ArenaBlock* b, void SerialArena::Init(ArenaBlock* b, size_t offset) { set_ptr(b->Pointer(offset)); limit_ = b->Limit(); - set_head(b); + head_.relaxed_set(b); space_used_.relaxed_set(0); - space_allocated_.relaxed_set(b->size()); + space_allocated_.relaxed_set(b->size); cached_block_length_ = 0; cached_blocks_ = nullptr; } @@ -164,11 +164,11 @@ SerialArena* SerialArena::New(Memory mem, ThreadSafeArena& parent) { template SerialArena::Memory SerialArena::Free(Deallocator deallocator) { ArenaBlock* b = head(); - Memory mem = {b, b->size()}; + Memory mem = {b, b->size}; while (b->next) { b = b->next; // We must first advance before deleting this block deallocator(mem); - mem = {b, b->size()}; + mem = {b, b->size}; } return mem; } @@ -204,7 +204,7 @@ void SerialArena::AllocateNewBlock(size_t n) { // Record how much used in this block. used = static_cast(ptr() - old_head->Pointer(kBlockHeaderSize)); - wasted = old_head->size() - used; + wasted = old_head->size - used; space_used_.relaxed_set(space_used_.relaxed_get() + used); } @@ -213,7 +213,7 @@ void SerialArena::AllocateNewBlock(size_t n) { // but with a CPU regression. The regression might have been an artifact of // the microbenchmark. - auto mem = AllocateMemory(parent_.AllocPolicy(), old_head->size(), n); + auto mem = AllocateMemory(parent_.AllocPolicy(), old_head->size, n); // We don't want to emit an expensive RMW instruction that requires // exclusive access to a cacheline. Hence we write it in terms of a // regular add. @@ -222,9 +222,10 @@ void SerialArena::AllocateNewBlock(size_t n) { /*used=*/used, /*allocated=*/mem.size, wasted); auto* new_head = new (mem.ptr) ArenaBlock{old_head, mem.size}; - set_head(new_head); set_ptr(new_head->Pointer(kBlockHeaderSize)); limit_ = new_head->Limit(); + // Previous writes must take effect before writing new head. + head_.atomic_set(new_head); #ifdef ADDRESS_SANITIZER ASAN_POISON_MEMORY_REGION(ptr(), limit_ - ptr()); @@ -239,10 +240,10 @@ uint64_t SerialArena::SpaceUsed() const { // usage of the *current* block. // TODO(mkruskal) Consider eliminating this race in exchange for a possible // performance hit on ARM (see cl/455186837). - const ArenaBlock* h = head(); + const ArenaBlock* h = head_.atomic_get(); if (h->IsSentry()) return 0; - const uint64_t current_block_size = h->size(); + const uint64_t current_block_size = h->size; uint64_t current_space_used = std::min( static_cast( ptr() - const_cast(h)->Pointer(kBlockHeaderSize)), @@ -400,7 +401,7 @@ class ThreadSafeArena::SerialArenaChunk { } id(idx).relaxed_set(me); - arena(idx).relaxed_set(serial); + arena(idx).atomic_set(serial); return true; } @@ -774,7 +775,7 @@ template void ThreadSafeArena::PerConstSerialArenaInChunk(Functor fn) const { WalkConstSerialArenaChunk([&fn](const SerialArenaChunk* chunk) { for (const auto& each : chunk->arenas()) { - const SerialArena* serial = each.relaxed_get(); + const SerialArena* serial = each.atomic_get(); // It is possible that newly added SerialArena is not updated although // size was. This is acceptable for SpaceAllocated and SpaceUsed. if (serial == nullptr) continue; diff --git a/src/google/protobuf/arena_impl.h b/src/google/protobuf/arena_impl.h index fa4d6f50c8..3bab4ae8f5 100644 --- a/src/google/protobuf/arena_impl.h +++ b/src/google/protobuf/arena_impl.h @@ -94,11 +94,9 @@ struct Atomic { PROTOBUF_CONSTEXPR explicit Atomic(T v) : val(v) {} T relaxed_get() const { return val.load(std::memory_order_relaxed); } - T relaxed_get() { return val.load(std::memory_order_relaxed); } void relaxed_set(T v) { val.store(v, std::memory_order_relaxed); } T atomic_get() const { return val.load(std::memory_order_acquire); } - T atomic_get() { return val.load(std::memory_order_acquire); } void atomic_set(T v) { val.store(v, std::memory_order_release); } T relaxed_fetch_add(T v) { @@ -115,27 +113,24 @@ struct ArenaBlock { // For the sentry block with zero-size where ptr_, limit_, cleanup_nodes all // point to "this". PROTOBUF_CONSTEXPR ArenaBlock() - : next(nullptr), cleanup_nodes(this), relaxed_size(0) {} + : next(nullptr), cleanup_nodes(this), size(0) {} ArenaBlock(ArenaBlock* next, size_t size) - : next(next), cleanup_nodes(nullptr), relaxed_size(size) { + : next(next), cleanup_nodes(nullptr), size(size) { GOOGLE_DCHECK_GT(size, sizeof(ArenaBlock)); } char* Pointer(size_t n) { - GOOGLE_DCHECK_LE(n, size()); + GOOGLE_DCHECK_LE(n, size); return reinterpret_cast(this) + n; } - char* Limit() { return Pointer(size() & static_cast(-8)); } + char* Limit() { return Pointer(size & static_cast(-8)); } - size_t size() const { return relaxed_size.relaxed_get(); } - bool IsSentry() const { return size() == 0; } + bool IsSentry() const { return size == 0; } ArenaBlock* const next; void* cleanup_nodes; - - private: - const Atomic relaxed_size; + const size_t size; // data follows }; @@ -618,7 +613,6 @@ class PROTOBUF_EXPORT SerialArena { // Helper getters/setters to handle relaxed operations on atomic variables. ArenaBlock* head() { return head_.relaxed_get(); } const ArenaBlock* head() const { return head_.relaxed_get(); } - void set_head(ArenaBlock* head) { return head_.relaxed_set(head); } char* ptr() { return ptr_.relaxed_get(); } const char* ptr() const { return ptr_.relaxed_get(); } diff --git a/src/google/protobuf/compiler/cpp/BUILD.bazel b/src/google/protobuf/compiler/cpp/BUILD.bazel index bf95ffef03..4463a09652 100644 --- a/src/google/protobuf/compiler/cpp/BUILD.bazel +++ b/src/google/protobuf/compiler/cpp/BUILD.bazel @@ -98,6 +98,7 @@ cc_library( "unittest.inc", ], strip_include_prefix = "/src", + visibility = ["//pkg:__pkg__"], ) cc_test( diff --git a/src/google/protobuf/compiler/cpp/message_size_unittest.cc b/src/google/protobuf/compiler/cpp/message_size_unittest.cc index a75d77a70c..ed4a90e223 100644 --- a/src/google/protobuf/compiler/cpp/message_size_unittest.cc +++ b/src/google/protobuf/compiler/cpp/message_size_unittest.cc @@ -139,9 +139,9 @@ TEST(GeneratedMessageTest, OneStringSize) { TEST(GeneratedMessageTest, MoreStringSize) { struct MockGenerated : public MockMessageBase { // 16 bytes - int has_bits[1]; // 4 bytes int cached_size; // 4 bytes MockRepeatedPtrField data; // 24 bytes + // + 4 bytes padding }; GOOGLE_CHECK_MESSAGE_SIZE(MockGenerated, 48); EXPECT_EQ(sizeof(protobuf_unittest::MoreString), sizeof(MockGenerated)); diff --git a/src/google/protobuf/compiler/java/enum_field.cc b/src/google/protobuf/compiler/java/enum_field.cc index ea74d8bc6f..2eccbd497b 100644 --- a/src/google/protobuf/compiler/java/enum_field.cc +++ b/src/google/protobuf/compiler/java/enum_field.cc @@ -116,13 +116,6 @@ void SetEnumVariables(const FieldDescriptor* descriptor, int messageBitIndex, (*variables)["set_mutable_bit_builder"] = GenerateSetBit(builderBitIndex); (*variables)["clear_mutable_bit_builder"] = GenerateClearBit(builderBitIndex); - // For repeated fields, one bit is used for whether the array is immutable - // in the parsing constructor. - (*variables)["get_mutable_bit_parser"] = - GenerateGetBitMutableLocal(builderBitIndex); - (*variables)["set_mutable_bit_parser"] = - GenerateSetBitMutableLocal(builderBitIndex); - (*variables)["get_has_field_bit_from_local"] = GenerateGetBitFromLocal(builderBitIndex); (*variables)["set_has_field_bit_to_local"] = @@ -276,7 +269,7 @@ void ImmutableEnumFieldGenerator::GenerateKotlinDslMembers( io::Printer* printer) const { WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print(variables_, - "$kt_deprecation$ var $kt_name$: $kt_type$\n" + "$kt_deprecation$public var $kt_name$: $kt_type$\n" " @JvmName(\"${$get$kt_capitalized_name$$}$\")\n" " get() = $kt_dsl_builder$.${$get$capitalized_name$$}$()\n" " @JvmName(\"${$set$kt_capitalized_name$$}$\")\n" @@ -299,7 +292,7 @@ void ImmutableEnumFieldGenerator::GenerateKotlinDslMembers( WriteFieldAccessorDocComment(printer, descriptor_, CLEARER, /* builder */ false, /* kdoc */ true); printer->Print(variables_, - "fun ${$clear$kt_capitalized_name$$}$() {\n" + "public fun ${$clear$kt_capitalized_name$$}$() {\n" " $kt_dsl_builder$.${$clear$capitalized_name$$}$()\n" "}\n"); @@ -308,7 +301,7 @@ void ImmutableEnumFieldGenerator::GenerateKotlinDslMembers( /* builder */ false, /* kdoc */ true); printer->Print( variables_, - "fun ${$has$kt_capitalized_name$$}$(): kotlin.Boolean {\n" + "public fun ${$has$kt_capitalized_name$$}$(): kotlin.Boolean {\n" " return $kt_dsl_builder$.${$has$capitalized_name$$}$()\n" "}\n"); } @@ -360,31 +353,26 @@ void ImmutableEnumFieldGenerator::GenerateBuildingCode( printer->Print(variables_, "result.$name$_ = $name$_;\n"); } -void ImmutableEnumFieldGenerator::GenerateParsingCode( +void ImmutableEnumFieldGenerator::GenerateBuilderParsingCode( io::Printer* printer) const { if (SupportUnknownEnumValue(descriptor_->file())) { printer->Print(variables_, - "int rawValue = input.readEnum();\n" - "$set_has_field_bit_message$\n" - "$name$_ = rawValue;\n"); + "$name$_ = input.readEnum();\n" + "$set_has_field_bit_builder$\n"); } else { printer->Print(variables_, - "int rawValue = input.readEnum();\n" - "$type$ value = $type$.forNumber(rawValue);\n" - "if (value == null) {\n" - " unknownFields.mergeVarintField($number$, rawValue);\n" + "int tmpRaw = input.readEnum();\n" + "$type$ tmpValue =\n" + " $type$.forNumber(tmpRaw);\n" + "if (tmpValue == null) {\n" + " mergeUnknownVarintField($number$, tmpRaw);\n" "} else {\n" - " $set_has_field_bit_message$\n" - " $name$_ = rawValue;\n" + " $name$_ = tmpRaw;\n" + " $set_has_field_bit_builder$\n" "}\n"); } } -void ImmutableEnumFieldGenerator::GenerateParsingDoneCode( - io::Printer* printer) const { - // noop for enums -} - void ImmutableEnumFieldGenerator::GenerateSerializationCode( io::Printer* printer) const { printer->Print(variables_, @@ -543,6 +531,11 @@ void ImmutableEnumOneofFieldGenerator::GenerateBuilderMembers( printer->Annotate("{", "}", descriptor_); } +void ImmutableEnumOneofFieldGenerator::GenerateBuilderClearCode( + io::Printer* printer) const { + // Enum fields in oneofs are correctly cleared by clearing the oneof +} + void ImmutableEnumOneofFieldGenerator::GenerateBuildingCode( io::Printer* printer) const { printer->Print(variables_, @@ -563,7 +556,7 @@ void ImmutableEnumOneofFieldGenerator::GenerateMergingCode( } } -void ImmutableEnumOneofFieldGenerator::GenerateParsingCode( +void ImmutableEnumOneofFieldGenerator::GenerateBuilderParsingCode( io::Printer* printer) const { if (SupportUnknownEnumValue(descriptor_->file())) { printer->Print(variables_, @@ -573,9 +566,10 @@ void ImmutableEnumOneofFieldGenerator::GenerateParsingCode( } else { printer->Print(variables_, "int rawValue = input.readEnum();\n" - "$type$ value = $type$.forNumber(rawValue);\n" + "$type$ value =\n" + " $type$.forNumber(rawValue);\n" "if (value == null) {\n" - " unknownFields.mergeVarintField($number$, rawValue);\n" + " mergeUnknownVarintField($number$, rawValue);\n" "} else {\n" " $set_oneof_case_message$;\n" " $oneof_name$_ = rawValue;\n" @@ -953,35 +947,29 @@ void RepeatedImmutableEnumFieldGenerator::GenerateBuildingCode( "result.$name$_ = $name$_;\n"); } -void RepeatedImmutableEnumFieldGenerator::GenerateParsingCode( +void RepeatedImmutableEnumFieldGenerator::GenerateBuilderParsingCode( io::Printer* printer) const { // Read and store the enum if (SupportUnknownEnumValue(descriptor_->file())) { printer->Print(variables_, - "int rawValue = input.readEnum();\n" - "if (!$get_mutable_bit_parser$) {\n" - " $name$_ = new java.util.ArrayList();\n" - " $set_mutable_bit_parser$;\n" - "}\n" - "$name$_.add(rawValue);\n"); + "int tmpRaw = input.readEnum();\n" + "ensure$capitalized_name$IsMutable();\n" + "$name$_.add(tmpRaw);\n"); } else { - printer->Print( - variables_, - "int rawValue = input.readEnum();\n" - "$type$ value = $type$.forNumber(rawValue);\n" - "if (value == null) {\n" - " unknownFields.mergeVarintField($number$, rawValue);\n" - "} else {\n" - " if (!$get_mutable_bit_parser$) {\n" - " $name$_ = new java.util.ArrayList();\n" - " $set_mutable_bit_parser$;\n" - " }\n" - " $name$_.add(rawValue);\n" - "}\n"); + printer->Print(variables_, + "int tmpRaw = input.readEnum();\n" + "$type$ tmpValue =\n" + " $type$.forNumber(tmpRaw);\n" + "if (tmpValue == null) {\n" + " mergeUnknownVarintField($number$, tmpRaw);\n" + "} else {\n" + " ensure$capitalized_name$IsMutable();\n" + " $name$_.add(tmpRaw);\n" + "}\n"); } } -void RepeatedImmutableEnumFieldGenerator::GenerateParsingCodeFromPacked( +void RepeatedImmutableEnumFieldGenerator::GenerateBuilderParsingCodeFromPacked( io::Printer* printer) const { // Wrap GenerateParsingCode's contents with a while loop. @@ -991,7 +979,7 @@ void RepeatedImmutableEnumFieldGenerator::GenerateParsingCodeFromPacked( "while(input.getBytesUntilLimit() > 0) {\n"); printer->Indent(); - GenerateParsingCode(printer); + GenerateBuilderParsingCode(printer); printer->Outdent(); printer->Print(variables_, @@ -999,15 +987,6 @@ void RepeatedImmutableEnumFieldGenerator::GenerateParsingCodeFromPacked( "input.popLimit(oldLimit);\n"); } -void RepeatedImmutableEnumFieldGenerator::GenerateParsingDoneCode( - io::Printer* printer) const { - printer->Print( - variables_, - "if ($get_mutable_bit_parser$) {\n" - " $name$_ = java.util.Collections.unmodifiableList($name$_);\n" - "}\n"); -} - void RepeatedImmutableEnumFieldGenerator::GenerateSerializationCode( io::Printer* printer) const { if (descriptor_->is_packed()) { @@ -1085,12 +1064,12 @@ void RepeatedImmutableEnumFieldGenerator::GenerateKotlinDslMembers( " */\n" "@kotlin.OptIn" "(com.google.protobuf.kotlin.OnlyForUseByGeneratedProtoCode::class)\n" - "class ${$$kt_capitalized_name$Proxy$}$ private constructor()" + "public class ${$$kt_capitalized_name$Proxy$}$ private constructor()" " : com.google.protobuf.kotlin.DslProxy()\n"); WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print(variables_, - "$kt_deprecation$ val $kt_name$: " + "$kt_deprecation$public val $kt_name$: " "com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>\n" " @kotlin.jvm.JvmSynthetic\n" @@ -1103,7 +1082,7 @@ void RepeatedImmutableEnumFieldGenerator::GenerateKotlinDslMembers( printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"add$kt_capitalized_name$\")\n" - "fun com.google.protobuf.kotlin.DslList" + "public fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "add(value: $kt_type$) {\n" " $kt_dsl_builder$.${$add$capitalized_name$$}$(value)\n" @@ -1115,7 +1094,7 @@ void RepeatedImmutableEnumFieldGenerator::GenerateKotlinDslMembers( "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"plusAssign$kt_capitalized_name$\")\n" "@Suppress(\"NOTHING_TO_INLINE\")\n" - "inline operator fun com.google.protobuf.kotlin.DslList" + "public inline operator fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "plusAssign(value: $kt_type$) {\n" " add(value)\n" @@ -1126,7 +1105,7 @@ void RepeatedImmutableEnumFieldGenerator::GenerateKotlinDslMembers( printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"addAll$kt_capitalized_name$\")\n" - "fun com.google.protobuf.kotlin.DslList" + "public fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "addAll(values: kotlin.collections.Iterable<$kt_type$>) {\n" " $kt_dsl_builder$.${$addAll$capitalized_name$$}$(values)\n" @@ -1139,7 +1118,7 @@ void RepeatedImmutableEnumFieldGenerator::GenerateKotlinDslMembers( "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"plusAssignAll$kt_capitalized_name$\")\n" "@Suppress(\"NOTHING_TO_INLINE\")\n" - "inline operator fun com.google.protobuf.kotlin.DslList" + "public inline operator fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "plusAssign(values: kotlin.collections.Iterable<$kt_type$>) {\n" " addAll(values)\n" @@ -1151,7 +1130,7 @@ void RepeatedImmutableEnumFieldGenerator::GenerateKotlinDslMembers( variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"set$kt_capitalized_name$\")\n" - "operator fun com.google.protobuf.kotlin.DslList" + "public operator fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "set(index: kotlin.Int, value: $kt_type$) {\n" " $kt_dsl_builder$.${$set$capitalized_name$$}$(index, value)\n" @@ -1162,7 +1141,7 @@ void RepeatedImmutableEnumFieldGenerator::GenerateKotlinDslMembers( printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"clear$kt_capitalized_name$\")\n" - "fun com.google.protobuf.kotlin.DslList" + "public fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "clear() {\n" " $kt_dsl_builder$.${$clear$capitalized_name$$}$()\n" diff --git a/src/google/protobuf/compiler/java/enum_field.h b/src/google/protobuf/compiler/java/enum_field.h index db2aa75edf..25ab0738da 100644 --- a/src/google/protobuf/compiler/java/enum_field.h +++ b/src/google/protobuf/compiler/java/enum_field.h @@ -75,10 +75,9 @@ class ImmutableEnumFieldGenerator : public ImmutableFieldGenerator { void GenerateBuilderMembers(io::Printer* printer) const override; void GenerateInitializationCode(io::Printer* printer) const override; void GenerateBuilderClearCode(io::Printer* printer) const override; + void GenerateBuilderParsingCode(io::Printer* printer) const override; void GenerateMergingCode(io::Printer* printer) const override; void GenerateBuildingCode(io::Printer* printer) const override; - void GenerateParsingCode(io::Printer* printer) const override; - void GenerateParsingDoneCode(io::Printer* printer) const override; void GenerateSerializationCode(io::Printer* printer) const override; void GenerateSerializedSizeCode(io::Printer* printer) const override; void GenerateFieldBuilderInitializationCode( @@ -108,9 +107,10 @@ class ImmutableEnumOneofFieldGenerator : public ImmutableEnumFieldGenerator { void GenerateMembers(io::Printer* printer) const override; void GenerateBuilderMembers(io::Printer* printer) const override; + void GenerateBuilderClearCode(io::Printer* printer) const override; void GenerateMergingCode(io::Printer* printer) const override; void GenerateBuildingCode(io::Printer* printer) const override; - void GenerateParsingCode(io::Printer* printer) const override; + void GenerateBuilderParsingCode(io::Printer* printer) const override; void GenerateSerializationCode(io::Printer* printer) const override; void GenerateSerializedSizeCode(io::Printer* printer) const override; void GenerateEqualsCode(io::Printer* printer) const override; @@ -138,9 +138,9 @@ class RepeatedImmutableEnumFieldGenerator : public ImmutableFieldGenerator { void GenerateBuilderClearCode(io::Printer* printer) const override; void GenerateMergingCode(io::Printer* printer) const override; void GenerateBuildingCode(io::Printer* printer) const override; - void GenerateParsingCode(io::Printer* printer) const override; - void GenerateParsingCodeFromPacked(io::Printer* printer) const override; - void GenerateParsingDoneCode(io::Printer* printer) const override; + void GenerateBuilderParsingCode(io::Printer* printer) const override; + void GenerateBuilderParsingCodeFromPacked( + io::Printer* printer) const override; void GenerateSerializationCode(io::Printer* printer) const override; void GenerateSerializedSizeCode(io::Printer* printer) const override; void GenerateFieldBuilderInitializationCode( diff --git a/src/google/protobuf/compiler/java/enum_field_lite.cc b/src/google/protobuf/compiler/java/enum_field_lite.cc index 8536eded40..1bdfc3ef56 100644 --- a/src/google/protobuf/compiler/java/enum_field_lite.cc +++ b/src/google/protobuf/compiler/java/enum_field_lite.cc @@ -318,7 +318,7 @@ void ImmutableEnumFieldLiteGenerator::GenerateKotlinDslMembers( io::Printer* printer) const { WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print(variables_, - "$kt_deprecation$var $kt_name$: $kt_type$\n" + "$kt_deprecation$public var $kt_name$: $kt_type$\n" " @JvmName(\"${$get$kt_capitalized_name$$}$\")\n" " get() = $kt_dsl_builder$.${$get$capitalized_name$$}$()\n" " @JvmName(\"${$set$kt_capitalized_name$$}$\")\n" @@ -341,17 +341,18 @@ void ImmutableEnumFieldLiteGenerator::GenerateKotlinDslMembers( WriteFieldAccessorDocComment(printer, descriptor_, CLEARER, /* builder */ false, /* kdoc */ true); printer->Print(variables_, - "fun ${$clear$kt_capitalized_name$$}$() {\n" + "public fun ${$clear$kt_capitalized_name$$}$() {\n" " $kt_dsl_builder$.${$clear$capitalized_name$$}$()\n" "}\n"); if (HasHazzer(descriptor_)) { WriteFieldAccessorDocComment(printer, descriptor_, HAZZER, /* builder */ false, /* kdoc */ true); - printer->Print(variables_, - "fun ${$has$kt_capitalized_name$$}$(): kotlin.Boolean {\n" - " return $kt_dsl_builder$.${$has$capitalized_name$$}$()\n" - "}\n"); + printer->Print( + variables_, + "public fun ${$has$kt_capitalized_name$$}$(): kotlin.Boolean {\n" + " return $kt_dsl_builder$.${$has$capitalized_name$$}$()\n" + "}\n"); } } @@ -884,12 +885,12 @@ void RepeatedImmutableEnumFieldLiteGenerator::GenerateKotlinDslMembers( " */\n" "@kotlin.OptIn" "(com.google.protobuf.kotlin.OnlyForUseByGeneratedProtoCode::class)\n" - "class ${$$kt_capitalized_name$Proxy$}$ private constructor()" + "public class ${$$kt_capitalized_name$Proxy$}$ private constructor()" " : com.google.protobuf.kotlin.DslProxy()\n"); WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print(variables_, - "$kt_deprecation$ val $kt_name$: " + "$kt_deprecation$ public val $kt_name$: " "com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>\n" " @kotlin.jvm.JvmSynthetic\n" @@ -902,7 +903,7 @@ void RepeatedImmutableEnumFieldLiteGenerator::GenerateKotlinDslMembers( printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"add$kt_capitalized_name$\")\n" - "fun com.google.protobuf.kotlin.DslList" + "public fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "add(value: $kt_type$) {\n" " $kt_dsl_builder$.${$add$capitalized_name$$}$(value)\n" @@ -914,7 +915,7 @@ void RepeatedImmutableEnumFieldLiteGenerator::GenerateKotlinDslMembers( "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"plusAssign$kt_capitalized_name$\")\n" "@Suppress(\"NOTHING_TO_INLINE\")\n" - "inline operator fun com.google.protobuf.kotlin.DslList" + "public inline operator fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "plusAssign(value: $kt_type$) {\n" " add(value)\n" @@ -925,7 +926,7 @@ void RepeatedImmutableEnumFieldLiteGenerator::GenerateKotlinDslMembers( printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"addAll$kt_capitalized_name$\")\n" - "fun com.google.protobuf.kotlin.DslList" + "public fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "addAll(values: kotlin.collections.Iterable<$kt_type$>) {\n" " $kt_dsl_builder$.${$addAll$capitalized_name$$}$(values)\n" @@ -938,7 +939,7 @@ void RepeatedImmutableEnumFieldLiteGenerator::GenerateKotlinDslMembers( "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"plusAssignAll$kt_capitalized_name$\")\n" "@Suppress(\"NOTHING_TO_INLINE\")\n" - "inline operator fun com.google.protobuf.kotlin.DslList" + "public inline operator fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "plusAssign(values: kotlin.collections.Iterable<$kt_type$>) {\n" " addAll(values)\n" @@ -950,7 +951,7 @@ void RepeatedImmutableEnumFieldLiteGenerator::GenerateKotlinDslMembers( variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"set$kt_capitalized_name$\")\n" - "operator fun com.google.protobuf.kotlin.DslList" + "public operator fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "set(index: kotlin.Int, value: $kt_type$) {\n" " $kt_dsl_builder$.${$set$capitalized_name$$}$(index, value)\n" @@ -961,7 +962,7 @@ void RepeatedImmutableEnumFieldLiteGenerator::GenerateKotlinDslMembers( printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"clear$kt_capitalized_name$\")\n" - "fun com.google.protobuf.kotlin.DslList" + "public fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "clear() {\n" " $kt_dsl_builder$.${$clear$capitalized_name$$}$()\n" diff --git a/src/google/protobuf/compiler/java/field.cc b/src/google/protobuf/compiler/java/field.cc index dd5e16cb5b..13aed84bd8 100644 --- a/src/google/protobuf/compiler/java/field.cc +++ b/src/google/protobuf/compiler/java/field.cc @@ -186,7 +186,7 @@ static inline void ReportUnexpectedPackedFieldsCall(io::Printer* printer) { // but this method should be overridden. // - This FieldGenerator doesn't support packing, and this method // should never have been called. - GOOGLE_LOG(FATAL) << "GenerateParsingCodeFromPacked() " + GOOGLE_LOG(FATAL) << "GenerateBuilderParsingCodeFromPacked() " << "called on field generator that does not support packing."; } @@ -194,7 +194,7 @@ static inline void ReportUnexpectedPackedFieldsCall(io::Printer* printer) { ImmutableFieldGenerator::~ImmutableFieldGenerator() {} -void ImmutableFieldGenerator::GenerateParsingCodeFromPacked( +void ImmutableFieldGenerator::GenerateBuilderParsingCodeFromPacked( io::Printer* printer) const { ReportUnexpectedPackedFieldsCall(printer); } diff --git a/src/google/protobuf/compiler/java/field.h b/src/google/protobuf/compiler/java/field.h index 4743f59539..5ac57cd4e1 100644 --- a/src/google/protobuf/compiler/java/field.h +++ b/src/google/protobuf/compiler/java/field.h @@ -80,9 +80,8 @@ class ImmutableFieldGenerator { virtual void GenerateBuilderClearCode(io::Printer* printer) const = 0; virtual void GenerateMergingCode(io::Printer* printer) const = 0; virtual void GenerateBuildingCode(io::Printer* printer) const = 0; - virtual void GenerateParsingCode(io::Printer* printer) const = 0; - virtual void GenerateParsingCodeFromPacked(io::Printer* printer) const; - virtual void GenerateParsingDoneCode(io::Printer* printer) const = 0; + virtual void GenerateBuilderParsingCode(io::Printer* printer) const = 0; + virtual void GenerateBuilderParsingCodeFromPacked(io::Printer* printer) const; virtual void GenerateSerializationCode(io::Printer* printer) const = 0; virtual void GenerateSerializedSizeCode(io::Printer* printer) const = 0; virtual void GenerateFieldBuilderInitializationCode( diff --git a/src/google/protobuf/compiler/java/map_field.cc b/src/google/protobuf/compiler/java/map_field.cc index 66d7fbb9c5..2f162af71c 100644 --- a/src/google/protobuf/compiler/java/map_field.cc +++ b/src/google/protobuf/compiler/java/map_field.cc @@ -180,13 +180,6 @@ void SetMessageVariables(const FieldDescriptor* descriptor, int messageBitIndex, : ""; (*variables)["on_changed"] = "onChanged();"; - // For repeated fields, one bit is used for whether the array is immutable - // in the parsing constructor. - (*variables)["get_mutable_bit_parser"] = - GenerateGetBitMutableLocal(builderBitIndex); - (*variables)["set_mutable_bit_parser"] = - GenerateSetBitMutableLocal(builderBitIndex); - (*variables)["default_entry"] = (*variables)["capitalized_name"] + "DefaultEntryHolder.defaultEntry"; (*variables)["map_field_parameter"] = (*variables)["default_entry"]; @@ -727,13 +720,13 @@ void ImmutableMapFieldGenerator::GenerateKotlinDslMembers( " */\n" "@kotlin.OptIn" "(com.google.protobuf.kotlin.OnlyForUseByGeneratedProtoCode::class)\n" - "class ${$$kt_capitalized_name$Proxy$}$ private constructor()" + "public class ${$$kt_capitalized_name$Proxy$}$ private constructor()" " : com.google.protobuf.kotlin.DslProxy()\n"); WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print( variables_, - "$kt_deprecation$ val $kt_name$: " + "$kt_deprecation$ public val $kt_name$: " "com.google.protobuf.kotlin.DslMap" "<$kt_key_type$, $kt_value_type$, ${$$kt_capitalized_name$Proxy$}$>\n" " @kotlin.jvm.JvmSynthetic\n" @@ -746,7 +739,7 @@ void ImmutableMapFieldGenerator::GenerateKotlinDslMembers( printer->Print( variables_, "@JvmName(\"put$kt_capitalized_name$\")\n" - "fun com.google.protobuf.kotlin.DslMap" + "public fun com.google.protobuf.kotlin.DslMap" "<$kt_key_type$, $kt_value_type$, ${$$kt_capitalized_name$Proxy$}$>\n" " .put(key: $kt_key_type$, value: $kt_value_type$) {\n" " $kt_dsl_builder$.${$put$capitalized_name$$}$(key, value)\n" @@ -758,7 +751,7 @@ void ImmutableMapFieldGenerator::GenerateKotlinDslMembers( "@kotlin.jvm.JvmSynthetic\n" "@JvmName(\"set$kt_capitalized_name$\")\n" "@Suppress(\"NOTHING_TO_INLINE\")\n" - "inline operator fun com.google.protobuf.kotlin.DslMap" + "public inline operator fun com.google.protobuf.kotlin.DslMap" "<$kt_key_type$, $kt_value_type$, ${$$kt_capitalized_name$Proxy$}$>\n" " .set(key: $kt_key_type$, value: $kt_value_type$) {\n" " put(key, value)\n" @@ -769,7 +762,7 @@ void ImmutableMapFieldGenerator::GenerateKotlinDslMembers( variables_, "@kotlin.jvm.JvmSynthetic\n" "@JvmName(\"remove$kt_capitalized_name$\")\n" - "fun com.google.protobuf.kotlin.DslMap" + "public fun com.google.protobuf.kotlin.DslMap" "<$kt_key_type$, $kt_value_type$, ${$$kt_capitalized_name$Proxy$}$>\n" " .remove(key: $kt_key_type$) {\n" " $kt_dsl_builder$.${$remove$capitalized_name$$}$(key)\n" @@ -780,7 +773,7 @@ void ImmutableMapFieldGenerator::GenerateKotlinDslMembers( variables_, "@kotlin.jvm.JvmSynthetic\n" "@JvmName(\"putAll$kt_capitalized_name$\")\n" - "fun com.google.protobuf.kotlin.DslMap" + "public fun com.google.protobuf.kotlin.DslMap" "<$kt_key_type$, $kt_value_type$, ${$$kt_capitalized_name$Proxy$}$>\n" " .putAll(map: kotlin.collections.Map<$kt_key_type$, $kt_value_type$>) " "{\n" @@ -792,7 +785,7 @@ void ImmutableMapFieldGenerator::GenerateKotlinDslMembers( variables_, "@kotlin.jvm.JvmSynthetic\n" "@JvmName(\"clear$kt_capitalized_name$\")\n" - "fun com.google.protobuf.kotlin.DslMap" + "public fun com.google.protobuf.kotlin.DslMap" "<$kt_key_type$, $kt_value_type$, ${$$kt_capitalized_name$Proxy$}$>\n" " .clear() {\n" " $kt_dsl_builder$.${$clear$capitalized_name$$}$()\n" @@ -829,27 +822,19 @@ void ImmutableMapFieldGenerator::GenerateBuildingCode( "result.$name$_.makeImmutable();\n"); } -void ImmutableMapFieldGenerator::GenerateParsingCode( +void ImmutableMapFieldGenerator::GenerateBuilderParsingCode( io::Printer* printer) const { - printer->Print(variables_, - "if (!$get_mutable_bit_parser$) {\n" - " $name$_ = com.google.protobuf.MapField.newMapField(\n" - " $map_field_parameter$);\n" - " $set_mutable_bit_parser$;\n" - "}\n"); if (!SupportUnknownEnumValue(descriptor_->file()) && GetJavaType(ValueField(descriptor_)) == JAVATYPE_ENUM) { printer->Print( variables_, "com.google.protobuf.ByteString bytes = input.readBytes();\n" "com.google.protobuf.MapEntry<$type_parameters$>\n" - "$name$__ = $default_entry$.getParserForType().parseFrom(bytes);\n"); - printer->Print( - variables_, + "$name$__ = $default_entry$.getParserForType().parseFrom(bytes);\n" "if ($value_enum_type$.forNumber($name$__.getValue()) == null) {\n" - " unknownFields.mergeLengthDelimitedField($number$, bytes);\n" + " mergeUnknownLengthDelimitedField($number$, bytes);\n" "} else {\n" - " $name$_.getMutableMap().put(\n" + " internalGetMutable$capitalized_name$().getMutableMap().put(\n" " $name$__.getKey(), $name$__.getValue());\n" "}\n"); } else { @@ -858,16 +843,11 @@ void ImmutableMapFieldGenerator::GenerateParsingCode( "com.google.protobuf.MapEntry<$type_parameters$>\n" "$name$__ = input.readMessage(\n" " $default_entry$.getParserForType(), extensionRegistry);\n" - "$name$_.getMutableMap().put(\n" + "internalGetMutable$capitalized_name$().getMutableMap().put(\n" " $name$__.getKey(), $name$__.getValue());\n"); } } -void ImmutableMapFieldGenerator::GenerateParsingDoneCode( - io::Printer* printer) const { - // Nothing to do here. -} - void ImmutableMapFieldGenerator::GenerateSerializationCode( io::Printer* printer) const { printer->Print(variables_, diff --git a/src/google/protobuf/compiler/java/map_field.h b/src/google/protobuf/compiler/java/map_field.h index 646c35a16e..434150820b 100644 --- a/src/google/protobuf/compiler/java/map_field.h +++ b/src/google/protobuf/compiler/java/map_field.h @@ -55,8 +55,7 @@ class ImmutableMapFieldGenerator : public ImmutableFieldGenerator { void GenerateBuilderClearCode(io::Printer* printer) const override; void GenerateMergingCode(io::Printer* printer) const override; void GenerateBuildingCode(io::Printer* printer) const override; - void GenerateParsingCode(io::Printer* printer) const override; - void GenerateParsingDoneCode(io::Printer* printer) const override; + void GenerateBuilderParsingCode(io::Printer* printer) const override; void GenerateSerializationCode(io::Printer* printer) const override; void GenerateSerializedSizeCode(io::Printer* printer) const override; void GenerateFieldBuilderInitializationCode( diff --git a/src/google/protobuf/compiler/java/map_field_lite.cc b/src/google/protobuf/compiler/java/map_field_lite.cc index a498050b7f..22bc306586 100644 --- a/src/google/protobuf/compiler/java/map_field_lite.cc +++ b/src/google/protobuf/compiler/java/map_field_lite.cc @@ -873,13 +873,13 @@ void ImmutableMapFieldLiteGenerator::GenerateKotlinDslMembers( " */\n" "@kotlin.OptIn" "(com.google.protobuf.kotlin.OnlyForUseByGeneratedProtoCode::class)\n" - "class ${$$kt_capitalized_name$Proxy$}$ private constructor()" + "public class ${$$kt_capitalized_name$Proxy$}$ private constructor()" " : com.google.protobuf.kotlin.DslProxy()\n"); WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print( variables_, - "$kt_deprecation$ val $kt_name$: " + "$kt_deprecation$ public val $kt_name$: " "com.google.protobuf.kotlin.DslMap" "<$kt_key_type$, $kt_value_type$, ${$$kt_capitalized_name$Proxy$}$>\n" " @kotlin.jvm.JvmSynthetic\n" @@ -892,7 +892,7 @@ void ImmutableMapFieldLiteGenerator::GenerateKotlinDslMembers( printer->Print( variables_, "@JvmName(\"put$kt_capitalized_name$\")\n" - "fun com.google.protobuf.kotlin.DslMap" + "public fun com.google.protobuf.kotlin.DslMap" "<$kt_key_type$, $kt_value_type$, ${$$kt_capitalized_name$Proxy$}$>\n" " .put(key: $kt_key_type$, value: $kt_value_type$) {\n" " $kt_dsl_builder$.${$put$capitalized_name$$}$(key, value)\n" @@ -904,7 +904,7 @@ void ImmutableMapFieldLiteGenerator::GenerateKotlinDslMembers( "@kotlin.jvm.JvmSynthetic\n" "@JvmName(\"set$kt_capitalized_name$\")\n" "@Suppress(\"NOTHING_TO_INLINE\")\n" - "inline operator fun com.google.protobuf.kotlin.DslMap" + "public inline operator fun com.google.protobuf.kotlin.DslMap" "<$kt_key_type$, $kt_value_type$, ${$$kt_capitalized_name$Proxy$}$>\n" " .set(key: $kt_key_type$, value: $kt_value_type$) {\n" " put(key, value)\n" @@ -915,7 +915,7 @@ void ImmutableMapFieldLiteGenerator::GenerateKotlinDslMembers( variables_, "@kotlin.jvm.JvmSynthetic\n" "@JvmName(\"remove$kt_capitalized_name$\")\n" - "fun com.google.protobuf.kotlin.DslMap" + "public fun com.google.protobuf.kotlin.DslMap" "<$kt_key_type$, $kt_value_type$, ${$$kt_capitalized_name$Proxy$}$>\n" " .remove(key: $kt_key_type$) {\n" " $kt_dsl_builder$.${$remove$capitalized_name$$}$(key)\n" @@ -926,7 +926,7 @@ void ImmutableMapFieldLiteGenerator::GenerateKotlinDslMembers( variables_, "@kotlin.jvm.JvmSynthetic\n" "@JvmName(\"putAll$kt_capitalized_name$\")\n" - "fun com.google.protobuf.kotlin.DslMap" + "public fun com.google.protobuf.kotlin.DslMap" "<$kt_key_type$, $kt_value_type$, ${$$kt_capitalized_name$Proxy$}$>\n" " .putAll(map: kotlin.collections.Map<$kt_key_type$, $kt_value_type$>) " "{\n" @@ -938,7 +938,7 @@ void ImmutableMapFieldLiteGenerator::GenerateKotlinDslMembers( variables_, "@kotlin.jvm.JvmSynthetic\n" "@JvmName(\"clear$kt_capitalized_name$\")\n" - "fun com.google.protobuf.kotlin.DslMap" + "public fun com.google.protobuf.kotlin.DslMap" "<$kt_key_type$, $kt_value_type$, ${$$kt_capitalized_name$Proxy$}$>\n" " .clear() {\n" " $kt_dsl_builder$.${$clear$capitalized_name$$}$()\n" diff --git a/src/google/protobuf/compiler/java/message.cc b/src/google/protobuf/compiler/java/message.cc index 52768bb477..3b5a3b5bfe 100644 --- a/src/google/protobuf/compiler/java/message.cc +++ b/src/google/protobuf/compiler/java/message.cc @@ -42,6 +42,7 @@ #include "google/protobuf/io/coded_stream.h" #include "google/protobuf/io/printer.h" +#include "google/protobuf/descriptor.h" #include "google/protobuf/wire_format.h" #include "google/protobuf/stubs/strutil.h" #include "absl/strings/ascii.h" @@ -389,6 +390,7 @@ void ImmutableMessageGenerator::Generate(io::Printer* printer) { "}\n" "\n"); + // TODO(b/248149118): Remove this superfluous override. printer->Print( "@java.lang.Override\n" "public final com.google.protobuf.UnknownFieldSet\n" @@ -396,10 +398,6 @@ void ImmutableMessageGenerator::Generate(io::Printer* printer) { " return this.unknownFields;\n" "}\n"); - if (context_->HasGeneratedMethods(descriptor_)) { - GenerateParsingConstructor(printer); - } - GenerateDescriptorMethods(printer); // Nested types @@ -639,9 +637,9 @@ void ImmutableMessageGenerator::GenerateMessageSerializationMethods( sorted_fields.get()); if (descriptor_->options().message_set_wire_format()) { - printer->Print("unknownFields.writeAsMessageSetTo(output);\n"); + printer->Print("getUnknownFields().writeAsMessageSetTo(output);\n"); } else { - printer->Print("unknownFields.writeTo(output);\n"); + printer->Print("getUnknownFields().writeTo(output);\n"); } printer->Outdent(); @@ -670,9 +668,10 @@ void ImmutableMessageGenerator::GenerateMessageSerializationMethods( } if (descriptor_->options().message_set_wire_format()) { - printer->Print("size += unknownFields.getSerializedSizeAsMessageSet();\n"); + printer->Print( + "size += getUnknownFields().getSerializedSizeAsMessageSet();\n"); } else { - printer->Print("size += unknownFields.getSerializedSize();\n"); + printer->Print("size += getUnknownFields().getSerializedSize();\n"); } printer->Print( @@ -1064,7 +1063,8 @@ void ImmutableMessageGenerator::GenerateEqualsAndHashCode( // false for non-canonical ordering when running in LITE_RUNTIME but it's // the best we can do. printer->Print( - "if (!unknownFields.equals(other.unknownFields)) return false;\n"); + "if (!getUnknownFields().equals(other.getUnknownFields())) return " + "false;\n"); if (descriptor_->extension_range_count() > 0) { printer->Print( "if (!getExtensionFields().equals(other.getExtensionFields()))\n" @@ -1138,7 +1138,7 @@ void ImmutableMessageGenerator::GenerateEqualsAndHashCode( printer->Print("hash = hashFields(hash, getExtensionFields());\n"); } - printer->Print("hash = (29 * hash) + unknownFields.hashCode();\n"); + printer->Print("hash = (29 * hash) + getUnknownFields().hashCode();\n"); printer->Print( "memoizedHashCode = hash;\n" "return hash;\n"); @@ -1163,189 +1163,33 @@ void ImmutableMessageGenerator::GenerateExtensionRegistrationCode( } } -// =================================================================== -void ImmutableMessageGenerator::GenerateParsingConstructor( - io::Printer* printer) { - std::unique_ptr sorted_fields( - SortFieldsByNumber(descriptor_)); - - printer->Print( - "private $classname$(\n" - " com.google.protobuf.CodedInputStream input,\n" - " com.google.protobuf.ExtensionRegistryLite extensionRegistry)\n" - " throws com.google.protobuf.InvalidProtocolBufferException {\n", - "classname", descriptor_->name()); - printer->Indent(); - - // Initialize all fields to default. - printer->Print( - "this();\n" - "if (extensionRegistry == null) {\n" - " throw new java.lang.NullPointerException();\n" - "}\n"); - - // Use builder bits to track mutable repeated fields. - int totalBuilderBits = 0; - for (int i = 0; i < descriptor_->field_count(); i++) { - const ImmutableFieldGenerator& field = - field_generators_.get(descriptor_->field(i)); - totalBuilderBits += field.GetNumBitsForBuilder(); - } - int totalBuilderInts = (totalBuilderBits + 31) / 32; - for (int i = 0; i < totalBuilderInts; i++) { - printer->Print("int mutable_$bit_field_name$ = 0;\n", "bit_field_name", - GetBitFieldName(i)); - } - - printer->Print( - "com.google.protobuf.UnknownFieldSet.Builder unknownFields =\n" - " com.google.protobuf.UnknownFieldSet.newBuilder();\n"); - - printer->Print("try {\n"); - printer->Indent(); - - printer->Print( - "boolean done = false;\n" - "while (!done) {\n"); - printer->Indent(); - - printer->Print( - "int tag = input.readTag();\n" - "switch (tag) {\n"); - printer->Indent(); - - printer->Print( - "case 0:\n" // zero signals EOF / limit reached - " done = true;\n" - " break;\n"); - - for (int i = 0; i < descriptor_->field_count(); i++) { - const FieldDescriptor* field = sorted_fields[i]; - uint32_t tag = WireFormatLite::MakeTag( - field->number(), WireFormat::WireTypeForFieldType(field->type())); - - printer->Print("case $tag$: {\n", "tag", - absl::StrCat(static_cast(tag))); - printer->Indent(); - - field_generators_.get(field).GenerateParsingCode(printer); - - printer->Outdent(); - printer->Print( - " break;\n" - "}\n"); - - if (field->is_packable()) { - // To make packed = true wire compatible, we generate parsing code from a - // packed version of this field regardless of field->options().packed(). - uint32_t packed_tag = WireFormatLite::MakeTag( - field->number(), WireFormatLite::WIRETYPE_LENGTH_DELIMITED); - printer->Print("case $tag$: {\n", "tag", - absl::StrCat(static_cast(packed_tag))); - printer->Indent(); - - field_generators_.get(field).GenerateParsingCodeFromPacked(printer); - - printer->Outdent(); - printer->Print( - " break;\n" - "}\n"); - } - } - - printer->Print( - "default: {\n" - " if (!parseUnknownField(\n" - " input, unknownFields, extensionRegistry, tag)) {\n" - " done = true;\n" // it's an endgroup tag - " }\n" - " break;\n" - "}\n"); - - printer->Outdent(); - printer->Outdent(); - printer->Print( - " }\n" // switch (tag) - "}\n"); // while (!done) - - printer->Outdent(); - printer->Print( - "} catch (com.google.protobuf.InvalidProtocolBufferException e) {\n" - " throw e.setUnfinishedMessage(this);\n" - "} catch (com.google.protobuf.UninitializedMessageException e) {\n" - " throw " - "e.asInvalidProtocolBufferException().setUnfinishedMessage(this);\n" - "} catch (java.io.IOException e) {\n" - " throw new com.google.protobuf.InvalidProtocolBufferException(\n" - " e).setUnfinishedMessage(this);\n" - "} finally {\n"); - printer->Indent(); - - // Make repeated field list immutable. - for (int i = 0; i < descriptor_->field_count(); i++) { - const FieldDescriptor* field = sorted_fields[i]; - field_generators_.get(field).GenerateParsingDoneCode(printer); - } - - // Make unknown fields immutable. - printer->Print("this.unknownFields = unknownFields.build();\n"); - - // Make extensions immutable. - printer->Print("makeExtensionsImmutable();\n"); - - printer->Outdent(); - printer->Outdent(); - printer->Print( - " }\n" // finally - "}\n"); -} - // =================================================================== void ImmutableMessageGenerator::GenerateParser(io::Printer* printer) { printer->Print( "$visibility$ static final com.google.protobuf.Parser<$classname$>\n" - " PARSER = new com.google.protobuf.AbstractParser<$classname$>() {\n", - "visibility", - ExposePublicParser(descriptor_->file()) ? "@java.lang.Deprecated public" - : "private", - "classname", descriptor_->name()); - printer->Indent(); - printer->Print( - "@java.lang.Override\n" - "public $classname$ parsePartialFrom(\n" - " com.google.protobuf.CodedInputStream input,\n" - " com.google.protobuf.ExtensionRegistryLite extensionRegistry)\n" - " throws com.google.protobuf.InvalidProtocolBufferException {\n", - "classname", descriptor_->name()); - if (context_->HasGeneratedMethods(descriptor_)) { - printer->Print(" return new $classname$(input, extensionRegistry);\n", - "classname", descriptor_->name()); - } else { - // When parsing constructor isn't generated, use builder to parse - // messages. Note, will fallback to use reflection based mergeFieldFrom() - // in AbstractMessage.Builder. - printer->Indent(); - printer->Print( - "Builder builder = newBuilder();\n" - "try {\n" - " builder.mergeFrom(input, extensionRegistry);\n" - "} catch (com.google.protobuf.InvalidProtocolBufferException e) {\n" - " throw e.setUnfinishedMessage(builder.buildPartial());\n" - "} catch (java.io.IOException e) {\n" - " throw new com.google.protobuf.InvalidProtocolBufferException(\n" - " e.getMessage()).setUnfinishedMessage(\n" - " builder.buildPartial());\n" - "}\n" - "return builder.buildPartial();\n"); - printer->Outdent(); - } - printer->Print("}\n"); - printer->Outdent(); - printer->Print( + " PARSER = new com.google.protobuf.AbstractParser<$classname$>() {\n" + " @java.lang.Override\n" + " public $classname$ parsePartialFrom(\n" + " com.google.protobuf.CodedInputStream input,\n" + " com.google.protobuf.ExtensionRegistryLite extensionRegistry)\n" + " throws com.google.protobuf.InvalidProtocolBufferException {\n" + " Builder builder = newBuilder();\n" + " try {\n" + " builder.mergeFrom(input, extensionRegistry);\n" + " } catch (com.google.protobuf.InvalidProtocolBufferException e) {\n" + " throw e.setUnfinishedMessage(builder.buildPartial());\n" + " } catch (com.google.protobuf.UninitializedMessageException e) {\n" + " throw " + "e.asInvalidProtocolBufferException().setUnfinishedMessage(builder." + "buildPartial());\n" + " } catch (java.io.IOException e) {\n" + " throw new com.google.protobuf.InvalidProtocolBufferException(e)\n" + " .setUnfinishedMessage(builder.buildPartial());\n" + " }\n" + " return builder.buildPartial();\n" + " }\n" "};\n" - "\n"); - - printer->Print( + "\n" "public static com.google.protobuf.Parser<$classname$> parser() {\n" " return PARSER;\n" "}\n" @@ -1355,6 +1199,9 @@ void ImmutableMessageGenerator::GenerateParser(io::Printer* printer) { " return PARSER;\n" "}\n" "\n", + "visibility", + ExposePublicParser(descriptor_->file()) ? "@java.lang.Deprecated public" + : "private", "classname", descriptor_->name()); } @@ -1409,10 +1256,10 @@ void ImmutableMessageGenerator::GenerateKotlinDsl(io::Printer* printer) const { "(com.google.protobuf.kotlin.OnlyForUseByGeneratedProtoCode::class)\n" "@com.google.protobuf.kotlin.ProtoDslMarker\n"); printer->Print( - "class Dsl private constructor(\n" + "public class Dsl private constructor(\n" " private val _builder: $message$.Builder\n" ") {\n" - " companion object {\n" + " public companion object {\n" " @kotlin.jvm.JvmSynthetic\n" " @kotlin.PublishedApi\n" " internal fun _create(builder: $message$.Builder): Dsl = " @@ -1435,10 +1282,10 @@ void ImmutableMessageGenerator::GenerateKotlinDsl(io::Printer* printer) const { for (auto oneof : oneofs_) { printer->Print( - "val $oneof_name$Case: $message$.$oneof_capitalized_name$Case\n" + "public val $oneof_name$Case: $message$.$oneof_capitalized_name$Case\n" " @JvmName(\"get$oneof_capitalized_name$Case\")\n" " get() = _builder.get$oneof_capitalized_name$Case()\n\n" - "fun clear$oneof_capitalized_name$() {\n" + "public fun clear$oneof_capitalized_name$() {\n" " _builder.clear$oneof_capitalized_name$()\n" "}\n", "oneof_name", context_->GetOneofGeneratorInfo(oneof)->name, @@ -1466,7 +1313,7 @@ void ImmutableMessageGenerator::GenerateKotlinMembers( } printer->Print( - "inline fun $camelcase_name$(block: $message_kt$.Dsl.() -> " + "public inline fun $camelcase_name$(block: $message_kt$.Dsl.() -> " "kotlin.Unit): " "$message$ " "=\n" @@ -1480,7 +1327,7 @@ void ImmutableMessageGenerator::GenerateKotlinMembers( EscapeKotlinKeywords(name_resolver_->GetClassName(descriptor_, true))); WriteMessageDocComment(printer, descriptor_, /* kdoc */ true); - printer->Print("object $name$Kt {\n", "name", descriptor_->name()); + printer->Print("public object $name$Kt {\n", "name", descriptor_->name()); printer->Indent(); GenerateKotlinDsl(printer); for (int i = 0; i < descriptor_->nested_type_count(); i++) { @@ -1501,7 +1348,7 @@ void ImmutableMessageGenerator::GenerateTopLevelKotlinMembers( } printer->Print( - "inline fun $message$.copy(block: $message_kt$.Dsl.() -> " + "public inline fun $message$.copy(block: $message_kt$.Dsl.() -> " "kotlin.Unit): $message$ =\n" " $message_kt$.Dsl._create(this.toBuilder()).apply { block() " "}._build()\n\n", @@ -1546,7 +1393,7 @@ void ImmutableMessageGenerator::GenerateKotlinExtensions( printer->Print( "@Suppress(\"UNCHECKED_CAST\")\n" "@kotlin.jvm.JvmSynthetic\n" - "operator fun get(extension: " + "public operator fun get(extension: " "com.google.protobuf.ExtensionLite<$message$, T>): T {\n" " return if (extension.isRepeated) {\n" " get(extension as com.google.protobuf.ExtensionLite<$message$, " @@ -1562,7 +1409,7 @@ void ImmutableMessageGenerator::GenerateKotlinExtensions( "@kotlin.OptIn" "(com.google.protobuf.kotlin.OnlyForUseByGeneratedProtoCode::class)\n" "@kotlin.jvm.JvmName(\"-getRepeatedExtension\")\n" - "operator fun get(\n" + "public operator fun get(\n" " extension: com.google.protobuf.ExtensionLite<$message$, List>\n" "): com.google.protobuf.kotlin.ExtensionList {\n" " return com.google.protobuf.kotlin.ExtensionList(extension, " @@ -1572,7 +1419,7 @@ void ImmutableMessageGenerator::GenerateKotlinExtensions( printer->Print( "@kotlin.jvm.JvmSynthetic\n" - "operator fun contains(extension: " + "public operator fun contains(extension: " "com.google.protobuf.ExtensionLite<$message$, *>): " "Boolean {\n" " return _builder.hasExtension(extension)\n" @@ -1581,7 +1428,7 @@ void ImmutableMessageGenerator::GenerateKotlinExtensions( printer->Print( "@kotlin.jvm.JvmSynthetic\n" - "fun clear(extension: " + "public fun clear(extension: " "com.google.protobuf.ExtensionLite<$message$, *>) " "{\n" " _builder.clearExtension(extension)\n" @@ -1601,7 +1448,7 @@ void ImmutableMessageGenerator::GenerateKotlinExtensions( printer->Print( "@kotlin.jvm.JvmSynthetic\n" "@Suppress(\"NOTHING_TO_INLINE\")\n" - "inline operator fun > set(\n" + "public inline operator fun > set(\n" " extension: com.google.protobuf.ExtensionLite<$message$, T>,\n" " value: T\n" ") {\n" @@ -1612,7 +1459,7 @@ void ImmutableMessageGenerator::GenerateKotlinExtensions( printer->Print( "@kotlin.jvm.JvmSynthetic\n" "@Suppress(\"NOTHING_TO_INLINE\")\n" - "inline operator fun set(\n" + "public inline operator fun set(\n" " extension: com.google.protobuf.ExtensionLite<$message$, " "com.google.protobuf.ByteString>,\n" " value: com.google.protobuf.ByteString\n" @@ -1624,7 +1471,7 @@ void ImmutableMessageGenerator::GenerateKotlinExtensions( printer->Print( "@kotlin.jvm.JvmSynthetic\n" "@Suppress(\"NOTHING_TO_INLINE\")\n" - "inline operator fun set(\n" + "public inline operator fun set(\n" " extension: com.google.protobuf.ExtensionLite<$message$, T>,\n" " value: T\n" ") {\n" @@ -1634,7 +1481,7 @@ void ImmutableMessageGenerator::GenerateKotlinExtensions( printer->Print( "@kotlin.jvm.JvmSynthetic\n" - "fun com.google.protobuf.kotlin.ExtensionList com.google.protobuf.kotlin.ExtensionList.add(value: E) {\n" " _builder.addExtension(this.extension, value)\n" "}\n\n", @@ -1643,7 +1490,7 @@ void ImmutableMessageGenerator::GenerateKotlinExtensions( printer->Print( "@kotlin.jvm.JvmSynthetic\n" "@Suppress(\"NOTHING_TO_INLINE\")\n" - "inline operator fun " + "public inline operator fun " "com.google.protobuf.kotlin.ExtensionList.plusAssign" "(value: E) {\n" @@ -1653,7 +1500,7 @@ void ImmutableMessageGenerator::GenerateKotlinExtensions( printer->Print( "@kotlin.jvm.JvmSynthetic\n" - "fun com.google.protobuf.kotlin.ExtensionList com.google.protobuf.kotlin.ExtensionList.addAll(values: Iterable) {\n" " for (value in values) {\n" " add(value)\n" @@ -1664,7 +1511,7 @@ void ImmutableMessageGenerator::GenerateKotlinExtensions( printer->Print( "@kotlin.jvm.JvmSynthetic\n" "@Suppress(\"NOTHING_TO_INLINE\")\n" - "inline operator fun " + "public inline operator fun " "com.google.protobuf.kotlin.ExtensionList.plusAssign(values: " "Iterable) {\n" @@ -1674,7 +1521,7 @@ void ImmutableMessageGenerator::GenerateKotlinExtensions( printer->Print( "@kotlin.jvm.JvmSynthetic\n" - "operator fun " + "public operator fun " "com.google.protobuf.kotlin.ExtensionList.set(index: Int, value: " "E) {\n" @@ -1685,7 +1532,7 @@ void ImmutableMessageGenerator::GenerateKotlinExtensions( printer->Print( "@kotlin.jvm.JvmSynthetic\n" "@Suppress(\"NOTHING_TO_INLINE\")\n" - "inline fun com.google.protobuf.kotlin.ExtensionList<*, " + "public inline fun com.google.protobuf.kotlin.ExtensionList<*, " "$message$>.clear() {\n" " clear(extension)\n" "}\n\n", diff --git a/src/google/protobuf/compiler/java/message_builder.cc b/src/google/protobuf/compiler/java/message_builder.cc index 0184ee843d..d378d0d2f6 100644 --- a/src/google/protobuf/compiler/java/message_builder.cc +++ b/src/google/protobuf/compiler/java/message_builder.cc @@ -63,6 +63,9 @@ namespace protobuf { namespace compiler { namespace java { +using internal::WireFormat; +using internal::WireFormatLite; + namespace { std::string MapValueImmutableClassdName(const Descriptor* descriptor, ClassNameResolver* name_resolver) { @@ -292,43 +295,60 @@ void MessageBuilderGenerator::GenerateDescriptorMethods(io::Printer* printer) { void MessageBuilderGenerator::GenerateCommonBuilderMethods( io::Printer* printer) { + bool need_maybe_force_builder_init = false; + for (int i = 0; i < descriptor_->field_count(); i++) { + if (descriptor_->field(i)->message_type() != nullptr && + !IsRealOneof(descriptor_->field(i)) && + HasHasbit(descriptor_->field(i))) { + need_maybe_force_builder_init = true; + break; + } + } + + const char* force_builder_init = need_maybe_force_builder_init + ? " maybeForceBuilderInitialization();" + : ""; + printer->Print( "// Construct using $classname$.newBuilder()\n" "private Builder() {\n" - " maybeForceBuilderInitialization();\n" + "$force_builder_init$\n" "}\n" "\n", - "classname", name_resolver_->GetImmutableClassName(descriptor_)); + "classname", name_resolver_->GetImmutableClassName(descriptor_), + "force_builder_init", force_builder_init); printer->Print( "private Builder(\n" " com.google.protobuf.GeneratedMessage$ver$.BuilderParent parent) {\n" " super(parent);\n" - " maybeForceBuilderInitialization();\n" + "$force_builder_init$\n" "}\n", "classname", name_resolver_->GetImmutableClassName(descriptor_), "ver", - GeneratedCodeVersionSuffix()); + GeneratedCodeVersionSuffix(), "force_builder_init", force_builder_init); - printer->Print( - "private void maybeForceBuilderInitialization() {\n" - " if (com.google.protobuf.GeneratedMessage$ver$\n" - " .alwaysUseFieldBuilders) {\n", - "ver", GeneratedCodeVersionSuffix()); + if (need_maybe_force_builder_init) { + printer->Print( + "private void maybeForceBuilderInitialization() {\n" + " if (com.google.protobuf.GeneratedMessage$ver$\n" + " .alwaysUseFieldBuilders) {\n", + "ver", GeneratedCodeVersionSuffix()); - printer->Indent(); - printer->Indent(); - for (int i = 0; i < descriptor_->field_count(); i++) { - if (!IsRealOneof(descriptor_->field(i))) { - field_generators_.get(descriptor_->field(i)) - .GenerateFieldBuilderInitializationCode(printer); + printer->Indent(); + printer->Indent(); + for (int i = 0; i < descriptor_->field_count(); i++) { + if (!IsRealOneof(descriptor_->field(i))) { + field_generators_.get(descriptor_->field(i)) + .GenerateFieldBuilderInitializationCode(printer); + } } - } - printer->Outdent(); - printer->Outdent(); + printer->Outdent(); + printer->Outdent(); - printer->Print( - " }\n" - "}\n"); + printer->Print( + " }\n" + "}\n"); + } printer->Print( "@java.lang.Override\n" @@ -338,10 +358,8 @@ void MessageBuilderGenerator::GenerateCommonBuilderMethods( printer->Indent(); for (int i = 0; i < descriptor_->field_count(); i++) { - if (!IsRealOneof(descriptor_->field(i))) { - field_generators_.get(descriptor_->field(i)) - .GenerateBuilderClearCode(printer); - } + field_generators_.get(descriptor_->field(i)) + .GenerateBuilderClearCode(printer); } for (auto oneof : oneofs_) { @@ -584,7 +602,7 @@ void MessageBuilderGenerator::GenerateCommonBuilderMethods( printer->Print(" this.mergeExtensionFields(other);\n"); } - printer->Print(" this.mergeUnknownFields(other.unknownFields);\n"); + printer->Print(" this.mergeUnknownFields(other.getUnknownFields());\n"); printer->Print(" onChanged();\n"); @@ -605,20 +623,92 @@ void MessageBuilderGenerator::GenerateBuilderParsingMethods( " com.google.protobuf.CodedInputStream input,\n" " com.google.protobuf.ExtensionRegistryLite extensionRegistry)\n" " throws java.io.IOException {\n" - " $classname$ parsedMessage = null;\n" + " if (extensionRegistry == null) {\n" + " throw new java.lang.NullPointerException();\n" + " }\n" " try {\n" - " parsedMessage = PARSER.parsePartialFrom(input, extensionRegistry);\n" + " boolean done = false;\n" + " while (!done) {\n" + " int tag = input.readTag();\n" + " switch (tag) {\n" + " case 0:\n" // zero signals EOF / limit reached + " done = true;\n" + " break;\n"); + printer->Indent(); // method + printer->Indent(); // try + printer->Indent(); // while + printer->Indent(); // switch + GenerateBuilderFieldParsingCases(printer); + printer->Outdent(); // switch + printer->Outdent(); // while + printer->Outdent(); // try + printer->Outdent(); // method + printer->Print( + " default: {\n" + " if (!super.parseUnknownField(input, extensionRegistry, tag)) " + "{\n" + " done = true; // was an endgroup tag\n" + " }\n" + " break;\n" + " } // default:\n" + " } // switch (tag)\n" + " } // while (!done)\n" " } catch (com.google.protobuf.InvalidProtocolBufferException e) {\n" - " parsedMessage = ($classname$) e.getUnfinishedMessage();\n" " throw e.unwrapIOException();\n" " } finally {\n" - " if (parsedMessage != null) {\n" - " mergeFrom(parsedMessage);\n" - " }\n" - " }\n" + " onChanged();\n" + " } // finally\n" " return this;\n" - "}\n", - "classname", name_resolver_->GetImmutableClassName(descriptor_)); + "}\n"); +} + +void MessageBuilderGenerator::GenerateBuilderFieldParsingCases( + io::Printer* printer) { + std::unique_ptr sorted_fields( + SortFieldsByNumber(descriptor_)); + for (int i = 0; i < descriptor_->field_count(); i++) { + const FieldDescriptor* field = sorted_fields[i]; + GenerateBuilderFieldParsingCase(printer, field); + if (field->is_packable()) { + GenerateBuilderPackedFieldParsingCase(printer, field); + } + } +} + +void MessageBuilderGenerator::GenerateBuilderFieldParsingCase( + io::Printer* printer, const FieldDescriptor* field) { + uint32_t tag = WireFormatLite::MakeTag( + field->number(), WireFormat::WireTypeForFieldType(field->type())); + std::string tagString = absl::StrCat(static_cast(tag)); + printer->Print("case $tag$: {\n", "tag", tagString); + printer->Indent(); + + field_generators_.get(field).GenerateBuilderParsingCode(printer); + + printer->Outdent(); + printer->Print( + " break;\n" + "} // case $tag$\n", + "tag", tagString); +} + +void MessageBuilderGenerator::GenerateBuilderPackedFieldParsingCase( + io::Printer* printer, const FieldDescriptor* field) { + // To make packed = true wire compatible, we generate parsing code from a + // packed version of this field regardless of field->options().packed(). + uint32_t tag = WireFormatLite::MakeTag( + field->number(), WireFormatLite::WIRETYPE_LENGTH_DELIMITED); + std::string tagString = absl::StrCat(static_cast(tag)); + printer->Print("case $tag$: {\n", "tag", tagString); + printer->Indent(); + + field_generators_.get(field).GenerateBuilderParsingCodeFromPacked(printer); + + printer->Outdent(); + printer->Print( + " break;\n" + "} // case $tag$\n", + "tag", tagString); } // =================================================================== diff --git a/src/google/protobuf/compiler/java/message_builder.h b/src/google/protobuf/compiler/java/message_builder.h index 5ecbb91bbc..5d37bb6f0f 100644 --- a/src/google/protobuf/compiler/java/message_builder.h +++ b/src/google/protobuf/compiler/java/message_builder.h @@ -73,6 +73,11 @@ class MessageBuilderGenerator { void GenerateCommonBuilderMethods(io::Printer* printer); void GenerateDescriptorMethods(io::Printer* printer); void GenerateBuilderParsingMethods(io::Printer* printer); + void GenerateBuilderFieldParsingCases(io::Printer* printer); + void GenerateBuilderFieldParsingCase(io::Printer* printer, + const FieldDescriptor* field); + void GenerateBuilderPackedFieldParsingCase(io::Printer* printer, + const FieldDescriptor* field); void GenerateIsInitialized(io::Printer* printer); const Descriptor* descriptor_; diff --git a/src/google/protobuf/compiler/java/message_field.cc b/src/google/protobuf/compiler/java/message_field.cc index 32da1b74eb..42905c34a0 100644 --- a/src/google/protobuf/compiler/java/message_field.cc +++ b/src/google/protobuf/compiler/java/message_field.cc @@ -116,13 +116,6 @@ void SetMessageVariables(const FieldDescriptor* descriptor, int messageBitIndex, (*variables)["set_mutable_bit_builder"] = GenerateSetBit(builderBitIndex); (*variables)["clear_mutable_bit_builder"] = GenerateClearBit(builderBitIndex); - // For repeated fields, one bit is used for whether the array is immutable - // in the parsing constructor. - (*variables)["get_mutable_bit_parser"] = - GenerateGetBitMutableLocal(builderBitIndex); - (*variables)["set_mutable_bit_parser"] = - GenerateSetBitMutableLocal(builderBitIndex); - (*variables)["get_has_field_bit_from_local"] = GenerateGetBitFromLocal(builderBitIndex); (*variables)["set_has_field_bit_to_local"] = @@ -426,7 +419,7 @@ void ImmutableMessageFieldGenerator::GenerateKotlinDslMembers( io::Printer* printer) const { WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print(variables_, - "$kt_deprecation$var $kt_name$: $kt_type$\n" + "$kt_deprecation$public var $kt_name$: $kt_type$\n" " @JvmName(\"${$get$kt_capitalized_name$$}$\")\n" " get() = $kt_dsl_builder$.${$get$capitalized_name$$}$()\n" " @JvmName(\"${$set$kt_capitalized_name$$}$\")\n" @@ -437,7 +430,7 @@ void ImmutableMessageFieldGenerator::GenerateKotlinDslMembers( WriteFieldAccessorDocComment(printer, descriptor_, CLEARER, /* builder */ false, /* kdoc */ true); printer->Print(variables_, - "fun ${$clear$kt_capitalized_name$$}$() {\n" + "public fun ${$clear$kt_capitalized_name$$}$() {\n" " $kt_dsl_builder$.${$clear$capitalized_name$$}$()\n" "}\n"); @@ -445,7 +438,7 @@ void ImmutableMessageFieldGenerator::GenerateKotlinDslMembers( /* builder */ false, /* kdoc */ true); printer->Print( variables_, - "fun ${$has$kt_capitalized_name$$}$(): kotlin.Boolean {\n" + "public fun ${$has$kt_capitalized_name$$}$(): kotlin.Boolean {\n" " return $kt_dsl_builder$.${$has$capitalized_name$$}$()\n" "}\n"); @@ -455,7 +448,7 @@ void ImmutableMessageFieldGenerator::GenerateKotlinDslMembers( void ImmutableMessageFieldGenerator::GenerateKotlinOrNull(io::Printer* printer) const { if (descriptor_->has_optional_keyword()) { printer->Print(variables_, - "val $classname$Kt.Dsl.$name$OrNull: $kt_type$?\n" + "public val $classname$Kt.Dsl.$name$OrNull: $kt_type$?\n" " get() = $kt_dsl_builder$.$name$OrNull\n"); } } @@ -510,35 +503,21 @@ void ImmutableMessageFieldGenerator::GenerateBuildingCode( } } -void ImmutableMessageFieldGenerator::GenerateParsingCode( +void ImmutableMessageFieldGenerator::GenerateBuilderParsingCode( io::Printer* printer) const { - printer->Print(variables_, - "$type$.Builder subBuilder = null;\n" - "if ($is_field_present_message$) {\n" - " subBuilder = $name$_.toBuilder();\n" - "}\n"); - if (GetType(descriptor_) == FieldDescriptor::TYPE_GROUP) { printer->Print(variables_, - "$name$_ = input.readGroup($number$, $type$.$get_parser$,\n" - " extensionRegistry);\n"); + "input.readGroup($number$,\n" + " get$capitalized_name$FieldBuilder().getBuilder(),\n" + " extensionRegistry);\n" + "$set_has_field_bit_builder$\n"); } else { printer->Print(variables_, - "$name$_ = input.readMessage($type$.$get_parser$, " - "extensionRegistry);\n"); + "input.readMessage(\n" + " get$capitalized_name$FieldBuilder().getBuilder(),\n" + " extensionRegistry);\n" + "$set_has_field_bit_builder$\n"); } - - printer->Print(variables_, - "if (subBuilder != null) {\n" - " subBuilder.mergeFrom($name$_);\n" - " $name$_ = subBuilder.buildPartial();\n" - "}\n" - "$set_has_field_bit_message$\n"); -} - -void ImmutableMessageFieldGenerator::GenerateParsingDoneCode( - io::Printer* printer) const { - // noop for messages. } void ImmutableMessageFieldGenerator::GenerateSerializationCode( @@ -791,6 +770,15 @@ void ImmutableMessageOneofFieldGenerator::GenerateBuilderMembers( printer->Annotate("{", "}", descriptor_); } +void ImmutableMessageOneofFieldGenerator::GenerateBuilderClearCode( + io::Printer* printer) const { + // Make sure the builder gets cleared. + printer->Print(variables_, + "if ($name$Builder_ != null) {\n" + " $name$Builder_.clear();\n" + "}\n"); +} + void ImmutableMessageOneofFieldGenerator::GenerateBuildingCode( io::Printer* printer) const { printer->Print(variables_, "if ($has_oneof_case_message$) {\n"); @@ -811,32 +799,21 @@ void ImmutableMessageOneofFieldGenerator::GenerateMergingCode( "merge$capitalized_name$(other.get$capitalized_name$());\n"); } -void ImmutableMessageOneofFieldGenerator::GenerateParsingCode( +void ImmutableMessageOneofFieldGenerator::GenerateBuilderParsingCode( io::Printer* printer) const { - printer->Print(variables_, - "$type$.Builder subBuilder = null;\n" - "if ($has_oneof_case_message$) {\n" - " subBuilder = (($type$) $oneof_name$_).toBuilder();\n" - "}\n"); - if (GetType(descriptor_) == FieldDescriptor::TYPE_GROUP) { - printer->Print( - variables_, - "$oneof_name$_ = input.readGroup($number$, $type$.$get_parser$,\n" - " extensionRegistry);\n"); + printer->Print(variables_, + "input.readGroup($number$,\n" + " get$capitalized_name$FieldBuilder().getBuilder(),\n" + " extensionRegistry);\n" + "$set_oneof_case_message$;\n"); } else { - printer->Print( - variables_, - "$oneof_name$_ =\n" - " input.readMessage($type$.$get_parser$, extensionRegistry);\n"); + printer->Print(variables_, + "input.readMessage(\n" + " get$capitalized_name$FieldBuilder().getBuilder(),\n" + " extensionRegistry);\n" + "$set_oneof_case_message$;\n"); } - - printer->Print(variables_, - "if (subBuilder != null) {\n" - " subBuilder.mergeFrom(($type$) $oneof_name$_);\n" - " $oneof_name$_ = subBuilder.buildPartial();\n" - "}\n"); - printer->Print(variables_, "$set_oneof_case_message$;\n"); } void ImmutableMessageOneofFieldGenerator::GenerateSerializationCode( @@ -1287,10 +1264,12 @@ void RepeatedImmutableMessageFieldGenerator::GenerateInitializationCode( void RepeatedImmutableMessageFieldGenerator::GenerateBuilderClearCode( io::Printer* printer) const { PrintNestedBuilderCondition(printer, - "$name$_ = java.util.Collections.emptyList();\n" - "$clear_mutable_bit_builder$;\n", + "$name$_ = java.util.Collections.emptyList();\n", + "$name$_ = null;\n" "$name$Builder_.clear();\n"); + + printer->Print(variables_, "$clear_mutable_bit_builder$;\n"); } void RepeatedImmutableMessageFieldGenerator::GenerateMergingCode( @@ -1345,34 +1324,25 @@ void RepeatedImmutableMessageFieldGenerator::GenerateBuildingCode( "result.$name$_ = $name$Builder_.build();\n"); } -void RepeatedImmutableMessageFieldGenerator::GenerateParsingCode( +void RepeatedImmutableMessageFieldGenerator::GenerateBuilderParsingCode( io::Printer* printer) const { - printer->Print(variables_, - "if (!$get_mutable_bit_parser$) {\n" - " $name$_ = new java.util.ArrayList<$type$>();\n" - " $set_mutable_bit_parser$;\n" - "}\n"); - if (GetType(descriptor_) == FieldDescriptor::TYPE_GROUP) { - printer->Print( - variables_, - "$name$_.add(input.readGroup($number$, $type$.$get_parser$,\n" - " extensionRegistry));\n"); + printer->Print(variables_, + "$type$ m =\n" + " input.readGroup($number$,\n" + " $type$.$get_parser$,\n" + " extensionRegistry);\n"); } else { - printer->Print( - variables_, - "$name$_.add(\n" - " input.readMessage($type$.$get_parser$, extensionRegistry));\n"); + printer->Print(variables_, + "$type$ m =\n" + " input.readMessage(\n" + " $type$.$get_parser$,\n" + " extensionRegistry);\n"); } -} - -void RepeatedImmutableMessageFieldGenerator::GenerateParsingDoneCode( - io::Printer* printer) const { - printer->Print( - variables_, - "if ($get_mutable_bit_parser$) {\n" - " $name$_ = java.util.Collections.unmodifiableList($name$_);\n" - "}\n"); + PrintNestedBuilderCondition(printer, + "ensure$capitalized_name$IsMutable();\n" + "$name$_.add(m);\n", + "$name$Builder_.addMessage(m);\n"); } void RepeatedImmutableMessageFieldGenerator::GenerateSerializationCode( @@ -1425,12 +1395,12 @@ void RepeatedImmutableMessageFieldGenerator::GenerateKotlinDslMembers( " */\n" "@kotlin.OptIn" "(com.google.protobuf.kotlin.OnlyForUseByGeneratedProtoCode::class)\n" - "class ${$$kt_capitalized_name$Proxy$}$ private constructor()" + "public class ${$$kt_capitalized_name$Proxy$}$ private constructor()" " : com.google.protobuf.kotlin.DslProxy()\n"); WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print(variables_, - "$kt_deprecation$ val $kt_name$: " + "$kt_deprecation$ public val $kt_name$: " "com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>\n" " @kotlin.jvm.JvmSynthetic\n" @@ -1443,7 +1413,7 @@ void RepeatedImmutableMessageFieldGenerator::GenerateKotlinDslMembers( printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"add$kt_capitalized_name$\")\n" - "fun com.google.protobuf.kotlin.DslList" + "public fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "add(value: $kt_type$) {\n" " $kt_dsl_builder$.${$add$capitalized_name$$}$(value)\n" @@ -1455,7 +1425,7 @@ void RepeatedImmutableMessageFieldGenerator::GenerateKotlinDslMembers( "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"plusAssign$kt_capitalized_name$\")\n" "@Suppress(\"NOTHING_TO_INLINE\")\n" - "inline operator fun com.google.protobuf.kotlin.DslList" + "public inline operator fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "plusAssign(value: $kt_type$) {\n" " add(value)\n" @@ -1466,7 +1436,7 @@ void RepeatedImmutableMessageFieldGenerator::GenerateKotlinDslMembers( printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"addAll$kt_capitalized_name$\")\n" - "fun com.google.protobuf.kotlin.DslList" + "public fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "addAll(values: kotlin.collections.Iterable<$kt_type$>) {\n" " $kt_dsl_builder$.${$addAll$capitalized_name$$}$(values)\n" @@ -1479,7 +1449,7 @@ void RepeatedImmutableMessageFieldGenerator::GenerateKotlinDslMembers( "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"plusAssignAll$kt_capitalized_name$\")\n" "@Suppress(\"NOTHING_TO_INLINE\")\n" - "inline operator fun com.google.protobuf.kotlin.DslList" + "public inline operator fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "plusAssign(values: kotlin.collections.Iterable<$kt_type$>) {\n" " addAll(values)\n" @@ -1491,7 +1461,7 @@ void RepeatedImmutableMessageFieldGenerator::GenerateKotlinDslMembers( variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"set$kt_capitalized_name$\")\n" - "operator fun com.google.protobuf.kotlin.DslList" + "public operator fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "set(index: kotlin.Int, value: $kt_type$) {\n" " $kt_dsl_builder$.${$set$capitalized_name$$}$(index, value)\n" @@ -1502,7 +1472,7 @@ void RepeatedImmutableMessageFieldGenerator::GenerateKotlinDslMembers( printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"clear$kt_capitalized_name$\")\n" - "fun com.google.protobuf.kotlin.DslList" + "public fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "clear() {\n" " $kt_dsl_builder$.${$clear$capitalized_name$$}$()\n" diff --git a/src/google/protobuf/compiler/java/message_field.h b/src/google/protobuf/compiler/java/message_field.h index 65ce3733bf..4fe5ac7df4 100644 --- a/src/google/protobuf/compiler/java/message_field.h +++ b/src/google/protobuf/compiler/java/message_field.h @@ -79,8 +79,7 @@ class ImmutableMessageFieldGenerator : public ImmutableFieldGenerator { void GenerateBuilderClearCode(io::Printer* printer) const override; void GenerateMergingCode(io::Printer* printer) const override; void GenerateBuildingCode(io::Printer* printer) const override; - void GenerateParsingCode(io::Printer* printer) const override; - void GenerateParsingDoneCode(io::Printer* printer) const override; + void GenerateBuilderParsingCode(io::Printer* printer) const override; void GenerateSerializationCode(io::Printer* printer) const override; void GenerateSerializedSizeCode(io::Printer* printer) const override; void GenerateFieldBuilderInitializationCode( @@ -124,9 +123,10 @@ class ImmutableMessageOneofFieldGenerator void GenerateMembers(io::Printer* printer) const override; void GenerateBuilderMembers(io::Printer* printer) const override; + void GenerateBuilderClearCode(io::Printer* printer) const override; void GenerateBuildingCode(io::Printer* printer) const override; void GenerateMergingCode(io::Printer* printer) const override; - void GenerateParsingCode(io::Printer* printer) const override; + void GenerateBuilderParsingCode(io::Printer* printer) const override; void GenerateSerializationCode(io::Printer* printer) const override; void GenerateSerializedSizeCode(io::Printer* printer) const override; }; @@ -152,8 +152,7 @@ class RepeatedImmutableMessageFieldGenerator : public ImmutableFieldGenerator { void GenerateBuilderClearCode(io::Printer* printer) const override; void GenerateMergingCode(io::Printer* printer) const override; void GenerateBuildingCode(io::Printer* printer) const override; - void GenerateParsingCode(io::Printer* printer) const override; - void GenerateParsingDoneCode(io::Printer* printer) const override; + void GenerateBuilderParsingCode(io::Printer* printer) const override; void GenerateSerializationCode(io::Printer* printer) const override; void GenerateSerializedSizeCode(io::Printer* printer) const override; void GenerateFieldBuilderInitializationCode( diff --git a/src/google/protobuf/compiler/java/message_field_lite.cc b/src/google/protobuf/compiler/java/message_field_lite.cc index 655d8eba74..bc7c99f5e1 100644 --- a/src/google/protobuf/compiler/java/message_field_lite.cc +++ b/src/google/protobuf/compiler/java/message_field_lite.cc @@ -300,7 +300,7 @@ void ImmutableMessageFieldLiteGenerator::GenerateKotlinDslMembers( io::Printer* printer) const { WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print(variables_, - "$kt_deprecation$var $kt_name$: $kt_type$\n" + "$kt_deprecation$public var $kt_name$: $kt_type$\n" " @JvmName(\"${$get$kt_capitalized_name$$}$\")\n" " get() = $kt_dsl_builder$.${$get$capitalized_name$$}$()\n" " @JvmName(\"${$set$kt_capitalized_name$$}$\")\n" @@ -311,7 +311,7 @@ void ImmutableMessageFieldLiteGenerator::GenerateKotlinDslMembers( WriteFieldAccessorDocComment(printer, descriptor_, CLEARER, /* builder */ false, /* kdoc */ true); printer->Print(variables_, - "fun ${$clear$kt_capitalized_name$$}$() {\n" + "public fun ${$clear$kt_capitalized_name$$}$() {\n" " $kt_dsl_builder$.${$clear$capitalized_name$$}$()\n" "}\n"); @@ -319,7 +319,7 @@ void ImmutableMessageFieldLiteGenerator::GenerateKotlinDslMembers( /* builder */ false, /* kdoc */ true); printer->Print( variables_, - "fun ${$has$kt_capitalized_name$$}$(): kotlin.Boolean {\n" + "public fun ${$has$kt_capitalized_name$$}$(): kotlin.Boolean {\n" " return $kt_dsl_builder$.${$has$capitalized_name$$}$()\n" "}\n"); GenerateKotlinOrNull(printer); @@ -328,7 +328,7 @@ void ImmutableMessageFieldLiteGenerator::GenerateKotlinDslMembers( void ImmutableMessageFieldLiteGenerator::GenerateKotlinOrNull(io::Printer* printer) const { if (descriptor_->has_optional_keyword()) { printer->Print(variables_, - "val $classname$Kt.Dsl.$name$OrNull: $kt_type$?\n" + "public val $classname$Kt.Dsl.$name$OrNull: $kt_type$?\n" " get() = $kt_dsl_builder$.$name$OrNull\n"); } } @@ -830,12 +830,12 @@ void RepeatedImmutableMessageFieldLiteGenerator::GenerateKotlinDslMembers( " */\n" "@kotlin.OptIn" "(com.google.protobuf.kotlin.OnlyForUseByGeneratedProtoCode::class)\n" - "class ${$$kt_capitalized_name$Proxy$}$ private constructor()" + "public class ${$$kt_capitalized_name$Proxy$}$ private constructor()" " : com.google.protobuf.kotlin.DslProxy()\n"); WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print(variables_, - "$kt_deprecation$ val $kt_name$: " + "$kt_deprecation$ public val $kt_name$: " "com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>\n" " @kotlin.jvm.JvmSynthetic\n" @@ -848,7 +848,7 @@ void RepeatedImmutableMessageFieldLiteGenerator::GenerateKotlinDslMembers( printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"add$kt_capitalized_name$\")\n" - "fun com.google.protobuf.kotlin.DslList" + "public fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "add(value: $kt_type$) {\n" " $kt_dsl_builder$.${$add$capitalized_name$$}$(value)\n" @@ -860,7 +860,7 @@ void RepeatedImmutableMessageFieldLiteGenerator::GenerateKotlinDslMembers( "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"plusAssign$kt_capitalized_name$\")\n" "@Suppress(\"NOTHING_TO_INLINE\")\n" - "inline operator fun com.google.protobuf.kotlin.DslList" + "public inline operator fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "plusAssign(value: $kt_type$) {\n" " add(value)\n" @@ -871,7 +871,7 @@ void RepeatedImmutableMessageFieldLiteGenerator::GenerateKotlinDslMembers( printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"addAll$kt_capitalized_name$\")\n" - "fun com.google.protobuf.kotlin.DslList" + "public fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "addAll(values: kotlin.collections.Iterable<$kt_type$>) {\n" " $kt_dsl_builder$.${$addAll$capitalized_name$$}$(values)\n" @@ -884,7 +884,7 @@ void RepeatedImmutableMessageFieldLiteGenerator::GenerateKotlinDslMembers( "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"plusAssignAll$kt_capitalized_name$\")\n" "@Suppress(\"NOTHING_TO_INLINE\")\n" - "inline operator fun com.google.protobuf.kotlin.DslList" + "public inline operator fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "plusAssign(values: kotlin.collections.Iterable<$kt_type$>) {\n" " addAll(values)\n" @@ -896,7 +896,7 @@ void RepeatedImmutableMessageFieldLiteGenerator::GenerateKotlinDslMembers( variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"set$kt_capitalized_name$\")\n" - "operator fun com.google.protobuf.kotlin.DslList" + "public operator fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "set(index: kotlin.Int, value: $kt_type$) {\n" " $kt_dsl_builder$.${$set$capitalized_name$$}$(index, value)\n" @@ -907,7 +907,7 @@ void RepeatedImmutableMessageFieldLiteGenerator::GenerateKotlinDslMembers( printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"clear$kt_capitalized_name$\")\n" - "fun com.google.protobuf.kotlin.DslList" + "public fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "clear() {\n" " $kt_dsl_builder$.${$clear$capitalized_name$$}$()\n" diff --git a/src/google/protobuf/compiler/java/message_lite.cc b/src/google/protobuf/compiler/java/message_lite.cc index 44568b9310..fc44cb172d 100644 --- a/src/google/protobuf/compiler/java/message_lite.cc +++ b/src/google/protobuf/compiler/java/message_lite.cc @@ -759,10 +759,10 @@ void ImmutableMessageLiteGenerator::GenerateKotlinDsl( "(com.google.protobuf.kotlin.OnlyForUseByGeneratedProtoCode::class)\n" "@com.google.protobuf.kotlin.ProtoDslMarker\n"); printer->Print( - "class Dsl private constructor(\n" + "public class Dsl private constructor(\n" " private val _builder: $message$.Builder\n" ") {\n" - " companion object {\n" + " public companion object {\n" " @kotlin.jvm.JvmSynthetic\n" " @kotlin.PublishedApi\n" " internal fun _create(builder: $message$.Builder): Dsl = " @@ -785,10 +785,10 @@ void ImmutableMessageLiteGenerator::GenerateKotlinDsl( for (auto oneof : oneofs_) { printer->Print( - "val $oneof_name$Case: $message$.$oneof_capitalized_name$Case\n" + "public val $oneof_name$Case: $message$.$oneof_capitalized_name$Case\n" " @JvmName(\"get$oneof_capitalized_name$Case\")\n" " get() = _builder.get$oneof_capitalized_name$Case()\n\n" - "fun clear$oneof_capitalized_name$() {\n" + "public fun clear$oneof_capitalized_name$() {\n" " _builder.clear$oneof_capitalized_name$()\n" "}\n", "oneof_name", context_->GetOneofGeneratorInfo(oneof)->name, @@ -815,7 +815,7 @@ void ImmutableMessageLiteGenerator::GenerateKotlinMembers( printer->Print("@com.google.errorprone.annotations.CheckReturnValue\n"); } printer->Print( - "inline fun $camelcase_name$(block: $message_kt$.Dsl.() -> " + "public inline fun $camelcase_name$(block: $message_kt$.Dsl.() -> " "kotlin.Unit): $message$ =\n" " $message_kt$.Dsl._create($message$.newBuilder()).apply { block() " "}._build()\n", @@ -827,7 +827,7 @@ void ImmutableMessageLiteGenerator::GenerateKotlinMembers( EscapeKotlinKeywords(name_resolver_->GetClassName(descriptor_, true))); WriteMessageDocComment(printer, descriptor_, /* kdoc */ true); - printer->Print("object $name$Kt {\n", "name", descriptor_->name()); + printer->Print("public object $name$Kt {\n", "name", descriptor_->name()); printer->Indent(); GenerateKotlinDsl(printer); for (int i = 0; i < descriptor_->nested_type_count(); i++) { @@ -845,7 +845,7 @@ void ImmutableMessageLiteGenerator::GenerateTopLevelKotlinMembers( printer->Print("@com.google.errorprone.annotations.CheckReturnValue\n"); } printer->Print( - "inline fun $message$.copy(block: $message_kt$.Dsl.() -> " + "public inline fun $message$.copy(block: $message_kt$.Dsl.() -> " "kotlin.Unit): $message$ =\n" " $message_kt$.Dsl._create(this.toBuilder()).apply { block() " "}._build()\n\n", @@ -892,7 +892,7 @@ void ImmutableMessageLiteGenerator::GenerateKotlinExtensions( printer->Print( "@Suppress(\"UNCHECKED_CAST\")\n" "@kotlin.jvm.JvmSynthetic\n" - "operator fun get(extension: " + "public operator fun get(extension: " "com.google.protobuf.ExtensionLite<$message$, T>): T {\n" " return if (extension.isRepeated) {\n" " get(extension as com.google.protobuf.ExtensionLite<$message$, " @@ -908,7 +908,7 @@ void ImmutableMessageLiteGenerator::GenerateKotlinExtensions( "@kotlin.OptIn" "(com.google.protobuf.kotlin.OnlyForUseByGeneratedProtoCode::class)\n" "@kotlin.jvm.JvmName(\"-getRepeatedExtension\")\n" - "operator fun get(\n" + "public operator fun get(\n" " extension: com.google.protobuf.ExtensionLite<$message$, List>\n" "): com.google.protobuf.kotlin.ExtensionList {\n" " return com.google.protobuf.kotlin.ExtensionList(extension, " @@ -918,7 +918,7 @@ void ImmutableMessageLiteGenerator::GenerateKotlinExtensions( printer->Print( "@kotlin.jvm.JvmSynthetic\n" - "operator fun contains(extension: " + "public operator fun contains(extension: " "com.google.protobuf.ExtensionLite<$message$, *>): " "Boolean {\n" " return _builder.hasExtension(extension)\n" @@ -927,7 +927,7 @@ void ImmutableMessageLiteGenerator::GenerateKotlinExtensions( printer->Print( "@kotlin.jvm.JvmSynthetic\n" - "fun clear(extension: " + "public fun clear(extension: " "com.google.protobuf.ExtensionLite<$message$, *>) " "{\n" " _builder.clearExtension(extension)\n" @@ -947,7 +947,7 @@ void ImmutableMessageLiteGenerator::GenerateKotlinExtensions( printer->Print( "@kotlin.jvm.JvmSynthetic\n" "@Suppress(\"NOTHING_TO_INLINE\")\n" - "inline operator fun > set(\n" + "public inline operator fun > set(\n" " extension: com.google.protobuf.ExtensionLite<$message$, T>,\n" " value: T\n" ") {\n" @@ -958,7 +958,7 @@ void ImmutableMessageLiteGenerator::GenerateKotlinExtensions( printer->Print( "@kotlin.jvm.JvmSynthetic\n" "@Suppress(\"NOTHING_TO_INLINE\")\n" - "inline operator fun set(\n" + "public inline operator fun set(\n" " extension: com.google.protobuf.ExtensionLite<$message$, " "com.google.protobuf.ByteString>,\n" " value: com.google.protobuf.ByteString\n" @@ -970,7 +970,7 @@ void ImmutableMessageLiteGenerator::GenerateKotlinExtensions( printer->Print( "@kotlin.jvm.JvmSynthetic\n" "@Suppress(\"NOTHING_TO_INLINE\")\n" - "inline operator fun set(\n" + "public inline operator fun set(\n" " extension: com.google.protobuf.ExtensionLite<$message$, T>,\n" " value: T\n" ") {\n" @@ -980,7 +980,7 @@ void ImmutableMessageLiteGenerator::GenerateKotlinExtensions( printer->Print( "@kotlin.jvm.JvmSynthetic\n" - "fun com.google.protobuf.kotlin.ExtensionList com.google.protobuf.kotlin.ExtensionList.add(value: E) {\n" " _builder.addExtension(this.extension, value)\n" "}\n\n", @@ -989,7 +989,7 @@ void ImmutableMessageLiteGenerator::GenerateKotlinExtensions( printer->Print( "@kotlin.jvm.JvmSynthetic\n" "@Suppress(\"NOTHING_TO_INLINE\")\n" - "inline operator fun " + "public inline operator fun " "com.google.protobuf.kotlin.ExtensionList.plusAssign" "(value: E) {\n" @@ -999,7 +999,7 @@ void ImmutableMessageLiteGenerator::GenerateKotlinExtensions( printer->Print( "@kotlin.jvm.JvmSynthetic\n" - "fun com.google.protobuf.kotlin.ExtensionList com.google.protobuf.kotlin.ExtensionList.addAll(values: Iterable) {\n" " for (value in values) {\n" " add(value)\n" @@ -1010,7 +1010,7 @@ void ImmutableMessageLiteGenerator::GenerateKotlinExtensions( printer->Print( "@kotlin.jvm.JvmSynthetic\n" "@Suppress(\"NOTHING_TO_INLINE\")\n" - "inline operator fun " + "public inline operator fun " "com.google.protobuf.kotlin.ExtensionList.plusAssign(values: " "Iterable) {\n" @@ -1020,7 +1020,7 @@ void ImmutableMessageLiteGenerator::GenerateKotlinExtensions( printer->Print( "@kotlin.jvm.JvmSynthetic\n" - "operator fun " + "public operator fun " "com.google.protobuf.kotlin.ExtensionList.set(index: Int, value: " "E) {\n" @@ -1031,7 +1031,7 @@ void ImmutableMessageLiteGenerator::GenerateKotlinExtensions( printer->Print( "@kotlin.jvm.JvmSynthetic\n" "@Suppress(\"NOTHING_TO_INLINE\")\n" - "inline fun com.google.protobuf.kotlin.ExtensionList<*, " + "public inline fun com.google.protobuf.kotlin.ExtensionList<*, " "$message$>.clear() {\n" " clear(extension)\n" "}\n\n", diff --git a/src/google/protobuf/compiler/java/primitive_field.cc b/src/google/protobuf/compiler/java/primitive_field.cc index 3706ef164f..a66b41f688 100644 --- a/src/google/protobuf/compiler/java/primitive_field.cc +++ b/src/google/protobuf/compiler/java/primitive_field.cc @@ -190,13 +190,6 @@ void SetPrimitiveVariables(const FieldDescriptor* descriptor, (*variables)["set_mutable_bit_builder"] = GenerateSetBit(builderBitIndex); (*variables)["clear_mutable_bit_builder"] = GenerateClearBit(builderBitIndex); - // For repeated fields, one bit is used for whether the array is immutable - // in the parsing constructor. - (*variables)["get_mutable_bit_parser"] = - GenerateGetBitMutableLocal(builderBitIndex); - (*variables)["set_mutable_bit_parser"] = - GenerateSetBitMutableLocal(builderBitIndex); - (*variables)["get_has_field_bit_from_local"] = GenerateGetBitFromLocal(builderBitIndex); (*variables)["set_has_field_bit_to_local"] = @@ -324,7 +317,7 @@ void ImmutablePrimitiveFieldGenerator::GenerateKotlinDslMembers( io::Printer* printer) const { WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print(variables_, - "$kt_deprecation$var $kt_name$: $kt_type$\n" + "$kt_deprecation$public var $kt_name$: $kt_type$\n" " @JvmName(\"${$get$kt_capitalized_name$$}$\")\n" " get() = $kt_dsl_builder$.${$get$capitalized_name$$}$()\n" " @JvmName(\"${$set$kt_capitalized_name$$}$\")\n" @@ -335,7 +328,7 @@ void ImmutablePrimitiveFieldGenerator::GenerateKotlinDslMembers( WriteFieldAccessorDocComment(printer, descriptor_, CLEARER, /* builder */ false, /* kdoc */ true); printer->Print(variables_, - "fun ${$clear$kt_capitalized_name$$}$() {\n" + "public fun ${$clear$kt_capitalized_name$$}$() {\n" " $kt_dsl_builder$.${$clear$capitalized_name$$}$()\n" "}\n"); @@ -344,7 +337,7 @@ void ImmutablePrimitiveFieldGenerator::GenerateKotlinDslMembers( /* builder */ false, /* kdoc */ true); printer->Print( variables_, - "fun ${$has$kt_capitalized_name$$}$(): kotlin.Boolean {\n" + "public fun ${$has$kt_capitalized_name$$}$(): kotlin.Boolean {\n" " return $kt_dsl_builder$.${$has$capitalized_name$$}$()\n" "}\n"); } @@ -405,16 +398,11 @@ void ImmutablePrimitiveFieldGenerator::GenerateBuildingCode( } } -void ImmutablePrimitiveFieldGenerator::GenerateParsingCode( +void ImmutablePrimitiveFieldGenerator::GenerateBuilderParsingCode( io::Printer* printer) const { printer->Print(variables_, - "$set_has_field_bit_message$\n" - "$name$_ = input.read$capitalized_type$();\n"); -} - -void ImmutablePrimitiveFieldGenerator::GenerateParsingDoneCode( - io::Printer* printer) const { - // noop for primitives. + "$name$_ = input.read$capitalized_type$();\n" + "$set_has_field_bit_builder$\n"); } void ImmutablePrimitiveFieldGenerator::GenerateSerializationCode( @@ -619,6 +607,11 @@ void ImmutablePrimitiveOneofFieldGenerator::GenerateBuilderMembers( printer->Annotate("{", "}", descriptor_); } +void ImmutablePrimitiveOneofFieldGenerator::GenerateBuilderClearCode( + io::Printer* printer) const { + // When a primitive field is in a oneof, clearing the oneof clears that field. +} + void ImmutablePrimitiveOneofFieldGenerator::GenerateBuildingCode( io::Printer* printer) const { printer->Print(variables_, @@ -633,7 +626,7 @@ void ImmutablePrimitiveOneofFieldGenerator::GenerateMergingCode( "set$capitalized_name$(other.get$capitalized_name$());\n"); } -void ImmutablePrimitiveOneofFieldGenerator::GenerateParsingCode( +void ImmutablePrimitiveOneofFieldGenerator::GenerateBuilderParsingCode( io::Printer* printer) const { printer->Print(variables_, "$oneof_name$_ = input.read$capitalized_type$();\n" @@ -854,12 +847,12 @@ void RepeatedImmutablePrimitiveFieldGenerator::GenerateKotlinDslMembers( " */\n" "@kotlin.OptIn" "(com.google.protobuf.kotlin.OnlyForUseByGeneratedProtoCode::class)\n" - "class ${$$kt_capitalized_name$Proxy$}$ private constructor()" + "public class ${$$kt_capitalized_name$Proxy$}$ private constructor()" " : com.google.protobuf.kotlin.DslProxy()\n"); WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print(variables_, - "$kt_deprecation$ val $kt_name$: " + "$kt_deprecation$ public val $kt_name$: " "com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>\n" " @kotlin.jvm.JvmSynthetic\n" @@ -872,7 +865,7 @@ void RepeatedImmutablePrimitiveFieldGenerator::GenerateKotlinDslMembers( printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"add$kt_capitalized_name$\")\n" - "fun com.google.protobuf.kotlin.DslList" + "public fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "add(value: $kt_type$) {\n" " $kt_dsl_builder$.${$add$capitalized_name$$}$(value)\n" @@ -884,7 +877,7 @@ void RepeatedImmutablePrimitiveFieldGenerator::GenerateKotlinDslMembers( "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"plusAssign$kt_capitalized_name$\")\n" "@Suppress(\"NOTHING_TO_INLINE\")\n" - "inline operator fun com.google.protobuf.kotlin.DslList" + "public inline operator fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "plusAssign(value: $kt_type$) {\n" " add(value)\n" @@ -895,7 +888,7 @@ void RepeatedImmutablePrimitiveFieldGenerator::GenerateKotlinDslMembers( printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"addAll$kt_capitalized_name$\")\n" - "fun com.google.protobuf.kotlin.DslList" + "public fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "addAll(values: kotlin.collections.Iterable<$kt_type$>) {\n" " $kt_dsl_builder$.${$addAll$capitalized_name$$}$(values)\n" @@ -908,7 +901,7 @@ void RepeatedImmutablePrimitiveFieldGenerator::GenerateKotlinDslMembers( "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"plusAssignAll$kt_capitalized_name$\")\n" "@Suppress(\"NOTHING_TO_INLINE\")\n" - "inline operator fun com.google.protobuf.kotlin.DslList" + "public inline operator fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "plusAssign(values: kotlin.collections.Iterable<$kt_type$>) {\n" " addAll(values)\n" @@ -920,7 +913,7 @@ void RepeatedImmutablePrimitiveFieldGenerator::GenerateKotlinDslMembers( variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"set$kt_capitalized_name$\")\n" - "operator fun com.google.protobuf.kotlin.DslList" + "public operator fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "set(index: kotlin.Int, value: $kt_type$) {\n" " $kt_dsl_builder$.${$set$capitalized_name$$}$(index, value)\n" @@ -931,7 +924,7 @@ void RepeatedImmutablePrimitiveFieldGenerator::GenerateKotlinDslMembers( printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"clear$kt_capitalized_name$\")\n" - "fun com.google.protobuf.kotlin.DslList" + "public fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "clear() {\n" " $kt_dsl_builder$.${$clear$capitalized_name$$}$()\n" @@ -987,38 +980,24 @@ void RepeatedImmutablePrimitiveFieldGenerator::GenerateBuildingCode( "result.$name$_ = $name$_;\n"); } -void RepeatedImmutablePrimitiveFieldGenerator::GenerateParsingCode( +void RepeatedImmutablePrimitiveFieldGenerator::GenerateBuilderParsingCode( io::Printer* printer) const { printer->Print(variables_, - "if (!$get_mutable_bit_parser$) {\n" - " $name$_ = $create_list$;\n" - " $set_mutable_bit_parser$;\n" - "}\n" - "$repeated_add$(input.read$capitalized_type$());\n"); + "$type$ v = input.read$capitalized_type$();\n" + "ensure$capitalized_name$IsMutable();\n" + "$repeated_add$(v);\n"); } -void RepeatedImmutablePrimitiveFieldGenerator::GenerateParsingCodeFromPacked( - io::Printer* printer) const { - printer->Print( - variables_, - "int length = input.readRawVarint32();\n" - "int limit = input.pushLimit(length);\n" - "if (!$get_mutable_bit_parser$ && input.getBytesUntilLimit() > 0) {\n" - " $name$_ = $create_list$;\n" - " $set_mutable_bit_parser$;\n" - "}\n" - "while (input.getBytesUntilLimit() > 0) {\n" - " $repeated_add$(input.read$capitalized_type$());\n" - "}\n" - "input.popLimit(limit);\n"); -} - -void RepeatedImmutablePrimitiveFieldGenerator::GenerateParsingDoneCode( - io::Printer* printer) const { +void RepeatedImmutablePrimitiveFieldGenerator:: + GenerateBuilderParsingCodeFromPacked(io::Printer* printer) const { printer->Print(variables_, - "if ($get_mutable_bit_parser$) {\n" - " $name_make_immutable$; // C\n" - "}\n"); + "int length = input.readRawVarint32();\n" + "int limit = input.pushLimit(length);\n" + "ensure$capitalized_name$IsMutable();\n" + "while (input.getBytesUntilLimit() > 0) {\n" + " $repeated_add$(input.read$capitalized_type$());\n" + "}\n" + "input.popLimit(limit);\n"); } void RepeatedImmutablePrimitiveFieldGenerator::GenerateSerializationCode( diff --git a/src/google/protobuf/compiler/java/primitive_field.h b/src/google/protobuf/compiler/java/primitive_field.h index 914772cf4b..d8104ba5f6 100644 --- a/src/google/protobuf/compiler/java/primitive_field.h +++ b/src/google/protobuf/compiler/java/primitive_field.h @@ -79,8 +79,7 @@ class ImmutablePrimitiveFieldGenerator : public ImmutableFieldGenerator { void GenerateBuilderClearCode(io::Printer* printer) const override; void GenerateMergingCode(io::Printer* printer) const override; void GenerateBuildingCode(io::Printer* printer) const override; - void GenerateParsingCode(io::Printer* printer) const override; - void GenerateParsingDoneCode(io::Printer* printer) const override; + void GenerateBuilderParsingCode(io::Printer* printer) const override; void GenerateSerializationCode(io::Printer* printer) const override; void GenerateSerializedSizeCode(io::Printer* printer) const override; void GenerateFieldBuilderInitializationCode( @@ -111,9 +110,10 @@ class ImmutablePrimitiveOneofFieldGenerator void GenerateMembers(io::Printer* printer) const override; void GenerateBuilderMembers(io::Printer* printer) const override; + void GenerateBuilderClearCode(io::Printer* printer) const override; void GenerateBuildingCode(io::Printer* printer) const override; void GenerateMergingCode(io::Printer* printer) const override; - void GenerateParsingCode(io::Printer* printer) const override; + void GenerateBuilderParsingCode(io::Printer* printer) const override; void GenerateSerializationCode(io::Printer* printer) const override; void GenerateSerializedSizeCode(io::Printer* printer) const override; }; @@ -140,9 +140,9 @@ class RepeatedImmutablePrimitiveFieldGenerator void GenerateBuilderClearCode(io::Printer* printer) const override; void GenerateMergingCode(io::Printer* printer) const override; void GenerateBuildingCode(io::Printer* printer) const override; - void GenerateParsingCode(io::Printer* printer) const override; - void GenerateParsingCodeFromPacked(io::Printer* printer) const override; - void GenerateParsingDoneCode(io::Printer* printer) const override; + void GenerateBuilderParsingCode(io::Printer* printer) const override; + void GenerateBuilderParsingCodeFromPacked( + io::Printer* printer) const override; void GenerateSerializationCode(io::Printer* printer) const override; void GenerateSerializedSizeCode(io::Printer* printer) const override; void GenerateFieldBuilderInitializationCode( diff --git a/src/google/protobuf/compiler/java/primitive_field_lite.cc b/src/google/protobuf/compiler/java/primitive_field_lite.cc index 3d9fa3d68a..6db3a8c657 100644 --- a/src/google/protobuf/compiler/java/primitive_field_lite.cc +++ b/src/google/protobuf/compiler/java/primitive_field_lite.cc @@ -348,7 +348,7 @@ void ImmutablePrimitiveFieldLiteGenerator::GenerateKotlinDslMembers( io::Printer* printer) const { WriteFieldDocComment(printer, descriptor_); printer->Print(variables_, - "$kt_deprecation$var $kt_name$: $kt_type$\n" + "$kt_deprecation$public var $kt_name$: $kt_type$\n" " @JvmName(\"${$get$kt_capitalized_name$$}$\")\n" " get() = $kt_dsl_builder$.${$get$capitalized_name$$}$()\n" " @JvmName(\"${$set$kt_capitalized_name$$}$\")\n" @@ -359,7 +359,7 @@ void ImmutablePrimitiveFieldLiteGenerator::GenerateKotlinDslMembers( WriteFieldAccessorDocComment(printer, descriptor_, CLEARER, /* builder */ false, /* kdoc */ true); printer->Print(variables_, - "fun ${$clear$kt_capitalized_name$$}$() {\n" + "public fun ${$clear$kt_capitalized_name$$}$() {\n" " $kt_dsl_builder$.${$clear$capitalized_name$$}$()\n" "}\n"); @@ -368,7 +368,7 @@ void ImmutablePrimitiveFieldLiteGenerator::GenerateKotlinDslMembers( /* builder */ false, /* kdoc */ true); printer->Print( variables_, - "fun ${$has$kt_capitalized_name$$}$(): kotlin.Boolean {\n" + "public fun ${$has$kt_capitalized_name$$}$(): kotlin.Boolean {\n" " return $kt_dsl_builder$.${$has$capitalized_name$$}$()\n" "}\n"); } @@ -699,12 +699,12 @@ void RepeatedImmutablePrimitiveFieldLiteGenerator::GenerateKotlinDslMembers( " */\n" "@kotlin.OptIn" "(com.google.protobuf.kotlin.OnlyForUseByGeneratedProtoCode::class)\n" - "class ${$$kt_capitalized_name$Proxy$}$ private constructor()" + "public class ${$$kt_capitalized_name$Proxy$}$ private constructor()" " : com.google.protobuf.kotlin.DslProxy()\n"); WriteFieldDocComment(printer, descriptor_); printer->Print(variables_, - "$kt_deprecation$ val $kt_name$: " + "$kt_deprecation$ public val $kt_name$: " "com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>\n" " @kotlin.jvm.JvmSynthetic\n" @@ -717,7 +717,7 @@ void RepeatedImmutablePrimitiveFieldLiteGenerator::GenerateKotlinDslMembers( printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"add$kt_capitalized_name$\")\n" - "fun com.google.protobuf.kotlin.DslList" + "public fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "add(value: $kt_type$) {\n" " $kt_dsl_builder$.${$add$capitalized_name$$}$(value)\n" @@ -729,7 +729,7 @@ void RepeatedImmutablePrimitiveFieldLiteGenerator::GenerateKotlinDslMembers( "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"plusAssign$kt_capitalized_name$\")\n" "@Suppress(\"NOTHING_TO_INLINE\")\n" - "inline operator fun com.google.protobuf.kotlin.DslList" + "public inline operator fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "plusAssign(value: $kt_type$) {\n" " add(value)\n" @@ -740,7 +740,7 @@ void RepeatedImmutablePrimitiveFieldLiteGenerator::GenerateKotlinDslMembers( printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"addAll$kt_capitalized_name$\")\n" - "fun com.google.protobuf.kotlin.DslList" + "public fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "addAll(values: kotlin.collections.Iterable<$kt_type$>) {\n" " $kt_dsl_builder$.${$addAll$capitalized_name$$}$(values)\n" @@ -753,7 +753,7 @@ void RepeatedImmutablePrimitiveFieldLiteGenerator::GenerateKotlinDslMembers( "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"plusAssignAll$kt_capitalized_name$\")\n" "@Suppress(\"NOTHING_TO_INLINE\")\n" - "inline operator fun com.google.protobuf.kotlin.DslList" + "public inline operator fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "plusAssign(values: kotlin.collections.Iterable<$kt_type$>) {\n" " addAll(values)\n" @@ -765,7 +765,7 @@ void RepeatedImmutablePrimitiveFieldLiteGenerator::GenerateKotlinDslMembers( variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"set$kt_capitalized_name$\")\n" - "operator fun com.google.protobuf.kotlin.DslList" + "public operator fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "set(index: kotlin.Int, value: $kt_type$) {\n" " $kt_dsl_builder$.${$set$capitalized_name$$}$(index, value)\n" @@ -776,7 +776,7 @@ void RepeatedImmutablePrimitiveFieldLiteGenerator::GenerateKotlinDslMembers( printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"clear$kt_capitalized_name$\")\n" - "fun com.google.protobuf.kotlin.DslList" + "public fun com.google.protobuf.kotlin.DslList" "<$kt_type$, ${$$kt_capitalized_name$Proxy$}$>." "clear() {\n" " $kt_dsl_builder$.${$clear$capitalized_name$$}$()\n" diff --git a/src/google/protobuf/compiler/java/string_field.cc b/src/google/protobuf/compiler/java/string_field.cc index d8b5d4ef6f..600e7f88a8 100644 --- a/src/google/protobuf/compiler/java/string_field.cc +++ b/src/google/protobuf/compiler/java/string_field.cc @@ -132,13 +132,6 @@ void SetPrimitiveVariables(const FieldDescriptor* descriptor, (*variables)["set_mutable_bit_builder"] = GenerateSetBit(builderBitIndex); (*variables)["clear_mutable_bit_builder"] = GenerateClearBit(builderBitIndex); - // For repeated fields, one bit is used for whether the array is immutable - // in the parsing constructor. - (*variables)["get_mutable_bit_parser"] = - GenerateGetBitMutableLocal(builderBitIndex); - (*variables)["set_mutable_bit_parser"] = - GenerateSetBitMutableLocal(builderBitIndex); - (*variables)["get_has_field_bit_from_local"] = GenerateGetBitFromLocal(builderBitIndex); (*variables)["set_has_field_bit_to_local"] = @@ -383,7 +376,7 @@ void ImmutableStringFieldGenerator::GenerateKotlinDslMembers( io::Printer* printer) const { WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print(variables_, - "$kt_deprecation$var $kt_name$: kotlin.String\n" + "$kt_deprecation$public var $kt_name$: kotlin.String\n" " @JvmName(\"${$get$kt_capitalized_name$$}$\")\n" " get() = $kt_dsl_builder$.${$get$capitalized_name$$}$()\n" " @JvmName(\"${$set$kt_capitalized_name$$}$\")\n" @@ -394,7 +387,7 @@ void ImmutableStringFieldGenerator::GenerateKotlinDslMembers( WriteFieldAccessorDocComment(printer, descriptor_, CLEARER, /* builder */ false, /* kdoc */ true); printer->Print(variables_, - "fun ${$clear$kt_capitalized_name$$}$() {\n" + "public fun ${$clear$kt_capitalized_name$$}$() {\n" " $kt_dsl_builder$.${$clear$capitalized_name$$}$()\n" "}\n"); @@ -403,7 +396,7 @@ void ImmutableStringFieldGenerator::GenerateKotlinDslMembers( /* builder */ false, /* kdoc */ true); printer->Print( variables_, - "fun ${$has$kt_capitalized_name$$}$(): kotlin.Boolean {\n" + "public fun ${$has$kt_capitalized_name$$}$(): kotlin.Boolean {\n" " return $kt_dsl_builder$.${$has$capitalized_name$$}$()\n" "}\n"); } @@ -457,26 +450,19 @@ void ImmutableStringFieldGenerator::GenerateBuildingCode( printer->Print(variables_, "result.$name$_ = $name$_;\n"); } -void ImmutableStringFieldGenerator::GenerateParsingCode( +void ImmutableStringFieldGenerator::GenerateBuilderParsingCode( io::Printer* printer) const { if (CheckUtf8(descriptor_)) { printer->Print(variables_, - "java.lang.String s = input.readStringRequireUtf8();\n" - "$set_has_field_bit_message$\n" - "$name$_ = s;\n"); + "$name$_ = input.readStringRequireUtf8();\n" + "$set_has_field_bit_builder$\n"); } else { printer->Print(variables_, - "com.google.protobuf.ByteString bs = input.readBytes();\n" - "$set_has_field_bit_message$\n" - "$name$_ = bs;\n"); + "$name$_ = input.readBytes();\n" + "$set_has_field_bit_builder$\n"); } } -void ImmutableStringFieldGenerator::GenerateParsingDoneCode( - io::Printer* printer) const { - // noop for strings. -} - void ImmutableStringFieldGenerator::GenerateSerializationCode( io::Printer* printer) const { printer->Print(variables_, @@ -700,6 +686,11 @@ void ImmutableStringOneofFieldGenerator::GenerateBuilderMembers( "}\n"); } +void ImmutableStringOneofFieldGenerator::GenerateBuilderClearCode( + io::Printer* printer) const { + // String fields in oneofs are correctly cleared by clearing the oneof +} + void ImmutableStringOneofFieldGenerator::GenerateMergingCode( io::Printer* printer) const { // Allow a slight breach of abstraction here in order to avoid forcing @@ -718,7 +709,7 @@ void ImmutableStringOneofFieldGenerator::GenerateBuildingCode( "}\n"); } -void ImmutableStringOneofFieldGenerator::GenerateParsingCode( +void ImmutableStringOneofFieldGenerator::GenerateBuilderParsingCode( io::Printer* printer) const { if (CheckUtf8(descriptor_)) { printer->Print(variables_, @@ -968,14 +959,14 @@ void RepeatedImmutableStringFieldGenerator::GenerateKotlinDslMembers( " */\n" "@kotlin.OptIn" "(com.google.protobuf.kotlin.OnlyForUseByGeneratedProtoCode::class)\n" - "class ${$$kt_capitalized_name$Proxy$}$ private constructor()" + "public class ${$$kt_capitalized_name$Proxy$}$ private constructor()" " : com.google.protobuf.kotlin.DslProxy()\n"); // property for List WriteFieldAccessorDocComment(printer, descriptor_, LIST_GETTER, /* builder */ false, /* kdoc */ true); printer->Print(variables_, - "$kt_deprecation$ val $kt_name$: " + "$kt_deprecation$public val $kt_name$: " "com.google.protobuf.kotlin.DslList" "\n" " @kotlin.jvm.JvmSynthetic\n" @@ -989,7 +980,7 @@ void RepeatedImmutableStringFieldGenerator::GenerateKotlinDslMembers( printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"add$kt_capitalized_name$\")\n" - "fun com.google.protobuf.kotlin.DslList" + "public fun com.google.protobuf.kotlin.DslList" "." "add(value: kotlin.String) {\n" " $kt_dsl_builder$.${$add$capitalized_name$$}$(value)\n" @@ -1002,7 +993,7 @@ void RepeatedImmutableStringFieldGenerator::GenerateKotlinDslMembers( "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"plusAssign$kt_capitalized_name$\")\n" "@Suppress(\"NOTHING_TO_INLINE\")\n" - "inline operator fun com.google.protobuf.kotlin.DslList" + "public inline operator fun com.google.protobuf.kotlin.DslList" "." "plusAssign(value: kotlin.String) {\n" " add(value)\n" @@ -1015,7 +1006,7 @@ void RepeatedImmutableStringFieldGenerator::GenerateKotlinDslMembers( variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"addAll$kt_capitalized_name$\")\n" - "fun com.google.protobuf.kotlin.DslList" + "public fun com.google.protobuf.kotlin.DslList" "." "addAll(values: kotlin.collections.Iterable) {\n" " $kt_dsl_builder$.${$addAll$capitalized_name$$}$(values)\n" @@ -1029,7 +1020,7 @@ void RepeatedImmutableStringFieldGenerator::GenerateKotlinDslMembers( "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"plusAssignAll$kt_capitalized_name$\")\n" "@Suppress(\"NOTHING_TO_INLINE\")\n" - "inline operator fun com.google.protobuf.kotlin.DslList" + "public inline operator fun com.google.protobuf.kotlin.DslList" "." "plusAssign(values: kotlin.collections.Iterable) {\n" " addAll(values)\n" @@ -1042,7 +1033,7 @@ void RepeatedImmutableStringFieldGenerator::GenerateKotlinDslMembers( variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"set$kt_capitalized_name$\")\n" - "operator fun com.google.protobuf.kotlin.DslList" + "public operator fun com.google.protobuf.kotlin.DslList" "." "set(index: kotlin.Int, value: kotlin.String) {\n" " $kt_dsl_builder$.${$set$capitalized_name$$}$(index, value)\n" @@ -1053,7 +1044,7 @@ void RepeatedImmutableStringFieldGenerator::GenerateKotlinDslMembers( printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"clear$kt_capitalized_name$\")\n" - "fun com.google.protobuf.kotlin.DslList" + "public fun com.google.protobuf.kotlin.DslList" "." "clear() {\n" " $kt_dsl_builder$.${$clear$capitalized_name$$}$()\n" @@ -1110,35 +1101,21 @@ void RepeatedImmutableStringFieldGenerator::GenerateBuildingCode( "result.$name$_ = $name$_;\n"); } -void RepeatedImmutableStringFieldGenerator::GenerateParsingCode( +void RepeatedImmutableStringFieldGenerator::GenerateBuilderParsingCode( io::Printer* printer) const { if (CheckUtf8(descriptor_)) { printer->Print(variables_, - "java.lang.String s = input.readStringRequireUtf8();\n"); + "java.lang.String s = input.readStringRequireUtf8();\n" + "ensure$capitalized_name$IsMutable();\n" + "$name$_.add(s);\n"); } else { printer->Print(variables_, - "com.google.protobuf.ByteString bs = input.readBytes();\n"); - } - printer->Print(variables_, - "if (!$get_mutable_bit_parser$) {\n" - " $name$_ = new com.google.protobuf.LazyStringArrayList();\n" - " $set_mutable_bit_parser$;\n" - "}\n"); - if (CheckUtf8(descriptor_)) { - printer->Print(variables_, "$name$_.add(s);\n"); - } else { - printer->Print(variables_, "$name$_.add(bs);\n"); + "com.google.protobuf.ByteString bs = input.readBytes();\n" + "ensure$capitalized_name$IsMutable();\n" + "$name$_.add(bs);\n"); } } -void RepeatedImmutableStringFieldGenerator::GenerateParsingDoneCode( - io::Printer* printer) const { - printer->Print(variables_, - "if ($get_mutable_bit_parser$) {\n" - " $name$_ = $name$_.getUnmodifiableView();\n" - "}\n"); -} - void RepeatedImmutableStringFieldGenerator::GenerateSerializationCode( io::Printer* printer) const { printer->Print(variables_, diff --git a/src/google/protobuf/compiler/java/string_field.h b/src/google/protobuf/compiler/java/string_field.h index b8bc921933..c9b6256d5d 100644 --- a/src/google/protobuf/compiler/java/string_field.h +++ b/src/google/protobuf/compiler/java/string_field.h @@ -78,8 +78,7 @@ class ImmutableStringFieldGenerator : public ImmutableFieldGenerator { void GenerateBuilderClearCode(io::Printer* printer) const override; void GenerateMergingCode(io::Printer* printer) const override; void GenerateBuildingCode(io::Printer* printer) const override; - void GenerateParsingCode(io::Printer* printer) const override; - void GenerateParsingDoneCode(io::Printer* printer) const override; + void GenerateBuilderParsingCode(io::Printer* printer) const override; void GenerateSerializationCode(io::Printer* printer) const override; void GenerateSerializedSizeCode(io::Printer* printer) const override; void GenerateFieldBuilderInitializationCode( @@ -111,9 +110,10 @@ class ImmutableStringOneofFieldGenerator private: void GenerateMembers(io::Printer* printer) const override; void GenerateBuilderMembers(io::Printer* printer) const override; + void GenerateBuilderClearCode(io::Printer* printer) const override; void GenerateMergingCode(io::Printer* printer) const override; void GenerateBuildingCode(io::Printer* printer) const override; - void GenerateParsingCode(io::Printer* printer) const override; + void GenerateBuilderParsingCode(io::Printer* printer) const override; void GenerateSerializationCode(io::Printer* printer) const override; void GenerateSerializedSizeCode(io::Printer* printer) const override; }; @@ -139,8 +139,7 @@ class RepeatedImmutableStringFieldGenerator : public ImmutableFieldGenerator { void GenerateBuilderClearCode(io::Printer* printer) const override; void GenerateMergingCode(io::Printer* printer) const override; void GenerateBuildingCode(io::Printer* printer) const override; - void GenerateParsingCode(io::Printer* printer) const override; - void GenerateParsingDoneCode(io::Printer* printer) const override; + void GenerateBuilderParsingCode(io::Printer* printer) const override; void GenerateSerializationCode(io::Printer* printer) const override; void GenerateSerializedSizeCode(io::Printer* printer) const override; void GenerateFieldBuilderInitializationCode( diff --git a/src/google/protobuf/compiler/java/string_field_lite.cc b/src/google/protobuf/compiler/java/string_field_lite.cc index 6dd1996503..13fb9296e6 100644 --- a/src/google/protobuf/compiler/java/string_field_lite.cc +++ b/src/google/protobuf/compiler/java/string_field_lite.cc @@ -348,7 +348,7 @@ void ImmutableStringFieldLiteGenerator::GenerateKotlinDslMembers( io::Printer* printer) const { WriteFieldDocComment(printer, descriptor_, /* kdoc */ true); printer->Print(variables_, - "$kt_deprecation$var $kt_name$: kotlin.String\n" + "$kt_deprecation$public var $kt_name$: kotlin.String\n" " @JvmName(\"${$get$kt_capitalized_name$$}$\")\n" " get() = $kt_dsl_builder$.${$get$capitalized_name$$}$()\n" " @JvmName(\"${$set$kt_capitalized_name$$}$\")\n" @@ -359,7 +359,7 @@ void ImmutableStringFieldLiteGenerator::GenerateKotlinDslMembers( WriteFieldAccessorDocComment(printer, descriptor_, CLEARER, /* builder */ false, /* kdoc */ true); printer->Print(variables_, - "fun ${$clear$kt_capitalized_name$$}$() {\n" + "public fun ${$clear$kt_capitalized_name$$}$() {\n" " $kt_dsl_builder$.${$clear$capitalized_name$$}$()\n" "}\n"); @@ -368,7 +368,7 @@ void ImmutableStringFieldLiteGenerator::GenerateKotlinDslMembers( /* builder */ false, /* kdoc */ true); printer->Print( variables_, - "fun ${$has$kt_capitalized_name$$}$(): kotlin.Boolean {\n" + "public fun ${$has$kt_capitalized_name$$}$(): kotlin.Boolean {\n" " return $kt_dsl_builder$.${$has$capitalized_name$$}$()\n" "}\n"); } @@ -798,7 +798,7 @@ void RepeatedImmutableStringFieldLiteGenerator::GenerateKotlinDslMembers( " */\n" "@kotlin.OptIn" "(com.google.protobuf.kotlin.OnlyForUseByGeneratedProtoCode::class)\n" - "class ${$$kt_capitalized_name$Proxy$}$ private constructor()" + "public class ${$$kt_capitalized_name$Proxy$}$ private constructor()" " : com.google.protobuf.kotlin.DslProxy()\n"); // property for List @@ -806,7 +806,7 @@ void RepeatedImmutableStringFieldLiteGenerator::GenerateKotlinDslMembers( /* builder */ false, /* kdoc */ true); printer->Print( variables_, - "$kt_deprecation$ val $kt_name$: " + "$kt_deprecation$public val $kt_name$: " "com.google.protobuf.kotlin.DslList" "\n" "@kotlin.OptIn" @@ -821,7 +821,7 @@ void RepeatedImmutableStringFieldLiteGenerator::GenerateKotlinDslMembers( printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"add$kt_capitalized_name$\")\n" - "fun com.google.protobuf.kotlin.DslList" + "public fun com.google.protobuf.kotlin.DslList" "." "add(value: kotlin.String) {\n" " $kt_dsl_builder$.${$add$capitalized_name$$}$(value)\n" @@ -834,7 +834,7 @@ void RepeatedImmutableStringFieldLiteGenerator::GenerateKotlinDslMembers( "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"plusAssign$kt_capitalized_name$\")\n" "@Suppress(\"NOTHING_TO_INLINE\")\n" - "inline operator fun com.google.protobuf.kotlin.DslList" + "public inline operator fun com.google.protobuf.kotlin.DslList" "." "plusAssign(value: kotlin.String) {\n" " add(value)\n" @@ -847,7 +847,7 @@ void RepeatedImmutableStringFieldLiteGenerator::GenerateKotlinDslMembers( variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"addAll$kt_capitalized_name$\")\n" - "fun com.google.protobuf.kotlin.DslList" + "public fun com.google.protobuf.kotlin.DslList" "." "addAll(values: kotlin.collections.Iterable) {\n" " $kt_dsl_builder$.${$addAll$capitalized_name$$}$(values)\n" @@ -861,7 +861,7 @@ void RepeatedImmutableStringFieldLiteGenerator::GenerateKotlinDslMembers( "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"plusAssignAll$kt_capitalized_name$\")\n" "@Suppress(\"NOTHING_TO_INLINE\")\n" - "inline operator fun com.google.protobuf.kotlin.DslList" + "public inline operator fun com.google.protobuf.kotlin.DslList" "." "plusAssign(values: kotlin.collections.Iterable) {\n" " addAll(values)\n" @@ -874,7 +874,7 @@ void RepeatedImmutableStringFieldLiteGenerator::GenerateKotlinDslMembers( variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"set$kt_capitalized_name$\")\n" - "operator fun com.google.protobuf.kotlin.DslList" + "public operator fun com.google.protobuf.kotlin.DslList" "." "set(index: kotlin.Int, value: kotlin.String) {\n" " $kt_dsl_builder$.${$set$capitalized_name$$}$(index, value)\n" @@ -885,7 +885,7 @@ void RepeatedImmutableStringFieldLiteGenerator::GenerateKotlinDslMembers( printer->Print(variables_, "@kotlin.jvm.JvmSynthetic\n" "@kotlin.jvm.JvmName(\"clear$kt_capitalized_name$\")\n" - "fun com.google.protobuf.kotlin.DslList" + "public fun com.google.protobuf.kotlin.DslList" "." "clear() {\n" " $kt_dsl_builder$.${$clear$capitalized_name$$}$()\n" diff --git a/src/google/protobuf/compiler/parser.cc b/src/google/protobuf/compiler/parser.cc index 4cae1a1187..5a835a0060 100644 --- a/src/google/protobuf/compiler/parser.cc +++ b/src/google/protobuf/compiler/parser.cc @@ -51,6 +51,7 @@ #include "absl/strings/ascii.h" #include "absl/strings/escaping.h" #include "absl/strings/str_cat.h" +#include "absl/strings/str_format.h" #include "google/protobuf/descriptor.h" #include "google/protobuf/descriptor.pb.h" #include "google/protobuf/io/tokenizer.h" @@ -288,6 +289,15 @@ bool Parser::ConsumeInteger64(uint64_t max_value, uint64_t* output, } } +bool Parser::TryConsumeInteger64(uint64_t max_value, uint64_t* output) { + if (LookingAtType(io::Tokenizer::TYPE_INTEGER) && + io::Tokenizer::ParseInteger(input_->current().text, max_value, output)) { + input_->Next(); + return true; + } + return false; +} + bool Parser::ConsumeNumber(double* output, const char* error) { if (LookingAtType(io::Tokenizer::TYPE_FLOAT)) { *output = io::Tokenizer::ParseFloat(input_->current().text); @@ -296,13 +306,19 @@ bool Parser::ConsumeNumber(double* output, const char* error) { } else if (LookingAtType(io::Tokenizer::TYPE_INTEGER)) { // Also accept integers. uint64_t value = 0; - if (!io::Tokenizer::ParseInteger(input_->current().text, - std::numeric_limits::max(), - &value)) { + if (io::Tokenizer::ParseInteger(input_->current().text, + std::numeric_limits::max(), + &value)) { + *output = value; + } else if (input_->current().text[0] == '0') { + // octal or hexadecimal; don't bother parsing as float + AddError("Integer out of range."); + // We still return true because we did, in fact, parse a number. + } else if (!io::Tokenizer::TryParseFloat(input_->current().text, output)) { + // out of int range, and not valid float? 🤷 AddError("Integer out of range."); // We still return true because we did, in fact, parse a number. } - *output = value; input_->Next(); return true; } else if (LookingAt("inf")) { @@ -389,13 +405,16 @@ void Parser::AddError(const std::string& error) { AddError(input_->current().line, input_->current().column, error); } -void Parser::AddWarning(const std::string& warning) { +void Parser::AddWarning(int line, int column, const std::string& warning) { if (error_collector_ != nullptr) { - error_collector_->AddWarning(input_->current().line, - input_->current().column, warning); + error_collector_->AddWarning(line, column, warning); } } +void Parser::AddWarning(const std::string& warning) { + AddWarning(input_->current().line, input_->current().column, warning); +} + // ------------------------------------------------------------------- Parser::LocationRecorder::LocationRecorder(Parser* parser) @@ -410,6 +429,11 @@ Parser::LocationRecorder::LocationRecorder(const LocationRecorder& parent) { Init(parent, parent.source_code_info_); } +Parser::LocationRecorder::LocationRecorder(const LocationRecorder& parent, + SourceCodeInfo* source_code_info) { + Init(parent, source_code_info); +} + Parser::LocationRecorder::LocationRecorder(const LocationRecorder& parent, int path1, SourceCodeInfo* source_code_info) { @@ -1239,13 +1263,22 @@ bool Parser::ParseDefaultAssignment( field->clear_default_value(); } + LocationRecorder location(field_location, + FieldDescriptorProto::kDefaultValueFieldNumber); + DO(Consume("default")); DO(Consume("=")); - LocationRecorder location(field_location, - FieldDescriptorProto::kDefaultValueFieldNumber); - location.RecordLegacyLocation(field, - DescriptorPool::ErrorCollector::DEFAULT_VALUE); + // We don't need to create separate spans in source code info for name and + // value, since there's no way to represent them distinctly in a location + // path. But we will want a separate recorder for the value, just to have more + // precise location info in error messages. So we let it create a location in + // no_op, so it doesn't add a span to the file descriptor. + SourceCodeInfo no_op; + LocationRecorder value_location(location, &no_op); + value_location.RecordLegacyLocation( + field, DescriptorPool::ErrorCollector::DEFAULT_VALUE); + std::string* default_value = field->mutable_default_value(); if (!field->has_type()) { @@ -1316,18 +1349,19 @@ bool Parser::ParseDefaultAssignment( } case FieldDescriptorProto::TYPE_FLOAT: - case FieldDescriptorProto::TYPE_DOUBLE: + case FieldDescriptorProto::TYPE_DOUBLE: { // These types can be negative. if (TryConsume("-")) { default_value->append("-"); } // Parse the integer because we have to convert hex integers to decimal // floats. - double value; + double value = 0; DO(ConsumeNumber(&value, "Expected number.")); // And stringify it again. default_value->append(SimpleDtoa(value)); break; + } case FieldDescriptorProto::TYPE_BOOL: if (TryConsume("true")) { @@ -1379,13 +1413,23 @@ bool Parser::ParseJsonName(FieldDescriptorProto* field, LocationRecorder location(field_location, FieldDescriptorProto::kJsonNameFieldNumber); - location.RecordLegacyLocation(field, - DescriptorPool::ErrorCollector::OPTION_NAME); - DO(Consume("json_name")); + // We don't need to create separate spans in source code info for name and + // value, since there's no way to represent them distinctly in a location + // path. But we will want a separate recorder for them, just to have more + // precise location info in error messages. So we let them create a location + // in no_op, so they don't add a span to the file descriptor. + SourceCodeInfo no_op; + { + LocationRecorder name_location(location, &no_op); + name_location.RecordLegacyLocation( + field, DescriptorPool::ErrorCollector::OPTION_NAME); + + DO(Consume("json_name")); + } DO(Consume("=")); - LocationRecorder value_location(location); + LocationRecorder value_location(location, &no_op); value_location.RecordLegacyLocation( field, DescriptorPool::ErrorCollector::OPTION_VALUE); @@ -1551,18 +1595,22 @@ bool Parser::ParseOption(Message* options, is_negative ? static_cast(std::numeric_limits::max()) + 1 : std::numeric_limits::max(); - DO(ConsumeInteger64(max_value, &value, "Expected integer.")); - if (is_negative) { - value_location.AddPath( - UninterpretedOption::kNegativeIntValueFieldNumber); - uninterpreted_option->set_negative_int_value( - static_cast(0 - value)); - } else { - value_location.AddPath( - UninterpretedOption::kPositiveIntValueFieldNumber); - uninterpreted_option->set_positive_int_value(value); + if (TryConsumeInteger64(max_value, &value)) { + if (is_negative) { + value_location.AddPath( + UninterpretedOption::kNegativeIntValueFieldNumber); + uninterpreted_option->set_negative_int_value( + static_cast(0 - value)); + } else { + value_location.AddPath( + UninterpretedOption::kPositiveIntValueFieldNumber); + uninterpreted_option->set_positive_int_value(value); + } + break; } - break; + // value too large for an integer; fall through below to treat as + // floating point + ABSL_FALLTHROUGH_INTENDED; } case io::Tokenizer::TYPE_FLOAT: { @@ -1728,11 +1776,25 @@ bool Parser::ParseReserved(DescriptorProto* message, } } +bool Parser::ParseReservedName(std::string* name, const char* error_message) { + // Capture the position of the token, in case we have to report an + // error after it is consumed. + int line = input_->current().line; + int col = input_->current().column; + DO(ConsumeString(name, error_message)); + if (!io::Tokenizer::IsIdentifier(*name)) { + AddWarning(line, col, + absl::StrFormat( + "Reserved name \"%s\" is not a valid identifier.", *name)); + } + return true; +} + bool Parser::ParseReservedNames(DescriptorProto* message, const LocationRecorder& parent_location) { do { LocationRecorder location(parent_location, message->reserved_name_size()); - DO(ConsumeString(message->add_reserved_name(), "Expected field name.")); + DO(ParseReservedName(message->add_reserved_name(), "Expected field name.")); } while (TryConsume(",")); DO(ConsumeEndOfDeclaration(";", &parent_location)); return true; @@ -1787,42 +1849,41 @@ bool Parser::ParseReservedNumbers(DescriptorProto* message, return true; } -bool Parser::ParseReserved(EnumDescriptorProto* message, - const LocationRecorder& message_location) { +bool Parser::ParseReserved(EnumDescriptorProto* proto, + const LocationRecorder& enum_location) { io::Tokenizer::Token start_token = input_->current(); // Parse the declaration. DO(Consume("reserved")); if (LookingAtType(io::Tokenizer::TYPE_STRING)) { - LocationRecorder location(message_location, + LocationRecorder location(enum_location, EnumDescriptorProto::kReservedNameFieldNumber); location.StartAt(start_token); - return ParseReservedNames(message, location); + return ParseReservedNames(proto, location); } else { - LocationRecorder location(message_location, + LocationRecorder location(enum_location, EnumDescriptorProto::kReservedRangeFieldNumber); location.StartAt(start_token); - return ParseReservedNumbers(message, location); + return ParseReservedNumbers(proto, location); } } -bool Parser::ParseReservedNames(EnumDescriptorProto* message, +bool Parser::ParseReservedNames(EnumDescriptorProto* proto, const LocationRecorder& parent_location) { do { - LocationRecorder location(parent_location, message->reserved_name_size()); - DO(ConsumeString(message->add_reserved_name(), "Expected enum value.")); + LocationRecorder location(parent_location, proto->reserved_name_size()); + DO(ParseReservedName(proto->add_reserved_name(), "Expected enum value.")); } while (TryConsume(",")); DO(ConsumeEndOfDeclaration(";", &parent_location)); return true; } -bool Parser::ParseReservedNumbers(EnumDescriptorProto* message, +bool Parser::ParseReservedNumbers(EnumDescriptorProto* proto, const LocationRecorder& parent_location) { bool first = true; do { - LocationRecorder location(parent_location, message->reserved_range_size()); + LocationRecorder location(parent_location, proto->reserved_range_size()); - EnumDescriptorProto::EnumReservedRange* range = - message->add_reserved_range(); + EnumDescriptorProto::EnumReservedRange* range = proto->add_reserved_range(); int start, end; io::Tokenizer::Token start_token; { diff --git a/src/google/protobuf/compiler/parser.h b/src/google/protobuf/compiler/parser.h index ccd3e5a5f7..65baf476b2 100644 --- a/src/google/protobuf/compiler/parser.h +++ b/src/google/protobuf/compiler/parser.h @@ -180,6 +180,9 @@ class PROTOBUF_EXPORT Parser { // is greater than max_value, an error will be reported. bool ConsumeInteger64(uint64_t max_value, uint64_t* output, const char* error); + // Try to consume a 64-bit integer and store its value in "output". No + // error is reported on failure, allowing caller to consume token another way. + bool TryConsumeInteger64(uint64_t max_value, uint64_t* output); // Consume a number and store its value in "output". This will accept // tokens of either INTEGER or FLOAT type. bool ConsumeNumber(double* output, const char* error); @@ -213,6 +216,9 @@ class PROTOBUF_EXPORT Parser { // of the current token. void AddError(const std::string& error); + // Invokes error_collector_->AddWarning(), if error_collector_ is not NULL. + void AddWarning(int line, int column, const std::string& warning); + // Invokes error_collector_->AddWarning() with the line and column number // of the current token. void AddWarning(const std::string& warning); @@ -239,6 +245,10 @@ class PROTOBUF_EXPORT Parser { LocationRecorder(const LocationRecorder& parent, int path1, int path2); // Creates a recorder that generates locations into given source code info. + LocationRecorder(const LocationRecorder& parent, + SourceCodeInfo* source_code_info); + // Creates a recorder that generates locations into given source code info + // and calls AddPath() one time. LocationRecorder(const LocationRecorder& parent, int path1, SourceCodeInfo* source_code_info); @@ -394,6 +404,7 @@ class PROTOBUF_EXPORT Parser { const LocationRecorder& message_location); bool ParseReservedNames(DescriptorProto* message, const LocationRecorder& parent_location); + bool ParseReservedName(std::string* name, const char* error_message); bool ParseReservedNumbers(DescriptorProto* message, const LocationRecorder& parent_location); bool ParseReserved(EnumDescriptorProto* message, diff --git a/src/google/protobuf/compiler/parser_unittest.cc b/src/google/protobuf/compiler/parser_unittest.cc index 3b32451916..3bc620f973 100644 --- a/src/google/protobuf/compiler/parser_unittest.cc +++ b/src/google/protobuf/compiler/parser_unittest.cc @@ -147,6 +147,16 @@ class ParserTest : public testing::Test { EXPECT_EQ(io::Tokenizer::TYPE_END, input_->current().type); } + // Parse the text and expect that the given warnings are reported. + void ExpectHasWarnings(const char* text, const char* expected_warnings) { + SetupParser(text); + FileDescriptorProto file; + ASSERT_TRUE(parser_->Parse(input_.get(), &file)); + EXPECT_EQ(io::Tokenizer::TYPE_END, input_->current().type); + ASSERT_EQ("", error_collector_.text_); + EXPECT_EQ(expected_warnings, error_collector_.warning_); + } + // Same as above but does not expect that the parser parses the complete // input. void ExpectHasEarlyExitErrors(const char* text, const char* expected_errors) { @@ -592,6 +602,56 @@ TEST_F(ParseMessageTest, FieldOptions) { "}"); } +TEST_F(ParseMessageTest, FieldOptionsSupportLargeDecimalLiteral) { + // decimal integer literal > uint64 max + ExpectParsesTo( + "import \"google/protobuf/descriptor.proto\";\n" + "extend google.protobuf.FieldOptions {\n" + " optional double f = 10101;\n" + "}\n" + "message TestMessage {\n" + " optional double a = 1 [default = 18446744073709551616];\n" + " optional double b = 2 [default = -18446744073709551616];\n" + " optional double c = 3 [(f) = 18446744073709551616];\n" + " optional double d = 4 [(f) = -18446744073709551616];\n" + "}\n", + + "dependency: \"google/protobuf/descriptor.proto\"" + "extension {" + " name: \"f\" label: LABEL_OPTIONAL type: TYPE_DOUBLE number: 10101" + " extendee: \"google.protobuf.FieldOptions\"" + "}" + "message_type {" + " name: \"TestMessage\"" + " field {" + " name: \"a\" label: LABEL_OPTIONAL type: TYPE_DOUBLE number: 1" + " default_value: \"1.8446744073709552e+19\"" + " }" + " field {" + " name: \"b\" label: LABEL_OPTIONAL type: TYPE_DOUBLE number: 2" + " default_value: \"-1.8446744073709552e+19\"" + " }" + " field {" + " name: \"c\" label: LABEL_OPTIONAL type: TYPE_DOUBLE number: 3" + " options{" + " uninterpreted_option{" + " name{ name_part: \"f\" is_extension: true }" + " double_value: 1.8446744073709552e+19" + " }" + " }" + " }" + " field {" + " name: \"d\" label: LABEL_OPTIONAL type: TYPE_DOUBLE number: 4" + " options{" + " uninterpreted_option{" + " name{ name_part: \"f\" is_extension: true }" + " double_value: -1.8446744073709552e+19" + " }" + " }" + " }" + "}"); +} + TEST_F(ParseMessageTest, Oneof) { ExpectParsesTo( "message TestMessage {\n" @@ -1675,6 +1735,15 @@ TEST_F(ParseErrorTest, EnumReservedMissingQuotes) { "2:11: Expected enum value or number range.\n"); } +TEST_F(ParseErrorTest, EnumReservedInvalidIdentifier) { + ExpectHasWarnings( + "enum TestEnum {\n" + " FOO = 1;\n" + " reserved \"foo bar\";\n" + "}\n", + "2:11: Reserved name \"foo bar\" is not a valid identifier.\n"); +} + // ------------------------------------------------------------------- // Reserved field number errors @@ -1702,6 +1771,14 @@ TEST_F(ParseErrorTest, ReservedMissingQuotes) { "1:11: Expected field name or number range.\n"); } +TEST_F(ParseErrorTest, ReservedInvalidIdentifier) { + ExpectHasWarnings( + "enum TestEnum {\n" + " reserved \"foo bar\";\n" + "}\n", + "1:11: Reserved name \"foo bar\" is not a valid identifier.\n"); +} + TEST_F(ParseErrorTest, ReservedNegativeNumber) { ExpectHasErrors( "message Foo {\n" @@ -1891,6 +1968,22 @@ TEST_F(ParserValidationErrorTest, FieldDefaultValueError) { "2:32: Enum type \"Baz\" has no value named \"NO_SUCH_VALUE\".\n"); } +TEST_F(ParserValidationErrorTest, FieldDefaultIntegerOutOfRange) { + ExpectHasErrors( + "message Foo {\n" + " optional double bar = 1 [default = 0x10000000000000000];\n" + "}\n", + "1:37: Integer out of range.\n"); +} + +TEST_F(ParserValidationErrorTest, FieldOptionOutOfRange) { + ExpectHasErrors( + "message Foo {\n" + " optional double bar = 1 [foo = 0x10000000000000000];\n" + "}\n", + "1:33: Integer out of range.\n"); +} + TEST_F(ParserValidationErrorTest, FileOptionNameError) { ExpectHasValidationErrors( "option foo = 5;", @@ -1977,8 +2070,117 @@ TEST_F(ParserValidationErrorTest, Proto3JsonConflictError) { " uint32 foo = 1;\n" " uint32 Foo = 2;\n" "}\n", - "3:9: The JSON camel-case name of field \"Foo\" conflicts with field " - "\"foo\". This is not allowed in proto3.\n"); + "3:9: The default JSON name of field \"Foo\" (\"Foo\") conflicts " + "with the default JSON name of field \"foo\" (\"foo\"). " + "This is not allowed in proto3.\n"); +} + +TEST_F(ParserValidationErrorTest, Proto2JsonConflictError) { + // conflicts with default JSON names are not errors in proto2 + ExpectParsesTo( + R"pb( + syntax = "proto2" + ; + message TestMessage { + optional uint32 foo = 1; + optional uint32 Foo = 2; + } + )pb", + R"pb( + syntax: 'proto2' + message_type { + name: 'TestMessage' + field { + label: LABEL_OPTIONAL + type: TYPE_UINT32 + name: 'foo' + number: 1 + } + field { + label: LABEL_OPTIONAL + type: TYPE_UINT32 + name: 'Foo' + number: 2 + } + } + )pb"); +} + +TEST_F(ParserValidationErrorTest, Proto2CustomJsonConflictWithDefaultError) { + // conflicts with default JSON names are not errors in proto2 + ExpectParsesTo( + R"pb( + syntax = 'proto2' + ; + message TestMessage { + optional uint32 foo = 1 [json_name='bar']; + optional uint32 bar = 2; + } + )pb", + R"pb( + syntax: 'proto2' + message_type { + name: 'TestMessage' + field { + label: LABEL_OPTIONAL + type: TYPE_UINT32 + name: 'foo' + number: 1 + json_name: 'bar' + } + field { + label: LABEL_OPTIONAL + type: TYPE_UINT32 + name: 'bar' + number: 2 + } + } + )pb"); +} + +// TODO(b/248626372) Re-enable these in google3. +TEST_F(ParserValidationErrorTest, Proto3CustomJsonConflictWithDefaultError) { + ExpectHasValidationErrors( + R"pb( + syntax = 'proto3' + ; + message TestMessage { + uint32 foo = 1 [json_name='bar']; + uint32 bar = 2; + } + )pb", + "5:15: The default JSON name of field \"bar\" (\"bar\") conflicts " + "with the custom JSON name of field \"foo\". " + "This is not allowed in proto3.\n"); +} + +TEST_F(ParserValidationErrorTest, Proto3CustomJsonConflictError) { + ExpectHasValidationErrors( + R"pb( + syntax = 'proto3' + ; + message TestMessage { + uint32 foo = 1 [json_name='baz']; + uint32 bar = 2 [json_name='baz']; + } + )pb", + "5:15: The custom JSON name of field \"bar\" (\"baz\") conflicts " + "with the custom JSON name of field \"foo\".\n"); +} + +TEST_F(ParserValidationErrorTest, Proto2CustomJsonConflictError) { + ExpectHasValidationErrors( + R"pb( + syntax = 'proto2' + ; + message TestMessage { + optional uint32 foo = 1 [json_name='baz']; + optional uint32 bar = 2 [json_name='baz']; + } + )pb", + // fails in proto2 also: can't explicitly configure bad custom JSON names + "5:24: The custom JSON name of field \"bar\" (\"baz\") conflicts " + "with the custom JSON name of field \"foo\".\n"); } TEST_F(ParserValidationErrorTest, EnumNameError) { @@ -3415,18 +3617,19 @@ TEST_F(SourceInfoTest, FieldOptions) { EXPECT_TRUE( Parse("message Foo {" " optional int32 bar = 1 " - "$a$[default=$b$123$c$,$d$opt1=123$e$," - "$f$opt2='hi'$g$]$h$;" + "$a$[$b$default=123$c$, $d$opt1=123$e$, " + "$f$opt2='hi'$g$, $h$json_name='barBar'$i$]$j$;" "}\n")); const FieldDescriptorProto& field = file_.message_type(0).field(0); const UninterpretedOption& option1 = field.options().uninterpreted_option(0); const UninterpretedOption& option2 = field.options().uninterpreted_option(1); - EXPECT_TRUE(HasSpan('a', 'h', field.options())); + EXPECT_TRUE(HasSpan('a', 'j', field.options())); EXPECT_TRUE(HasSpan('b', 'c', field, "default_value")); EXPECT_TRUE(HasSpan('d', 'e', option1)); EXPECT_TRUE(HasSpan('f', 'g', option2)); + EXPECT_TRUE(HasSpan('h', 'i', field, "json_name")); // Ignore these. EXPECT_TRUE(HasSpan(file_)); diff --git a/src/google/protobuf/compiler/plugin.pb.h b/src/google/protobuf/compiler/plugin.pb.h index b4489e5f08..5abaad9dd3 100644 --- a/src/google/protobuf/compiler/plugin.pb.h +++ b/src/google/protobuf/compiler/plugin.pb.h @@ -15,7 +15,7 @@ #error "your headers." #endif // PROTOBUF_VERSION -#if 3021005 < PROTOBUF_MIN_PROTOC_VERSION +#if 3021006 < PROTOBUF_MIN_PROTOC_VERSION #error "This file was generated by an older version of protoc which is" #error "incompatible with your Protocol Buffer headers. Please" #error "regenerate this file with a newer version of protoc." diff --git a/src/google/protobuf/descriptor.cc b/src/google/protobuf/descriptor.cc index c995cb263c..36892913df 100644 --- a/src/google/protobuf/descriptor.cc +++ b/src/google/protobuf/descriptor.cc @@ -3855,8 +3855,6 @@ class DescriptorBuilder { internal::FlatAllocator& alloc); void BuildOneof(const OneofDescriptorProto& proto, Descriptor* parent, OneofDescriptor* result, internal::FlatAllocator& alloc); - void CheckEnumValueUniqueness(const EnumDescriptorProto& proto, - const EnumDescriptor* result); void BuildEnum(const EnumDescriptorProto& proto, const Descriptor* parent, EnumDescriptor* result, internal::FlatAllocator& alloc); void BuildEnumValue(const EnumValueDescriptorProto& proto, @@ -3868,6 +3866,15 @@ class DescriptorBuilder { const ServiceDescriptor* parent, MethodDescriptor* result, internal::FlatAllocator& alloc); + void CheckFieldJsonNameUniqueness(const DescriptorProto& proto, + const Descriptor* result); + void CheckFieldJsonNameUniqueness(const std::string& message_name, + const DescriptorProto& message, + FileDescriptor::Syntax syntax, + bool use_custom_names); + void CheckEnumValueUniqueness(const EnumDescriptorProto& proto, + const EnumDescriptor* result); + void LogUnusedDependency(const FileDescriptorProto& proto, const FileDescriptor* result); @@ -5423,7 +5430,11 @@ void DescriptorBuilder::BuildMessage(const DescriptorProto& proto, } } + // TODO(b/248626372) Make this consistent with OSS behavior. + CheckFieldJsonNameUniqueness(proto, result); + // Check that fields aren't using reserved names or numbers and that they + // aren't using extension numbers. for (int i = 0; i < result->field_count(); i++) { const FieldDescriptor* field = result->field(i); for (int j = 0; j < result->extension_range_count(); j++) { @@ -5488,6 +5499,83 @@ void DescriptorBuilder::BuildMessage(const DescriptorProto& proto, } } +void DescriptorBuilder::CheckFieldJsonNameUniqueness( + const DescriptorProto& proto, const Descriptor* result) { + FileDescriptor::Syntax syntax = result->file()->syntax(); + std::string message_name = result->full_name(); + // two passes: one looking only at default JSON names, and one that considers + // custom JSON names + CheckFieldJsonNameUniqueness(message_name, proto, syntax, false); + CheckFieldJsonNameUniqueness(message_name, proto, syntax, true); +} + +namespace { +// Helpers for function below + +struct JsonNameDetails { + const FieldDescriptorProto* field; + std::string orig_name; + bool is_custom; +}; + +JsonNameDetails GetJsonNameDetails(const FieldDescriptorProto* field, + bool use_custom) { + if (use_custom && field->has_json_name()) { + return {field, field->json_name(), true}; + } + return {field, ToJsonName(field->name()), false}; +} + +} // namespace + +void DescriptorBuilder::CheckFieldJsonNameUniqueness( + const std::string& message_name, const DescriptorProto& message, + FileDescriptor::Syntax syntax, bool use_custom_names) { + absl::flat_hash_map name_to_field; + for (const FieldDescriptorProto& field : message.field()) { + JsonNameDetails details = GetJsonNameDetails(&field, use_custom_names); + std::string lowercase_name = absl::AsciiStrToLower(details.orig_name); + auto existing = name_to_field.find(lowercase_name); + if (existing == name_to_field.end()) { + name_to_field[lowercase_name] = details; + continue; + } + JsonNameDetails& match = existing->second; + if (use_custom_names && !details.is_custom && !match.is_custom) { + // if this pass is considering custom JSON names, but neither of the + // names involved in the conflict are custom, don't bother with a + // message. That will have been reported from other pass (non-custom + // JSON names). + continue; + } + absl::string_view this_type = details.is_custom ? "custom" : "default"; + absl::string_view existing_type = match.is_custom ? "custom" : "default"; + // If the matched name differs (which it can only differ in case), include + // it in the error message, for maximum clarity to user. + std::string name_suffix; + if (details.orig_name != match.orig_name) { + name_suffix = absl::StrCat(" (\"", match.orig_name, "\")"); + } + std::string error_message = absl::StrFormat( + "The %s JSON name of field \"%s\" (\"%s\") conflicts " + "with the %s JSON name of field \"%s\"%s.", + this_type, field.name(), details.orig_name, existing_type, + match.field->name(), name_suffix); + + bool involves_default = !details.is_custom || !match.is_custom; + if (syntax == FileDescriptor::SYNTAX_PROTO2 && involves_default) { + AddWarning(message_name, field, DescriptorPool::ErrorCollector::NAME, + error_message); + } else { + if (involves_default) { + absl::StrAppend(&error_message, " This is not allowed in proto3."); + } + AddError(message_name, field, DescriptorPool::ErrorCollector::NAME, + error_message); + } + } +} + void DescriptorBuilder::BuildFieldOrExtension(const FieldDescriptorProto& proto, Descriptor* parent, FieldDescriptor* result, @@ -5891,13 +5979,12 @@ void DescriptorBuilder::CheckEnumValueUniqueness( // NAME_TYPE_LAST_NAME = 2, // } PrefixRemover remover(result->name()); - std::map values; + absl::flat_hash_map values; for (int i = 0; i < result->value_count(); i++) { const EnumValueDescriptor* value = result->value(i); std::string stripped = EnumValueToPascalCase(remover.MaybeRemove(value->name())); - std::pair::iterator, bool> - insert_result = values.insert(std::make_pair(stripped, value)); + auto insert_result = values.insert(std::make_pair(stripped, value)); bool inserted = insert_result.second; // We don't throw the error if the two conflicting symbols are identical, or @@ -5908,19 +5995,18 @@ void DescriptorBuilder::CheckEnumValueUniqueness( // stripping should de-dup the labels in this case). if (!inserted && insert_result.first->second->name() != value->name() && insert_result.first->second->number() != value->number()) { - std::string error_message = - "Enum name " + value->name() + " has the same name as " + - values[stripped]->name() + - " if you ignore case and strip out the enum name prefix (if any). " - "This is error-prone and can lead to undefined behavior. " - "Please avoid doing this. If you are using allow_alias, please " - "assign the same numeric value to both enums."; + std::string error_message = absl::StrFormat( + "Enum name %s has the same name as %s if you ignore case and strip " + "out the enum name prefix (if any). (If you are using allow_alias, " + "please assign the same numeric value to both enums.)", + value->name(), values[stripped]->name()); // There are proto2 enums out there with conflicting names, so to preserve // compatibility we issue only a warning for proto2. if (result->file()->syntax() == FileDescriptor::SYNTAX_PROTO2) { AddWarning(value->full_name(), proto.value(i), DescriptorPool::ErrorCollector::NAME, error_message); } else { + absl::StrAppend(&error_message, " This is not allowed in proto3."); AddError(value->full_name(), proto.value(i), DescriptorPool::ErrorCollector::NAME, error_message); } @@ -6787,19 +6873,6 @@ void DescriptorBuilder::ValidateProto3(FileDescriptor* file, } } -static std::string ToLowercaseWithoutUnderscores(const std::string& name) { - std::string result; - for (char character : name) { - if (character != '_') { - if (character >= 'A' && character <= 'Z') { - result.push_back(character - 'A' + 'a'); - } else { - result.push_back(character); - } - } - } - return result; -} void DescriptorBuilder::ValidateProto3Message(Descriptor* message, const DescriptorProto& proto) { @@ -6826,24 +6899,6 @@ void DescriptorBuilder::ValidateProto3Message(Descriptor* message, "MessageSet is not supported in proto3."); } - // In proto3, we reject field names if they conflict in camelCase. - // Note that we currently enforce a stricter rule: Field names must be - // unique after being converted to lowercase with underscores removed. - std::map name_to_field; - for (int i = 0; i < message->field_count(); ++i) { - std::string lowercase_name = - ToLowercaseWithoutUnderscores(message->field(i)->name()); - if (name_to_field.find(lowercase_name) != name_to_field.end()) { - AddError(message->full_name(), proto.field(i), - DescriptorPool::ErrorCollector::NAME, - "The JSON camel-case name of field \"" + - message->field(i)->name() + "\" conflicts with field \"" + - name_to_field[lowercase_name]->name() + "\". This is not " + - "allowed in proto3."); - } else { - name_to_field[lowercase_name] = message->field(i); - } - } } void DescriptorBuilder::ValidateProto3Field(FieldDescriptor* field, @@ -7683,6 +7738,28 @@ bool DescriptorBuilder::OptionInterpreter::ExamineIfOptionIsSet( return true; } +namespace { +// Helpers for method below + +template +std::string ValueOutOfRange(absl::string_view type_name, + absl::string_view option_name) { + return absl::StrFormat("Value out of range, %d to %d, for %s option \"%s\".", + std::numeric_limits::min(), + std::numeric_limits::max(), type_name, option_name); +} + +template +std::string ValueMustBeInt(absl::string_view type_name, + absl::string_view option_name) { + return absl::StrFormat( + "Value must be integer, from %d to %d, for %s option \"%s\".", + std::numeric_limits::min(), std::numeric_limits::max(), type_name, + option_name); +} + +} // namespace + bool DescriptorBuilder::OptionInterpreter::SetOptionValue( const FieldDescriptor* option_field, UnknownFieldSet* unknown_fields) { // We switch on the CppType to validate. @@ -7691,8 +7768,8 @@ bool DescriptorBuilder::OptionInterpreter::SetOptionValue( if (uninterpreted_option_->has_positive_int_value()) { if (uninterpreted_option_->positive_int_value() > static_cast(std::numeric_limits::max())) { - return AddValueError("Value out of range for int32 option \"" + - option_field->full_name() + "\"."); + return AddValueError( + ValueOutOfRange("int32", option_field->full_name())); } else { SetInt32(option_field->number(), uninterpreted_option_->positive_int_value(), @@ -7701,16 +7778,16 @@ bool DescriptorBuilder::OptionInterpreter::SetOptionValue( } else if (uninterpreted_option_->has_negative_int_value()) { if (uninterpreted_option_->negative_int_value() < static_cast(std::numeric_limits::min())) { - return AddValueError("Value out of range for int32 option \"" + - option_field->full_name() + "\"."); + return AddValueError( + ValueOutOfRange("int32", option_field->full_name())); } else { SetInt32(option_field->number(), uninterpreted_option_->negative_int_value(), option_field->type(), unknown_fields); } } else { - return AddValueError("Value must be integer for int32 option \"" + - option_field->full_name() + "\"."); + return AddValueError( + ValueMustBeInt("int32", option_field->full_name())); } break; @@ -7718,8 +7795,8 @@ bool DescriptorBuilder::OptionInterpreter::SetOptionValue( if (uninterpreted_option_->has_positive_int_value()) { if (uninterpreted_option_->positive_int_value() > static_cast(std::numeric_limits::max())) { - return AddValueError("Value out of range for int64 option \"" + - option_field->full_name() + "\"."); + return AddValueError( + ValueOutOfRange("int64", option_field->full_name())); } else { SetInt64(option_field->number(), uninterpreted_option_->positive_int_value(), @@ -7730,8 +7807,8 @@ bool DescriptorBuilder::OptionInterpreter::SetOptionValue( uninterpreted_option_->negative_int_value(), option_field->type(), unknown_fields); } else { - return AddValueError("Value must be integer for int64 option \"" + - option_field->full_name() + "\"."); + return AddValueError( + ValueMustBeInt("int64", option_field->full_name())); } break; @@ -7739,8 +7816,8 @@ bool DescriptorBuilder::OptionInterpreter::SetOptionValue( if (uninterpreted_option_->has_positive_int_value()) { if (uninterpreted_option_->positive_int_value() > std::numeric_limits::max()) { - return AddValueError("Value out of range for uint32 option \"" + - option_field->name() + "\"."); + return AddValueError( + ValueOutOfRange("uint32", option_field->full_name())); } else { SetUInt32(option_field->number(), uninterpreted_option_->positive_int_value(), @@ -7748,9 +7825,7 @@ bool DescriptorBuilder::OptionInterpreter::SetOptionValue( } } else { return AddValueError( - "Value must be non-negative integer for uint32 " - "option \"" + - option_field->full_name() + "\"."); + ValueMustBeInt("uint32", option_field->full_name())); } break; @@ -7761,9 +7836,7 @@ bool DescriptorBuilder::OptionInterpreter::SetOptionValue( option_field->type(), unknown_fields); } else { return AddValueError( - "Value must be non-negative integer for uint64 " - "option \"" + - option_field->full_name() + "\"."); + ValueMustBeInt("uint64", option_field->full_name())); } break; diff --git a/src/google/protobuf/descriptor.pb.h b/src/google/protobuf/descriptor.pb.h index 9a5db1db2a..6c664ab20b 100644 --- a/src/google/protobuf/descriptor.pb.h +++ b/src/google/protobuf/descriptor.pb.h @@ -15,7 +15,7 @@ #error "your headers." #endif // PROTOBUF_VERSION -#if 3021005 < PROTOBUF_MIN_PROTOC_VERSION +#if 3021006 < PROTOBUF_MIN_PROTOC_VERSION #error "This file was generated by an older version of protoc which is" #error "incompatible with your Protocol Buffer headers. Please" #error "regenerate this file with a newer version of protoc." diff --git a/src/google/protobuf/descriptor_unittest.cc b/src/google/protobuf/descriptor_unittest.cc index 1b44dd363d..87954f4986 100644 --- a/src/google/protobuf/descriptor_unittest.cc +++ b/src/google/protobuf/descriptor_unittest.cc @@ -5624,7 +5624,8 @@ TEST_F(ValidationErrorTest, Int32OptionValueOutOfPositiveRange) { " positive_int_value: 0x80000000 } " "}", - "foo.proto: foo.proto: OPTION_VALUE: Value out of range " + "foo.proto: foo.proto: OPTION_VALUE: Value out of range, -2147483648 to " + "2147483647, " "for int32 option \"foo\".\n"); } @@ -5641,7 +5642,8 @@ TEST_F(ValidationErrorTest, Int32OptionValueOutOfNegativeRange) { " negative_int_value: -0x80000001 } " "}", - "foo.proto: foo.proto: OPTION_VALUE: Value out of range " + "foo.proto: foo.proto: OPTION_VALUE: Value out of range, -2147483648 to " + "2147483647, " "for int32 option \"foo\".\n"); } @@ -5657,7 +5659,8 @@ TEST_F(ValidationErrorTest, Int32OptionValueIsNotPositiveInt) { " is_extension: true } " " string_value: \"5\" } }", - "foo.proto: foo.proto: OPTION_VALUE: Value must be integer " + "foo.proto: foo.proto: OPTION_VALUE: Value must be integer, from " + "-2147483648 to 2147483647, " "for int32 option \"foo\".\n"); } @@ -5675,7 +5678,8 @@ TEST_F(ValidationErrorTest, Int64OptionValueOutOfRange) { "} " "}", - "foo.proto: foo.proto: OPTION_VALUE: Value out of range " + "foo.proto: foo.proto: OPTION_VALUE: Value out of range, " + "-9223372036854775808 to 9223372036854775807, " "for int64 option \"foo\".\n"); } @@ -5691,7 +5695,8 @@ TEST_F(ValidationErrorTest, Int64OptionValueIsNotPositiveInt) { " is_extension: true } " " identifier_value: \"5\" } }", - "foo.proto: foo.proto: OPTION_VALUE: Value must be integer " + "foo.proto: foo.proto: OPTION_VALUE: Value must be integer, from " + "-9223372036854775808 to 9223372036854775807, " "for int64 option \"foo\".\n"); } @@ -5707,7 +5712,8 @@ TEST_F(ValidationErrorTest, UInt32OptionValueOutOfRange) { " is_extension: true } " " positive_int_value: 0x100000000 } }", - "foo.proto: foo.proto: OPTION_VALUE: Value out of range " + "foo.proto: foo.proto: OPTION_VALUE: Value out of range, 0 to " + "4294967295, " "for uint32 option \"foo\".\n"); } @@ -5723,7 +5729,8 @@ TEST_F(ValidationErrorTest, UInt32OptionValueIsNotPositiveInt) { " is_extension: true } " " double_value: -5.6 } }", - "foo.proto: foo.proto: OPTION_VALUE: Value must be non-negative integer " + "foo.proto: foo.proto: OPTION_VALUE: Value must be integer, from 0 to " + "4294967295, " "for uint32 option \"foo\".\n"); } @@ -5739,7 +5746,8 @@ TEST_F(ValidationErrorTest, UInt64OptionValueIsNotPositiveInt) { " is_extension: true } " " negative_int_value: -5 } }", - "foo.proto: foo.proto: OPTION_VALUE: Value must be non-negative integer " + "foo.proto: foo.proto: OPTION_VALUE: Value must be integer, from 0 to " + "18446744073709551615, " "for uint64 option \"foo\".\n"); } @@ -6478,9 +6486,10 @@ TEST_F(ValidationErrorTest, EnumValuesConflictWithDifferentCasing) { "}", "foo.proto: bar: NAME: Enum name bar has the same name as BAR " "if you ignore case and strip out the enum name prefix (if any). " - "This is error-prone and can lead to undefined behavior. " - "Please avoid doing this. If you are using allow_alias, please assign " - "the same numeric value to both enums.\n"); + "(If you are using allow_alias, please assign the same numeric " + "value to both enums.)" + " This is not allowed in proto3.\n" + ); // Not an error because both enums are mapped to the same value. BuildFile( @@ -6496,6 +6505,20 @@ TEST_F(ValidationErrorTest, EnumValuesConflictWithDifferentCasing) { } TEST_F(ValidationErrorTest, EnumValuesConflictWhenPrefixesStripped) { + BuildFileWithErrors( + "syntax: 'proto3'" + "name: 'foo.proto' " + "enum_type {" + " name: 'FooEnum' " + " value { name: 'BAR' number: 0 }" + " value { name: 'bar' number: 1 }" + "}", + "foo.proto: bar: NAME: Enum name bar has the same name as BAR " + "if you ignore case and strip out the enum name prefix (if any). " + "(If you are using allow_alias, please assign the same numeric " + "value to both enums.)" + " This is not allowed in proto3.\n" + ); BuildFileWithErrors( "syntax: 'proto3'" "name: 'foo.proto' " @@ -6506,9 +6529,10 @@ TEST_F(ValidationErrorTest, EnumValuesConflictWhenPrefixesStripped) { "}", "foo.proto: BAZ: NAME: Enum name BAZ has the same name as FOO_ENUM_BAZ " "if you ignore case and strip out the enum name prefix (if any). " - "This is error-prone and can lead to undefined behavior. " - "Please avoid doing this. If you are using allow_alias, please assign " - "the same numeric value to both enums.\n"); + "(If you are using allow_alias, please assign the same numeric value " + "to both enums.)" + " This is not allowed in proto3.\n" + ); BuildFileWithErrors( "syntax: 'proto3'" @@ -6520,9 +6544,10 @@ TEST_F(ValidationErrorTest, EnumValuesConflictWhenPrefixesStripped) { "}", "foo.proto: BAZ: NAME: Enum name BAZ has the same name as FOOENUM_BAZ " "if you ignore case and strip out the enum name prefix (if any). " - "This is error-prone and can lead to undefined behavior. " - "Please avoid doing this. If you are using allow_alias, please assign " - "the same numeric value to both enums.\n"); + "(If you are using allow_alias, please assign the same numeric value " + "to both enums.)" + " This is not allowed in proto3.\n" + ); BuildFileWithErrors( "syntax: 'proto3'" @@ -6534,9 +6559,10 @@ TEST_F(ValidationErrorTest, EnumValuesConflictWhenPrefixesStripped) { "}", "foo.proto: BAR__BAZ: NAME: Enum name BAR__BAZ has the same name as " "FOO_ENUM_BAR_BAZ if you ignore case and strip out the enum name prefix " - "(if any). This is error-prone and can lead to undefined behavior. " - "Please avoid doing this. If you are using allow_alias, please assign " - "the same numeric value to both enums.\n"); + "(if any). (If you are using allow_alias, please assign the same numeric " + "value to both enums.)" + " This is not allowed in proto3.\n" + ); BuildFileWithErrors( "syntax: 'proto3'" @@ -6548,9 +6574,10 @@ TEST_F(ValidationErrorTest, EnumValuesConflictWhenPrefixesStripped) { "}", "foo.proto: BAR_BAZ: NAME: Enum name BAR_BAZ has the same name as " "FOO_ENUM__BAR_BAZ if you ignore case and strip out the enum name prefix " - "(if any). This is error-prone and can lead to undefined behavior. " - "Please avoid doing this. If you are using allow_alias, please assign " - "the same numeric value to both enums.\n"); + "(if any). (If you are using allow_alias, please assign the same numeric " + "value to both enums.)" + " This is not allowed in proto3.\n" + ); // This isn't an error because the underscore will cause the PascalCase to // differ by case (BarBaz vs. Barbaz). @@ -6886,6 +6913,7 @@ TEST_F(ValidationErrorTest, ValidateProto3Extension) { // Test that field names that may conflict in JSON is not allowed by protoc. TEST_F(ValidationErrorTest, ValidateProto3JsonName) { + // TODO(b/248626372) Re-enable this once they're errors in google3. // The comparison is case-insensitive. BuildFileWithErrors( "name: 'foo.proto' " @@ -6895,8 +6923,11 @@ TEST_F(ValidationErrorTest, ValidateProto3JsonName) { " field { name:'name' number:1 label:LABEL_OPTIONAL type:TYPE_INT32 }" " field { name:'Name' number:2 label:LABEL_OPTIONAL type:TYPE_INT32 }" "}", - "foo.proto: Foo: NAME: The JSON camel-case name of field \"Name\" " - "conflicts with field \"name\". This is not allowed in proto3.\n"); + "foo.proto: Foo: NAME: The default JSON name of field \"Name\" " + "(\"Name\") " + "conflicts with the default JSON name of field \"name\" (\"name\"). This " + "is " + "not allowed in proto3.\n"); // Underscores are ignored. BuildFileWithErrors( "name: 'foo.proto' " @@ -6906,8 +6937,11 @@ TEST_F(ValidationErrorTest, ValidateProto3JsonName) { " field { name:'ab' number:1 label:LABEL_OPTIONAL type:TYPE_INT32 }" " field { name:'_a__b_' number:2 label:LABEL_OPTIONAL type:TYPE_INT32 }" "}", - "foo.proto: Foo: NAME: The JSON camel-case name of field \"_a__b_\" " - "conflicts with field \"ab\". This is not allowed in proto3.\n"); + "foo.proto: Foo: NAME: The default JSON name of field \"_a__b_\" " + "(\"AB\") " + "conflicts with the default JSON name of field \"ab\" (\"ab\"). This is " + "not " + "allowed in proto3.\n"); } diff --git a/src/google/protobuf/io/tokenizer.cc b/src/google/protobuf/io/tokenizer.cc index 58bd8e3054..992d8c0b17 100644 --- a/src/google/protobuf/io/tokenizer.cc +++ b/src/google/protobuf/io/tokenizer.cc @@ -1002,9 +1002,20 @@ bool Tokenizer::ParseInteger(const std::string& text, uint64_t max_value, } double Tokenizer::ParseFloat(const std::string& text) { + double result = 0; + if (!TryParseFloat(text, &result)) { + GOOGLE_LOG(DFATAL) + << " Tokenizer::ParseFloat() passed text that could not have been" + " tokenized as a float: " + << absl::CEscape(text); + } + return result; +} + +bool Tokenizer::TryParseFloat(const std::string& text, double* result) { const char* start = text.c_str(); char* end; - double result = NoLocaleStrtod(start, &end); + *result = NoLocaleStrtod(start, &end); // "1e" is not a valid float, but if the tokenizer reads it, it will // report an error but still return it as a valid token. We need to @@ -1020,12 +1031,7 @@ double Tokenizer::ParseFloat(const std::string& text) { ++end; } - GOOGLE_LOG_IF(DFATAL, - static_cast(end - start) != text.size() || *start == '-') - << " Tokenizer::ParseFloat() passed text that could not have been" - " tokenized as a float: " - << absl::CEscape(text); - return result; + return static_cast(end - start) == text.size() && *start != '-'; } // Helper to append a Unicode code point to a string as UTF8, without bringing diff --git a/src/google/protobuf/io/tokenizer.h b/src/google/protobuf/io/tokenizer.h index cab1faf917..73877ccc24 100644 --- a/src/google/protobuf/io/tokenizer.h +++ b/src/google/protobuf/io/tokenizer.h @@ -214,6 +214,10 @@ class PROTOBUF_EXPORT Tokenizer { // result is undefined (possibly an assert failure). static double ParseFloat(const std::string& text); + // Parses given text as if it were a TYPE_FLOAT token. Returns false if the + // given text is not actually a valid float literal. + static bool TryParseFloat(const std::string& text, double* result); + // Parses a TYPE_STRING token. This never fails, so long as the text actually // comes from a TYPE_STRING token parsed by Tokenizer. If it doesn't, the // result is undefined (possibly an assert failure). diff --git a/src/google/protobuf/io/zero_copy_stream_unittest.cc b/src/google/protobuf/io/zero_copy_stream_unittest.cc index d82354e571..4b8c2afe67 100644 --- a/src/google/protobuf/io/zero_copy_stream_unittest.cc +++ b/src/google/protobuf/io/zero_copy_stream_unittest.cc @@ -720,6 +720,9 @@ TEST_F(IoTest, StringIo) { // Verifies that outputs up to kint32max can be created. TEST_F(IoTest, LargeOutput) { + // Filter out this test on 32-bit architectures. + if (sizeof(void*) < 8) return; + std::string str; StringOutputStream output(&str); void* unused_data; diff --git a/src/google/protobuf/port_def.inc b/src/google/protobuf/port_def.inc index 3486294fec..0fcc90d815 100644 --- a/src/google/protobuf/port_def.inc +++ b/src/google/protobuf/port_def.inc @@ -221,7 +221,7 @@ #ifdef PROTOBUF_VERSION #error PROTOBUF_VERSION was previously defined #endif -#define PROTOBUF_VERSION 3021005 +#define PROTOBUF_VERSION 3021006 #ifdef PROTOBUF_MIN_HEADER_VERSION_FOR_PROTOC #error PROTOBUF_MIN_HEADER_VERSION_FOR_PROTOC was previously defined diff --git a/src/google/protobuf/repeated_field_unittest.cc b/src/google/protobuf/repeated_field_unittest.cc index eb0b9091cf..3baf6f25bb 100644 --- a/src/google/protobuf/repeated_field_unittest.cc +++ b/src/google/protobuf/repeated_field_unittest.cc @@ -429,14 +429,14 @@ TEST(RepeatedField, ReserveNothing) { TEST(RepeatedField, ReserveLowerClamp) { int clamped_value = internal::CalculateReserveSize(0, 1); - EXPECT_GE(clamped_value, 8 / sizeof(bool)); + EXPECT_GE(clamped_value, sizeof(void*) / sizeof(bool)); EXPECT_EQ((internal::RepeatedFieldLowerClampLimit()), clamped_value); // EXPECT_EQ(clamped_value, (internal::CalculateReserveSize( clamped_value, 2))); clamped_value = internal::CalculateReserveSize(0, 1); - EXPECT_GE(clamped_value, 8 / sizeof(int)); + EXPECT_GE(clamped_value, sizeof(void*) / sizeof(int)); EXPECT_EQ((internal::RepeatedFieldLowerClampLimit()), clamped_value); // EXPECT_EQ(clamped_value, (internal::CalculateReserveSize= sizeof(uint64_t)) { + Timestamp begin, end; + EXPECT_TRUE(TimeUtil::FromString("0001-01-01T00:00:00Z", &begin)); + EXPECT_EQ(TimeUtil::kTimestampMinSeconds, begin.seconds()); + EXPECT_EQ(0, begin.nanos()); + EXPECT_TRUE(TimeUtil::FromString("9999-12-31T23:59:59.999999999Z", &end)); + EXPECT_EQ(TimeUtil::kTimestampMaxSeconds, end.seconds()); + EXPECT_EQ(999999999, end.nanos()); + EXPECT_EQ("0001-01-01T00:00:00Z", TimeUtil::ToString(begin)); + EXPECT_EQ("9999-12-31T23:59:59.999999999Z", TimeUtil::ToString(end)); + } // Test negative timestamps. Timestamp time = TimeUtil::NanosecondsToTimestamp(-1); @@ -94,9 +97,12 @@ TEST(TimeUtilTest, DurationStringFormat) { EXPECT_TRUE(TimeUtil::FromString("0001-01-01T00:00:00Z", &begin)); EXPECT_TRUE(TimeUtil::FromString("9999-12-31T23:59:59.999999999Z", &end)); - EXPECT_EQ("315537897599.999999999s", TimeUtil::ToString(end - begin)); + // These these are out of bounds for 32-bit architectures. + if (sizeof(time_t) >= sizeof(uint64_t)) { + EXPECT_EQ("315537897599.999999999s", TimeUtil::ToString(end - begin)); + EXPECT_EQ("-315537897599.999999999s", TimeUtil::ToString(begin - end)); + } EXPECT_EQ(999999999, (end - begin).nanos()); - EXPECT_EQ("-315537897599.999999999s", TimeUtil::ToString(begin - end)); EXPECT_EQ(-999999999, (begin - end).nanos()); // Generated output should contain 3, 6, or 9 fractional digits.