diff --git a/BUILD b/BUILD index 07ee629a18..1788cfc06e 100644 --- a/BUILD +++ b/BUILD @@ -369,6 +369,7 @@ cc_library( "src/google/protobuf/compiler/cpp/cpp_message.cc", "src/google/protobuf/compiler/cpp/cpp_message_field.cc", "src/google/protobuf/compiler/cpp/cpp_padding_optimizer.cc", + "src/google/protobuf/compiler/cpp/cpp_parse_function_generator.cc", "src/google/protobuf/compiler/cpp/cpp_primitive_field.cc", "src/google/protobuf/compiler/cpp/cpp_service.cc", "src/google/protobuf/compiler/cpp/cpp_string_field.cc", diff --git a/CHANGES.txt b/CHANGES.txt index 93ee2b8e86..9e6eb44885 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,5 +1,16 @@ + Unreleased Changes (C++/Java/Python/PHP/Objective-C/C#/Ruby/JavaScript) + C++ + * Fix bug where `Descriptor::DebugString()` printed proto3 synthetic oneofs. + * Provide stable versions of `SortAndUnique()`. + * Make sure to cache proto3 optional message fields when they are cleared. + + Kotlin + * Restrict extension setter and getter operators to non-nullable T. + +3.16.0 (C++/Java/Python/PHP/Objective-C/C#/Ruby/JavaScript) + C++ * The ::pb namespace is no longer exposed due to conflicts. * Allow MessageDifferencer::TreatAsSet() (and friends) to override previous diff --git a/build_files_updated_unittest.sh b/build_files_updated_unittest.sh index c86307193c..87541c35fa 100755 --- a/build_files_updated_unittest.sh +++ b/build_files_updated_unittest.sh @@ -58,5 +58,5 @@ bash "${test_dir}/update_file_lists.sh" # Test whether there are any differences for file in ${generated_files[@]}; do - diff "${golden_dir}/${file}" "${test_dir}/${file}" + diff -du "${golden_dir}/${file}" "${test_dir}/${file}" done diff --git a/cmake/libprotoc.cmake b/cmake/libprotoc.cmake index ecb5a851b8..899304dc2e 100644 --- a/cmake/libprotoc.cmake +++ b/cmake/libprotoc.cmake @@ -12,6 +12,7 @@ set(libprotoc_files ${protobuf_source_dir}/src/google/protobuf/compiler/cpp/cpp_message.cc ${protobuf_source_dir}/src/google/protobuf/compiler/cpp/cpp_message_field.cc ${protobuf_source_dir}/src/google/protobuf/compiler/cpp/cpp_padding_optimizer.cc + ${protobuf_source_dir}/src/google/protobuf/compiler/cpp/cpp_parse_function_generator.cc ${protobuf_source_dir}/src/google/protobuf/compiler/cpp/cpp_primitive_field.cc ${protobuf_source_dir}/src/google/protobuf/compiler/cpp/cpp_service.cc ${protobuf_source_dir}/src/google/protobuf/compiler/cpp/cpp_string_field.cc @@ -96,6 +97,7 @@ set(libprotoc_headers ${protobuf_source_dir}/src/google/protobuf/compiler/cpp/cpp_names.h ${protobuf_source_dir}/src/google/protobuf/compiler/cpp/cpp_options.h ${protobuf_source_dir}/src/google/protobuf/compiler/cpp/cpp_padding_optimizer.h + ${protobuf_source_dir}/src/google/protobuf/compiler/cpp/cpp_parse_function_generator.h ${protobuf_source_dir}/src/google/protobuf/compiler/cpp/cpp_primitive_field.h ${protobuf_source_dir}/src/google/protobuf/compiler/cpp/cpp_service.h ${protobuf_source_dir}/src/google/protobuf/compiler/cpp/cpp_string_field.h diff --git a/conformance/ConformanceJava.java b/conformance/ConformanceJava.java index 531998262d..100bec4b54 100644 --- a/conformance/ConformanceJava.java +++ b/conformance/ConformanceJava.java @@ -54,7 +54,7 @@ class ConformanceJava { while (len > 0) { int read = System.in.read(buf, ofs, len); if (read == -1) { - return false; // EOF + return false; // EOF } ofs += read; len -= read; @@ -81,10 +81,10 @@ class ConformanceJava { private void writeLittleEndianIntToStdout(int val) throws Exception { byte[] buf = new byte[4]; - buf[0] = (byte)val; - buf[1] = (byte)(val >> 8); - buf[2] = (byte)(val >> 16); - buf[3] = (byte)(val >> 24); + buf[0] = (byte) val; + buf[1] = (byte) (val >> 8); + buf[2] = (byte) (val >> 16); + buf[3] = (byte) (val >> 24); writeToStdout(buf); } @@ -98,85 +98,71 @@ class ConformanceJava { INPUT_STREAM_DECODER; } - private static class BinaryDecoder { - public MessageType decode (ByteString bytes, BinaryDecoderType type, - Parser parser, ExtensionRegistry extensions) - throws InvalidProtocolBufferException { + private static class BinaryDecoder { + public T decode( + ByteString bytes, BinaryDecoderType type, Parser parser, ExtensionRegistry extensions) + throws InvalidProtocolBufferException { switch (type) { case BTYE_STRING_DECODER: - return parser.parseFrom(bytes, extensions); case BYTE_ARRAY_DECODER: - return parser.parseFrom(bytes.toByteArray(), extensions); - case ARRAY_BYTE_BUFFER_DECODER: { - ByteBuffer buffer = ByteBuffer.allocate(bytes.size()); - bytes.copyTo(buffer); - buffer.flip(); - try { + return parser.parseFrom(bytes, extensions); + case ARRAY_BYTE_BUFFER_DECODER: + { + ByteBuffer buffer = ByteBuffer.allocate(bytes.size()); + bytes.copyTo(buffer); + buffer.flip(); return parser.parseFrom(CodedInputStream.newInstance(buffer), extensions); - } catch (InvalidProtocolBufferException e) { - throw e; } - } - case READONLY_ARRAY_BYTE_BUFFER_DECODER: { - try { + case READONLY_ARRAY_BYTE_BUFFER_DECODER: + { return parser.parseFrom( CodedInputStream.newInstance(bytes.asReadOnlyByteBuffer()), extensions); - } catch (InvalidProtocolBufferException e) { - throw e; } - } - case DIRECT_BYTE_BUFFER_DECODER: { - ByteBuffer buffer = ByteBuffer.allocateDirect(bytes.size()); - bytes.copyTo(buffer); - buffer.flip(); - try { + case DIRECT_BYTE_BUFFER_DECODER: + { + ByteBuffer buffer = ByteBuffer.allocateDirect(bytes.size()); + bytes.copyTo(buffer); + buffer.flip(); return parser.parseFrom(CodedInputStream.newInstance(buffer), extensions); - } catch (InvalidProtocolBufferException e) { - throw e; } - } - case READONLY_DIRECT_BYTE_BUFFER_DECODER: { - ByteBuffer buffer = ByteBuffer.allocateDirect(bytes.size()); - bytes.copyTo(buffer); - buffer.flip(); - try { + case READONLY_DIRECT_BYTE_BUFFER_DECODER: + { + ByteBuffer buffer = ByteBuffer.allocateDirect(bytes.size()); + bytes.copyTo(buffer); + buffer.flip(); return parser.parseFrom( CodedInputStream.newInstance(buffer.asReadOnlyBuffer()), extensions); - } catch (InvalidProtocolBufferException e) { - throw e; } - } - case INPUT_STREAM_DECODER: { - try { + case INPUT_STREAM_DECODER: + { return parser.parseFrom(bytes.newInput(), extensions); - } catch (InvalidProtocolBufferException e) { - throw e; } - } - default : - return null; } + return null; } } - private MessageType parseBinary( - ByteString bytes, Parser parser, ExtensionRegistry extensions) + private T parseBinary( + ByteString bytes, Parser parser, ExtensionRegistry extensions) throws InvalidProtocolBufferException { - ArrayList messages = new ArrayList (); - ArrayList exceptions = - new ArrayList (); + ArrayList messages = new ArrayList<>(); + ArrayList exceptions = new ArrayList<>(); for (int i = 0; i < BinaryDecoderType.values().length; i++) { messages.add(null); exceptions.add(null); } - BinaryDecoder decoder = new BinaryDecoder (); + if (messages.isEmpty()) { + throw new RuntimeException("binary decoder types missing"); + } + + BinaryDecoder decoder = new BinaryDecoder<>(); boolean hasMessage = false; boolean hasException = false; for (int i = 0; i < BinaryDecoderType.values().length; ++i) { try { - //= BinaryDecoderType.values()[i].parseProto3(bytes); + // = BinaryDecoderType.values()[i].parseProto3(bytes); messages.set(i, decoder.decode(bytes, BinaryDecoderType.values()[i], parser, extensions)); hasMessage = true; } catch (InvalidProtocolBufferException e) { @@ -202,7 +188,15 @@ class ConformanceJava { if (hasException) { // We do not check if exceptions are equal. Different implementations may return different // exception messages. Throw an arbitrary one out instead. - throw exceptions.get(0); + InvalidProtocolBufferException exception = null; + for (InvalidProtocolBufferException e : exceptions) { + if (exception != null) { + exception.addSuppressed(e); + } else { + exception = e; + } + } + throw exception; } // Fast path comparing all the messages with the first message, assuming equality being @@ -242,115 +236,138 @@ class ConformanceJava { request.getMessageType().equals("protobuf_test_messages.proto2.TestAllTypesProto2"); switch (request.getPayloadCase()) { - case PROTOBUF_PAYLOAD: { - if (isProto3) { - try { - ExtensionRegistry extensions = ExtensionRegistry.newInstance(); - TestMessagesProto3.registerAllExtensions(extensions); - testMessage = parseBinary(request.getProtobufPayload(), TestAllTypesProto3.parser(), extensions); - } catch (InvalidProtocolBufferException e) { - return Conformance.ConformanceResponse.newBuilder().setParseError(e.getMessage()).build(); - } - } else if (isProto2) { - try { - ExtensionRegistry extensions = ExtensionRegistry.newInstance(); - TestMessagesProto2.registerAllExtensions(extensions); - testMessage = parseBinary(request.getProtobufPayload(), TestAllTypesProto2.parser(), extensions); - } catch (InvalidProtocolBufferException e) { - return Conformance.ConformanceResponse.newBuilder().setParseError(e.getMessage()).build(); - } - } else { - throw new RuntimeException("Protobuf request doesn't have specific payload type."); - } - break; - } - case JSON_PAYLOAD: { - try { - JsonFormat.Parser parser = JsonFormat.parser().usingTypeRegistry(typeRegistry); - if (request.getTestCategory() - == Conformance.TestCategory.JSON_IGNORE_UNKNOWN_PARSING_TEST) { - parser = parser.ignoringUnknownFields(); - } + case PROTOBUF_PAYLOAD: + { if (isProto3) { - TestMessagesProto3.TestAllTypesProto3.Builder builder = - TestMessagesProto3.TestAllTypesProto3.newBuilder(); - parser.merge(request.getJsonPayload(), builder); - testMessage = builder.build(); + try { + ExtensionRegistry extensions = ExtensionRegistry.newInstance(); + TestMessagesProto3.registerAllExtensions(extensions); + testMessage = + parseBinary( + request.getProtobufPayload(), TestAllTypesProto3.parser(), extensions); + } catch (InvalidProtocolBufferException e) { + return Conformance.ConformanceResponse.newBuilder() + .setParseError(e.getMessage()) + .build(); + } } else if (isProto2) { - TestMessagesProto2.TestAllTypesProto2.Builder builder = - TestMessagesProto2.TestAllTypesProto2.newBuilder(); - parser.merge(request.getJsonPayload(), builder); - testMessage = builder.build(); + try { + ExtensionRegistry extensions = ExtensionRegistry.newInstance(); + TestMessagesProto2.registerAllExtensions(extensions); + testMessage = + parseBinary( + request.getProtobufPayload(), TestAllTypesProto2.parser(), extensions); + } catch (InvalidProtocolBufferException e) { + return Conformance.ConformanceResponse.newBuilder() + .setParseError(e.getMessage()) + .build(); + } } else { throw new RuntimeException("Protobuf request doesn't have specific payload type."); } - } catch (InvalidProtocolBufferException e) { - return Conformance.ConformanceResponse.newBuilder().setParseError(e.getMessage()).build(); + break; } - break; - } - case TEXT_PAYLOAD: { - if (isProto3) { + case JSON_PAYLOAD: + { try { - TestMessagesProto3.TestAllTypesProto3.Builder builder = - TestMessagesProto3.TestAllTypesProto3.newBuilder(); - TextFormat.merge(request.getTextPayload(), builder); - testMessage = builder.build(); - } catch (TextFormat.ParseException e) { + JsonFormat.Parser parser = JsonFormat.parser().usingTypeRegistry(typeRegistry); + if (request.getTestCategory() + == Conformance.TestCategory.JSON_IGNORE_UNKNOWN_PARSING_TEST) { + parser = parser.ignoringUnknownFields(); + } + if (isProto3) { + TestMessagesProto3.TestAllTypesProto3.Builder builder = + TestMessagesProto3.TestAllTypesProto3.newBuilder(); + parser.merge(request.getJsonPayload(), builder); + testMessage = builder.build(); + } else if (isProto2) { + TestMessagesProto2.TestAllTypesProto2.Builder builder = + TestMessagesProto2.TestAllTypesProto2.newBuilder(); + parser.merge(request.getJsonPayload(), builder); + testMessage = builder.build(); + } else { + throw new RuntimeException("Protobuf request doesn't have specific payload type."); + } + } catch (InvalidProtocolBufferException e) { + return Conformance.ConformanceResponse.newBuilder() + .setParseError(e.getMessage()) + .build(); + } + break; + } + case TEXT_PAYLOAD: + { + if (isProto3) { + try { + TestMessagesProto3.TestAllTypesProto3.Builder builder = + TestMessagesProto3.TestAllTypesProto3.newBuilder(); + TextFormat.merge(request.getTextPayload(), builder); + testMessage = builder.build(); + } catch (TextFormat.ParseException e) { return Conformance.ConformanceResponse.newBuilder() .setParseError(e.getMessage()) .build(); - } - } else if (isProto2) { - try { - TestMessagesProto2.TestAllTypesProto2.Builder builder = - TestMessagesProto2.TestAllTypesProto2.newBuilder(); - TextFormat.merge(request.getTextPayload(), builder); - testMessage = builder.build(); - } catch (TextFormat.ParseException e) { + } + } else if (isProto2) { + try { + TestMessagesProto2.TestAllTypesProto2.Builder builder = + TestMessagesProto2.TestAllTypesProto2.newBuilder(); + TextFormat.merge(request.getTextPayload(), builder); + testMessage = builder.build(); + } catch (TextFormat.ParseException e) { return Conformance.ConformanceResponse.newBuilder() .setParseError(e.getMessage()) .build(); + } + } else { + throw new RuntimeException("Protobuf request doesn't have specific payload type."); } - } else { - throw new RuntimeException("Protobuf request doesn't have specific payload type."); + break; + } + case PAYLOAD_NOT_SET: + { + throw new RuntimeException("Request didn't have payload."); } - break; - } - case PAYLOAD_NOT_SET: { - throw new RuntimeException("Request didn't have payload."); - } - default: { - throw new RuntimeException("Unexpected payload case."); - } + default: + { + throw new RuntimeException("Unexpected payload case."); + } } switch (request.getRequestedOutputFormat()) { case UNSPECIFIED: throw new RuntimeException("Unspecified output format."); - case PROTOBUF: { - ByteString MessageString = testMessage.toByteString(); - return Conformance.ConformanceResponse.newBuilder().setProtobufPayload(MessageString).build(); - } + case PROTOBUF: + { + ByteString messageString = testMessage.toByteString(); + return Conformance.ConformanceResponse.newBuilder() + .setProtobufPayload(messageString) + .build(); + } case JSON: try { - return Conformance.ConformanceResponse.newBuilder().setJsonPayload( - JsonFormat.printer().usingTypeRegistry(typeRegistry).print(testMessage)).build(); + return Conformance.ConformanceResponse.newBuilder() + .setJsonPayload( + JsonFormat.printer().usingTypeRegistry(typeRegistry).print(testMessage)) + .build(); } catch (InvalidProtocolBufferException | IllegalArgumentException e) { - return Conformance.ConformanceResponse.newBuilder().setSerializeError( - e.getMessage()).build(); + return Conformance.ConformanceResponse.newBuilder() + .setSerializeError(e.getMessage()) + .build(); } case TEXT_FORMAT: - return Conformance.ConformanceResponse.newBuilder().setTextPayload( - TextFormat.printToString(testMessage)).build(); + return Conformance.ConformanceResponse.newBuilder() + .setTextPayload(TextFormat.printToString(testMessage)) + .build(); - default: { - throw new RuntimeException("Unexpected request output."); - } + default: + { + throw new RuntimeException("Unexpected request output."); + } } } @@ -358,7 +375,7 @@ class ConformanceJava { int bytes = readLittleEndianIntFromStdin(); if (bytes == -1) { - return false; // EOF + return false; // EOF } byte[] serializedInput = new byte[bytes]; @@ -379,14 +396,16 @@ class ConformanceJava { } public void run() throws Exception { - typeRegistry = TypeRegistry.newBuilder().add( - TestMessagesProto3.TestAllTypesProto3.getDescriptor()).build(); + typeRegistry = + TypeRegistry.newBuilder() + .add(TestMessagesProto3.TestAllTypesProto3.getDescriptor()) + .build(); while (doTestIo()) { this.testCount++; } - System.err.println("ConformanceJava: received EOF from test runner after " + - this.testCount + " tests"); + System.err.println( + "ConformanceJava: received EOF from test runner after " + this.testCount + " tests"); } public static void main(String[] args) throws Exception { diff --git a/conformance/ConformanceJavaLite.java b/conformance/ConformanceJavaLite.java index 147738dcbe..eb3d06afc5 100644 --- a/conformance/ConformanceJavaLite.java +++ b/conformance/ConformanceJavaLite.java @@ -28,8 +28,19 @@ // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -import com.google.protobuf.conformance.Conformance; +import com.google.protobuf.ByteString; +import com.google.protobuf.CodedInputStream; +import com.google.protobuf.ExtensionRegistryLite; import com.google.protobuf.InvalidProtocolBufferException; +import com.google.protobuf.MessageLite; +import com.google.protobuf.Parser; +import com.google.protobuf.conformance.Conformance; +import com.google.protobuf_test_messages.proto2.TestMessagesProto2; +import com.google.protobuf_test_messages.proto2.TestMessagesProto2.TestAllTypesProto2; +import com.google.protobuf_test_messages.proto3.TestMessagesProto3; +import com.google.protobuf_test_messages.proto3.TestMessagesProto3.TestAllTypesProto3; +import java.nio.ByteBuffer; +import java.util.ArrayList; class ConformanceJavaLite { private int testCount = 0; @@ -39,7 +50,7 @@ class ConformanceJavaLite { while (len > 0) { int read = System.in.read(buf, ofs, len); if (read == -1) { - return false; // EOF + return false; // EOF } ofs += read; len -= read; @@ -66,36 +77,214 @@ class ConformanceJavaLite { private void writeLittleEndianIntToStdout(int val) throws Exception { byte[] buf = new byte[4]; - buf[0] = (byte)val; - buf[1] = (byte)(val >> 8); - buf[2] = (byte)(val >> 16); - buf[3] = (byte)(val >> 24); + buf[0] = (byte) val; + buf[1] = (byte) (val >> 8); + buf[2] = (byte) (val >> 16); + buf[3] = (byte) (val >> 24); writeToStdout(buf); } - private Conformance.ConformanceResponse doTest(Conformance.ConformanceRequest request) { - Conformance.TestAllTypes testMessage; + private enum BinaryDecoderType { + BTYE_STRING_DECODER, + BYTE_ARRAY_DECODER, + ARRAY_BYTE_BUFFER_DECODER, + READONLY_ARRAY_BYTE_BUFFER_DECODER, + DIRECT_BYTE_BUFFER_DECODER, + READONLY_DIRECT_BYTE_BUFFER_DECODER, + INPUT_STREAM_DECODER; + } - switch (request.getPayloadCase()) { - case PROTOBUF_PAYLOAD: { - try { - testMessage = Conformance.TestAllTypes.parseFrom(request.getProtobufPayload()); - } catch (InvalidProtocolBufferException e) { - return Conformance.ConformanceResponse.newBuilder().setParseError(e.getMessage()).build(); + private static class BinaryDecoder { + public T decode( + ByteString bytes, + BinaryDecoderType type, + Parser parser, + ExtensionRegistryLite extensions) + throws InvalidProtocolBufferException { + switch (type) { + case BTYE_STRING_DECODER: + case BYTE_ARRAY_DECODER: + return parser.parseFrom(bytes, extensions); + case ARRAY_BYTE_BUFFER_DECODER: + { + ByteBuffer buffer = ByteBuffer.allocate(bytes.size()); + bytes.copyTo(buffer); + buffer.flip(); + return parser.parseFrom(CodedInputStream.newInstance(buffer), extensions); + } + case READONLY_ARRAY_BYTE_BUFFER_DECODER: + { + return parser.parseFrom( + CodedInputStream.newInstance(bytes.asReadOnlyByteBuffer()), extensions); + } + case DIRECT_BYTE_BUFFER_DECODER: + { + ByteBuffer buffer = ByteBuffer.allocateDirect(bytes.size()); + bytes.copyTo(buffer); + buffer.flip(); + return parser.parseFrom(CodedInputStream.newInstance(buffer), extensions); + } + case READONLY_DIRECT_BYTE_BUFFER_DECODER: + { + ByteBuffer buffer = ByteBuffer.allocateDirect(bytes.size()); + bytes.copyTo(buffer); + buffer.flip(); + return parser.parseFrom( + CodedInputStream.newInstance(buffer.asReadOnlyBuffer()), extensions); + } + case INPUT_STREAM_DECODER: + { + return parser.parseFrom(bytes.newInput(), extensions); + } + } + return null; + } + } + + private T parseBinary( + ByteString bytes, Parser parser, ExtensionRegistryLite extensions) + throws InvalidProtocolBufferException { + ArrayList messages = new ArrayList<>(); + ArrayList exceptions = new ArrayList<>(); + + for (int i = 0; i < BinaryDecoderType.values().length; i++) { + messages.add(null); + exceptions.add(null); + } + if (messages.isEmpty()) { + throw new RuntimeException("binary decoder types missing"); + } + + BinaryDecoder decoder = new BinaryDecoder<>(); + + boolean hasMessage = false; + boolean hasException = false; + for (int i = 0; i < BinaryDecoderType.values().length; ++i) { + try { + messages.set(i, decoder.decode(bytes, BinaryDecoderType.values()[i], parser, extensions)); + hasMessage = true; + } catch (InvalidProtocolBufferException e) { + exceptions.set(i, e); + hasException = true; + } + } + + if (hasMessage && hasException) { + StringBuilder sb = + new StringBuilder("Binary decoders disagreed on whether the payload was valid.\n"); + for (int i = 0; i < BinaryDecoderType.values().length; ++i) { + sb.append(BinaryDecoderType.values()[i].name()); + if (messages.get(i) != null) { + sb.append(" accepted the payload.\n"); + } else { + sb.append(" rejected the payload.\n"); } - break; } - case JSON_PAYLOAD: { - return Conformance.ConformanceResponse.newBuilder().setSkipped( - "Lite runtime does not support JSON format.").build(); + throw new RuntimeException(sb.toString()); + } + + if (hasException) { + // We do not check if exceptions are equal. Different implementations may return different + // exception messages. Throw an arbitrary one out instead. + InvalidProtocolBufferException exception = null; + for (InvalidProtocolBufferException e : exceptions) { + if (exception != null) { + exception.addSuppressed(e); + } else { + exception = e; + } } - case PAYLOAD_NOT_SET: { - throw new RuntimeException("Request didn't have payload."); + throw exception; + } + + // Fast path comparing all the messages with the first message, assuming equality being + // symmetric and transitive. + boolean allEqual = true; + for (int i = 1; i < messages.size(); ++i) { + if (!messages.get(0).equals(messages.get(i))) { + allEqual = false; + break; } + } - default: { - throw new RuntimeException("Unexpected payload case."); + // Slow path: compare and find out all unequal pairs. + if (!allEqual) { + StringBuilder sb = new StringBuilder(); + for (int i = 0; i < messages.size() - 1; ++i) { + for (int j = i + 1; j < messages.size(); ++j) { + if (!messages.get(i).equals(messages.get(j))) { + sb.append(BinaryDecoderType.values()[i].name()) + .append(" and ") + .append(BinaryDecoderType.values()[j].name()) + .append(" parsed the payload differently.\n"); + } + } } + throw new RuntimeException(sb.toString()); + } + + return messages.get(0); + } + + private Conformance.ConformanceResponse doTest(Conformance.ConformanceRequest request) { + com.google.protobuf.MessageLite testMessage; + boolean isProto3 = + request.getMessageType().equals("protobuf_test_messages.proto3.TestAllTypesProto3"); + boolean isProto2 = + request.getMessageType().equals("protobuf_test_messages.proto2.TestAllTypesProto2"); + + switch (request.getPayloadCase()) { + case PROTOBUF_PAYLOAD: + { + if (isProto3) { + try { + ExtensionRegistryLite extensions = ExtensionRegistryLite.newInstance(); + TestMessagesProto3.registerAllExtensions(extensions); + testMessage = + parseBinary( + request.getProtobufPayload(), TestAllTypesProto3.parser(), extensions); + } catch (InvalidProtocolBufferException e) { + return Conformance.ConformanceResponse.newBuilder() + .setParseError(e.getMessage()) + .build(); + } + } else if (isProto2) { + try { + ExtensionRegistryLite extensions = ExtensionRegistryLite.newInstance(); + TestMessagesProto2.registerAllExtensions(extensions); + testMessage = + parseBinary( + request.getProtobufPayload(), TestAllTypesProto2.parser(), extensions); + } catch (InvalidProtocolBufferException e) { + return Conformance.ConformanceResponse.newBuilder() + .setParseError(e.getMessage()) + .build(); + } + } else { + throw new RuntimeException("Protobuf request doesn't have specific payload type."); + } + break; + } + case JSON_PAYLOAD: + { + return Conformance.ConformanceResponse.newBuilder() + .setSkipped("Lite runtime does not support JSON format.") + .build(); + } + case TEXT_PAYLOAD: + { + return Conformance.ConformanceResponse.newBuilder() + .setSkipped("Lite runtime does not support Text format.") + .build(); + } + case PAYLOAD_NOT_SET: + { + throw new RuntimeException("Request didn't have payload."); + } + default: + { + throw new RuntimeException("Unexpected payload case."); + } } switch (request.getRequestedOutputFormat()) { @@ -103,15 +292,23 @@ class ConformanceJavaLite { throw new RuntimeException("Unspecified output format."); case PROTOBUF: - return Conformance.ConformanceResponse.newBuilder().setProtobufPayload(testMessage.toByteString()).build(); + return Conformance.ConformanceResponse.newBuilder() + .setProtobufPayload(testMessage.toByteString()) + .build(); case JSON: - return Conformance.ConformanceResponse.newBuilder().setSkipped( - "Lite runtime does not support JSON format.").build(); + return Conformance.ConformanceResponse.newBuilder() + .setSkipped("Lite runtime does not support JSON format.") + .build(); - default: { - throw new RuntimeException("Unexpected request output."); - } + case TEXT_FORMAT: + return Conformance.ConformanceResponse.newBuilder() + .setSkipped("Lite runtime does not support Text format.") + .build(); + default: + { + throw new RuntimeException("Unexpected request output."); + } } } @@ -119,7 +316,7 @@ class ConformanceJavaLite { int bytes = readLittleEndianIntFromStdin(); if (bytes == -1) { - return false; // EOF + return false; // EOF } byte[] serializedInput = new byte[bytes]; @@ -144,8 +341,8 @@ class ConformanceJavaLite { this.testCount++; } - System.err.println("ConformanceJavaLite: received EOF from test runner after " + - this.testCount + " tests"); + System.err.println( + "ConformanceJavaLite: received EOF from test runner after " + this.testCount + " tests"); } public static void main(String[] args) throws Exception { diff --git a/csharp/src/AddressBook/Addressbook.cs b/csharp/src/AddressBook/Addressbook.cs index 042f69ee60..8b21683db4 100644 --- a/csharp/src/AddressBook/Addressbook.cs +++ b/csharp/src/AddressBook/Addressbook.cs @@ -32,9 +32,9 @@ namespace Google.Protobuf.Examples.AddressBook { "Eg4KBm51bWJlchgBIAEoCRIoCgR0eXBlGAIgASgOMhoudHV0b3JpYWwuUGVy", "c29uLlBob25lVHlwZSIrCglQaG9uZVR5cGUSCgoGTU9CSUxFEAASCAoESE9N", "RRABEggKBFdPUksQAiIvCgtBZGRyZXNzQm9vaxIgCgZwZW9wbGUYASADKAsy", - "EC50dXRvcmlhbC5QZXJzb25CUAoUY29tLmV4YW1wbGUudHV0b3JpYWxCEUFk", - "ZHJlc3NCb29rUHJvdG9zqgIkR29vZ2xlLlByb3RvYnVmLkV4YW1wbGVzLkFk", - "ZHJlc3NCb29rYgZwcm90bzM=")); + "EC50dXRvcmlhbC5QZXJzb25CWQobY29tLmV4YW1wbGUudHV0b3JpYWwucHJv", + "dG9zQhFBZGRyZXNzQm9va1Byb3Rvc1ABqgIkR29vZ2xlLlByb3RvYnVmLkV4", + "YW1wbGVzLkFkZHJlc3NCb29rYgZwcm90bzM=")); descriptor = pbr::FileDescriptor.FromGeneratedCode(descriptorData, new pbr::FileDescriptor[] { global::Google.Protobuf.WellKnownTypes.TimestampReflection.Descriptor, }, new pbr::GeneratedClrTypeInfo(null, null, new pbr::GeneratedClrTypeInfo[] { diff --git a/python/google/protobuf/internal/api_implementation.py b/python/google/protobuf/internal/api_implementation.py index 7592be4eed..be1af7df6b 100644 --- a/python/google/protobuf/internal/api_implementation.py +++ b/python/google/protobuf/internal/api_implementation.py @@ -32,8 +32,8 @@ """ import os -import warnings import sys +import warnings try: # pylint: disable=g-import-not-at-top @@ -60,10 +60,11 @@ if _api_version < 0: # Still unspecified? raise ImportError('_use_fast_cpp_protos import succeeded but was None') del _use_fast_cpp_protos _api_version = 2 - # Can not import both use_fast_cpp_protos and use_pure_python. from google.protobuf import use_pure_python raise RuntimeError( - 'Conflict depend on both use_fast_cpp_protos and use_pure_python') + 'Conflicting deps on both :use_fast_cpp_protos and :use_pure_python.\n' + ' go/build_deps_on_BOTH_use_fast_cpp_protos_AND_use_pure_python\n' + 'This should be impossible via a link error at build time...') except ImportError: try: # pylint: disable=g-import-not-at-top diff --git a/src/Makefile.am b/src/Makefile.am index 49fe6ed933..18b1a431f7 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -345,6 +345,8 @@ libprotoc_la_SOURCES = \ google/protobuf/compiler/cpp/cpp_options.h \ google/protobuf/compiler/cpp/cpp_padding_optimizer.cc \ google/protobuf/compiler/cpp/cpp_padding_optimizer.h \ + google/protobuf/compiler/cpp/cpp_parse_function_generator.cc \ + google/protobuf/compiler/cpp/cpp_parse_function_generator.h \ google/protobuf/compiler/cpp/cpp_primitive_field.cc \ google/protobuf/compiler/cpp/cpp_primitive_field.h \ google/protobuf/compiler/cpp/cpp_service.cc \ diff --git a/src/google/protobuf/any.pb.cc b/src/google/protobuf/any.pb.cc index 5ecf63f8c4..f061f5a038 100644 --- a/src/google/protobuf/any.pb.cc +++ b/src/google/protobuf/any.pb.cc @@ -315,7 +315,7 @@ bool Any::IsInitialized() const { void Any::InternalSwap(Any* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); type_url_.Swap(&other->type_url_, &::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); value_.Swap(&other->value_, &::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); } diff --git a/src/google/protobuf/api.pb.cc b/src/google/protobuf/api.pb.cc index 0f4facd1f4..e818046f31 100644 --- a/src/google/protobuf/api.pb.cc +++ b/src/google/protobuf/api.pb.cc @@ -558,7 +558,7 @@ bool Api::IsInitialized() const { void Api::InternalSwap(Api* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); methods_.InternalSwap(&other->methods_); options_.InternalSwap(&other->options_); mixins_.InternalSwap(&other->mixins_); @@ -953,7 +953,7 @@ bool Method::IsInitialized() const { void Method::InternalSwap(Method* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); options_.InternalSwap(&other->options_); name_.Swap(&other->name_, &::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); request_type_url_.Swap(&other->request_type_url_, &::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); @@ -1200,7 +1200,7 @@ bool Mixin::IsInitialized() const { void Mixin::InternalSwap(Mixin* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); name_.Swap(&other->name_, &::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); root_.Swap(&other->root_, &::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); } diff --git a/src/google/protobuf/api.pb.h b/src/google/protobuf/api.pb.h index 75b0428cb3..15ee4c11a0 100644 --- a/src/google/protobuf/api.pb.h +++ b/src/google/protobuf/api.pb.h @@ -279,7 +279,7 @@ class PROTOBUF_EXPORT Api PROTOBUF_FINAL : public: void clear_source_context(); const PROTOBUF_NAMESPACE_ID::SourceContext& source_context() const; - PROTOBUF_NAMESPACE_ID::SourceContext* release_source_context(); + PROTOBUF_FUTURE_MUST_USE_RESULT PROTOBUF_NAMESPACE_ID::SourceContext* release_source_context(); PROTOBUF_NAMESPACE_ID::SourceContext* mutable_source_context(); void set_allocated_source_context(PROTOBUF_NAMESPACE_ID::SourceContext* source_context); private: diff --git a/src/google/protobuf/arena.h b/src/google/protobuf/arena.h index 8b5c7f1826..59c9317f21 100644 --- a/src/google/protobuf/arena.h +++ b/src/google/protobuf/arena.h @@ -618,7 +618,6 @@ class PROTOBUF_EXPORT PROTOBUF_ALIGNAS(8) Arena final { new (arena->AllocateInternal(sizeof(T), alignof(T), destructor, RTTI_TYPE_ID(T))) T(std::forward(args)...); - result->SetOwningArena(arena); return result; } } @@ -646,7 +645,6 @@ class PROTOBUF_EXPORT PROTOBUF_ALIGNAS(8) Arena final { PROTOBUF_ALWAYS_INLINE void OwnInternal(T* object, std::true_type) { if (object != NULL) { impl_.AddCleanup(object, &internal::arena_delete_object); - object->SetOwningArena(this); } } template diff --git a/src/google/protobuf/arena_test_util.h b/src/google/protobuf/arena_test_util.h index 8ceab91aab..33f5cf4192 100644 --- a/src/google/protobuf/arena_test_util.h +++ b/src/google/protobuf/arena_test_util.h @@ -92,6 +92,33 @@ class NoHeapChecker { } capture_alloc; }; +// Owns the internal T only if it's not owned by an arena. +// T needs to be arena constructible and destructor skippable. +template +class ArenaHolder { + public: + explicit ArenaHolder(Arena* arena) + : field_(Arena::CreateMessage(arena)), + owned_by_arena_(arena != nullptr) { + GOOGLE_DCHECK(google::protobuf::Arena::is_arena_constructable::value); + GOOGLE_DCHECK(google::protobuf::Arena::is_destructor_skippable::value); + } + + ~ArenaHolder() { + if (!owned_by_arena_) { + delete field_; + } + } + + T* get() { return field_; } + T* operator->() { return field_; } + T& operator*() { return *field_; } + + private: + T* field_; + bool owned_by_arena_; +}; + } // namespace internal } // namespace protobuf } // namespace google diff --git a/src/google/protobuf/compiler/cpp/cpp_helpers.cc b/src/google/protobuf/compiler/cpp/cpp_helpers.cc index dcbb14f1ba..fbca72c412 100644 --- a/src/google/protobuf/compiler/cpp/cpp_helpers.cc +++ b/src/google/protobuf/compiler/cpp/cpp_helpers.cc @@ -206,10 +206,6 @@ void SetIntVar(const Options& options, const std::string& type, (*variables)[type] = IntTypeName(options, type); } -bool HasInternalAccessors(const FieldOptions::CType ctype) { - return ctype == FieldOptions::STRING || ctype == FieldOptions::CORD; -} - } // namespace void SetCommonVars(const Options& options, @@ -1349,523 +1345,6 @@ bool MaybeBootstrap(const Options& options, GeneratorContext* generator_context, } } -class ParseLoopGenerator { - public: - ParseLoopGenerator(int num_hasbits, const Options& options, - MessageSCCAnalyzer* scc_analyzer, io::Printer* printer) - : scc_analyzer_(scc_analyzer), - options_(options), - format_(printer), - num_hasbits_(num_hasbits) {} - - void GenerateParserLoop(const Descriptor* descriptor) { - format_.Set("classname", ClassName(descriptor)); - format_.Set("p_ns", "::" + ProtobufNamespace(options_)); - format_.Set("pi_ns", - StrCat("::", ProtobufNamespace(options_), "::internal")); - format_.Set("GOOGLE_PROTOBUF", MacroPrefix(options_)); - std::map vars; - SetCommonVars(options_, &vars); - SetUnknkownFieldsVariable(descriptor, options_, &vars); - format_.AddMap(vars); - - std::vector ordered_fields; - for (auto field : FieldRange(descriptor)) { - if (!IsFieldStripped(field, options_)) { - ordered_fields.push_back(field); - } - } - std::sort(ordered_fields.begin(), ordered_fields.end(), - [](const FieldDescriptor* a, const FieldDescriptor* b) { - return a->number() < b->number(); - }); - - format_( - "const char* $classname$::_InternalParse(const char* ptr, " - "$pi_ns$::ParseContext* ctx) {\n" - "#define CHK_(x) if (PROTOBUF_PREDICT_FALSE(!(x))) goto failure\n"); - format_.Indent(); - int hasbits_size = 0; - if (num_hasbits_ > 0) { - hasbits_size = (num_hasbits_ + 31) / 32; - } - // For now only optimize small hasbits. - if (hasbits_size != 1) hasbits_size = 0; - if (hasbits_size) { - format_("_Internal::HasBits has_bits{};\n"); - format_.Set("has_bits", "has_bits"); - } else { - format_.Set("has_bits", "_has_bits_"); - } - - GenerateParseLoop(descriptor, ordered_fields); - format_.Outdent(); - format_("success:\n"); - if (hasbits_size) format_(" _has_bits_.Or(has_bits);\n"); - - format_( - " return ptr;\n" - "failure:\n" - " ptr = nullptr;\n" - " goto success;\n" - "#undef CHK_\n" - "}\n"); - } - - private: - MessageSCCAnalyzer* scc_analyzer_; - const Options& options_; - Formatter format_; - int num_hasbits_; - - using WireFormat = internal::WireFormat; - using WireFormatLite = internal::WireFormatLite; - - void GenerateArenaString(const FieldDescriptor* field) { - if (HasHasbit(field)) { - format_("_Internal::set_has_$1$(&$has_bits$);\n", FieldName(field)); - } - std::string default_string = - field->default_value_string().empty() - ? "::" + ProtobufNamespace(options_) + - "::internal::GetEmptyStringAlreadyInited()" - : QualifiedClassName(field->containing_type(), options_) + - "::" + MakeDefaultName(field) + ".get()"; - format_( - "if (arena != nullptr) {\n" - " ptr = ctx->ReadArenaString(ptr, &$1$_, arena);\n" - "} else {\n" - " ptr = " - "$pi_ns$::InlineGreedyStringParser($1$_.MutableNoArenaNoDefault(&$2$" - "), ptr, ctx);" - "\n}\n" - "const std::string* str = &$1$_.Get(); (void)str;\n", - FieldName(field), default_string); - } - - void GenerateStrings(const FieldDescriptor* field, bool check_utf8) { - FieldOptions::CType ctype = FieldOptions::STRING; - if (!options_.opensource_runtime) { - // Open source doesn't support other ctypes; - ctype = field->options().ctype(); - } - if (!field->is_repeated() && !options_.opensource_runtime && - GetOptimizeFor(field->file(), options_) != FileOptions::LITE_RUNTIME && - // For now only use arena string for strings with empty defaults. - field->default_value_string().empty() && - !field->real_containing_oneof() && ctype == FieldOptions::STRING) { - GenerateArenaString(field); - } else { - std::string name; - switch (ctype) { - case FieldOptions::STRING: - name = "GreedyStringParser"; - break; - case FieldOptions::CORD: - name = "CordParser"; - break; - case FieldOptions::STRING_PIECE: - name = "StringPieceParser"; - break; - } - format_( - "auto str = $1$$2$_$3$();\n" - "ptr = $pi_ns$::Inline$4$(str, ptr, ctx);\n", - HasInternalAccessors(ctype) ? "_internal_" : "", - field->is_repeated() && !field->is_packable() ? "add" : "mutable", - FieldName(field), name); - } - if (!check_utf8) return; // return if this is a bytes field - auto level = GetUtf8CheckMode(field, options_); - switch (level) { - case Utf8CheckMode::kNone: - return; - case Utf8CheckMode::kVerify: - format_("#ifndef NDEBUG\n"); - break; - case Utf8CheckMode::kStrict: - format_("CHK_("); - break; - } - std::string field_name; - field_name = "nullptr"; - if (HasDescriptorMethods(field->file(), options_)) { - field_name = StrCat("\"", field->full_name(), "\""); - } - format_("$pi_ns$::VerifyUTF8(str, $1$)", field_name); - switch (level) { - case Utf8CheckMode::kNone: - return; - case Utf8CheckMode::kVerify: - format_( - ";\n" - "#endif // !NDEBUG\n"); - break; - case Utf8CheckMode::kStrict: - format_(");\n"); - break; - } - } - - void GenerateLengthDelim(const FieldDescriptor* field) { - if (field->is_packable()) { - std::string enum_validator; - if (field->type() == FieldDescriptor::TYPE_ENUM && - !HasPreservingUnknownEnumSemantics(field)) { - enum_validator = - StrCat(", ", QualifiedClassName(field->enum_type(), options_), - "_IsValid, &_internal_metadata_, ", field->number()); - format_( - "ptr = " - "$pi_ns$::Packed$1$Parser<$unknown_fields_type$>(_internal_mutable_" - "$2$(), ptr, " - "ctx$3$);\n", - DeclaredTypeMethodName(field->type()), FieldName(field), - enum_validator); - } else { - format_( - "ptr = $pi_ns$::Packed$1$Parser(_internal_mutable_$2$(), ptr, " - "ctx$3$);\n", - DeclaredTypeMethodName(field->type()), FieldName(field), - enum_validator); - } - } else { - auto field_type = field->type(); - switch (field_type) { - case FieldDescriptor::TYPE_STRING: - GenerateStrings(field, true /* utf8 */); - break; - case FieldDescriptor::TYPE_BYTES: - GenerateStrings(field, false /* utf8 */); - break; - case FieldDescriptor::TYPE_MESSAGE: { - if (field->is_map()) { - const FieldDescriptor* val = - field->message_type()->FindFieldByName("value"); - GOOGLE_CHECK(val); - if (val->type() == FieldDescriptor::TYPE_ENUM && - !HasPreservingUnknownEnumSemantics(field)) { - format_( - "auto object = " - "::$proto_ns$::internal::InitEnumParseWrapper<$unknown_" - "fields_type$>(" - "&$1$_, $2$_IsValid, $3$, &_internal_metadata_);\n" - "ptr = ctx->ParseMessage(&object, ptr);\n", - FieldName(field), QualifiedClassName(val->enum_type()), - field->number()); - } else { - format_("ptr = ctx->ParseMessage(&$1$_, ptr);\n", - FieldName(field)); - } - } else if (IsLazy(field, options_)) { - if (field->real_containing_oneof()) { - format_( - "if (!_internal_has_$1$()) {\n" - " clear_$2$();\n" - " $2$_.$1$_ = ::$proto_ns$::Arena::CreateMessage<\n" - " $pi_ns$::LazyField>(GetArena());\n" - " set_has_$1$();\n" - "}\n" - "ptr = ctx->ParseMessage($2$_.$1$_, ptr);\n", - FieldName(field), field->containing_oneof()->name()); - } else if (HasHasbit(field)) { - format_( - "_Internal::set_has_$1$(&$has_bits$);\n" - "ptr = ctx->ParseMessage(&$1$_, ptr);\n", - FieldName(field)); - } else { - format_("ptr = ctx->ParseMessage(&$1$_, ptr);\n", - FieldName(field)); - } - } else if (IsImplicitWeakField(field, options_, scc_analyzer_)) { - if (!field->is_repeated()) { - format_( - "ptr = ctx->ParseMessage(_Internal::mutable_$1$(this), " - "ptr);\n", - FieldName(field)); - } else { - format_( - "ptr = ctx->ParseMessage($1$_.AddWeak(reinterpret_cast($2$::_$3$_default_instance_ptr_)" - "), ptr);\n", - FieldName(field), Namespace(field->message_type(), options_), - ClassName(field->message_type())); - } - } else if (IsWeak(field, options_)) { - format_( - "{\n" - " auto* default_ = &reinterpret_cast($1$);\n" - " ptr = ctx->ParseMessage(_weak_field_map_.MutableMessage($2$," - " default_), ptr);\n" - "}\n", - QualifiedDefaultInstanceName(field->message_type(), options_), - field->number()); - } else { - format_("ptr = ctx->ParseMessage(_internal_$1$_$2$(), ptr);\n", - field->is_repeated() ? "add" : "mutable", FieldName(field)); - } - break; - } - default: - GOOGLE_LOG(FATAL) << "Illegal combination for length delimited wiretype " - << " filed type is " << field->type(); - } - } - } - - // Convert a 1 or 2 byte varint into the equivalent value upon a direct load. - static uint32_t SmallVarintValue(uint32_t x) { - GOOGLE_DCHECK(x < 128 * 128); - if (x >= 128) x += (x & 0xFF80) + 128; - return x; - } - - static bool ShouldRepeat(const FieldDescriptor* descriptor, - internal::WireFormatLite::WireType wiretype) { - constexpr int kMaxTwoByteFieldNumber = 16 * 128; - return descriptor->number() < kMaxTwoByteFieldNumber && - descriptor->is_repeated() && - (!descriptor->is_packable() || - wiretype != internal::WireFormatLite::WIRETYPE_LENGTH_DELIMITED); - } - - void GenerateFieldBody(internal::WireFormatLite::WireType wiretype, - const FieldDescriptor* field) { - uint32_t tag = WireFormatLite::MakeTag(field->number(), wiretype); - switch (wiretype) { - case WireFormatLite::WIRETYPE_VARINT: { - std::string type = PrimitiveTypeName(options_, field->cpp_type()); - std::string prefix = field->is_repeated() ? "add" : "set"; - if (field->type() == FieldDescriptor::TYPE_ENUM) { - format_( - "$uint64$ val = $pi_ns$::ReadVarint64(&ptr);\n" - "CHK_(ptr);\n"); - if (!HasPreservingUnknownEnumSemantics(field)) { - format_("if (PROTOBUF_PREDICT_TRUE($1$_IsValid(val))) {\n", - QualifiedClassName(field->enum_type(), options_)); - format_.Indent(); - } - format_("_internal_$1$_$2$(static_cast<$3$>(val));\n", prefix, - FieldName(field), - QualifiedClassName(field->enum_type(), options_)); - if (!HasPreservingUnknownEnumSemantics(field)) { - format_.Outdent(); - format_( - "} else {\n" - " $pi_ns$::WriteVarint($1$, val, mutable_unknown_fields());\n" - "}\n", - field->number()); - } - } else { - std::string size = (field->type() == FieldDescriptor::TYPE_SINT32 || - field->type() == FieldDescriptor::TYPE_UINT32) - ? "32" - : "64"; - std::string zigzag; - if ((field->type() == FieldDescriptor::TYPE_SINT32 || - field->type() == FieldDescriptor::TYPE_SINT64)) { - zigzag = "ZigZag"; - } - if (field->is_repeated() || field->real_containing_oneof()) { - std::string prefix = field->is_repeated() ? "add" : "set"; - format_( - "_internal_$1$_$2$($pi_ns$::ReadVarint$3$$4$(&ptr));\n" - "CHK_(ptr);\n", - prefix, FieldName(field), zigzag, size); - } else { - if (HasHasbit(field)) { - format_("_Internal::set_has_$1$(&$has_bits$);\n", - FieldName(field)); - } - format_( - "$1$_ = $pi_ns$::ReadVarint$2$$3$(&ptr);\n" - "CHK_(ptr);\n", - FieldName(field), zigzag, size); - } - } - break; - } - case WireFormatLite::WIRETYPE_FIXED32: - case WireFormatLite::WIRETYPE_FIXED64: { - std::string type = PrimitiveTypeName(options_, field->cpp_type()); - if (field->is_repeated() || field->real_containing_oneof()) { - std::string prefix = field->is_repeated() ? "add" : "set"; - format_( - "_internal_$1$_$2$($pi_ns$::UnalignedLoad<$3$>(ptr));\n" - "ptr += sizeof($3$);\n", - prefix, FieldName(field), type); - } else { - if (HasHasbit(field)) { - format_("_Internal::set_has_$1$(&$has_bits$);\n", FieldName(field)); - } - format_( - "$1$_ = $pi_ns$::UnalignedLoad<$2$>(ptr);\n" - "ptr += sizeof($2$);\n", - FieldName(field), type); - } - break; - } - case WireFormatLite::WIRETYPE_LENGTH_DELIMITED: { - GenerateLengthDelim(field); - format_("CHK_(ptr);\n"); - break; - } - case WireFormatLite::WIRETYPE_START_GROUP: { - format_( - "ptr = ctx->ParseGroup(_internal_$1$_$2$(), ptr, $3$);\n" - "CHK_(ptr);\n", - field->is_repeated() ? "add" : "mutable", FieldName(field), tag); - break; - } - case WireFormatLite::WIRETYPE_END_GROUP: { - GOOGLE_LOG(FATAL) << "Can't have end group field\n"; - break; - } - } // switch (wire_type) - } - - // Returns the tag for this field and in case of repeated packable fields, - // sets a fallback tag in fallback_tag_ptr. - static uint32_t ExpectedTag(const FieldDescriptor* field, - uint32_t* fallback_tag_ptr) { - uint32_t expected_tag; - if (field->is_packable()) { - auto expected_wiretype = WireFormat::WireTypeForFieldType(field->type()); - expected_tag = - WireFormatLite::MakeTag(field->number(), expected_wiretype); - GOOGLE_CHECK(expected_wiretype != WireFormatLite::WIRETYPE_LENGTH_DELIMITED); - auto fallback_wiretype = WireFormatLite::WIRETYPE_LENGTH_DELIMITED; - uint32_t fallback_tag = - WireFormatLite::MakeTag(field->number(), fallback_wiretype); - - if (field->is_packed()) std::swap(expected_tag, fallback_tag); - *fallback_tag_ptr = fallback_tag; - } else { - auto expected_wiretype = WireFormat::WireTypeForField(field); - expected_tag = - WireFormatLite::MakeTag(field->number(), expected_wiretype); - } - return expected_tag; - } - - void GenerateParseLoop( - const Descriptor* descriptor, - const std::vector& ordered_fields) { - format_( - "while (!ctx->Done(&ptr)) {\n" - " $uint32$ tag;\n" - " ptr = $pi_ns$::ReadTag(ptr, &tag);\n"); - if (!ordered_fields.empty()) format_(" switch (tag >> 3) {\n"); - - format_.Indent(); - format_.Indent(); - - for (const auto* field : ordered_fields) { - PrintFieldComment(format_, field); - format_("case $1$:\n", field->number()); - format_.Indent(); - uint32_t fallback_tag = 0; - uint32_t expected_tag = ExpectedTag(field, &fallback_tag); - format_( - "if (PROTOBUF_PREDICT_TRUE(static_cast<$uint8$>(tag) == $1$)) {\n", - expected_tag & 0xFF); - format_.Indent(); - auto wiretype = WireFormatLite::GetTagWireType(expected_tag); - uint32_t tag = WireFormatLite::MakeTag(field->number(), wiretype); - int tag_size = io::CodedOutputStream::VarintSize32(tag); - bool is_repeat = ShouldRepeat(field, wiretype); - if (is_repeat) { - format_( - "ptr -= $1$;\n" - "do {\n" - " ptr += $1$;\n", - tag_size); - format_.Indent(); - } - GenerateFieldBody(wiretype, field); - if (is_repeat) { - format_.Outdent(); - format_( - " if (!ctx->DataAvailable(ptr)) break;\n" - "} while ($pi_ns$::ExpectTag<$1$>(ptr));\n", - tag); - } - format_.Outdent(); - if (fallback_tag) { - format_("} else if (static_cast<$uint8$>(tag) == $1$) {\n", - fallback_tag & 0xFF); - format_.Indent(); - GenerateFieldBody(WireFormatLite::GetTagWireType(fallback_tag), field); - format_.Outdent(); - } - format_.Outdent(); - format_( - " } else goto handle_unusual;\n" - " continue;\n"); - } // for loop over ordered fields - - // Default case - if (!ordered_fields.empty()) format_("default: {\n"); - if (!ordered_fields.empty()) format_("handle_unusual:\n"); - format_( - " if ((tag == 0) || ((tag & 7) == 4)) {\n" - " CHK_(ptr);\n" - " ctx->SetLastTag(tag);\n" - " goto success;\n" - " }\n"); - if (IsMapEntryMessage(descriptor)) { - format_(" continue;\n"); - } else { - if (descriptor->extension_range_count() > 0) { - format_("if ("); - for (int i = 0; i < descriptor->extension_range_count(); i++) { - const Descriptor::ExtensionRange* range = - descriptor->extension_range(i); - if (i > 0) format_(" ||\n "); - - uint32_t start_tag = WireFormatLite::MakeTag( - range->start, static_cast(0)); - uint32_t end_tag = WireFormatLite::MakeTag( - range->end, static_cast(0)); - - if (range->end > FieldDescriptor::kMaxNumber) { - format_("($1$u <= tag)", start_tag); - } else { - format_("($1$u <= tag && tag < $2$u)", start_tag, end_tag); - } - } - format_(") {\n"); - format_( - " ptr = _extensions_.ParseField(tag, ptr,\n" - " internal_default_instance(), &_internal_metadata_, ctx);\n" - " CHK_(ptr != nullptr);\n" - " continue;\n" - "}\n"); - } - format_( - " ptr = UnknownFieldParse(tag,\n" - " _internal_metadata_.mutable_unknown_fields<$unknown_" - "fields_type$>(),\n" - " ptr, ctx);\n" - " CHK_(ptr != nullptr);\n" - " continue;\n"); - } - if (!ordered_fields.empty()) format_("}\n"); // default case - format_.Outdent(); - format_.Outdent(); - if (!ordered_fields.empty()) format_(" } // switch\n"); - format_("} // while\n"); - } -}; - -void GenerateParserLoop(const Descriptor* descriptor, int num_hasbits, - const Options& options, - MessageSCCAnalyzer* scc_analyzer, - io::Printer* printer) { - ParseLoopGenerator generator(num_hasbits, options, scc_analyzer, printer); - generator.GenerateParserLoop(descriptor); -} - static bool HasExtensionFromFile(const Message& msg, const FileDescriptor* file, const Options& options, bool* has_opt_codesize_extension) { diff --git a/src/google/protobuf/compiler/cpp/cpp_helpers.h b/src/google/protobuf/compiler/cpp/cpp_helpers.h index 8e509c18d5..5fa019d216 100644 --- a/src/google/protobuf/compiler/cpp/cpp_helpers.h +++ b/src/google/protobuf/compiler/cpp/cpp_helpers.h @@ -418,8 +418,8 @@ bool IsStringOrMessage(const FieldDescriptor* field); std::string UnderscoresToCamelCase(const std::string& input, bool cap_next_letter); -inline bool HasFieldPresence(const FileDescriptor* file) { - return file->syntax() != FileDescriptor::SYNTAX_PROTO3; +inline bool IsProto3(const FileDescriptor* file) { + return file->syntax() == FileDescriptor::SYNTAX_PROTO3; } inline bool HasHasbit(const FieldDescriptor* field) { @@ -865,10 +865,6 @@ struct OneOfRangeImpl { inline OneOfRangeImpl OneOfRange(const Descriptor* desc) { return {desc}; } -void GenerateParserLoop(const Descriptor* descriptor, int num_hasbits, - const Options& options, - MessageSCCAnalyzer* scc_analyzer, io::Printer* printer); - PROTOC_EXPORT std::string StripProto(const std::string& filename); } // namespace cpp diff --git a/src/google/protobuf/compiler/cpp/cpp_map_field.cc b/src/google/protobuf/compiler/cpp/cpp_map_field.cc index c9c30e0c63..44c913868a 100644 --- a/src/google/protobuf/compiler/cpp/cpp_map_field.cc +++ b/src/google/protobuf/compiler/cpp/cpp_map_field.cc @@ -157,7 +157,7 @@ void MapFieldGenerator::GenerateMergingCode(io::Printer* printer) const { void MapFieldGenerator::GenerateSwappingCode(io::Printer* printer) const { Formatter format(printer, variables_); - format("$name$_.Swap(&other->$name$_);\n"); + format("$name$_.InternalSwap(&other->$name$_);\n"); } void MapFieldGenerator::GenerateCopyConstructorCode( diff --git a/src/google/protobuf/compiler/cpp/cpp_message.cc b/src/google/protobuf/compiler/cpp/cpp_message.cc index 0a41acfd5f..f2d1aabf5f 100644 --- a/src/google/protobuf/compiler/cpp/cpp_message.cc +++ b/src/google/protobuf/compiler/cpp/cpp_message.cc @@ -48,6 +48,7 @@ #include #include #include +#include #include #include #include @@ -237,7 +238,7 @@ bool EmitFieldNonDefaultCondition(io::Printer* printer, // Does the given field have a has_$name$() method? bool HasHasMethod(const FieldDescriptor* field) { - if (HasFieldPresence(field->file())) { + if (!IsProto3(field->file())) { // In proto1/proto2, every field has a has_$name$() method. return true; } @@ -276,7 +277,7 @@ void CollectMapInfo(const Options& options, const Descriptor* descriptor, bool HasPrivateHasMethod(const FieldDescriptor* field) { // Only for oneofs in message types with no field presence. has_$name$(), // based on the oneof case, is still useful internally for generated code. - return (!HasFieldPresence(field->file()) && field->real_containing_oneof()); + return IsProto3(field->file()) && field->real_containing_oneof(); } // TODO(ckennelly): Cull these exclusions if/when these protos do not have @@ -2896,15 +2897,13 @@ void MessageGenerator::GenerateSwap(io::Printer* printer) { if (HasGeneratedMethods(descriptor_->file(), options_)) { if (descriptor_->extension_range_count() > 0) { - format("_extensions_.Swap(&other->_extensions_);\n"); + format("_extensions_.InternalSwap(&other->_extensions_);\n"); } std::map vars; SetUnknkownFieldsVariable(descriptor_, options_, &vars); format.AddMap(vars); - format( - "_internal_metadata_.Swap<$unknown_fields_type$>(&other->_internal_" - "metadata_);\n"); + format("_internal_metadata_.InternalSwap(&other->_internal_metadata_);\n"); if (!has_bit_indices_.empty()) { for (int i = 0; i < HasBitsSize(); ++i) { @@ -3255,8 +3254,8 @@ void MessageGenerator::GenerateMergeFromCodedStream(io::Printer* printer) { "}\n"); return; } - GenerateParserLoop(descriptor_, max_has_bit_index_, options_, scc_analyzer_, - printer); + GenerateParseFunction(descriptor_, max_has_bit_index_, options_, + scc_analyzer_, printer); } void MessageGenerator::GenerateSerializeOneofFields( @@ -3410,7 +3409,7 @@ void MessageGenerator::GenerateSerializeWithCachedSizesBody( LazySerializerEmitter(MessageGenerator* mg, io::Printer* printer) : mg_(mg), format_(printer), - eager_(!HasFieldPresence(mg->descriptor_->file())), + eager_(IsProto3(mg->descriptor_->file())), cached_has_bit_index_(kNoHasbit) {} ~LazySerializerEmitter() { Flush(); } diff --git a/src/google/protobuf/compiler/cpp/cpp_message_field.cc b/src/google/protobuf/compiler/cpp/cpp_message_field.cc index 7bad24ca84..18c6f0930a 100644 --- a/src/google/protobuf/compiler/cpp/cpp_message_field.cc +++ b/src/google/protobuf/compiler/cpp/cpp_message_field.cc @@ -125,7 +125,8 @@ void MessageFieldGenerator::GenerateAccessorDeclarations( } format( "$deprecated_attr$const $type$& ${1$$name$$}$() const;\n" - "$deprecated_attr$$type$* ${1$$release_name$$}$();\n" + "PROTOBUF_FUTURE_MUST_USE_RESULT $deprecated_attr$$type$* " + "${1$$release_name$$}$();\n" "$deprecated_attr$$type$* ${1$mutable_$name$$}$();\n" "$deprecated_attr$void ${1$set_allocated_$name$$}$" "($type$* $name$);\n", @@ -320,7 +321,7 @@ void MessageFieldGenerator::GenerateInternalAccessorDefinitions( format( "::$proto_ns$::MessageLite*\n" "$classname$::_Internal::mutable_$name$($classname$* msg) {\n"); - if (HasFieldPresence(descriptor_->file())) { + if (HasHasbit(descriptor_)) { format(" msg->$set_hasbit$\n"); } format( @@ -353,7 +354,7 @@ void MessageFieldGenerator::GenerateClearingCode(io::Printer* printer) const { GOOGLE_CHECK(!IsFieldStripped(descriptor_, options_)); Formatter format(printer, variables_); - if (!HasFieldPresence(descriptor_->file())) { + if (!HasHasbit(descriptor_)) { // If we don't have has-bits, message presence is indicated only by ptr != // NULL. Thus on clear, we need to delete the object. format( @@ -371,7 +372,7 @@ void MessageFieldGenerator::GenerateMessageClearingCode( GOOGLE_CHECK(!IsFieldStripped(descriptor_, options_)); Formatter format(printer, variables_); - if (!HasFieldPresence(descriptor_->file())) { + if (!HasHasbit(descriptor_)) { // If we don't have has-bits, message presence is indicated only by ptr != // NULL. Thus on clear, we need to delete the object. format( diff --git a/src/google/protobuf/compiler/cpp/cpp_parse_function_generator.cc b/src/google/protobuf/compiler/cpp/cpp_parse_function_generator.cc new file mode 100644 index 0000000000..41be0b51fd --- /dev/null +++ b/src/google/protobuf/compiler/cpp/cpp_parse_function_generator.cc @@ -0,0 +1,580 @@ +// Protocol Buffers - Google's data interchange format +// Copyright 2008 Google Inc. All rights reserved. +// https://developers.google.com/protocol-buffers/ +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +#include + +#include +#include + +namespace google { +namespace protobuf { +namespace compiler { +namespace cpp { + +namespace { +using google::protobuf::internal::WireFormat; +using google::protobuf::internal::WireFormatLite; + +std::vector GetOrderedFields( + const Descriptor* descriptor, const Options& options) { + std::vector ordered_fields; + for (auto field : FieldRange(descriptor)) { + if (!IsFieldStripped(field, options)) { + ordered_fields.push_back(field); + } + } + std::sort(ordered_fields.begin(), ordered_fields.end(), + [](const FieldDescriptor* a, const FieldDescriptor* b) { + return a->number() < b->number(); + }); + return ordered_fields; +} + +bool HasInternalAccessors(const FieldOptions::CType ctype) { + return ctype == FieldOptions::STRING || ctype == FieldOptions::CORD; +} + +class ParseFunctionGenerator { + public: + ParseFunctionGenerator(const Descriptor* descriptor, int num_hasbits, + const Options& options, + MessageSCCAnalyzer* scc_analyzer, io::Printer* printer) + : descriptor_(descriptor), + scc_analyzer_(scc_analyzer), + options_(options), + format_(printer), + num_hasbits_(num_hasbits) { + format_.Set("classname", ClassName(descriptor)); + format_.Set("p_ns", "::" + ProtobufNamespace(options_)); + format_.Set("pi_ns", + StrCat("::", ProtobufNamespace(options_), "::internal")); + format_.Set("GOOGLE_PROTOBUF", MacroPrefix(options_)); + std::map vars; + SetCommonVars(options_, &vars); + SetUnknkownFieldsVariable(descriptor, options_, &vars); + format_.AddMap(vars); + } + + void GenerateLoopingParseFunction() { + format_( + "const char* $classname$::_InternalParse(const char* ptr, " + "$pi_ns$::ParseContext* ctx) {\n" + "#define CHK_(x) if (PROTOBUF_PREDICT_FALSE(!(x))) goto failure\n"); + format_.Indent(); + int hasbits_size = 0; + if (num_hasbits_ > 0) { + hasbits_size = (num_hasbits_ + 31) / 32; + } + // For now only optimize small hasbits. + if (hasbits_size != 1) hasbits_size = 0; + if (hasbits_size) { + format_("_Internal::HasBits has_bits{};\n"); + format_.Set("has_bits", "has_bits"); + } else { + format_.Set("has_bits", "_has_bits_"); + } + format_.Set("continue", "continue"); + format_("while (!ctx->Done(&ptr)) {\n"); + format_.Indent(); + + GenerateParseIterationBody(descriptor_, + GetOrderedFields(descriptor_, options_)); + + format_.Outdent(); + format_("} // while\n"); + + format_.Outdent(); + format_("success:\n"); + if (hasbits_size) format_(" _has_bits_.Or(has_bits);\n"); + + format_( + " return ptr;\n" + "failure:\n" + " ptr = nullptr;\n" + " goto success;\n" + "#undef CHK_\n" + "}\n"); + } + + private: + const Descriptor* descriptor_; + MessageSCCAnalyzer* scc_analyzer_; + const Options& options_; + Formatter format_; + int num_hasbits_; + + void GenerateArenaString(const FieldDescriptor* field) { + if (HasHasbit(field)) { + format_("_Internal::set_has_$1$(&$has_bits$);\n", FieldName(field)); + } + std::string default_string = + field->default_value_string().empty() + ? "::" + ProtobufNamespace(options_) + + "::internal::GetEmptyStringAlreadyInited()" + : QualifiedClassName(field->containing_type(), options_) + + "::" + MakeDefaultName(field) + ".get()"; + format_( + "if (arena != nullptr) {\n" + " ptr = ctx->ReadArenaString(ptr, &$1$_, arena);\n" + "} else {\n" + " ptr = " + "$pi_ns$::InlineGreedyStringParser($1$_.MutableNoArenaNoDefault(&$2$" + "), ptr, ctx);" + "\n}\n" + "const std::string* str = &$1$_.Get(); (void)str;\n", + FieldName(field), default_string); + } + + void GenerateStrings(const FieldDescriptor* field, bool check_utf8) { + FieldOptions::CType ctype = FieldOptions::STRING; + if (!options_.opensource_runtime) { + // Open source doesn't support other ctypes; + ctype = field->options().ctype(); + } + if (!field->is_repeated() && !options_.opensource_runtime && + GetOptimizeFor(field->file(), options_) != FileOptions::LITE_RUNTIME && + // For now only use arena string for strings with empty defaults. + field->default_value_string().empty() && + !field->real_containing_oneof() && ctype == FieldOptions::STRING) { + GenerateArenaString(field); + } else { + std::string name; + switch (ctype) { + case FieldOptions::STRING: + name = "GreedyStringParser"; + break; + case FieldOptions::CORD: + name = "CordParser"; + break; + case FieldOptions::STRING_PIECE: + name = "StringPieceParser"; + break; + } + format_( + "auto str = $1$$2$_$3$();\n" + "ptr = $pi_ns$::Inline$4$(str, ptr, ctx);\n", + HasInternalAccessors(ctype) ? "_internal_" : "", + field->is_repeated() && !field->is_packable() ? "add" : "mutable", + FieldName(field), name); + } + if (!check_utf8) return; // return if this is a bytes field + auto level = GetUtf8CheckMode(field, options_); + switch (level) { + case Utf8CheckMode::kNone: + return; + case Utf8CheckMode::kVerify: + format_("#ifndef NDEBUG\n"); + break; + case Utf8CheckMode::kStrict: + format_("CHK_("); + break; + } + std::string field_name; + field_name = "nullptr"; + if (HasDescriptorMethods(field->file(), options_)) { + field_name = StrCat("\"", field->full_name(), "\""); + } + format_("$pi_ns$::VerifyUTF8(str, $1$)", field_name); + switch (level) { + case Utf8CheckMode::kNone: + return; + case Utf8CheckMode::kVerify: + format_( + ";\n" + "#endif // !NDEBUG\n"); + break; + case Utf8CheckMode::kStrict: + format_(");\n"); + break; + } + } + + void GenerateLengthDelim(const FieldDescriptor* field) { + if (field->is_packable()) { + std::string enum_validator; + if (field->type() == FieldDescriptor::TYPE_ENUM && + !HasPreservingUnknownEnumSemantics(field)) { + enum_validator = + StrCat(", ", QualifiedClassName(field->enum_type(), options_), + "_IsValid, &_internal_metadata_, ", field->number()); + format_( + "ptr = " + "$pi_ns$::Packed$1$Parser<$unknown_fields_type$>(_internal_mutable_" + "$2$(), ptr, " + "ctx$3$);\n", + DeclaredTypeMethodName(field->type()), FieldName(field), + enum_validator); + } else { + format_( + "ptr = $pi_ns$::Packed$1$Parser(_internal_mutable_$2$(), ptr, " + "ctx$3$);\n", + DeclaredTypeMethodName(field->type()), FieldName(field), + enum_validator); + } + } else { + auto field_type = field->type(); + switch (field_type) { + case FieldDescriptor::TYPE_STRING: + GenerateStrings(field, true /* utf8 */); + break; + case FieldDescriptor::TYPE_BYTES: + GenerateStrings(field, false /* utf8 */); + break; + case FieldDescriptor::TYPE_MESSAGE: { + if (field->is_map()) { + const FieldDescriptor* val = + field->message_type()->FindFieldByName("value"); + GOOGLE_CHECK(val); + if (val->type() == FieldDescriptor::TYPE_ENUM && + !HasPreservingUnknownEnumSemantics(field)) { + format_( + "auto object = " + "::$proto_ns$::internal::InitEnumParseWrapper<$unknown_" + "fields_type$>(" + "&$1$_, $2$_IsValid, $3$, &_internal_metadata_);\n" + "ptr = ctx->ParseMessage(&object, ptr);\n", + FieldName(field), QualifiedClassName(val->enum_type()), + field->number()); + } else { + format_("ptr = ctx->ParseMessage(&$1$_, ptr);\n", + FieldName(field)); + } + } else if (IsLazy(field, options_)) { + if (field->real_containing_oneof()) { + format_( + "if (!_internal_has_$1$()) {\n" + " clear_$2$();\n" + " $2$_.$1$_ = ::$proto_ns$::Arena::CreateMessage<\n" + " $pi_ns$::LazyField>(GetArena());\n" + " set_has_$1$();\n" + "}\n" + "ptr = ctx->ParseMessage($2$_.$1$_, ptr);\n", + FieldName(field), field->containing_oneof()->name()); + } else if (HasHasbit(field)) { + format_( + "_Internal::set_has_$1$(&$has_bits$);\n" + "ptr = ctx->ParseMessage(&$1$_, ptr);\n", + FieldName(field)); + } else { + format_("ptr = ctx->ParseMessage(&$1$_, ptr);\n", + FieldName(field)); + } + } else if (IsImplicitWeakField(field, options_, scc_analyzer_)) { + if (!field->is_repeated()) { + format_( + "ptr = ctx->ParseMessage(_Internal::mutable_$1$(this), " + "ptr);\n", + FieldName(field)); + } else { + format_( + "ptr = ctx->ParseMessage($1$_.AddWeak(reinterpret_cast($2$::_$3$_default_instance_ptr_)" + "), ptr);\n", + FieldName(field), Namespace(field->message_type(), options_), + ClassName(field->message_type())); + } + } else if (IsWeak(field, options_)) { + format_( + "{\n" + " auto* default_ = &reinterpret_cast($1$);\n" + " ptr = ctx->ParseMessage(_weak_field_map_.MutableMessage($2$," + " default_), ptr);\n" + "}\n", + QualifiedDefaultInstanceName(field->message_type(), options_), + field->number()); + } else { + format_("ptr = ctx->ParseMessage(_internal_$1$_$2$(), ptr);\n", + field->is_repeated() ? "add" : "mutable", FieldName(field)); + } + break; + } + default: + GOOGLE_LOG(FATAL) << "Illegal combination for length delimited wiretype " + << " filed type is " << field->type(); + } + } + } + + // Convert a 1 or 2 byte varint into the equivalent value upon a direct load. + static uint32_t SmallVarintValue(uint32_t x) { + GOOGLE_DCHECK(x < 128 * 128); + if (x >= 128) x += (x & 0xFF80) + 128; + return x; + } + + static bool ShouldRepeat(const FieldDescriptor* descriptor, + internal::WireFormatLite::WireType wiretype) { + constexpr int kMaxTwoByteFieldNumber = 16 * 128; + return descriptor->number() < kMaxTwoByteFieldNumber && + descriptor->is_repeated() && + (!descriptor->is_packable() || + wiretype != internal::WireFormatLite::WIRETYPE_LENGTH_DELIMITED); + } + + void GenerateFieldBody(internal::WireFormatLite::WireType wiretype, + const FieldDescriptor* field) { + uint32_t tag = WireFormatLite::MakeTag(field->number(), wiretype); + switch (wiretype) { + case WireFormatLite::WIRETYPE_VARINT: { + std::string type = PrimitiveTypeName(options_, field->cpp_type()); + std::string prefix = field->is_repeated() ? "add" : "set"; + if (field->type() == FieldDescriptor::TYPE_ENUM) { + format_( + "$uint64$ val = $pi_ns$::ReadVarint64(&ptr);\n" + "CHK_(ptr);\n"); + if (!HasPreservingUnknownEnumSemantics(field)) { + format_("if (PROTOBUF_PREDICT_TRUE($1$_IsValid(val))) {\n", + QualifiedClassName(field->enum_type(), options_)); + format_.Indent(); + } + format_("_internal_$1$_$2$(static_cast<$3$>(val));\n", prefix, + FieldName(field), + QualifiedClassName(field->enum_type(), options_)); + if (!HasPreservingUnknownEnumSemantics(field)) { + format_.Outdent(); + format_( + "} else {\n" + " $pi_ns$::WriteVarint($1$, val, mutable_unknown_fields());\n" + "}\n", + field->number()); + } + } else { + std::string size = (field->type() == FieldDescriptor::TYPE_SINT32 || + field->type() == FieldDescriptor::TYPE_UINT32) + ? "32" + : "64"; + std::string zigzag; + if ((field->type() == FieldDescriptor::TYPE_SINT32 || + field->type() == FieldDescriptor::TYPE_SINT64)) { + zigzag = "ZigZag"; + } + if (field->is_repeated() || field->real_containing_oneof()) { + std::string prefix = field->is_repeated() ? "add" : "set"; + format_( + "_internal_$1$_$2$($pi_ns$::ReadVarint$3$$4$(&ptr));\n" + "CHK_(ptr);\n", + prefix, FieldName(field), zigzag, size); + } else { + if (HasHasbit(field)) { + format_("_Internal::set_has_$1$(&$has_bits$);\n", + FieldName(field)); + } + format_( + "$1$_ = $pi_ns$::ReadVarint$2$$3$(&ptr);\n" + "CHK_(ptr);\n", + FieldName(field), zigzag, size); + } + } + break; + } + case WireFormatLite::WIRETYPE_FIXED32: + case WireFormatLite::WIRETYPE_FIXED64: { + std::string type = PrimitiveTypeName(options_, field->cpp_type()); + if (field->is_repeated() || field->real_containing_oneof()) { + std::string prefix = field->is_repeated() ? "add" : "set"; + format_( + "_internal_$1$_$2$($pi_ns$::UnalignedLoad<$3$>(ptr));\n" + "ptr += sizeof($3$);\n", + prefix, FieldName(field), type); + } else { + if (HasHasbit(field)) { + format_("_Internal::set_has_$1$(&$has_bits$);\n", FieldName(field)); + } + format_( + "$1$_ = $pi_ns$::UnalignedLoad<$2$>(ptr);\n" + "ptr += sizeof($2$);\n", + FieldName(field), type); + } + break; + } + case WireFormatLite::WIRETYPE_LENGTH_DELIMITED: { + GenerateLengthDelim(field); + format_("CHK_(ptr);\n"); + break; + } + case WireFormatLite::WIRETYPE_START_GROUP: { + format_( + "ptr = ctx->ParseGroup(_internal_$1$_$2$(), ptr, $3$);\n" + "CHK_(ptr);\n", + field->is_repeated() ? "add" : "mutable", FieldName(field), tag); + break; + } + case WireFormatLite::WIRETYPE_END_GROUP: { + GOOGLE_LOG(FATAL) << "Can't have end group field\n"; + break; + } + } // switch (wire_type) + } + + // Returns the tag for this field and in case of repeated packable fields, + // sets a fallback tag in fallback_tag_ptr. + static uint32_t ExpectedTag(const FieldDescriptor* field, + uint32_t* fallback_tag_ptr) { + uint32_t expected_tag; + if (field->is_packable()) { + auto expected_wiretype = WireFormat::WireTypeForFieldType(field->type()); + expected_tag = + WireFormatLite::MakeTag(field->number(), expected_wiretype); + GOOGLE_CHECK(expected_wiretype != WireFormatLite::WIRETYPE_LENGTH_DELIMITED); + auto fallback_wiretype = WireFormatLite::WIRETYPE_LENGTH_DELIMITED; + uint32_t fallback_tag = + WireFormatLite::MakeTag(field->number(), fallback_wiretype); + + if (field->is_packed()) std::swap(expected_tag, fallback_tag); + *fallback_tag_ptr = fallback_tag; + } else { + auto expected_wiretype = WireFormat::WireTypeForField(field); + expected_tag = + WireFormatLite::MakeTag(field->number(), expected_wiretype); + } + return expected_tag; + } + + void GenerateParseIterationBody( + const Descriptor* descriptor, + const std::vector& ordered_fields) { + format_( + "$uint32$ tag;\n" + "ptr = $pi_ns$::ReadTag(ptr, &tag);\n"); + if (!ordered_fields.empty()) format_("switch (tag >> 3) {\n"); + + format_.Indent(); + + for (const auto* field : ordered_fields) { + PrintFieldComment(format_, field); + format_("case $1$:\n", field->number()); + format_.Indent(); + uint32_t fallback_tag = 0; + uint32_t expected_tag = ExpectedTag(field, &fallback_tag); + format_( + "if (PROTOBUF_PREDICT_TRUE(static_cast<$uint8$>(tag) == $1$)) {\n", + expected_tag & 0xFF); + format_.Indent(); + auto wiretype = WireFormatLite::GetTagWireType(expected_tag); + uint32_t tag = WireFormatLite::MakeTag(field->number(), wiretype); + int tag_size = io::CodedOutputStream::VarintSize32(tag); + bool is_repeat = ShouldRepeat(field, wiretype); + if (is_repeat) { + format_( + "ptr -= $1$;\n" + "do {\n" + " ptr += $1$;\n", + tag_size); + format_.Indent(); + } + GenerateFieldBody(wiretype, field); + if (is_repeat) { + format_.Outdent(); + format_( + " if (!ctx->DataAvailable(ptr)) break;\n" + "} while ($pi_ns$::ExpectTag<$1$>(ptr));\n", + tag); + } + format_.Outdent(); + if (fallback_tag) { + format_("} else if (static_cast<$uint8$>(tag) == $1$) {\n", + fallback_tag & 0xFF); + format_.Indent(); + GenerateFieldBody(WireFormatLite::GetTagWireType(fallback_tag), field); + format_.Outdent(); + } + format_.Outdent(); + format_( + " } else goto handle_unusual;\n" + " $continue$;\n"); + } // for loop over ordered fields + + // Default case + if (!ordered_fields.empty()) format_("default: {\n"); + if (!ordered_fields.empty()) format_("handle_unusual:\n"); + format_( + " if ((tag == 0) || ((tag & 7) == 4)) {\n" + " CHK_(ptr);\n" + " ctx->SetLastTag(tag);\n" + " goto success;\n" + " }\n"); + if (IsMapEntryMessage(descriptor)) { + format_(" $continue$;\n"); + } else { + if (descriptor->extension_range_count() > 0) { + format_("if ("); + for (int i = 0; i < descriptor->extension_range_count(); i++) { + const Descriptor::ExtensionRange* range = + descriptor->extension_range(i); + if (i > 0) format_(" ||\n "); + + uint32_t start_tag = WireFormatLite::MakeTag( + range->start, static_cast(0)); + uint32_t end_tag = WireFormatLite::MakeTag( + range->end, static_cast(0)); + + if (range->end > FieldDescriptor::kMaxNumber) { + format_("($1$u <= tag)", start_tag); + } else { + format_("($1$u <= tag && tag < $2$u)", start_tag, end_tag); + } + } + format_(") {\n"); + format_( + " ptr = _extensions_.ParseField(tag, ptr,\n" + " internal_default_instance(), &_internal_metadata_, ctx);\n" + " CHK_(ptr != nullptr);\n" + " $continue$;\n" + "}\n"); + } + format_( + " ptr = UnknownFieldParse(tag,\n" + " _internal_metadata_.mutable_unknown_fields<$unknown_" + "fields_type$>(),\n" + " ptr, ctx);\n" + " CHK_(ptr != nullptr);\n" + " $continue$;\n"); + } + if (!ordered_fields.empty()) format_("}\n"); // default case + format_.Outdent(); + if (!ordered_fields.empty()) format_("} // switch\n"); + } +}; + +} // namespace + +void GenerateParseFunction(const Descriptor* descriptor, int num_hasbits, + const Options& options, + MessageSCCAnalyzer* scc_analyzer, + io::Printer* printer) { + ParseFunctionGenerator generator(descriptor, num_hasbits, options, + scc_analyzer, printer); + generator.GenerateLoopingParseFunction(); +} + +} // namespace cpp +} // namespace compiler +} // namespace protobuf +} // namespace google diff --git a/src/google/protobuf/compiler/cpp/cpp_parse_function_generator.h b/src/google/protobuf/compiler/cpp/cpp_parse_function_generator.h new file mode 100644 index 0000000000..5166903f62 --- /dev/null +++ b/src/google/protobuf/compiler/cpp/cpp_parse_function_generator.h @@ -0,0 +1,54 @@ +// Protocol Buffers - Google's data interchange format +// Copyright 2008 Google Inc. All rights reserved. +// https://developers.google.com/protocol-buffers/ +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +#ifndef GOOGLE_PROTOBUF_COMPILER_CPP_PARSE_FUNCTION_GENERATOR_H__ +#define GOOGLE_PROTOBUF_COMPILER_CPP_PARSE_FUNCTION_GENERATOR_H__ + +#include +#include +#include +#include + +namespace google { +namespace protobuf { +namespace compiler { +namespace cpp { + +void GenerateParseFunction(const Descriptor* descriptor, int num_hasbits, + const Options& options, + MessageSCCAnalyzer* scc_analyzer, + io::Printer* printer); + +} // namespace cpp +} // namespace compiler +} // namespace protobuf +} // namespace google + +#endif // GOOGLE_PROTOBUF_COMPILER_CPP_PARSE_FUNCTION_GENERATOR_H__ diff --git a/src/google/protobuf/compiler/parser.cc b/src/google/protobuf/compiler/parser.cc index b1bf2882e3..bbcc8e592b 100644 --- a/src/google/protobuf/compiler/parser.cc +++ b/src/google/protobuf/compiler/parser.cc @@ -2215,7 +2215,6 @@ bool Parser::ParseServiceMethod(MethodDescriptorProto* method, return true; } - bool Parser::ParseMethodOptions(const LocationRecorder& parent_location, const FileDescriptorProto* containing_file, const int optionsFieldNumber, diff --git a/src/google/protobuf/compiler/parser.h b/src/google/protobuf/compiler/parser.h index e0ba90f921..b5a5df8996 100644 --- a/src/google/protobuf/compiler/parser.h +++ b/src/google/protobuf/compiler/parser.h @@ -523,7 +523,6 @@ class PROTOBUF_EXPORT Parser { return syntax_identifier_ == "proto3"; } - bool ValidateEnum(const EnumDescriptorProto* proto); // ================================================================= diff --git a/src/google/protobuf/compiler/plugin.pb.cc b/src/google/protobuf/compiler/plugin.pb.cc index 374a444a9e..1fd80a19fb 100644 --- a/src/google/protobuf/compiler/plugin.pb.cc +++ b/src/google/protobuf/compiler/plugin.pb.cc @@ -516,7 +516,7 @@ bool Version::IsInitialized() const { void Version::InternalSwap(Version* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); swap(_has_bits_[0], other->_has_bits_[0]); suffix_.Swap(&other->suffix_, &::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); ::PROTOBUF_NAMESPACE_ID::internal::memswap< @@ -863,7 +863,7 @@ bool CodeGeneratorRequest::IsInitialized() const { void CodeGeneratorRequest::InternalSwap(CodeGeneratorRequest* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); swap(_has_bits_[0], other->_has_bits_[0]); file_to_generate_.InternalSwap(&other->file_to_generate_); proto_file_.InternalSwap(&other->proto_file_); @@ -1226,7 +1226,7 @@ bool CodeGeneratorResponse_File::IsInitialized() const { void CodeGeneratorResponse_File::InternalSwap(CodeGeneratorResponse_File* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); swap(_has_bits_[0], other->_has_bits_[0]); name_.Swap(&other->name_, &::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); insertion_point_.Swap(&other->insertion_point_, &::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); @@ -1513,7 +1513,7 @@ bool CodeGeneratorResponse::IsInitialized() const { void CodeGeneratorResponse::InternalSwap(CodeGeneratorResponse* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); swap(_has_bits_[0], other->_has_bits_[0]); file_.InternalSwap(&other->file_); error_.Swap(&other->error_, &::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); diff --git a/src/google/protobuf/compiler/plugin.pb.h b/src/google/protobuf/compiler/plugin.pb.h index 9ac275a66e..67eeabbbca 100644 --- a/src/google/protobuf/compiler/plugin.pb.h +++ b/src/google/protobuf/compiler/plugin.pb.h @@ -490,7 +490,7 @@ class PROTOC_EXPORT CodeGeneratorRequest PROTOBUF_FINAL : public: void clear_compiler_version(); const PROTOBUF_NAMESPACE_ID::compiler::Version& compiler_version() const; - PROTOBUF_NAMESPACE_ID::compiler::Version* release_compiler_version(); + PROTOBUF_FUTURE_MUST_USE_RESULT PROTOBUF_NAMESPACE_ID::compiler::Version* release_compiler_version(); PROTOBUF_NAMESPACE_ID::compiler::Version* mutable_compiler_version(); void set_allocated_compiler_version(PROTOBUF_NAMESPACE_ID::compiler::Version* compiler_version); private: @@ -698,7 +698,7 @@ class PROTOC_EXPORT CodeGeneratorResponse_File PROTOBUF_FINAL : public: void clear_generated_code_info(); const PROTOBUF_NAMESPACE_ID::GeneratedCodeInfo& generated_code_info() const; - PROTOBUF_NAMESPACE_ID::GeneratedCodeInfo* release_generated_code_info(); + PROTOBUF_FUTURE_MUST_USE_RESULT PROTOBUF_NAMESPACE_ID::GeneratedCodeInfo* release_generated_code_info(); PROTOBUF_NAMESPACE_ID::GeneratedCodeInfo* mutable_generated_code_info(); void set_allocated_generated_code_info(PROTOBUF_NAMESPACE_ID::GeneratedCodeInfo* generated_code_info); private: diff --git a/src/google/protobuf/descriptor.cc b/src/google/protobuf/descriptor.cc index 03c4e2b516..40ded3a1eb 100644 --- a/src/google/protobuf/descriptor.cc +++ b/src/google/protobuf/descriptor.cc @@ -2591,7 +2591,7 @@ void Descriptor::DebugString(int depth, std::string* contents, enum_type(i)->DebugString(depth, contents, debug_string_options); } for (int i = 0; i < field_count(); i++) { - if (field(i)->containing_oneof() == nullptr) { + if (field(i)->real_containing_oneof() == nullptr) { field(i)->DebugString(depth, contents, debug_string_options); } else if (field(i)->containing_oneof()->field(0) == field(i)) { // This is the first field in this oneof, so print the whole oneof. @@ -2701,7 +2701,7 @@ void FieldDescriptor::DebugString( std::string label = StrCat(kLabelToName[this->label()], " "); // Label is omitted for maps, oneof, and plain proto3 fields. - if (is_map() || containing_oneof() || + if (is_map() || real_containing_oneof() || (is_optional() && !has_optional_keyword())) { label.clear(); } diff --git a/src/google/protobuf/descriptor.pb.cc b/src/google/protobuf/descriptor.pb.cc index c22ae431c0..047a401265 100644 --- a/src/google/protobuf/descriptor.pb.cc +++ b/src/google/protobuf/descriptor.pb.cc @@ -1418,7 +1418,7 @@ bool FileDescriptorSet::IsInitialized() const { void FileDescriptorSet::InternalSwap(FileDescriptorSet* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); file_.InternalSwap(&other->file_); } @@ -2056,7 +2056,7 @@ bool FileDescriptorProto::IsInitialized() const { void FileDescriptorProto::InternalSwap(FileDescriptorProto* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); swap(_has_bits_[0], other->_has_bits_[0]); dependency_.InternalSwap(&other->dependency_); message_type_.InternalSwap(&other->message_type_); @@ -2362,7 +2362,7 @@ bool DescriptorProto_ExtensionRange::IsInitialized() const { void DescriptorProto_ExtensionRange::InternalSwap(DescriptorProto_ExtensionRange* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); swap(_has_bits_[0], other->_has_bits_[0]); ::PROTOBUF_NAMESPACE_ID::internal::memswap< PROTOBUF_FIELD_OFFSET(DescriptorProto_ExtensionRange, end_) @@ -2612,7 +2612,7 @@ bool DescriptorProto_ReservedRange::IsInitialized() const { void DescriptorProto_ReservedRange::InternalSwap(DescriptorProto_ReservedRange* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); swap(_has_bits_[0], other->_has_bits_[0]); ::PROTOBUF_NAMESPACE_ID::internal::memswap< PROTOBUF_FIELD_OFFSET(DescriptorProto_ReservedRange, end_) @@ -3149,7 +3149,7 @@ bool DescriptorProto::IsInitialized() const { void DescriptorProto::InternalSwap(DescriptorProto* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); swap(_has_bits_[0], other->_has_bits_[0]); field_.InternalSwap(&other->field_); nested_type_.InternalSwap(&other->nested_type_); @@ -3375,8 +3375,8 @@ bool ExtensionRangeOptions::IsInitialized() const { void ExtensionRangeOptions::InternalSwap(ExtensionRangeOptions* other) { using std::swap; - _extensions_.Swap(&other->_extensions_); - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _extensions_.InternalSwap(&other->_extensions_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); uninterpreted_option_.InternalSwap(&other->uninterpreted_option_); } @@ -3987,7 +3987,7 @@ bool FieldDescriptorProto::IsInitialized() const { void FieldDescriptorProto::InternalSwap(FieldDescriptorProto* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); swap(_has_bits_[0], other->_has_bits_[0]); name_.Swap(&other->name_, &::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); extendee_.Swap(&other->extendee_, &::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); @@ -4270,7 +4270,7 @@ bool OneofDescriptorProto::IsInitialized() const { void OneofDescriptorProto::InternalSwap(OneofDescriptorProto* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); swap(_has_bits_[0], other->_has_bits_[0]); name_.Swap(&other->name_, &::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); swap(options_, other->options_); @@ -4516,7 +4516,7 @@ bool EnumDescriptorProto_EnumReservedRange::IsInitialized() const { void EnumDescriptorProto_EnumReservedRange::InternalSwap(EnumDescriptorProto_EnumReservedRange* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); swap(_has_bits_[0], other->_has_bits_[0]); ::PROTOBUF_NAMESPACE_ID::internal::memswap< PROTOBUF_FIELD_OFFSET(EnumDescriptorProto_EnumReservedRange, end_) @@ -4893,7 +4893,7 @@ bool EnumDescriptorProto::IsInitialized() const { void EnumDescriptorProto::InternalSwap(EnumDescriptorProto* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); swap(_has_bits_[0], other->_has_bits_[0]); value_.InternalSwap(&other->value_); reserved_range_.InternalSwap(&other->reserved_range_); @@ -5201,7 +5201,7 @@ bool EnumValueDescriptorProto::IsInitialized() const { void EnumValueDescriptorProto::InternalSwap(EnumValueDescriptorProto* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); swap(_has_bits_[0], other->_has_bits_[0]); name_.Swap(&other->name_, &::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); ::PROTOBUF_NAMESPACE_ID::internal::memswap< @@ -5510,7 +5510,7 @@ bool ServiceDescriptorProto::IsInitialized() const { void ServiceDescriptorProto::InternalSwap(ServiceDescriptorProto* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); swap(_has_bits_[0], other->_has_bits_[0]); method_.InternalSwap(&other->method_); name_.Swap(&other->name_, &::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); @@ -5931,7 +5931,7 @@ bool MethodDescriptorProto::IsInitialized() const { void MethodDescriptorProto::InternalSwap(MethodDescriptorProto* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); swap(_has_bits_[0], other->_has_bits_[0]); name_.Swap(&other->name_, &::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); input_type_.Swap(&other->input_type_, &::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); @@ -6900,8 +6900,8 @@ bool FileOptions::IsInitialized() const { void FileOptions::InternalSwap(FileOptions* other) { using std::swap; - _extensions_.Swap(&other->_extensions_); - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _extensions_.InternalSwap(&other->_extensions_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); swap(_has_bits_[0], other->_has_bits_[0]); uninterpreted_option_.InternalSwap(&other->uninterpreted_option_); java_package_.Swap(&other->java_package_, &::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); @@ -7259,8 +7259,8 @@ bool MessageOptions::IsInitialized() const { void MessageOptions::InternalSwap(MessageOptions* other) { using std::swap; - _extensions_.Swap(&other->_extensions_); - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _extensions_.InternalSwap(&other->_extensions_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); swap(_has_bits_[0], other->_has_bits_[0]); uninterpreted_option_.InternalSwap(&other->uninterpreted_option_); ::PROTOBUF_NAMESPACE_ID::internal::memswap< @@ -7671,8 +7671,8 @@ bool FieldOptions::IsInitialized() const { void FieldOptions::InternalSwap(FieldOptions* other) { using std::swap; - _extensions_.Swap(&other->_extensions_); - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _extensions_.InternalSwap(&other->_extensions_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); swap(_has_bits_[0], other->_has_bits_[0]); uninterpreted_option_.InternalSwap(&other->uninterpreted_option_); ::PROTOBUF_NAMESPACE_ID::internal::memswap< @@ -7895,8 +7895,8 @@ bool OneofOptions::IsInitialized() const { void OneofOptions::InternalSwap(OneofOptions* other) { using std::swap; - _extensions_.Swap(&other->_extensions_); - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _extensions_.InternalSwap(&other->_extensions_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); uninterpreted_option_.InternalSwap(&other->uninterpreted_option_); } @@ -8185,8 +8185,8 @@ bool EnumOptions::IsInitialized() const { void EnumOptions::InternalSwap(EnumOptions* other) { using std::swap; - _extensions_.Swap(&other->_extensions_); - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _extensions_.InternalSwap(&other->_extensions_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); swap(_has_bits_[0], other->_has_bits_[0]); uninterpreted_option_.InternalSwap(&other->uninterpreted_option_); ::PROTOBUF_NAMESPACE_ID::internal::memswap< @@ -8444,8 +8444,8 @@ bool EnumValueOptions::IsInitialized() const { void EnumValueOptions::InternalSwap(EnumValueOptions* other) { using std::swap; - _extensions_.Swap(&other->_extensions_); - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _extensions_.InternalSwap(&other->_extensions_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); swap(_has_bits_[0], other->_has_bits_[0]); uninterpreted_option_.InternalSwap(&other->uninterpreted_option_); swap(deprecated_, other->deprecated_); @@ -8698,8 +8698,8 @@ bool ServiceOptions::IsInitialized() const { void ServiceOptions::InternalSwap(ServiceOptions* other) { using std::swap; - _extensions_.Swap(&other->_extensions_); - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _extensions_.InternalSwap(&other->_extensions_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); swap(_has_bits_[0], other->_has_bits_[0]); uninterpreted_option_.InternalSwap(&other->uninterpreted_option_); swap(deprecated_, other->deprecated_); @@ -8999,8 +8999,8 @@ bool MethodOptions::IsInitialized() const { void MethodOptions::InternalSwap(MethodOptions* other) { using std::swap; - _extensions_.Swap(&other->_extensions_); - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _extensions_.InternalSwap(&other->_extensions_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); swap(_has_bits_[0], other->_has_bits_[0]); uninterpreted_option_.InternalSwap(&other->uninterpreted_option_); ::PROTOBUF_NAMESPACE_ID::internal::memswap< @@ -9276,7 +9276,7 @@ bool UninterpretedOption_NamePart::IsInitialized() const { void UninterpretedOption_NamePart::InternalSwap(UninterpretedOption_NamePart* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); swap(_has_bits_[0], other->_has_bits_[0]); name_part_.Swap(&other->name_part_, &::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); swap(is_extension_, other->is_extension_); @@ -9706,7 +9706,7 @@ bool UninterpretedOption::IsInitialized() const { void UninterpretedOption::InternalSwap(UninterpretedOption* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); swap(_has_bits_[0], other->_has_bits_[0]); name_.InternalSwap(&other->name_); identifier_value_.Swap(&other->identifier_value_, &::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); @@ -10097,7 +10097,7 @@ bool SourceCodeInfo_Location::IsInitialized() const { void SourceCodeInfo_Location::InternalSwap(SourceCodeInfo_Location* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); swap(_has_bits_[0], other->_has_bits_[0]); path_.InternalSwap(&other->path_); span_.InternalSwap(&other->span_); @@ -10297,7 +10297,7 @@ bool SourceCodeInfo::IsInitialized() const { void SourceCodeInfo::InternalSwap(SourceCodeInfo* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); location_.InternalSwap(&other->location_); } @@ -10623,7 +10623,7 @@ bool GeneratedCodeInfo_Annotation::IsInitialized() const { void GeneratedCodeInfo_Annotation::InternalSwap(GeneratedCodeInfo_Annotation* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); swap(_has_bits_[0], other->_has_bits_[0]); path_.InternalSwap(&other->path_); source_file_.Swap(&other->source_file_, &::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); @@ -10826,7 +10826,7 @@ bool GeneratedCodeInfo::IsInitialized() const { void GeneratedCodeInfo::InternalSwap(GeneratedCodeInfo* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); annotation_.InternalSwap(&other->annotation_); } diff --git a/src/google/protobuf/descriptor.pb.h b/src/google/protobuf/descriptor.pb.h index 8f109ab3ea..c3b7ed22bb 100644 --- a/src/google/protobuf/descriptor.pb.h +++ b/src/google/protobuf/descriptor.pb.h @@ -804,7 +804,7 @@ class PROTOBUF_EXPORT FileDescriptorProto PROTOBUF_FINAL : public: void clear_options(); const PROTOBUF_NAMESPACE_ID::FileOptions& options() const; - PROTOBUF_NAMESPACE_ID::FileOptions* release_options(); + PROTOBUF_FUTURE_MUST_USE_RESULT PROTOBUF_NAMESPACE_ID::FileOptions* release_options(); PROTOBUF_NAMESPACE_ID::FileOptions* mutable_options(); void set_allocated_options(PROTOBUF_NAMESPACE_ID::FileOptions* options); private: @@ -822,7 +822,7 @@ class PROTOBUF_EXPORT FileDescriptorProto PROTOBUF_FINAL : public: void clear_source_code_info(); const PROTOBUF_NAMESPACE_ID::SourceCodeInfo& source_code_info() const; - PROTOBUF_NAMESPACE_ID::SourceCodeInfo* release_source_code_info(); + PROTOBUF_FUTURE_MUST_USE_RESULT PROTOBUF_NAMESPACE_ID::SourceCodeInfo* release_source_code_info(); PROTOBUF_NAMESPACE_ID::SourceCodeInfo* mutable_source_code_info(); void set_allocated_source_code_info(PROTOBUF_NAMESPACE_ID::SourceCodeInfo* source_code_info); private: @@ -983,7 +983,7 @@ class PROTOBUF_EXPORT DescriptorProto_ExtensionRange PROTOBUF_FINAL : public: void clear_options(); const PROTOBUF_NAMESPACE_ID::ExtensionRangeOptions& options() const; - PROTOBUF_NAMESPACE_ID::ExtensionRangeOptions* release_options(); + PROTOBUF_FUTURE_MUST_USE_RESULT PROTOBUF_NAMESPACE_ID::ExtensionRangeOptions* release_options(); PROTOBUF_NAMESPACE_ID::ExtensionRangeOptions* mutable_options(); void set_allocated_options(PROTOBUF_NAMESPACE_ID::ExtensionRangeOptions* options); private: @@ -1497,7 +1497,7 @@ class PROTOBUF_EXPORT DescriptorProto PROTOBUF_FINAL : public: void clear_options(); const PROTOBUF_NAMESPACE_ID::MessageOptions& options() const; - PROTOBUF_NAMESPACE_ID::MessageOptions* release_options(); + PROTOBUF_FUTURE_MUST_USE_RESULT PROTOBUF_NAMESPACE_ID::MessageOptions* release_options(); PROTOBUF_NAMESPACE_ID::MessageOptions* mutable_options(); void set_allocated_options(PROTOBUF_NAMESPACE_ID::MessageOptions* options); private: @@ -1998,7 +1998,7 @@ class PROTOBUF_EXPORT FieldDescriptorProto PROTOBUF_FINAL : public: void clear_options(); const PROTOBUF_NAMESPACE_ID::FieldOptions& options() const; - PROTOBUF_NAMESPACE_ID::FieldOptions* release_options(); + PROTOBUF_FUTURE_MUST_USE_RESULT PROTOBUF_NAMESPACE_ID::FieldOptions* release_options(); PROTOBUF_NAMESPACE_ID::FieldOptions* mutable_options(); void set_allocated_options(PROTOBUF_NAMESPACE_ID::FieldOptions* options); private: @@ -2240,7 +2240,7 @@ class PROTOBUF_EXPORT OneofDescriptorProto PROTOBUF_FINAL : public: void clear_options(); const PROTOBUF_NAMESPACE_ID::OneofOptions& options() const; - PROTOBUF_NAMESPACE_ID::OneofOptions* release_options(); + PROTOBUF_FUTURE_MUST_USE_RESULT PROTOBUF_NAMESPACE_ID::OneofOptions* release_options(); PROTOBUF_NAMESPACE_ID::OneofOptions* mutable_options(); void set_allocated_options(PROTOBUF_NAMESPACE_ID::OneofOptions* options); private: @@ -2631,7 +2631,7 @@ class PROTOBUF_EXPORT EnumDescriptorProto PROTOBUF_FINAL : public: void clear_options(); const PROTOBUF_NAMESPACE_ID::EnumOptions& options() const; - PROTOBUF_NAMESPACE_ID::EnumOptions* release_options(); + PROTOBUF_FUTURE_MUST_USE_RESULT PROTOBUF_NAMESPACE_ID::EnumOptions* release_options(); PROTOBUF_NAMESPACE_ID::EnumOptions* mutable_options(); void set_allocated_options(PROTOBUF_NAMESPACE_ID::EnumOptions* options); private: @@ -2803,7 +2803,7 @@ class PROTOBUF_EXPORT EnumValueDescriptorProto PROTOBUF_FINAL : public: void clear_options(); const PROTOBUF_NAMESPACE_ID::EnumValueOptions& options() const; - PROTOBUF_NAMESPACE_ID::EnumValueOptions* release_options(); + PROTOBUF_FUTURE_MUST_USE_RESULT PROTOBUF_NAMESPACE_ID::EnumValueOptions* release_options(); PROTOBUF_NAMESPACE_ID::EnumValueOptions* mutable_options(); void set_allocated_options(PROTOBUF_NAMESPACE_ID::EnumValueOptions* options); private: @@ -3004,7 +3004,7 @@ class PROTOBUF_EXPORT ServiceDescriptorProto PROTOBUF_FINAL : public: void clear_options(); const PROTOBUF_NAMESPACE_ID::ServiceOptions& options() const; - PROTOBUF_NAMESPACE_ID::ServiceOptions* release_options(); + PROTOBUF_FUTURE_MUST_USE_RESULT PROTOBUF_NAMESPACE_ID::ServiceOptions* release_options(); PROTOBUF_NAMESPACE_ID::ServiceOptions* mutable_options(); void set_allocated_options(PROTOBUF_NAMESPACE_ID::ServiceOptions* options); private: @@ -3213,7 +3213,7 @@ class PROTOBUF_EXPORT MethodDescriptorProto PROTOBUF_FINAL : public: void clear_options(); const PROTOBUF_NAMESPACE_ID::MethodOptions& options() const; - PROTOBUF_NAMESPACE_ID::MethodOptions* release_options(); + PROTOBUF_FUTURE_MUST_USE_RESULT PROTOBUF_NAMESPACE_ID::MethodOptions* release_options(); PROTOBUF_NAMESPACE_ID::MethodOptions* mutable_options(); void set_allocated_options(PROTOBUF_NAMESPACE_ID::MethodOptions* options); private: diff --git a/src/google/protobuf/duration.pb.cc b/src/google/protobuf/duration.pb.cc index 01cd8d75b3..53307632b3 100644 --- a/src/google/protobuf/duration.pb.cc +++ b/src/google/protobuf/duration.pb.cc @@ -285,7 +285,7 @@ bool Duration::IsInitialized() const { void Duration::InternalSwap(Duration* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); ::PROTOBUF_NAMESPACE_ID::internal::memswap< PROTOBUF_FIELD_OFFSET(Duration, nanos_) + sizeof(Duration::nanos_) diff --git a/src/google/protobuf/empty.pb.cc b/src/google/protobuf/empty.pb.cc index b2149abb7b..2adc793c10 100644 --- a/src/google/protobuf/empty.pb.cc +++ b/src/google/protobuf/empty.pb.cc @@ -219,7 +219,7 @@ bool Empty::IsInitialized() const { void Empty::InternalSwap(Empty* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); } ::PROTOBUF_NAMESPACE_ID::Metadata Empty::GetMetadata() const { diff --git a/src/google/protobuf/extension_set.cc b/src/google/protobuf/extension_set.cc index bc53480794..aa57e67ec5 100644 --- a/src/google/protobuf/extension_set.cc +++ b/src/google/protobuf/extension_set.cc @@ -1055,10 +1055,7 @@ void ExtensionSet::InternalExtensionMergeFrom( void ExtensionSet::Swap(ExtensionSet* x) { if (GetArena() == x->GetArena()) { - using std::swap; - swap(flat_capacity_, x->flat_capacity_); - swap(flat_size_, x->flat_size_); - swap(map_, x->map_); + InternalSwap(x); } else { // TODO(cfallin, rohananil): We maybe able to optimize a case where we are // swapping from heap to arena-allocated extension set, by just Own()'ing @@ -1072,6 +1069,13 @@ void ExtensionSet::Swap(ExtensionSet* x) { } } +void ExtensionSet::InternalSwap(ExtensionSet* other) { + using std::swap; + swap(flat_capacity_, other->flat_capacity_); + swap(flat_size_, other->flat_size_); + swap(map_, other->map_); +} + void ExtensionSet::SwapExtension(ExtensionSet* other, int number) { if (this == other) return; Extension* this_ext = FindOrNull(number); diff --git a/src/google/protobuf/extension_set.h b/src/google/protobuf/extension_set.h index c4b845a8ee..55492a6d31 100644 --- a/src/google/protobuf/extension_set.h +++ b/src/google/protobuf/extension_set.h @@ -369,6 +369,7 @@ class PROTOBUF_EXPORT ExtensionSet { void Clear(); void MergeFrom(const ExtensionSet& other); void Swap(ExtensionSet* other); + void InternalSwap(ExtensionSet* other); void SwapExtension(ExtensionSet* other, int number); bool IsInitialized() const; diff --git a/src/google/protobuf/extension_set_unittest.cc b/src/google/protobuf/extension_set_unittest.cc index 6a6fc25b3c..7971fe59f9 100644 --- a/src/google/protobuf/extension_set_unittest.cc +++ b/src/google/protobuf/extension_set_unittest.cc @@ -198,7 +198,8 @@ TEST(ExtensionSetTest, ReleaseExtension) { // ReleaseExtension will return the underlying object even after // ClearExtension is called. message.SetAllocatedExtension( - unittest::TestMessageSetExtension1::message_set_extension, extension); + unittest::TestMessageSetExtension1::message_set_extension, + released_extension); message.ClearExtension( unittest::TestMessageSetExtension1::message_set_extension); released_extension = message.ReleaseExtension( @@ -1335,5 +1336,3 @@ TEST(ExtensionSetTest, ConstInit) { } // namespace internal } // namespace protobuf } // namespace google - -#include diff --git a/src/google/protobuf/field_mask.pb.cc b/src/google/protobuf/field_mask.pb.cc index 5890bb2c70..f83f62d183 100644 --- a/src/google/protobuf/field_mask.pb.cc +++ b/src/google/protobuf/field_mask.pb.cc @@ -263,7 +263,7 @@ bool FieldMask::IsInitialized() const { void FieldMask::InternalSwap(FieldMask* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); paths_.InternalSwap(&other->paths_); } diff --git a/src/google/protobuf/generated_message_reflection_unittest.cc b/src/google/protobuf/generated_message_reflection_unittest.cc index 7234f5f932..8b14662f1b 100644 --- a/src/google/protobuf/generated_message_reflection_unittest.cc +++ b/src/google/protobuf/generated_message_reflection_unittest.cc @@ -55,6 +55,9 @@ #include #include +// Must be included last. +#include + namespace google { namespace protobuf { @@ -358,6 +361,7 @@ TEST(GeneratedMessageReflectionTest, ReleaseLast) { ASSERT_EQ(2, message.repeated_foreign_message_size()); const protobuf_unittest::ForeignMessage* expected = message.mutable_repeated_foreign_message(1); + (void)expected; // unused in somce configurations std::unique_ptr released(message.GetReflection()->ReleaseLast( &message, descriptor->FindFieldByName("repeated_foreign_message"))); EXPECT_EQ(expected, released.get()); @@ -781,6 +785,7 @@ TEST(GeneratedMessageReflectionTest, SetAllocatedOneofMessageTest) { &to_message); const Message& sub_message = reflection->GetMessage( to_message, descriptor->FindFieldByName("foo_lazy_message")); + (void)sub_message; // unused in somce configurations released = reflection->ReleaseMessage( &to_message, descriptor->FindFieldByName("foo_lazy_message")); EXPECT_TRUE(released != NULL); @@ -798,6 +803,7 @@ TEST(GeneratedMessageReflectionTest, SetAllocatedOneofMessageTest) { const Message& sub_message2 = reflection->GetMessage( to_message, descriptor->FindFieldByName("foo_message")); + (void)sub_message2; // unused in somce configurations released = reflection->ReleaseMessage( &to_message, descriptor->FindFieldByName("foo_message")); EXPECT_TRUE(released != NULL); @@ -917,6 +923,7 @@ TEST(GeneratedMessageReflectionTest, ReleaseOneofMessageTest) { const Reflection* reflection = message.GetReflection(); const Message& sub_message = reflection->GetMessage( message, descriptor->FindFieldByName("foo_lazy_message")); + (void)sub_message; // unused in somce configurations Message* released = reflection->ReleaseMessage( &message, descriptor->FindFieldByName("foo_lazy_message")); diff --git a/src/google/protobuf/map.h b/src/google/protobuf/map.h index 7efee12526..df84e1efcb 100644 --- a/src/google/protobuf/map.h +++ b/src/google/protobuf/map.h @@ -1321,7 +1321,7 @@ class Map { void swap(Map& other) { if (arena() == other.arena()) { - elements_.Swap(&other.elements_); + InternalSwap(other); } else { // TODO(zuguang): optimize this. The temporary copy can be allocated // in the same arena as the other message, and the "other = copy" can @@ -1332,6 +1332,8 @@ class Map { } } + void InternalSwap(Map& other) { elements_.Swap(&other.elements_); } + // Access to hasher. Currently this returns a copy, but it may // be modified to return a const reference in the future. hasher hash_function() const { return elements_.hash_function(); } diff --git a/src/google/protobuf/map_field.cc b/src/google/protobuf/map_field.cc index 542a1f8335..23f36d03ed 100644 --- a/src/google/protobuf/map_field.cc +++ b/src/google/protobuf/map_field.cc @@ -56,6 +56,20 @@ RepeatedPtrFieldBase* MapFieldBase::MutableRepeatedField() { return reinterpret_cast(repeated_field_); } +void MapFieldBase::Swap(MapFieldBase* other) { + // TODO(teboring): This is incorrect when on different arenas. + InternalSwap(other); +} + +void MapFieldBase::InternalSwap(MapFieldBase* other) { + std::swap(repeated_field_, other->repeated_field_); + // a relaxed swap of the atomic + auto other_state = other->state_.load(std::memory_order_relaxed); + auto this_state = state_.load(std::memory_order_relaxed); + other->state_.store(this_state, std::memory_order_relaxed); + state_.store(other_state, std::memory_order_relaxed); +} + size_t MapFieldBase::SpaceUsedExcludingSelfLong() const { ConstAccess(); mutex_.Lock(); diff --git a/src/google/protobuf/map_field.h b/src/google/protobuf/map_field.h index e05d3ee04d..452c463a01 100644 --- a/src/google/protobuf/map_field.h +++ b/src/google/protobuf/map_field.h @@ -371,7 +371,7 @@ class PROTOBUF_EXPORT MapFieldBase { virtual void MapBegin(MapIterator* map_iter) const = 0; virtual void MapEnd(MapIterator* map_iter) const = 0; virtual void MergeFrom(const MapFieldBase& other) = 0; - virtual void Swap(MapFieldBase* other) = 0; + virtual void Swap(MapFieldBase* other); // Sync Map with repeated field and returns the size of map. virtual int size() const = 0; virtual void Clear() = 0; @@ -407,6 +407,8 @@ class PROTOBUF_EXPORT MapFieldBase { // Provides derived class the access to repeated field. void* MutableRepeatedPtrField() const; + void InternalSwap(MapFieldBase* other); + // Support thread sanitizer (tsan) by making const / mutable races // more apparent. If one thread calls MutableAccess() while another // thread calls either ConstAccess() or MutableAccess(), on the same @@ -571,6 +573,7 @@ class MapField : public TypeDefinedMapFieldBase { void Clear() override; void MergeFrom(const MapFieldBase& other) override; void Swap(MapFieldBase* other) override; + void InternalSwap(MapField* other); // Used in the implementation of parsing. Caller should take the ownership iff // arena_ is NULL. diff --git a/src/google/protobuf/map_field_inl.h b/src/google/protobuf/map_field_inl.h index 8e921708de..55648951d7 100644 --- a/src/google/protobuf/map_field_inl.h +++ b/src/google/protobuf/map_field_inl.h @@ -277,14 +277,18 @@ template void MapField::Swap( MapFieldBase* other) { + MapFieldBase::Swap(other); MapField* other_field = down_cast(other); - std::swap(this->MapFieldBase::repeated_field_, other_field->repeated_field_); impl_.Swap(&other_field->impl_); - // a relaxed swap of the atomic - auto other_state = other_field->state_.load(std::memory_order_relaxed); - auto this_state = this->MapFieldBase::state_.load(std::memory_order_relaxed); - other_field->state_.store(this_state, std::memory_order_relaxed); - this->MapFieldBase::state_.store(other_state, std::memory_order_relaxed); +} + +template +void MapField::InternalSwap( + MapField* other) { + MapFieldBase::InternalSwap(other); + impl_.InternalSwap(&other->impl_); } template map_); } + void InternalSwap(MapFieldLite* other) { map_.InternalSwap(other->map_); } // Used in the implementation of parsing. Caller should take the ownership iff // arena_ is NULL. diff --git a/src/google/protobuf/map_field_test.cc b/src/google/protobuf/map_field_test.cc index dd70e989d0..e581f20382 100644 --- a/src/google/protobuf/map_field_test.cc +++ b/src/google/protobuf/map_field_test.cc @@ -102,14 +102,17 @@ class MapFieldBaseStub : public MapFieldBase { void IncreaseIterator(MapIterator* map_iter) const override {} }; -class MapFieldBasePrimitiveTest : public ::testing::Test { +class MapFieldBasePrimitiveTest : public testing::TestWithParam { protected: typedef unittest::TestMap_MapInt32Int32Entry_DoNotUse EntryType; typedef MapField MapFieldType; - MapFieldBasePrimitiveTest() { + MapFieldBasePrimitiveTest() + : arena_(GetParam() ? new Arena() : nullptr), + map_field_(arena_.get()), + map_field_base_(map_field_.get()) { // Get descriptors map_descriptor_ = unittest::TestMap::descriptor() ->FindFieldByName("map_int32_int32") @@ -118,7 +121,6 @@ class MapFieldBasePrimitiveTest : public ::testing::Test { value_descriptor_ = map_descriptor_->map_value(); // Build map field - map_field_.reset(new MapFieldType); map_field_base_ = map_field_.get(); map_ = map_field_->MutableMap(); initial_value_map_[0] = 100; @@ -127,7 +129,8 @@ class MapFieldBasePrimitiveTest : public ::testing::Test { EXPECT_EQ(2, map_->size()); } - std::unique_ptr map_field_; + std::unique_ptr arena_; + ArenaHolder map_field_; MapFieldBase* map_field_base_; Map* map_; const Descriptor* map_descriptor_; @@ -136,11 +139,15 @@ class MapFieldBasePrimitiveTest : public ::testing::Test { std::map initial_value_map_; // copy of initial values inserted }; -TEST_F(MapFieldBasePrimitiveTest, SpaceUsedExcludingSelf) { +INSTANTIATE_TEST_SUITE_P(MapFieldBasePrimitiveTestInstance, + MapFieldBasePrimitiveTest, + testing::Values(true, false)); + +TEST_P(MapFieldBasePrimitiveTest, SpaceUsedExcludingSelf) { EXPECT_LT(0, map_field_base_->SpaceUsedExcludingSelf()); } -TEST_F(MapFieldBasePrimitiveTest, GetRepeatedField) { +TEST_P(MapFieldBasePrimitiveTest, GetRepeatedField) { const RepeatedPtrField& repeated = reinterpret_cast&>( map_field_base_->GetRepeatedField()); @@ -153,7 +160,7 @@ TEST_F(MapFieldBasePrimitiveTest, GetRepeatedField) { } } -TEST_F(MapFieldBasePrimitiveTest, MutableRepeatedField) { +TEST_P(MapFieldBasePrimitiveTest, MutableRepeatedField) { RepeatedPtrField* repeated = reinterpret_cast*>( map_field_base_->MutableRepeatedField()); @@ -166,7 +173,7 @@ TEST_F(MapFieldBasePrimitiveTest, MutableRepeatedField) { } } -TEST_F(MapFieldBasePrimitiveTest, Arena) { +TEST_P(MapFieldBasePrimitiveTest, Arena) { // Allocate a large initial block to avoid mallocs during hooked test. std::vector arena_block(128 * 1024); ArenaOptions options; @@ -205,18 +212,19 @@ namespace { enum State { CLEAN, MAP_DIRTY, REPEATED_DIRTY }; } // anonymous namespace -class MapFieldStateTest : public testing::TestWithParam { - public: +class MapFieldStateTest + : public testing::TestWithParam> { protected: typedef unittest::TestMap_MapInt32Int32Entry_DoNotUse EntryType; typedef MapField MapFieldType; - MapFieldStateTest() : state_(GetParam()) { + MapFieldStateTest() + : arena_(std::get<1>(GetParam()) ? new Arena() : nullptr), + map_field_(arena_.get()), + map_field_base_(map_field_.get()), + state_(std::get<0>(GetParam())) { // Build map field - map_field_.reset(new MapFieldType()); - map_field_base_ = map_field_.get(); - Expect(map_field_.get(), MAP_DIRTY, 0, 0, true); switch (state_) { case CLEAN: @@ -297,13 +305,16 @@ class MapFieldStateTest : public testing::TestWithParam { } } - std::unique_ptr map_field_; + std::unique_ptr arena_; + ArenaHolder map_field_; MapFieldBase* map_field_base_; State state_; }; INSTANTIATE_TEST_SUITE_P(MapFieldStateTestInstance, MapFieldStateTest, - ::testing::Values(CLEAN, MAP_DIRTY, REPEATED_DIRTY)); + testing::Combine(testing::Values(CLEAN, MAP_DIRTY, + REPEATED_DIRTY), + testing::Values(true, false))); TEST_P(MapFieldStateTest, GetMap) { map_field_->GetMap(); @@ -324,10 +335,10 @@ TEST_P(MapFieldStateTest, MutableMap) { } TEST_P(MapFieldStateTest, MergeFromClean) { - MapFieldType other; - AddOneStillClean(&other); + ArenaHolder other(arena_.get()); + AddOneStillClean(other.get()); - map_field_->MergeFrom(other); + map_field_->MergeFrom(*other); if (state_ != MAP_DIRTY) { Expect(map_field_.get(), MAP_DIRTY, 1, 1, false); @@ -335,14 +346,14 @@ TEST_P(MapFieldStateTest, MergeFromClean) { Expect(map_field_.get(), MAP_DIRTY, 1, 0, true); } - Expect(&other, CLEAN, 1, 1, false); + Expect(other.get(), CLEAN, 1, 1, false); } TEST_P(MapFieldStateTest, MergeFromMapDirty) { - MapFieldType other; - MakeMapDirty(&other); + ArenaHolder other(arena_.get()); + MakeMapDirty(other.get()); - map_field_->MergeFrom(other); + map_field_->MergeFrom(*other); if (state_ != MAP_DIRTY) { Expect(map_field_.get(), MAP_DIRTY, 1, 1, false); @@ -350,14 +361,14 @@ TEST_P(MapFieldStateTest, MergeFromMapDirty) { Expect(map_field_.get(), MAP_DIRTY, 1, 0, true); } - Expect(&other, MAP_DIRTY, 1, 0, true); + Expect(other.get(), MAP_DIRTY, 1, 0, true); } TEST_P(MapFieldStateTest, MergeFromRepeatedDirty) { - MapFieldType other; - MakeRepeatedDirty(&other); + ArenaHolder other(arena_.get()); + MakeRepeatedDirty(other.get()); - map_field_->MergeFrom(other); + map_field_->MergeFrom(*other); if (state_ != MAP_DIRTY) { Expect(map_field_.get(), MAP_DIRTY, 1, 1, false); @@ -365,26 +376,26 @@ TEST_P(MapFieldStateTest, MergeFromRepeatedDirty) { Expect(map_field_.get(), MAP_DIRTY, 1, 0, true); } - Expect(&other, CLEAN, 1, 1, false); + Expect(other.get(), CLEAN, 1, 1, false); } TEST_P(MapFieldStateTest, SwapClean) { - MapFieldType other; - AddOneStillClean(&other); + ArenaHolder other(arena_.get()); + AddOneStillClean(other.get()); - map_field_->Swap(&other); + map_field_->Swap(other.get()); Expect(map_field_.get(), CLEAN, 1, 1, false); switch (state_) { case CLEAN: - Expect(&other, CLEAN, 1, 1, false); + Expect(other.get(), CLEAN, 1, 1, false); break; case MAP_DIRTY: - Expect(&other, MAP_DIRTY, 1, 0, true); + Expect(other.get(), MAP_DIRTY, 1, 0, true); break; case REPEATED_DIRTY: - Expect(&other, REPEATED_DIRTY, 0, 1, false); + Expect(other.get(), REPEATED_DIRTY, 0, 1, false); break; default: break; @@ -392,22 +403,22 @@ TEST_P(MapFieldStateTest, SwapClean) { } TEST_P(MapFieldStateTest, SwapMapDirty) { - MapFieldType other; - MakeMapDirty(&other); + ArenaHolder other(arena_.get()); + MakeMapDirty(other.get()); - map_field_->Swap(&other); + map_field_->Swap(other.get()); Expect(map_field_.get(), MAP_DIRTY, 1, 0, true); switch (state_) { case CLEAN: - Expect(&other, CLEAN, 1, 1, false); + Expect(other.get(), CLEAN, 1, 1, false); break; case MAP_DIRTY: - Expect(&other, MAP_DIRTY, 1, 0, true); + Expect(other.get(), MAP_DIRTY, 1, 0, true); break; case REPEATED_DIRTY: - Expect(&other, REPEATED_DIRTY, 0, 1, false); + Expect(other.get(), REPEATED_DIRTY, 0, 1, false); break; default: break; @@ -415,22 +426,22 @@ TEST_P(MapFieldStateTest, SwapMapDirty) { } TEST_P(MapFieldStateTest, SwapRepeatedDirty) { - MapFieldType other; - MakeRepeatedDirty(&other); + ArenaHolder other(arena_.get()); + MakeRepeatedDirty(other.get()); - map_field_->Swap(&other); + map_field_->Swap(other.get()); Expect(map_field_.get(), REPEATED_DIRTY, 0, 1, false); switch (state_) { case CLEAN: - Expect(&other, CLEAN, 1, 1, false); + Expect(other.get(), CLEAN, 1, 1, false); break; case MAP_DIRTY: - Expect(&other, MAP_DIRTY, 1, 0, true); + Expect(other.get(), MAP_DIRTY, 1, 0, true); break; case REPEATED_DIRTY: - Expect(&other, REPEATED_DIRTY, 0, 1, false); + Expect(other.get(), REPEATED_DIRTY, 0, 1, false); break; default: break; @@ -501,5 +512,3 @@ TEST(MapFieldTest, ConstInit) { } // namespace internal } // namespace protobuf } // namespace google - -#include diff --git a/src/google/protobuf/map_test.cc b/src/google/protobuf/map_test.cc index 768858f766..335756eec9 100644 --- a/src/google/protobuf/map_test.cc +++ b/src/google/protobuf/map_test.cc @@ -1471,19 +1471,19 @@ TEST_F(MapFieldReflectionTest, RepeatedFieldRefForRegularFields) { std::unique_ptr entry_int32_int32( MessageFactory::generated_factory() ->GetPrototype(fd_map_int32_int32->message_type()) - ->New()); + ->New(message.GetArena())); std::unique_ptr entry_int32_double( MessageFactory::generated_factory() ->GetPrototype(fd_map_int32_double->message_type()) - ->New()); + ->New(message.GetArena())); std::unique_ptr entry_string_string( MessageFactory::generated_factory() ->GetPrototype(fd_map_string_string->message_type()) - ->New()); + ->New(message.GetArena())); std::unique_ptr entry_int32_foreign_message( MessageFactory::generated_factory() ->GetPrototype(fd_map_int32_foreign_message->message_type()) - ->New()); + ->New(message.GetArena())); EXPECT_EQ(10, mf_int32_int32.size()); EXPECT_EQ(10, mmf_int32_int32.size()); @@ -1890,6 +1890,15 @@ TEST_F(MapFieldReflectionTest, RepeatedFieldRefForRegularFields) { EXPECT_EQ(int32_value9a, int32_value0b); EXPECT_EQ(int32_value0a, int32_value9b); } + + // TODO(b/181148674): After supporting arena agnostic delete or let map entry + // handle heap allocation, this could be removed. + if (message.GetArena() != nullptr) { + entry_int32_int32.release(); + entry_int32_double.release(); + entry_string_string.release(); + entry_int32_foreign_message.release(); + } } TEST_F(MapFieldReflectionTest, RepeatedFieldRefMergeFromAndSwap) { @@ -3771,5 +3780,3 @@ TEST(MoveTest, MoveAssignmentWorks) { } // namespace internal } // namespace protobuf } // namespace google - -#include diff --git a/src/google/protobuf/message.h b/src/google/protobuf/message.h index 52e6a1db19..c9db4e8403 100644 --- a/src/google/protobuf/message.h +++ b/src/google/protobuf/message.h @@ -476,7 +476,8 @@ class PROTOBUF_EXPORT Reflection final { void RemoveLast(Message* message, const FieldDescriptor* field) const; // Removes the last element of a repeated message field, and returns the // pointer to the caller. Caller takes ownership of the returned pointer. - Message* ReleaseLast(Message* message, const FieldDescriptor* field) const; + PROTOBUF_FUTURE_MUST_USE_RESULT Message* ReleaseLast( + Message* message, const FieldDescriptor* field) const; // Swap the complete contents of two messages. void Swap(Message* message1, Message* message2) const; @@ -604,8 +605,9 @@ class PROTOBUF_EXPORT Reflection final { // If the field existed (HasField() is true), then the returned pointer will // be the same as the pointer returned by MutableMessage(). // This function has the same effect as ClearField(). - Message* ReleaseMessage(Message* message, const FieldDescriptor* field, - MessageFactory* factory = nullptr) const; + PROTOBUF_FUTURE_MUST_USE_RESULT Message* ReleaseMessage( + Message* message, const FieldDescriptor* field, + MessageFactory* factory = nullptr) const; // Repeated field getters ------------------------------------------ diff --git a/src/google/protobuf/message_lite.h b/src/google/protobuf/message_lite.h index 8a9b79dfb1..68cfa039b6 100644 --- a/src/google/protobuf/message_lite.h +++ b/src/google/protobuf/message_lite.h @@ -83,7 +83,6 @@ struct ConstantInitialized { // See parse_context.h for explanation class ParseContext; -class Proto3ArenaTestHelper; class RepeatedPtrFieldBase; class WireFormatLite; class WeakFieldMap; @@ -210,11 +209,11 @@ class PROTOBUF_EXPORT MessageLite { // if arena is a NULL. Default implementation for backwards compatibility. virtual MessageLite* New(Arena* arena) const; - // Get the arena for allocating submessages, if any, associated with this - // message. Virtual method required for generic operations but most - // arena-related operations should use the GetArena() generated-code method. - // Default implementation to reduce code size by avoiding the need for - // per-type implementations when types do not implement arena support. + // Get the arena, if any, associated with this message. Virtual method + // required for generic operations but most arena-related operations should + // use the GetArena() generated-code method. Default implementation + // to reduce code size by avoiding the need for per-type implementations + // when types do not implement arena support. Arena* GetArena() const { return _internal_metadata_.arena(); } // Get a pointer that may be equal to this message's arena, or may not be. @@ -503,18 +502,8 @@ class PROTOBUF_EXPORT MessageLite { // TODO(gerbens) make this a pure abstract function virtual const void* InternalGetTable() const { return NULL; } - // Get the arena that owns this message. - Arena* GetOwningArena() const { return _internal_metadata_.GetOwningArena(); } - - // Set the owning arena to the given one. - void SetOwningArena(Arena* arena) { - _internal_metadata_.SetOwningArena(arena); - } - - friend class Arena; friend class internal::WireFormatLite; friend class Message; - friend class internal::Proto3ArenaTestHelper; friend class internal::WeakFieldMap; void LogInitializationErrorMessage() const; diff --git a/src/google/protobuf/message_unittest.inc b/src/google/protobuf/message_unittest.inc index 0d97d407f1..78a12972ef 100644 --- a/src/google/protobuf/message_unittest.inc +++ b/src/google/protobuf/message_unittest.inc @@ -374,6 +374,17 @@ TEST(MESSAGE_TEST_NAME, FindInitializationErrors) { EXPECT_EQ("c", errors[2]); } +TEST(MESSAGE_TEST_NAME, ReleaseMustUseResult) { + UNITTEST::TestAllTypes message; + auto* f = new UNITTEST::ForeignMessage(); + f->set_c(1000); + message.set_allocated_optional_foreign_message(f); + auto* mf = message.mutable_optional_foreign_message(); + EXPECT_EQ(mf, f); + EXPECT_NE(message.release_optional_foreign_message(), nullptr); + delete f; +} + TEST(MESSAGE_TEST_NAME, ParseFailsOnInvalidMessageEnd) { UNITTEST::TestAllTypes message; diff --git a/src/google/protobuf/metadata_lite.h b/src/google/protobuf/metadata_lite.h index 1f8d8fcb63..ad5710c473 100644 --- a/src/google/protobuf/metadata_lite.h +++ b/src/google/protobuf/metadata_lite.h @@ -73,17 +73,15 @@ class InternalMetadata { } PROTOBUF_NDEBUG_INLINE Arena* arena() const { - if (PROTOBUF_PREDICT_TRUE(!has_tag())) { - return PtrValue(); - } else if (is_heap_allocating()) { - return nullptr; - } else { + if (PROTOBUF_PREDICT_FALSE(have_unknown_fields())) { return PtrValue()->arena; + } else { + return PtrValue(); } } PROTOBUF_NDEBUG_INLINE bool have_unknown_fields() const { - return UnknownTag() == kUnknownTagMask; + return PtrTag() == kTagContainer; } PROTOBUF_NDEBUG_INLINE void* raw_arena_ptr() const { return ptr_; } @@ -120,6 +118,10 @@ class InternalMetadata { } } + PROTOBUF_NDEBUG_INLINE void InternalSwap(InternalMetadata* other) { + std::swap(ptr_, other->ptr_); + } + template PROTOBUF_NDEBUG_INLINE void MergeFrom(const InternalMetadata& other) { if (other.have_unknown_fields()) { @@ -134,56 +136,23 @@ class InternalMetadata { } } - PROTOBUF_ALWAYS_INLINE Arena* GetOwningArena() const { - if (PROTOBUF_PREDICT_FALSE(have_unknown_fields())) { - return PtrValue()->arena; - } else { - return PtrValue(); - } - } - - PROTOBUF_ALWAYS_INLINE void SetOwningArena(Arena* arena) { - Arena* owning_arena = GetOwningArena(); - GOOGLE_DCHECK(arena != nullptr); // Heap can't own. - GOOGLE_DCHECK(owning_arena == nullptr); // Only heap can be owned. - - if (have_unknown_fields()) { - ContainerBase* container = PtrValue(); - container->arena = arena; - ptr_ = reinterpret_cast(reinterpret_cast(ptr_) | - kHeapAllocatingTagMask); - } else { - ptr_ = reinterpret_cast(reinterpret_cast(arena) | - kHeapAllocatingTagMask); - } - } - private: void* ptr_; // Tagged pointer implementation. - static constexpr intptr_t kUnknownTagMask = 1; - static constexpr intptr_t kHeapAllocatingTagMask = 2; - static constexpr intptr_t kPtrTagMask = - kUnknownTagMask | kHeapAllocatingTagMask; + enum { + // ptr_ is an Arena*. + kTagArena = 0, + // ptr_ is a Container*. + kTagContainer = 1, + }; + static constexpr intptr_t kPtrTagMask = 1; static constexpr intptr_t kPtrValueMask = ~kPtrTagMask; // Accessors for pointer tag and pointer value. PROTOBUF_NDEBUG_INLINE int PtrTag() const { return reinterpret_cast(ptr_) & kPtrTagMask; } - PROTOBUF_ALWAYS_INLINE int UnknownTag() const { - return reinterpret_cast(ptr_) & kUnknownTagMask; - } - PROTOBUF_ALWAYS_INLINE int HeapAllocatingTag() const { - return reinterpret_cast(ptr_) & kHeapAllocatingTagMask; - } - PROTOBUF_ALWAYS_INLINE bool has_tag() const { - return (reinterpret_cast(ptr_) & kPtrTagMask) != 0; - } - PROTOBUF_ALWAYS_INLINE bool is_heap_allocating() const { - return HeapAllocatingTag() == kHeapAllocatingTagMask; - } template U* PtrValue() const { @@ -191,8 +160,7 @@ class InternalMetadata { kPtrValueMask); } - // If ptr_'s tag is kUnknownTagMask, it points to an instance of this - // struct. + // If ptr_'s tag is kTagContainer, it points to an instance of this struct. struct ContainerBase { Arena* arena; }; @@ -212,16 +180,13 @@ class InternalMetadata { template PROTOBUF_NOINLINE T* mutable_unknown_fields_slow() { Arena* my_arena = arena(); - Arena* owning_arena = GetOwningArena(); Container* container = Arena::Create>(my_arena); // Two-step assignment works around a bug in clang's static analyzer: // https://bugs.llvm.org/show_bug.cgi?id=34198. - intptr_t allocating_tag = - reinterpret_cast(ptr_) & kHeapAllocatingTagMask; ptr_ = container; ptr_ = reinterpret_cast(reinterpret_cast(ptr_) | - kUnknownTagMask | allocating_tag); - container->arena = owning_arena; + kTagContainer); + container->arena = my_arena; return &(container->unknown_fields); } diff --git a/src/google/protobuf/port_def.inc b/src/google/protobuf/port_def.inc index 4c29134e3c..8a64749578 100644 --- a/src/google/protobuf/port_def.inc +++ b/src/google/protobuf/port_def.inc @@ -48,10 +48,10 @@ // detect/prohibit anytime it is #included twice without a corresponding // #undef. -// These macros are private and should always be -// ::util::RetrieveErrorSpace(*this) headers. If any of these errors fire, you -// should either properly #include port_undef.h at the end of your header that -// #includes port.h, or don't #include port.h twice in a .cc file. +// These macros are private and should always be #undef'd from headers. +// If any of these errors fire, you should either properly #include +// port_undef.h at the end of your header that #includes port.h, or +// don't #include port.h twice in a .cc file. #ifdef PROTOBUF_NAMESPACE #error PROTOBUF_NAMESPACE was previously defined #endif @@ -160,6 +160,15 @@ #ifdef PROTOBUF_ATTRIBUTE_WEAK #error PROTOBUF_ATTRIBUTE_WEAK was previously defined #endif +#ifdef PROTOBUF_ASAN +#error PROTOBUF_ASAN was previously defined +#endif +#ifdef PROTOBUF_MSAN +#error PROTOBUF_MSAN was previously defined +#endif +#ifdef PROTOBUF_TSAN +#error PROTOBUF_TSAN was previously defined +#endif #define PROTOBUF_NAMESPACE "google::protobuf" @@ -191,6 +200,7 @@ #define PROTOBUF_SECTION_VARIABLE(x) #define PROTOBUF_MUST_USE_RESULT +#define PROTOBUF_FUTURE_MUST_USE_RESULT // ---------------------------------------------------------------------------- // Annotations: Some parts of the code have been annotated in ways that might @@ -335,8 +345,6 @@ #endif -// Shared google3/opensource definitions. ////////////////////////////////////// - #define PROTOBUF_VERSION 3015008 #define PROTOBUF_MIN_HEADER_VERSION_FOR_PROTOC 3015000 #define PROTOBUF_MIN_PROTOC_VERSION 3015000 @@ -372,7 +380,7 @@ #if defined(__clang__) // For Clang we use __builtin_offsetof() and suppress the warning, // to avoid Control Flow Integrity and UBSan vptr sanitizers from -// crashing while trying to validate the invalid reinterpet_casts. +// crashing while trying to validate the invalid reinterpret_casts. #define PROTOBUF_FIELD_OFFSET(TYPE, FIELD) \ _Pragma("clang diagnostic push") \ _Pragma("clang diagnostic ignored \"-Winvalid-offsetof\"") \ @@ -444,6 +452,8 @@ #undef ERROR #pragma push_macro("ERROR_BUSY") #undef ERROR_BUSY +#pragma push_macro("ERROR_INSTALL_FAILED") +#undef ERROR_INSTALL_FAILED #pragma push_macro("ERROR_NOT_FOUND") #undef ERROR_NOT_FOUND #pragma push_macro("GetMessage") @@ -619,6 +629,22 @@ #define PROTOBUF_ATTRIBUTE_WEAK #endif +// Macros to detect sanitizers. +#if defined(__clang__) +# if __has_feature(address_sanitizer) +# define PROTOBUF_ASAN 1 +# endif +# if __has_feature(thread_sanitizer) +# define PROTOBUF_TSAN 1 +# endif +# if __has_feature(memory_sanitizer) +# define PROTOBUF_MSAN 1 +# endif +#elif defined(__GNUC__) +# define PROTOBUF_ASAN __SANITIZE_ADDRESS__ +# define PROTOBUF_TSAN __SANITIZE_THREAD__ +#endif + // Silence some MSVC warnings in all our code. #if _MSC_VER #pragma warning(push) diff --git a/src/google/protobuf/port_undef.inc b/src/google/protobuf/port_undef.inc index 80bfd30e04..e40ac0da9f 100644 --- a/src/google/protobuf/port_undef.inc +++ b/src/google/protobuf/port_undef.inc @@ -78,6 +78,9 @@ #undef PROTOBUF_ATTRIBUTE_NO_DESTROY #undef PROTOBUF_ATTRIBUTE_INIT_PRIORITY #undef PROTOBUF_PRAGMA_INIT_SEG +#undef PROTOBUF_ASAN +#undef PROTOBUF_MSAN +#undef PROTOBUF_TSAN #ifdef PROTOBUF_FUTURE_BREAKING_CHANGES #undef PROTOBUF_FUTURE_BREAKING_CHANGES @@ -90,6 +93,7 @@ #pragma pop_macro("DOUBLE_CLICK") #pragma pop_macro("ERROR") #pragma pop_macro("ERROR_BUSY") +#pragma pop_macro("ERROR_INSTALL_FAILED") #pragma pop_macro("ERROR_NOT_FOUND") #pragma pop_macro("GetMessage") #pragma pop_macro("IGNORE") diff --git a/src/google/protobuf/proto3_arena_unittest.cc b/src/google/protobuf/proto3_arena_unittest.cc index 83be3d83b4..5588f912a6 100644 --- a/src/google/protobuf/proto3_arena_unittest.cc +++ b/src/google/protobuf/proto3_arena_unittest.cc @@ -42,6 +42,9 @@ #include #include +// Must be included last. +#include + using proto3_arena_unittest::ForeignMessage; using proto3_arena_unittest::TestAllTypes; @@ -166,8 +169,6 @@ TEST(Proto3ArenaTest, GetArena) { auto* arena_repeated_submessage1 = arena_message1->add_repeated_foreign_message(); EXPECT_EQ(&arena, arena_message1->GetArena()); - EXPECT_EQ(&arena, - internal::Proto3ArenaTestHelper::GetOwningArena(*arena_message1)); EXPECT_EQ(&arena, arena_submessage1->GetArena()); EXPECT_EQ(&arena, arena_repeated_submessage1->GetArena()); @@ -180,17 +181,11 @@ TEST(Proto3ArenaTest, GetArena) { const auto& repeated_submessage2 = arena_message2->repeated_foreign_message(0); EXPECT_EQ(nullptr, submessage2.GetArena()); - EXPECT_EQ(&arena, - internal::Proto3ArenaTestHelper::GetOwningArena(submessage2)); EXPECT_EQ(nullptr, repeated_submessage2.GetArena()); - EXPECT_EQ(&arena, internal::Proto3ArenaTestHelper::GetOwningArena( - repeated_submessage2)); // Tests message created by Arena::Create. auto* arena_message3 = Arena::Create(&arena); EXPECT_EQ(nullptr, arena_message3->GetArena()); - EXPECT_EQ(&arena, - internal::Proto3ArenaTestHelper::GetOwningArena(*arena_message3)); } TEST(Proto3ArenaTest, GetArenaWithUnknown) { @@ -206,8 +201,6 @@ TEST(Proto3ArenaTest, GetArenaWithUnknown) { arena_repeated_submessage1->GetReflection()->MutableUnknownFields( arena_repeated_submessage1); EXPECT_EQ(&arena, arena_message1->GetArena()); - EXPECT_EQ(&arena, - internal::Proto3ArenaTestHelper::GetOwningArena(*arena_message1)); EXPECT_EQ(&arena, arena_submessage1->GetArena()); EXPECT_EQ(&arena, arena_repeated_submessage1->GetArena()); @@ -223,11 +216,7 @@ TEST(Proto3ArenaTest, GetArenaWithUnknown) { repeated_submessage2->GetReflection()->MutableUnknownFields( repeated_submessage2); EXPECT_EQ(nullptr, submessage2->GetArena()); - EXPECT_EQ(&arena, - internal::Proto3ArenaTestHelper::GetOwningArena(*submessage2)); EXPECT_EQ(nullptr, repeated_submessage2->GetArena()); - EXPECT_EQ(&arena, internal::Proto3ArenaTestHelper::GetOwningArena( - *repeated_submessage2)); } TEST(Proto3ArenaTest, Swap) { diff --git a/src/google/protobuf/repeated_field.h b/src/google/protobuf/repeated_field.h index 58190e3d85..ebccf1896d 100644 --- a/src/google/protobuf/repeated_field.h +++ b/src/google/protobuf/repeated_field.h @@ -688,7 +688,7 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase { void UnsafeArenaAddAllocated(typename TypeHandler::Type* value); template - typename TypeHandler::Type* ReleaseLast() { + PROTOBUF_FUTURE_MUST_USE_RESULT typename TypeHandler::Type* ReleaseLast() { typename TypeImplementsMergeBehavior::type t; return ReleaseLastInternal(t); } @@ -702,7 +702,7 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase { template void AddCleared(typename TypeHandler::Type* value); template - typename TypeHandler::Type* ReleaseCleared(); + PROTOBUF_FUTURE_MUST_USE_RESULT typename TypeHandler::Type* ReleaseCleared(); template void AddAllocatedInternal(typename TypeHandler::Type* value, std::true_type); @@ -1084,7 +1084,7 @@ class RepeatedPtrField final : private internal::RepeatedPtrFieldBase { // If this RepeatedPtrField is on an arena, an object copy is required to pass // ownership back to the user (for compatible semantics). Use // UnsafeArenaReleaseLast() if this behavior is undesired. - Element* ReleaseLast(); + PROTOBUF_FUTURE_MUST_USE_RESULT Element* ReleaseLast(); // Add an already-allocated object, skipping arena-ownership checks. The user // must guarantee that the given object is in the same arena as this @@ -1154,7 +1154,7 @@ class RepeatedPtrField final : private internal::RepeatedPtrFieldBase { // // This method cannot be called when the repeated field is on an arena; doing // so will trigger a GOOGLE_DCHECK-failure. - Element* ReleaseCleared(); + PROTOBUF_FUTURE_MUST_USE_RESULT Element* ReleaseCleared(); // Removes the element referenced by position. // diff --git a/src/google/protobuf/repeated_field_unittest.cc b/src/google/protobuf/repeated_field_unittest.cc index f3e8c2d5fa..b2708853cc 100644 --- a/src/google/protobuf/repeated_field_unittest.cc +++ b/src/google/protobuf/repeated_field_unittest.cc @@ -333,7 +333,7 @@ TEST(RepeatedField, ReserveHuge) { EXPECT_GE(huge_field.Capacity(), min_clamping_size); ASSERT_LT(huge_field.Capacity(), std::numeric_limits::max() - 1); -#ifndef ADDRESS_SANITIZER +#ifndef PROTOBUF_ASAN // The array containing all the fields is, in theory, up to MAXINT-1 in size. // However, some compilers can't handle a struct whose size is larger // than 2GB, and the protocol buffer format doesn't handle more than 2GB of @@ -344,7 +344,7 @@ TEST(RepeatedField, ReserveHuge) { // size must still be clamped to a valid range. huge_field.Reserve(huge_field.Capacity() + 1); EXPECT_EQ(huge_field.Capacity(), std::numeric_limits::max()); -#endif +#endif // PROTOBUF_ASAN #endif // PROTOBUF_TEST_ALLOW_LARGE_ALLOC } diff --git a/src/google/protobuf/source_context.pb.cc b/src/google/protobuf/source_context.pb.cc index 3994ceba89..4dae9363ba 100644 --- a/src/google/protobuf/source_context.pb.cc +++ b/src/google/protobuf/source_context.pb.cc @@ -264,7 +264,7 @@ bool SourceContext::IsInitialized() const { void SourceContext::InternalSwap(SourceContext* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); file_name_.Swap(&other->file_name_, &::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); } diff --git a/src/google/protobuf/struct.pb.cc b/src/google/protobuf/struct.pb.cc index fbe98e786d..1b2fbc7ae7 100644 --- a/src/google/protobuf/struct.pb.cc +++ b/src/google/protobuf/struct.pb.cc @@ -403,8 +403,8 @@ bool Struct::IsInitialized() const { void Struct::InternalSwap(Struct* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); - fields_.Swap(&other->fields_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); + fields_.InternalSwap(&other->fields_); } ::PROTOBUF_NAMESPACE_ID::Metadata Struct::GetMetadata() const { @@ -842,7 +842,7 @@ bool Value::IsInitialized() const { void Value::InternalSwap(Value* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); swap(kind_, other->kind_); swap(_oneof_case_[0], other->_oneof_case_[0]); } @@ -1038,7 +1038,7 @@ bool ListValue::IsInitialized() const { void ListValue::InternalSwap(ListValue* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); values_.InternalSwap(&other->values_); } diff --git a/src/google/protobuf/struct.pb.h b/src/google/protobuf/struct.pb.h index 56df12e258..a6f5e78242 100644 --- a/src/google/protobuf/struct.pb.h +++ b/src/google/protobuf/struct.pb.h @@ -462,7 +462,7 @@ class PROTOBUF_EXPORT Value PROTOBUF_FINAL : public: void clear_struct_value(); const PROTOBUF_NAMESPACE_ID::Struct& struct_value() const; - PROTOBUF_NAMESPACE_ID::Struct* release_struct_value(); + PROTOBUF_FUTURE_MUST_USE_RESULT PROTOBUF_NAMESPACE_ID::Struct* release_struct_value(); PROTOBUF_NAMESPACE_ID::Struct* mutable_struct_value(); void set_allocated_struct_value(PROTOBUF_NAMESPACE_ID::Struct* struct_value); private: @@ -480,7 +480,7 @@ class PROTOBUF_EXPORT Value PROTOBUF_FINAL : public: void clear_list_value(); const PROTOBUF_NAMESPACE_ID::ListValue& list_value() const; - PROTOBUF_NAMESPACE_ID::ListValue* release_list_value(); + PROTOBUF_FUTURE_MUST_USE_RESULT PROTOBUF_NAMESPACE_ID::ListValue* release_list_value(); PROTOBUF_NAMESPACE_ID::ListValue* mutable_list_value(); void set_allocated_list_value(PROTOBUF_NAMESPACE_ID::ListValue* list_value); private: diff --git a/src/google/protobuf/text_format.cc b/src/google/protobuf/text_format.cc index 2abd92766c..449a5a66e1 100644 --- a/src/google/protobuf/text_format.cc +++ b/src/google/protobuf/text_format.cc @@ -65,6 +65,7 @@ // Must be included last. #include +#define DEBUG_STRING_SILENT_MARKER "\t " namespace google { namespace protobuf { @@ -1247,6 +1248,18 @@ class TextFormat::Printer::TextGenerator buffer_size_(0), at_start_of_line_(true), failed_(false), + insert_silent_marker_(false), + indent_level_(initial_indent_level), + initial_indent_level_(initial_indent_level) {} + + explicit TextGenerator(io::ZeroCopyOutputStream* output, + bool insert_silent_marker, int initial_indent_level) + : output_(output), + buffer_(nullptr), + buffer_size_(0), + at_start_of_line_(true), + failed_(false), + insert_silent_marker_(insert_silent_marker), indent_level_(initial_indent_level), initial_indent_level_(initial_indent_level) {} @@ -1309,6 +1322,22 @@ class TextFormat::Printer::TextGenerator // error.) bool failed() const { return failed_; } + void PrintMaybeWithMarker(StringPiece text) { + Print(text.data(), text.size()); + if (ConsumeInsertSilentMarker()) { + PrintLiteral(DEBUG_STRING_SILENT_MARKER); + } + } + + void PrintMaybeWithMarker(StringPiece text_head, + StringPiece text_tail) { + Print(text_head.data(), text_head.size()); + if (ConsumeInsertSilentMarker()) { + PrintLiteral(DEBUG_STRING_SILENT_MARKER); + } + Print(text_tail.data(), text_tail.size()); + } + private: GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(TextGenerator); @@ -1369,16 +1398,67 @@ class TextFormat::Printer::TextGenerator buffer_size_ -= size; } + // Return the current value of insert_silent_marker_. If it is true, set it + // to false as we assume that a silent marker is inserted after a call to this + // function. + bool ConsumeInsertSilentMarker() { + if (insert_silent_marker_) { + insert_silent_marker_ = false; + return true; + } + return false; + } + io::ZeroCopyOutputStream* const output_; char* buffer_; int buffer_size_; bool at_start_of_line_; bool failed_; + // This flag is false when inserting silent marker is disabled or a silent + // marker has been inserted. + bool insert_silent_marker_; int indent_level_; int initial_indent_level_; }; +// =========================================================================== +// An internal field value printer that may insert a silent marker in +// DebugStrings. +class TextFormat::Printer::DebugStringFieldValuePrinter + : public TextFormat::FastFieldValuePrinter { + public: + void PrintMessageStart(const Message& message, int field_index, + int field_count, bool single_line_mode, + BaseTextGenerator* generator) const override { + // This is safe as only TextGenerator is used with + // DebugStringFieldValuePrinter. + TextGenerator* text_generator = static_cast(generator); + if (single_line_mode) { + text_generator->PrintMaybeWithMarker(" ", "{ "); + } else { + text_generator->PrintMaybeWithMarker(" ", "{\n"); + } + } +}; + +// =========================================================================== +// An internal field value printer that escape UTF8 strings. +class TextFormat::Printer::FastFieldValuePrinterUtf8Escaping + : public TextFormat::Printer::DebugStringFieldValuePrinter { + public: + void PrintString(const std::string& val, + TextFormat::BaseTextGenerator* generator) const override { + generator->PrintLiteral("\""); + generator->PrintString(strings::Utf8SafeCEscape(val)); + generator->PrintLiteral("\""); + } + void PrintBytes(const std::string& val, + TextFormat::BaseTextGenerator* generator) const override { + return FastFieldValuePrinter::PrintString(val, generator); + } +}; + // =========================================================================== // Implementation of the default Finder for extensions. TextFormat::Finder::~Finder() {} @@ -1808,22 +1888,6 @@ class FieldValuePrinterWrapper : public TextFormat::FastFieldValuePrinter { std::unique_ptr delegate_; }; -// Our own specialization: for UTF8 escaped strings. -class FastFieldValuePrinterUtf8Escaping - : public TextFormat::FastFieldValuePrinter { - public: - void PrintString(const std::string& val, - TextFormat::BaseTextGenerator* generator) const override { - generator->PrintLiteral("\""); - generator->PrintString(strings::Utf8SafeCEscape(val)); - generator->PrintLiteral("\""); - } - void PrintBytes(const std::string& val, - TextFormat::BaseTextGenerator* generator) const override { - return FastFieldValuePrinter::PrintString(val, generator); - } -}; - } // namespace const char* const TextFormat::Printer::kDoNotParse = @@ -1834,6 +1898,7 @@ TextFormat::Printer::Printer() single_line_mode_(false), use_field_number_(false), use_short_repeated_primitives_(false), + insert_silent_marker_(false), hide_unknown_fields_(false), print_message_fields_in_index_order_(false), expand_any_(false), @@ -1844,7 +1909,7 @@ TextFormat::Printer::Printer() void TextFormat::Printer::SetUseUtf8StringEscaping(bool as_utf8) { SetDefaultFieldValuePrinter(as_utf8 ? new FastFieldValuePrinterUtf8Escaping() - : new FastFieldValuePrinter()); + : new DebugStringFieldValuePrinter()); } void TextFormat::Printer::SetDefaultFieldValuePrinter( @@ -1924,7 +1989,7 @@ bool TextFormat::Printer::PrintUnknownFieldsToString( bool TextFormat::Printer::Print(const Message& message, io::ZeroCopyOutputStream* output) const { - TextGenerator generator(output, initial_indent_level_); + TextGenerator generator(output, insert_silent_marker_, initial_indent_level_); Print(message, &generator); @@ -2302,7 +2367,7 @@ void TextFormat::Printer::PrintField(const Message& message, printer->PrintMessageEnd(sub_message, field_index, count, single_line_mode_, generator); } else { - generator->PrintLiteral(": "); + generator->PrintMaybeWithMarker(": "); // Write the field value. PrintFieldValue(message, reflection, field, field_index, generator); if (single_line_mode_) { @@ -2327,7 +2392,7 @@ void TextFormat::Printer::PrintShortRepeatedField( int size = reflection->FieldSize(message, field); PrintFieldName(message, /*field_index=*/-1, /*field_count=*/size, reflection, field, generator); - generator->PrintLiteral(": ["); + generator->PrintMaybeWithMarker(": ", "["); for (int i = 0; i < size; i++) { if (i > 0) generator->PrintLiteral(", "); PrintFieldValue(message, reflection, field, i, generator); @@ -2481,7 +2546,7 @@ void TextFormat::Printer::PrintUnknownFields( switch (field.type()) { case UnknownField::TYPE_VARINT: generator->PrintString(field_number); - generator->PrintLiteral(": "); + generator->PrintMaybeWithMarker(": "); generator->PrintString(StrCat(field.varint())); if (single_line_mode_) { generator->PrintLiteral(" "); @@ -2491,7 +2556,7 @@ void TextFormat::Printer::PrintUnknownFields( break; case UnknownField::TYPE_FIXED32: { generator->PrintString(field_number); - generator->PrintLiteral(": 0x"); + generator->PrintMaybeWithMarker(": ", "0x"); generator->PrintString( StrCat(strings::Hex(field.fixed32(), strings::ZERO_PAD_8))); if (single_line_mode_) { @@ -2503,7 +2568,7 @@ void TextFormat::Printer::PrintUnknownFields( } case UnknownField::TYPE_FIXED64: { generator->PrintString(field_number); - generator->PrintLiteral(": 0x"); + generator->PrintMaybeWithMarker(": ", "0x"); generator->PrintString( StrCat(strings::Hex(field.fixed64(), strings::ZERO_PAD_16))); if (single_line_mode_) { @@ -2528,9 +2593,9 @@ void TextFormat::Printer::PrintUnknownFields( // This field is parseable as a Message. // So it is probably an embedded message. if (single_line_mode_) { - generator->PrintLiteral(" { "); + generator->PrintMaybeWithMarker(" ", "{ "); } else { - generator->PrintLiteral(" {\n"); + generator->PrintMaybeWithMarker(" ", "{\n"); generator->Indent(); } PrintUnknownFields(embedded_unknown_fields, generator, @@ -2544,7 +2609,7 @@ void TextFormat::Printer::PrintUnknownFields( } else { // This field is not parseable as a Message (or we ran out of // recursion budget). So it is probably just a plain string. - generator->PrintLiteral(": \""); + generator->PrintMaybeWithMarker(": ", "\""); generator->PrintString(CEscape(value)); if (single_line_mode_) { generator->PrintLiteral("\" "); @@ -2557,9 +2622,9 @@ void TextFormat::Printer::PrintUnknownFields( case UnknownField::TYPE_GROUP: generator->PrintString(field_number); if (single_line_mode_) { - generator->PrintLiteral(" { "); + generator->PrintMaybeWithMarker(" ", "{ "); } else { - generator->PrintLiteral(" {\n"); + generator->PrintMaybeWithMarker(" ", "{\n"); generator->Indent(); } // For groups, we recurse without checking the budget. This is OK, diff --git a/src/google/protobuf/text_format.h b/src/google/protobuf/text_format.h index 3a3ee1563a..ce4d1f44a7 100644 --- a/src/google/protobuf/text_format.h +++ b/src/google/protobuf/text_format.h @@ -62,9 +62,9 @@ namespace io { class ErrorCollector; // tokenizer.h } -// This class implements protocol buffer text format. Printing and parsing -// protocol messages in text format is useful for debugging and human editing -// of messages. +// This class implements protocol buffer text format, colloquially known as text +// proto. Printing and parsing protocol messages in text format is useful for +// debugging and human editing of messages. // // This class is really a namespace that contains only static methods. class PROTOBUF_EXPORT TextFormat { @@ -369,6 +369,14 @@ class PROTOBUF_EXPORT TextFormat { // output to the OutputStream (see text_format.cc for implementation). class TextGenerator; + // Forward declaration of an internal class used to print field values for + // DebugString APIs (see text_format.cc for implementation). + class DebugStringFieldValuePrinter; + + // Forward declaration of an internal class used to print UTF-8 escaped + // strings (see text_format.cc for implementation). + class FastFieldValuePrinterUtf8Escaping; + static const char* const kDoNotParse; // Internal Print method, used for writing to the OutputStream via @@ -419,6 +427,7 @@ class PROTOBUF_EXPORT TextFormat { bool single_line_mode_; bool use_field_number_; bool use_short_repeated_primitives_; + bool insert_silent_marker_; bool hide_unknown_fields_; bool print_message_fields_in_index_order_; bool expand_any_; diff --git a/src/google/protobuf/timestamp.pb.cc b/src/google/protobuf/timestamp.pb.cc index a3040a62b3..1828f839e9 100644 --- a/src/google/protobuf/timestamp.pb.cc +++ b/src/google/protobuf/timestamp.pb.cc @@ -285,7 +285,7 @@ bool Timestamp::IsInitialized() const { void Timestamp::InternalSwap(Timestamp* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); ::PROTOBUF_NAMESPACE_ID::internal::memswap< PROTOBUF_FIELD_OFFSET(Timestamp, nanos_) + sizeof(Timestamp::nanos_) diff --git a/src/google/protobuf/type.pb.cc b/src/google/protobuf/type.pb.cc index 024b277519..214d205a69 100644 --- a/src/google/protobuf/type.pb.cc +++ b/src/google/protobuf/type.pb.cc @@ -696,7 +696,7 @@ bool Type::IsInitialized() const { void Type::InternalSwap(Type* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); fields_.InternalSwap(&other->fields_); oneofs_.InternalSwap(&other->oneofs_); options_.InternalSwap(&other->options_); @@ -1173,7 +1173,7 @@ bool Field::IsInitialized() const { void Field::InternalSwap(Field* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); options_.InternalSwap(&other->options_); name_.Swap(&other->name_, &::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); type_url_.Swap(&other->type_url_, &::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); @@ -1522,7 +1522,7 @@ bool Enum::IsInitialized() const { void Enum::InternalSwap(Enum* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); enumvalue_.InternalSwap(&other->enumvalue_); options_.InternalSwap(&other->options_); name_.Swap(&other->name_, &::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); @@ -1788,7 +1788,7 @@ bool EnumValue::IsInitialized() const { void EnumValue::InternalSwap(EnumValue* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); options_.InternalSwap(&other->options_); name_.Swap(&other->name_, &::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); swap(number_, other->number_); @@ -2038,7 +2038,7 @@ bool Option::IsInitialized() const { void Option::InternalSwap(Option* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); name_.Swap(&other->name_, &::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); swap(value_, other->value_); } diff --git a/src/google/protobuf/type.pb.h b/src/google/protobuf/type.pb.h index acc86ed93b..e6a93f1245 100644 --- a/src/google/protobuf/type.pb.h +++ b/src/google/protobuf/type.pb.h @@ -373,7 +373,7 @@ class PROTOBUF_EXPORT Type PROTOBUF_FINAL : public: void clear_source_context(); const PROTOBUF_NAMESPACE_ID::SourceContext& source_context() const; - PROTOBUF_NAMESPACE_ID::SourceContext* release_source_context(); + PROTOBUF_FUTURE_MUST_USE_RESULT PROTOBUF_NAMESPACE_ID::SourceContext* release_source_context(); PROTOBUF_NAMESPACE_ID::SourceContext* mutable_source_context(); void set_allocated_source_context(PROTOBUF_NAMESPACE_ID::SourceContext* source_context); private: @@ -938,7 +938,7 @@ class PROTOBUF_EXPORT Enum PROTOBUF_FINAL : public: void clear_source_context(); const PROTOBUF_NAMESPACE_ID::SourceContext& source_context() const; - PROTOBUF_NAMESPACE_ID::SourceContext* release_source_context(); + PROTOBUF_FUTURE_MUST_USE_RESULT PROTOBUF_NAMESPACE_ID::SourceContext* release_source_context(); PROTOBUF_NAMESPACE_ID::SourceContext* mutable_source_context(); void set_allocated_source_context(PROTOBUF_NAMESPACE_ID::SourceContext* source_context); private: @@ -1273,7 +1273,7 @@ class PROTOBUF_EXPORT Option PROTOBUF_FINAL : public: void clear_value(); const PROTOBUF_NAMESPACE_ID::Any& value() const; - PROTOBUF_NAMESPACE_ID::Any* release_value(); + PROTOBUF_FUTURE_MUST_USE_RESULT PROTOBUF_NAMESPACE_ID::Any* release_value(); PROTOBUF_NAMESPACE_ID::Any* mutable_value(); void set_allocated_value(PROTOBUF_NAMESPACE_ID::Any* value); private: diff --git a/src/google/protobuf/unittest_proto3_arena.proto b/src/google/protobuf/unittest_proto3_arena.proto index fa26488d98..1752939797 100644 --- a/src/google/protobuf/unittest_proto3_arena.proto +++ b/src/google/protobuf/unittest_proto3_arena.proto @@ -116,6 +116,23 @@ message TestAllTypes { repeated string repeated_string = 44; repeated bytes repeated_bytes = 45; + // Optional + optional int32 proto3_optional_int32 = 116; + optional int64 proto3_optional_int64 = 117; + optional uint32 proto3_optional_uint32 = 118; + optional uint64 proto3_optional_uint64 = 119; + optional sint32 proto3_optional_sint32 = 120; + optional sint64 proto3_optional_sint64 = 121; + optional fixed32 proto3_optional_fixed32 = 122; + optional fixed64 proto3_optional_fixed64 = 123; + optional sfixed32 proto3_optional_sfixed32 = 124; + optional sfixed64 proto3_optional_sfixed64 = 125; + optional float proto3_optional_float = 126; + optional double proto3_optional_double = 127; + optional bool proto3_optional_bool = 128; + optional string proto3_optional_string = 129; + optional bytes proto3_optional_bytes = 130; + // Groups are not allowed in proto3. // repeated group RepeatedGroup = 46 { // optional int32 a = 47; diff --git a/src/google/protobuf/util/message_differencer.cc b/src/google/protobuf/util/message_differencer.cc index 4815178cd4..88e01df647 100644 --- a/src/google/protobuf/util/message_differencer.cc +++ b/src/google/protobuf/util/message_differencer.cc @@ -266,7 +266,6 @@ bool MessageDifferencer::ApproximatelyEquivalent(const Message& message1, MessageDifferencer::MessageDifferencer() : reporter_(NULL), - field_comparator_(NULL), message_field_comparison_(EQUAL), scope_(FULL), repeated_field_comparison_(AS_LIST), @@ -289,9 +288,19 @@ MessageDifferencer::~MessageDifferencer() { void MessageDifferencer::set_field_comparator(FieldComparator* comparator) { GOOGLE_CHECK(comparator) << "Field comparator can't be NULL."; - field_comparator_ = comparator; + field_comparator_kind_ = kFCBase; + field_comparator_.base = comparator; } +#ifdef PROTOBUF_FUTURE_BREAKING_CHANGES +void MessageDifferencer::set_field_comparator( + DefaultFieldComparator* comparator) { + GOOGLE_CHECK(comparator) << "Field comparator can't be NULL."; + field_comparator_kind_ = kFCDefault; + field_comparator_.default_impl = comparator; +} +#endif // PROTOBUF_FUTURE_BREAKING_CHANGES + void MessageDifferencer::set_message_field_comparison( MessageFieldComparison comparison) { message_field_comparison_ = comparison; @@ -1008,44 +1017,29 @@ bool MessageDifferencer::CompareMapField( const Reflection* reflection2 = message2.GetReflection(); // When both map fields are on map, do not sync to repeated field. - // TODO(jieluo): Add support for reporter - if (reporter_ == nullptr && + if (reflection1->GetMapData(message1, repeated_field)->IsMapValid() && + reflection2->GetMapData(message2, repeated_field)->IsMapValid() && + // TODO(jieluo): Add support for reporter + reporter_ == nullptr && // Users didn't set custom map field key comparator map_field_key_comparator_.find(repeated_field) == map_field_key_comparator_.end() && // Users didn't set repeated field comparison - repeated_field_comparison_ == AS_LIST) { - DefaultFieldComparator* map_field_comparator = - field_comparator_ ? nullptr : &default_field_comparator_; -#if PROTOBUF_RTTI - // Inherit class from DefaultFieldComparator can not get the benefit - // because DefaultFieldComparator::Compare() method might be overwrote. - if (field_comparator_ && - typeid(*field_comparator_) == typeid(default_field_comparator_)) { - map_field_comparator = - static_cast(field_comparator_); - } -#endif - if (map_field_comparator) { - const FieldDescriptor* key_des = - repeated_field->message_type()->map_key(); - const FieldDescriptor* val_des = - repeated_field->message_type()->map_value(); - const internal::MapFieldBase* map_field1 = - reflection1->GetMapData(message1, repeated_field); - const internal::MapFieldBase* map_field2 = - reflection2->GetMapData(message2, repeated_field); - std::vector current_parent_fields(*parent_fields); - SpecificField specific_field; - specific_field.field = repeated_field; - current_parent_fields.push_back(specific_field); - if (map_field1->IsMapValid() && map_field2->IsMapValid() && - !IsIgnored(message1, message2, key_des, current_parent_fields) && - !IsIgnored(message1, message2, val_des, current_parent_fields)) { - return CompareMapFieldByMapReflection( - message1, message2, repeated_field, ¤t_parent_fields, - map_field_comparator); - } + repeated_field_comparison_ == AS_LIST && + // Users didn't set their own FieldComparator implementation + field_comparator_kind_ == kFCDefault) { + const FieldDescriptor* key_des = repeated_field->message_type()->map_key(); + const FieldDescriptor* val_des = + repeated_field->message_type()->map_value(); + std::vector current_parent_fields(*parent_fields); + SpecificField specific_field; + specific_field.field = repeated_field; + current_parent_fields.push_back(specific_field); + if (!IsIgnored(message1, message2, key_des, current_parent_fields) && + !IsIgnored(message1, message2, val_des, current_parent_fields)) { + return CompareMapFieldByMapReflection(message1, message2, repeated_field, + ¤t_parent_fields, + field_comparator_.default_impl); } } @@ -1869,9 +1863,9 @@ FieldComparator::ComparisonResult MessageDifferencer::GetFieldComparisonResult( const Message& message1, const Message& message2, const FieldDescriptor* field, int index1, int index2, const FieldContext* field_context) { - FieldComparator* comparator = field_comparator_ != NULL - ? field_comparator_ - : &default_field_comparator_; + FieldComparator* comparator = field_comparator_kind_ == kFCBase + ? field_comparator_.base + : field_comparator_.default_impl; return comparator->Compare(message1, message2, field, index1, index2, field_context); } diff --git a/src/google/protobuf/util/message_differencer.h b/src/google/protobuf/util/message_differencer.h index 943a0db2d3..4cdb817304 100644 --- a/src/google/protobuf/util/message_differencer.h +++ b/src/google/protobuf/util/message_differencer.h @@ -533,6 +533,9 @@ class PROTOBUF_EXPORT MessageDifferencer { // Note that this method must be called before Compare for the comparator to // be used. void set_field_comparator(FieldComparator* comparator); +#ifdef PROTOBUF_FUTURE_BREAKING_CHANGES + void set_field_comparator(DefaultFieldComparator* comparator); +#endif // PROTOBUF_FUTURE_BREAKING_CHANGES // DEPRECATED. Pass a DefaultFieldComparator instance instead. // Sets the fraction and margin for the float comparison of a given field. @@ -904,7 +907,6 @@ class PROTOBUF_EXPORT MessageDifferencer { Reporter* reporter_; DefaultFieldComparator default_field_comparator_; - FieldComparator* field_comparator_; MessageFieldComparison message_field_comparison_; Scope scope_; RepeatedFieldComparison repeated_field_comparison_; @@ -925,6 +927,12 @@ class PROTOBUF_EXPORT MessageDifferencer { FieldSet ignored_fields_; + union { + DefaultFieldComparator* default_impl; + FieldComparator* base; + } field_comparator_ = {&default_field_comparator_}; + enum { kFCDefault, kFCBase } field_comparator_kind_ = kFCDefault; + bool report_matches_; bool report_moves_; bool report_ignores_; diff --git a/src/google/protobuf/wrappers.pb.cc b/src/google/protobuf/wrappers.pb.cc index c791d05bd6..0f16f6630d 100644 --- a/src/google/protobuf/wrappers.pb.cc +++ b/src/google/protobuf/wrappers.pb.cc @@ -417,7 +417,7 @@ bool DoubleValue::IsInitialized() const { void DoubleValue::InternalSwap(DoubleValue* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); swap(value_, other->value_); } @@ -605,7 +605,7 @@ bool FloatValue::IsInitialized() const { void FloatValue::InternalSwap(FloatValue* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); swap(value_, other->value_); } @@ -795,7 +795,7 @@ bool Int64Value::IsInitialized() const { void Int64Value::InternalSwap(Int64Value* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); swap(value_, other->value_); } @@ -985,7 +985,7 @@ bool UInt64Value::IsInitialized() const { void UInt64Value::InternalSwap(UInt64Value* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); swap(value_, other->value_); } @@ -1175,7 +1175,7 @@ bool Int32Value::IsInitialized() const { void Int32Value::InternalSwap(Int32Value* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); swap(value_, other->value_); } @@ -1365,7 +1365,7 @@ bool UInt32Value::IsInitialized() const { void UInt32Value::InternalSwap(UInt32Value* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); swap(value_, other->value_); } @@ -1553,7 +1553,7 @@ bool BoolValue::IsInitialized() const { void BoolValue::InternalSwap(BoolValue* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); swap(value_, other->value_); } @@ -1754,7 +1754,7 @@ bool StringValue::IsInitialized() const { void StringValue::InternalSwap(StringValue* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); value_.Swap(&other->value_, &::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); } @@ -1950,7 +1950,7 @@ bool BytesValue::IsInitialized() const { void BytesValue::InternalSwap(BytesValue* other) { using std::swap; - _internal_metadata_.Swap<::PROTOBUF_NAMESPACE_ID::UnknownFieldSet>(&other->_internal_metadata_); + _internal_metadata_.InternalSwap(&other->_internal_metadata_); value_.Swap(&other->value_, &::PROTOBUF_NAMESPACE_ID::internal::GetEmptyStringAlreadyInited(), GetArena()); }