diff --git a/.bazelrc b/.bazelrc index 65cc8c491e52..f8d9557ef1a5 100644 --- a/.bazelrc +++ b/.bazelrc @@ -22,6 +22,8 @@ build:ubsan --action_env=UBSAN_OPTIONS=halt_on_error=1:print_stacktrace=1 # Workaround for the fact that Bazel links with $CC, not $CXX # https://github.com/bazelbuild/bazel/issues/11122#issuecomment-613746748 build:ubsan --copt=-fno-sanitize=function --copt=-fno-sanitize=vptr +# Abseil passes nullptr to memcmp with 0 size +build:ubsan --copt=-fno-sanitize=nonnull-attribute # TODO: migrate all dependencies from WORKSPACE to MODULE.bazel # https://github.com/protocolbuffers/protobuf/issues/14313 diff --git a/ci/common.bazelrc b/ci/common.bazelrc index b0501d5d202c..ae483fa7cdfe 100644 --- a/ci/common.bazelrc +++ b/ci/common.bazelrc @@ -27,6 +27,8 @@ build:ubsan --action_env=UBSAN_OPTIONS=halt_on_error=1:print_stacktrace=1 # Workaround for the fact that Bazel links with $CC, not $CXX # https://github.com/bazelbuild/bazel/issues/11122#issuecomment-613746748 build:ubsan --copt=-fno-sanitize=function --copt=-fno-sanitize=vptr +# Abseil passes nullptr to memcmp with 0 size +build:ubsan --copt=-fno-sanitize=nonnull-attribute # Workaround Bazel 7 remote cache issues. # See https://github.com/bazelbuild/bazel/issues/20161 diff --git a/java/core/src/test/java/com/google/protobuf/CodedOutputStreamTest.java b/java/core/src/test/java/com/google/protobuf/CodedOutputStreamTest.java index 51e66b9759ff..1f17f24fe3c7 100644 --- a/java/core/src/test/java/com/google/protobuf/CodedOutputStreamTest.java +++ b/java/core/src/test/java/com/google/protobuf/CodedOutputStreamTest.java @@ -13,7 +13,6 @@ import com.google.protobuf.CodedOutputStream.OutOfSpaceException; import protobuf_unittest.UnittestProto.SparseEnumMessage; import protobuf_unittest.UnittestProto.TestAllTypes; -import protobuf_unittest.UnittestProto.TestPackedTypes; import protobuf_unittest.UnittestProto.TestSparseEnum; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; @@ -402,47 +401,6 @@ public void computeTagSize() { assertThat(CodedOutputStream.computeTagSize((1 << 30) + 1)).isEqualTo(1); } - /** Tests writing a whole message with every field type. */ - @Test - public void testWriteWholeMessage() throws Exception { - final byte[] expectedBytes = TestUtil.getGoldenMessage().toByteArray(); - TestAllTypes message = TestUtil.getAllSet(); - - for (OutputType outputType : OutputType.values()) { - Coder coder = outputType.newCoder(message.getSerializedSize()); - message.writeTo(coder.stream()); - coder.stream().flush(); - byte[] rawBytes = coder.toByteArray(); - assertEqualBytes(outputType, expectedBytes, rawBytes); - } - - // Try different block sizes. - for (int blockSize = 1; blockSize < 256; blockSize *= 2) { - Coder coder = OutputType.STREAM.newCoder(blockSize); - message.writeTo(coder.stream()); - coder.stream().flush(); - assertEqualBytes(OutputType.STREAM, expectedBytes, coder.toByteArray()); - } - } - - /** - * Tests writing a whole message with every packed field type. Ensures the wire format of packed - * fields is compatible with C++. - */ - @Test - public void testWriteWholePackedFieldsMessage() throws Exception { - byte[] expectedBytes = TestUtil.getGoldenPackedFieldsMessage().toByteArray(); - TestPackedTypes message = TestUtil.getPackedSet(); - - for (OutputType outputType : OutputType.values()) { - Coder coder = outputType.newCoder(message.getSerializedSize()); - message.writeTo(coder.stream()); - coder.stream().flush(); - byte[] rawBytes = coder.toByteArray(); - assertEqualBytes(outputType, expectedBytes, rawBytes); - } - } - /** * Test writing a message containing a negative enum value. This used to fail because the size was * not properly computed as a sign-extended varint. diff --git a/java/core/src/test/java/com/google/protobuf/TestUtil.java b/java/core/src/test/java/com/google/protobuf/TestUtil.java index 0f57ecc9acca..598a086f71b6 100644 --- a/java/core/src/test/java/com/google/protobuf/TestUtil.java +++ b/java/core/src/test/java/com/google/protobuf/TestUtil.java @@ -210,10 +210,6 @@ import protobuf_unittest.UnittestProto.TestPackedTypes; import protobuf_unittest.UnittestProto.TestRequired; import protobuf_unittest.UnittestProto.TestUnpackedTypes; -import java.io.File; -import java.io.IOException; -import java.io.InputStream; -import java.io.RandomAccessFile; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -234,6 +230,281 @@ public final class TestUtil { private TestUtil() {} + public static final String ALL_FIELDS_SET_TEXT = + "" + + "optional_int32: 101\n" + + "optional_int64: 102\n" + + "optional_uint32: 103\n" + + "optional_uint64: 104\n" + + "optional_sint32: 105\n" + + "optional_sint64: 106\n" + + "optional_fixed32: 107\n" + + "optional_fixed64: 108\n" + + "optional_sfixed32: 109\n" + + "optional_sfixed64: 110\n" + + "optional_float: 111.0\n" + + "optional_double: 112.0\n" + + "optional_bool: true\n" + + "optional_string: \"115\"\n" + + "optional_bytes: \"116\"\n" + + "OptionalGroup {\n" + + " a: 117\n" + + "}\n" + + "optional_nested_message {\n" + + " bb: 118\n" + + "}\n" + + "optional_foreign_message {\n" + + " c: 119\n" + + "}\n" + + "optional_import_message {\n" + + " d: 120\n" + + "}\n" + + "optional_nested_enum: BAZ\n" + + "optional_foreign_enum: FOREIGN_BAZ\n" + + "optional_import_enum: IMPORT_BAZ\n" + + "optional_string_piece: \"124\"\n" + + "optional_cord: \"125\"\n" + + "optional_public_import_message {\n" + + " e: 126\n" + + "}\n" + + "optional_lazy_message {\n" + + " bb: 127\n" + + "}\n" + + "optional_unverified_lazy_message {\n" + + " bb: 128\n" + + "}\n" + + "repeated_int32: 201\n" + + "repeated_int32: 301\n" + + "repeated_int64: 202\n" + + "repeated_int64: 302\n" + + "repeated_uint32: 203\n" + + "repeated_uint32: 303\n" + + "repeated_uint64: 204\n" + + "repeated_uint64: 304\n" + + "repeated_sint32: 205\n" + + "repeated_sint32: 305\n" + + "repeated_sint64: 206\n" + + "repeated_sint64: 306\n" + + "repeated_fixed32: 207\n" + + "repeated_fixed32: 307\n" + + "repeated_fixed64: 208\n" + + "repeated_fixed64: 308\n" + + "repeated_sfixed32: 209\n" + + "repeated_sfixed32: 309\n" + + "repeated_sfixed64: 210\n" + + "repeated_sfixed64: 310\n" + + "repeated_float: 211.0\n" + + "repeated_float: 311.0\n" + + "repeated_double: 212.0\n" + + "repeated_double: 312.0\n" + + "repeated_bool: true\n" + + "repeated_bool: false\n" + + "repeated_string: \"215\"\n" + + "repeated_string: \"315\"\n" + + "repeated_bytes: \"216\"\n" + + "repeated_bytes: \"316\"\n" + + "RepeatedGroup {\n" + + " a: 217\n" + + "}\n" + + "RepeatedGroup {\n" + + " a: 317\n" + + "}\n" + + "repeated_nested_message {\n" + + " bb: 218\n" + + "}\n" + + "repeated_nested_message {\n" + + " bb: 318\n" + + "}\n" + + "repeated_foreign_message {\n" + + " c: 219\n" + + "}\n" + + "repeated_foreign_message {\n" + + " c: 319\n" + + "}\n" + + "repeated_import_message {\n" + + " d: 220\n" + + "}\n" + + "repeated_import_message {\n" + + " d: 320\n" + + "}\n" + + "repeated_nested_enum: BAR\n" + + "repeated_nested_enum: BAZ\n" + + "repeated_foreign_enum: FOREIGN_BAR\n" + + "repeated_foreign_enum: FOREIGN_BAZ\n" + + "repeated_import_enum: IMPORT_BAR\n" + + "repeated_import_enum: IMPORT_BAZ\n" + + "repeated_string_piece: \"224\"\n" + + "repeated_string_piece: \"324\"\n" + + "repeated_cord: \"225\"\n" + + "repeated_cord: \"325\"\n" + + "repeated_lazy_message {\n" + + " bb: 227\n" + + "}\n" + + "repeated_lazy_message {\n" + + " bb: 327\n" + + "}\n" + + "default_int32: 401\n" + + "default_int64: 402\n" + + "default_uint32: 403\n" + + "default_uint64: 404\n" + + "default_sint32: 405\n" + + "default_sint64: 406\n" + + "default_fixed32: 407\n" + + "default_fixed64: 408\n" + + "default_sfixed32: 409\n" + + "default_sfixed64: 410\n" + + "default_float: 411.0\n" + + "default_double: 412.0\n" + + "default_bool: false\n" + + "default_string: \"415\"\n" + + "default_bytes: \"416\"\n" + + "default_nested_enum: FOO\n" + + "default_foreign_enum: FOREIGN_FOO\n" + + "default_import_enum: IMPORT_FOO\n" + + "default_string_piece: \"424\"\n" + + "default_cord: \"425\"\n" + + "oneof_bytes: \"604\"\n"; + + public static final String ALL_EXTENSIONS_SET_TEXT = + "" + + "[protobuf_unittest.optional_int32_extension]: 101\n" + + "[protobuf_unittest.optional_int64_extension]: 102\n" + + "[protobuf_unittest.optional_uint32_extension]: 103\n" + + "[protobuf_unittest.optional_uint64_extension]: 104\n" + + "[protobuf_unittest.optional_sint32_extension]: 105\n" + + "[protobuf_unittest.optional_sint64_extension]: 106\n" + + "[protobuf_unittest.optional_fixed32_extension]: 107\n" + + "[protobuf_unittest.optional_fixed64_extension]: 108\n" + + "[protobuf_unittest.optional_sfixed32_extension]: 109\n" + + "[protobuf_unittest.optional_sfixed64_extension]: 110\n" + + "[protobuf_unittest.optional_float_extension]: 111.0\n" + + "[protobuf_unittest.optional_double_extension]: 112.0\n" + + "[protobuf_unittest.optional_bool_extension]: true\n" + + "[protobuf_unittest.optional_string_extension]: \"115\"\n" + + "[protobuf_unittest.optional_bytes_extension]: \"116\"\n" + + "[protobuf_unittest.optionalgroup_extension] {\n" + + " a: 117\n" + + "}\n" + + "[protobuf_unittest.optional_nested_message_extension] {\n" + + " bb: 118\n" + + "}\n" + + "[protobuf_unittest.optional_foreign_message_extension] {\n" + + " c: 119\n" + + "}\n" + + "[protobuf_unittest.optional_import_message_extension] {\n" + + " d: 120\n" + + "}\n" + + "[protobuf_unittest.optional_nested_enum_extension]: BAZ\n" + + "[protobuf_unittest.optional_foreign_enum_extension]: FOREIGN_BAZ\n" + + "[protobuf_unittest.optional_import_enum_extension]: IMPORT_BAZ\n" + + "[protobuf_unittest.optional_string_piece_extension]: \"124\"\n" + + "[protobuf_unittest.optional_cord_extension]: \"125\"\n" + + "[protobuf_unittest.optional_public_import_message_extension] {\n" + + " e: 126\n" + + "}\n" + + "[protobuf_unittest.optional_lazy_message_extension] {\n" + + " bb: 127\n" + + "}\n" + + "[protobuf_unittest.optional_unverified_lazy_message_extension] {\n" + + " bb: 128\n" + + "}\n" + + "[protobuf_unittest.repeated_int32_extension]: 201\n" + + "[protobuf_unittest.repeated_int32_extension]: 301\n" + + "[protobuf_unittest.repeated_int64_extension]: 202\n" + + "[protobuf_unittest.repeated_int64_extension]: 302\n" + + "[protobuf_unittest.repeated_uint32_extension]: 203\n" + + "[protobuf_unittest.repeated_uint32_extension]: 303\n" + + "[protobuf_unittest.repeated_uint64_extension]: 204\n" + + "[protobuf_unittest.repeated_uint64_extension]: 304\n" + + "[protobuf_unittest.repeated_sint32_extension]: 205\n" + + "[protobuf_unittest.repeated_sint32_extension]: 305\n" + + "[protobuf_unittest.repeated_sint64_extension]: 206\n" + + "[protobuf_unittest.repeated_sint64_extension]: 306\n" + + "[protobuf_unittest.repeated_fixed32_extension]: 207\n" + + "[protobuf_unittest.repeated_fixed32_extension]: 307\n" + + "[protobuf_unittest.repeated_fixed64_extension]: 208\n" + + "[protobuf_unittest.repeated_fixed64_extension]: 308\n" + + "[protobuf_unittest.repeated_sfixed32_extension]: 209\n" + + "[protobuf_unittest.repeated_sfixed32_extension]: 309\n" + + "[protobuf_unittest.repeated_sfixed64_extension]: 210\n" + + "[protobuf_unittest.repeated_sfixed64_extension]: 310\n" + + "[protobuf_unittest.repeated_float_extension]: 211.0\n" + + "[protobuf_unittest.repeated_float_extension]: 311.0\n" + + "[protobuf_unittest.repeated_double_extension]: 212.0\n" + + "[protobuf_unittest.repeated_double_extension]: 312.0\n" + + "[protobuf_unittest.repeated_bool_extension]: true\n" + + "[protobuf_unittest.repeated_bool_extension]: false\n" + + "[protobuf_unittest.repeated_string_extension]: \"215\"\n" + + "[protobuf_unittest.repeated_string_extension]: \"315\"\n" + + "[protobuf_unittest.repeated_bytes_extension]: \"216\"\n" + + "[protobuf_unittest.repeated_bytes_extension]: \"316\"\n" + + "[protobuf_unittest.repeatedgroup_extension] {\n" + + " a: 217\n" + + "}\n" + + "[protobuf_unittest.repeatedgroup_extension] {\n" + + " a: 317\n" + + "}\n" + + "[protobuf_unittest.repeated_nested_message_extension] {\n" + + " bb: 218\n" + + "}\n" + + "[protobuf_unittest.repeated_nested_message_extension] {\n" + + " bb: 318\n" + + "}\n" + + "[protobuf_unittest.repeated_foreign_message_extension] {\n" + + " c: 219\n" + + "}\n" + + "[protobuf_unittest.repeated_foreign_message_extension] {\n" + + " c: 319\n" + + "}\n" + + "[protobuf_unittest.repeated_import_message_extension] {\n" + + " d: 220\n" + + "}\n" + + "[protobuf_unittest.repeated_import_message_extension] {\n" + + " d: 320\n" + + "}\n" + + "[protobuf_unittest.repeated_nested_enum_extension]: BAR\n" + + "[protobuf_unittest.repeated_nested_enum_extension]: BAZ\n" + + "[protobuf_unittest.repeated_foreign_enum_extension]: FOREIGN_BAR\n" + + "[protobuf_unittest.repeated_foreign_enum_extension]: FOREIGN_BAZ\n" + + "[protobuf_unittest.repeated_import_enum_extension]: IMPORT_BAR\n" + + "[protobuf_unittest.repeated_import_enum_extension]: IMPORT_BAZ\n" + + "[protobuf_unittest.repeated_string_piece_extension]: \"224\"\n" + + "[protobuf_unittest.repeated_string_piece_extension]: \"324\"\n" + + "[protobuf_unittest.repeated_cord_extension]: \"225\"\n" + + "[protobuf_unittest.repeated_cord_extension]: \"325\"\n" + + "[protobuf_unittest.repeated_lazy_message_extension] {\n" + + " bb: 227\n" + + "}\n" + + "[protobuf_unittest.repeated_lazy_message_extension] {\n" + + " bb: 327\n" + + "}\n" + + "[protobuf_unittest.default_int32_extension]: 401\n" + + "[protobuf_unittest.default_int64_extension]: 402\n" + + "[protobuf_unittest.default_uint32_extension]: 403\n" + + "[protobuf_unittest.default_uint64_extension]: 404\n" + + "[protobuf_unittest.default_sint32_extension]: 405\n" + + "[protobuf_unittest.default_sint64_extension]: 406\n" + + "[protobuf_unittest.default_fixed32_extension]: 407\n" + + "[protobuf_unittest.default_fixed64_extension]: 408\n" + + "[protobuf_unittest.default_sfixed32_extension]: 409\n" + + "[protobuf_unittest.default_sfixed64_extension]: 410\n" + + "[protobuf_unittest.default_float_extension]: 411.0\n" + + "[protobuf_unittest.default_double_extension]: 412.0\n" + + "[protobuf_unittest.default_bool_extension]: false\n" + + "[protobuf_unittest.default_string_extension]: \"415\"\n" + + "[protobuf_unittest.default_bytes_extension]: \"416\"\n" + + "[protobuf_unittest.default_nested_enum_extension]: FOO\n" + + "[protobuf_unittest.default_foreign_enum_extension]: FOREIGN_FOO\n" + + "[protobuf_unittest.default_import_enum_extension]: IMPORT_FOO\n" + + "[protobuf_unittest.default_string_piece_extension]: \"424\"\n" + + "[protobuf_unittest.default_cord_extension]: \"425\"\n" + + "[protobuf_unittest.oneof_uint32_extension]: 601\n" + + "[protobuf_unittest.oneof_nested_message_extension] {\n" + + " bb: 602\n" + + "}\n" + + "[protobuf_unittest.oneof_string_extension]: \"603\"\n" + + "[protobuf_unittest.oneof_bytes_extension]: \"604\"\n"; + public static final TestRequired TEST_REQUIRED_UNINITIALIZED = TestRequired.newBuilder().setA(1).buildPartial(); public static final TestRequired TEST_REQUIRED_INITIALIZED = @@ -3764,108 +4035,6 @@ public void assertReflectionRepeatedSettersRejectNull(Message.Builder builder) } } - /** @param filePath The path relative to {@link #getTestDataDir}. */ - public static String readTextFromFile(String filePath) { - return readBytesFromFile(filePath) - .toStringUtf8() - .replace(System.getProperty("line.separator"), "\n"); - } - - private static File getTestDataDir() { - // Search each parent directory looking for "src/google/protobuf". - File ancestor = new File(System.getProperty("protobuf.dir", ".")); - String initialPath = ancestor.getAbsolutePath(); - try { - ancestor = ancestor.getCanonicalFile(); - } catch (IOException e) { - throw new RuntimeException("Couldn't get canonical name of working directory.", e); - } - - String srcRootCheck = "src/google/protobuf"; - - // If we're running w/ Bazel on Windows, we're not in a sandbox, so we - // we must change our source root check condition to find the true test data dir. - String testBinaryName = System.getenv("TEST_BINARY"); - if (testBinaryName != null && testBinaryName.endsWith(".exe")) { - srcRootCheck = srcRootCheck + "/descriptor.cc"; - } - - while (ancestor != null && ancestor.exists()) { - // Identify the true source root. - if (new File(ancestor, srcRootCheck).exists()) { - return new File(ancestor, "src/google/protobuf/testdata"); - } - ancestor = ancestor.getParentFile(); - } - - throw new RuntimeException( - "Could not find golden files. This test must be run from within the " - + "protobuf source package so that it can read test data files from the " - + "C++ source tree: " - + initialPath); - } - - /** @param filename The path relative to {@link #getTestDataDir}. */ - public static ByteString readBytesFromFile(String filename) { - File fullPath = new File(getTestDataDir(), filename); - try { - RandomAccessFile file = new RandomAccessFile(fullPath, "r"); - byte[] content = new byte[(int) file.length()]; - file.readFully(content); - return ByteString.copyFrom(content); - } catch (IOException e) { - // Throw a RuntimeException here so that we can call this function from - // static initializers. - throw new IllegalArgumentException("Couldn't read file: " + fullPath.getPath(), e); - } - } - // END FULL-RUNTIME - - private static ByteString readBytesFromResource(String name) { - try { - InputStream in = TestUtil.class.getResourceAsStream(name); - if (in == null) { // - throw new RuntimeException("Tests data file " + name + " is missing."); - } - return ByteString.readFrom(in); - } catch (IOException e) { - throw new RuntimeException(e); - } - } - - /** - * Get the bytes of the "golden message". This is a serialized TestAllTypes with all fields set as - * they would be by {@link #setAllFields(TestAllTypes.Builder)}, but it is loaded from a file on - * disk rather than generated dynamically. The file is actually generated by C++ code, so testing - * against it verifies compatibility with C++. - */ - public static ByteString getGoldenMessage() { - if (goldenMessage == null) { - goldenMessage = - readBytesFromResource("/google/protobuf/testdata/golden_message_oneof_implemented"); - } - return goldenMessage; - } - - private static ByteString goldenMessage = null; - - /** - * Get the bytes of the "golden packed fields message". This is a serialized TestPackedTypes with - * all fields set as they would be by {@link #setPackedFields(TestPackedTypes.Builder)}, but it is - * loaded from a file on disk rather than generated dynamically. The file is actually generated by - * C++ code, so testing against it verifies compatibility with C++. - */ - public static ByteString getGoldenPackedFieldsMessage() { - if (goldenPackedFieldsMessage == null) { - goldenPackedFieldsMessage = - readBytesFromResource("/google/protobuf/testdata/golden_packed_fields_message"); - } - return goldenPackedFieldsMessage; - } - - private static ByteString goldenPackedFieldsMessage = null; - - // BEGIN FULL-RUNTIME /** * Mock implementation of {@link GeneratedMessage.BuilderParent} for testing. * diff --git a/java/core/src/test/java/com/google/protobuf/TextFormatTest.java b/java/core/src/test/java/com/google/protobuf/TextFormatTest.java index d66a8b0f981d..54d0f3ba3b72 100644 --- a/java/core/src/test/java/com/google/protobuf/TextFormatTest.java +++ b/java/core/src/test/java/com/google/protobuf/TextFormatTest.java @@ -65,11 +65,6 @@ public class TextFormatTest { "\\\"A string with \\' characters \\n and \\r newlines " + "and \\t tabs and \\001 slashes \\\\"; - private static final String ALL_FIELDS_SET_TEXT = - TestUtil.readTextFromFile("text_format_unittest_data_oneof_implemented.txt"); - private static final String ALL_EXTENSIONS_SET_TEXT = - TestUtil.readTextFromFile("text_format_unittest_extensions_data.txt"); - private static final String EXOTIC_TEXT = "" + "repeated_int32: -1\n" @@ -145,12 +140,7 @@ public class TextFormatTest { public void testPrintMessage() throws Exception { String javaText = TextFormat.printer().printToString(TestUtil.getAllSet()); - // Java likes to add a trailing ".0" to floats and doubles. C printf - // (with %g format) does not. Our golden files are used for both - // C++ and Java TextFormat classes, so we need to conform. - javaText = javaText.replace(".0\n", "\n"); - - assertThat(javaText).isEqualTo(ALL_FIELDS_SET_TEXT); + assertThat(javaText).isEqualTo(TestUtil.ALL_FIELDS_SET_TEXT); } @Test @@ -165,12 +155,7 @@ public void testCharacterNotInUnicodeBlock() throws TextFormat.InvalidEscapeSequ public void testPrintMessageBuilder() throws Exception { String javaText = TextFormat.printer().printToString(TestUtil.getAllSetBuilder()); - // Java likes to add a trailing ".0" to floats and doubles. C printf - // (with %g format) does not. Our golden files are used for both - // C++ and Java TextFormat classes, so we need to conform. - javaText = javaText.replace(".0\n", "\n"); - - assertThat(javaText).isEqualTo(ALL_FIELDS_SET_TEXT); + assertThat(javaText).isEqualTo(TestUtil.ALL_FIELDS_SET_TEXT); } /** Print TestAllExtensions and compare with golden file. */ @@ -178,12 +163,7 @@ public void testPrintMessageBuilder() throws Exception { public void testPrintExtensions() throws Exception { String javaText = TextFormat.printer().printToString(TestUtil.getAllExtensionsSet()); - // Java likes to add a trailing ".0" to floats and doubles. C printf - // (with %g format) does not. Our golden files are used for both - // C++ and Java TextFormat classes, so we need to conform. - javaText = javaText.replace(".0\n", "\n"); - - assertThat(javaText).isEqualTo(ALL_EXTENSIONS_SET_TEXT); + assertThat(javaText).isEqualTo(TestUtil.ALL_EXTENSIONS_SET_TEXT); } // Creates an example unknown field set. @@ -359,13 +339,13 @@ public void testPrintMessageSet() throws Exception { @Test public void testMerge() throws Exception { TestAllTypes.Builder builder = TestAllTypes.newBuilder(); - TextFormat.merge(ALL_FIELDS_SET_TEXT, builder); + TextFormat.merge(TestUtil.ALL_FIELDS_SET_TEXT, builder); TestUtil.assertAllFieldsSet(builder.build()); } @Test public void testParse() throws Exception { - TestUtil.assertAllFieldsSet(TextFormat.parse(ALL_FIELDS_SET_TEXT, TestAllTypes.class)); + TestUtil.assertAllFieldsSet(TextFormat.parse(TestUtil.ALL_FIELDS_SET_TEXT, TestAllTypes.class)); } @Test @@ -405,14 +385,15 @@ public void testParseUninitialized() throws Exception { @Test public void testMergeReader() throws Exception { TestAllTypes.Builder builder = TestAllTypes.newBuilder(); - TextFormat.merge(new StringReader(ALL_FIELDS_SET_TEXT), builder); + TextFormat.merge(new StringReader(TestUtil.ALL_FIELDS_SET_TEXT), builder); TestUtil.assertAllFieldsSet(builder.build()); } @Test public void testMergeExtensions() throws Exception { TestAllExtensions.Builder builder = TestAllExtensions.newBuilder(); - TextFormat.merge(ALL_EXTENSIONS_SET_TEXT, TestUtil.getFullExtensionRegistry(), builder); + TextFormat.merge( + TestUtil.ALL_EXTENSIONS_SET_TEXT, TestUtil.getFullExtensionRegistry(), builder); TestUtil.assertAllExtensionsSet(builder.build()); } @@ -420,7 +401,9 @@ public void testMergeExtensions() throws Exception { public void testParseExtensions() throws Exception { TestUtil.assertAllExtensionsSet( TextFormat.parse( - ALL_EXTENSIONS_SET_TEXT, TestUtil.getFullExtensionRegistry(), TestAllExtensions.class)); + TestUtil.ALL_EXTENSIONS_SET_TEXT, + TestUtil.getFullExtensionRegistry(), + TestAllExtensions.class)); } @Test diff --git a/python/google/protobuf/internal/descriptor_test.py b/python/google/protobuf/internal/descriptor_test.py index 2c8423efac79..b34109e906e8 100755 --- a/python/google/protobuf/internal/descriptor_test.py +++ b/python/google/protobuf/internal/descriptor_test.py @@ -726,7 +726,7 @@ def CheckDescriptorMapping(self, mapping): excepted_dict['new_key'] = 'new' self.assertNotEqual(mapping, excepted_dict) self.assertRaises(KeyError, mapping.__getitem__, 'key_error') - self.assertRaises(KeyError, mapping.__getitem__, len(mapping) + 1) + self.assertRaises(KeyError, mapping.__getitem__, len(mapping) * 2) # TODO: Add __repr__ support for DescriptorMapping. if api_implementation.Type() == 'cpp': self.assertEqual(str(mapping)[0], '<') diff --git a/python/google/protobuf/internal/field_mask_test.py b/python/google/protobuf/internal/field_mask_test.py index ed7c9ef60e26..ef286f8ba5d0 100644 --- a/python/google/protobuf/internal/field_mask_test.py +++ b/python/google/protobuf/internal/field_mask_test.py @@ -53,7 +53,7 @@ def testDescriptorToFieldMask(self): mask = field_mask_pb2.FieldMask() msg_descriptor = unittest_pb2.TestAllTypes.DESCRIPTOR mask.AllFieldsFromDescriptor(msg_descriptor) - self.assertEqual(79, len(mask.paths)) + self.assertEqual(80, len(mask.paths)) self.assertTrue(mask.IsValidForDescriptor(msg_descriptor)) for field in msg_descriptor.fields: self.assertTrue(field.name in mask.paths) diff --git a/python/google/protobuf/internal/message_test.py b/python/google/protobuf/internal/message_test.py index bf91424cd576..2326ed9687fd 100755 --- a/python/google/protobuf/internal/message_test.py +++ b/python/google/protobuf/internal/message_test.py @@ -65,48 +65,6 @@ def testBadUtf8String(self, message_module): message_module.TestAllTypes.FromString(bad_utf8_data) self.assertIn('TestAllTypes.optional_string', str(context.exception)) - def testGoldenMessage(self, message_module): - # Proto3 doesn't have the "default_foo" members or foreign enums, - # and doesn't preserve unknown fields, so for proto3 we use a golden - # message that doesn't have these fields set. - if message_module is unittest_pb2: - golden_data = test_util.GoldenFileData('golden_message_oneof_implemented') - else: - golden_data = test_util.GoldenFileData('golden_message_proto3') - - golden_message = message_module.TestAllTypes() - golden_message.ParseFromString(golden_data) - if message_module is unittest_pb2: - test_util.ExpectAllFieldsSet(self, golden_message) - self.assertEqual(golden_data, golden_message.SerializeToString()) - golden_copy = copy.deepcopy(golden_message) - self.assertEqual(golden_data, golden_copy.SerializeToString()) - - def testGoldenMessageBytearray(self, message_module): - # bytearray was broken, test that it works again - if message_module is unittest_pb2: - golden_data = test_util.GoldenFileData('golden_message_oneof_implemented') - else: - golden_data = test_util.GoldenFileData('golden_message_proto3') - - golden_message = message_module.TestAllTypes() - golden_message.ParseFromString(bytearray(golden_data)) - if message_module is unittest_pb2: - test_util.ExpectAllFieldsSet(self, golden_message) - self.assertEqual(golden_data, golden_message.SerializeToString()) - - def testGoldenPackedMessage(self, message_module): - golden_data = test_util.GoldenFileData('golden_packed_fields_message') - golden_message = message_module.TestPackedTypes() - parsed_bytes = golden_message.ParseFromString(golden_data) - all_set = message_module.TestPackedTypes() - test_util.SetAllPackedFields(all_set) - self.assertEqual(parsed_bytes, len(golden_data)) - self.assertEqual(all_set, golden_message) - self.assertEqual(golden_data, all_set.SerializeToString()) - golden_copy = copy.deepcopy(golden_message) - self.assertEqual(golden_data, golden_copy.SerializeToString()) - def testParseErrors(self, message_module): msg = message_module.TestAllTypes() self.assertRaises(TypeError, msg.FromString, 0) @@ -162,7 +120,9 @@ def __bool__(self): golden_message.SerializeToString(deterministic=BadArg()) def testPickleSupport(self, message_module): - golden_data = test_util.GoldenFileData('golden_message') + golden_message = message_module.TestAllTypes() + test_util.SetAllFields(golden_message) + golden_data = golden_message.SerializeToString() golden_message = message_module.TestAllTypes() golden_message.ParseFromString(golden_data) pickled_message = pickle.dumps(golden_message) @@ -1537,36 +1497,6 @@ def testCopyFromAllPackedExtensions(self): all_set.Extensions[unittest_pb2.packed_float_extension].extend([61.0, 71.0]) self.assertNotEqual(all_set, copy) - def testGoldenExtensions(self): - golden_data = test_util.GoldenFileData('golden_message') - golden_message = unittest_pb2.TestAllExtensions() - golden_message.ParseFromString(golden_data) - all_set = unittest_pb2.TestAllExtensions() - test_util.SetAllExtensions(all_set) - self.assertEqual(all_set, golden_message) - self.assertEqual(golden_data, golden_message.SerializeToString()) - golden_copy = copy.deepcopy(golden_message) - self.assertEqual(golden_message, golden_copy) - # Depend on a specific serialization order for extensions is not - # reasonable to guarantee. - if api_implementation.Type() != 'upb': - self.assertEqual(golden_data, golden_copy.SerializeToString()) - - def testGoldenPackedExtensions(self): - golden_data = test_util.GoldenFileData('golden_packed_fields_message') - golden_message = unittest_pb2.TestPackedExtensions() - golden_message.ParseFromString(golden_data) - all_set = unittest_pb2.TestPackedExtensions() - test_util.SetAllPackedExtensions(all_set) - self.assertEqual(all_set, golden_message) - self.assertEqual(golden_data, all_set.SerializeToString()) - golden_copy = copy.deepcopy(golden_message) - self.assertEqual(golden_message, golden_copy) - # Depend on a specific serialization order for extensions is not - # reasonable to guarantee. - if api_implementation.Type() != 'upb': - self.assertEqual(golden_data, golden_copy.SerializeToString()) - def testPickleIncompleteProto(self): golden_message = unittest_pb2.TestRequired(a=1) pickled_message = pickle.dumps(golden_message) diff --git a/python/google/protobuf/internal/test_util.py b/python/google/protobuf/internal/test_util.py index 53492d8a1587..46059fe1eba2 100755 --- a/python/google/protobuf/internal/test_util.py +++ b/python/google/protobuf/internal/test_util.py @@ -77,6 +77,7 @@ def SetAllNonLazyFields(message): message.optional_string_piece = u'124' message.optional_cord = u'125' + message.optional_bytes_cord = b'optional bytes cord' # # Repeated fields. @@ -247,6 +248,7 @@ def SetAllExtensions(message): extensions[pb2.optional_string_piece_extension] = u'124' extensions[pb2.optional_cord_extension] = u'125' + extensions[pb2.optional_bytes_cord_extension] = b'optional bytes cord' # # Repeated fields. @@ -423,6 +425,7 @@ def ExpectAllFieldsSet(test_case, message): test_case.assertTrue(message.HasField('optional_string_piece')) test_case.assertTrue(message.HasField('optional_cord')) + test_case.assertTrue(message.HasField('optional_bytes_cord')) test_case.assertEqual(101, message.optional_int32) test_case.assertEqual(102, message.optional_int64) diff --git a/python/google/protobuf/internal/unknown_fields_test.py b/python/google/protobuf/internal/unknown_fields_test.py index 57224ec74c74..33ad76653799 100755 --- a/python/google/protobuf/internal/unknown_fields_test.py +++ b/python/google/protobuf/internal/unknown_fields_test.py @@ -212,7 +212,7 @@ def testCheckUnknownFieldValue(self): unknown_field_set, (17, 0, 117)) - self.assertEqual(98, len(unknown_field_set)) + self.assertEqual(99, len(unknown_field_set)) def testCopyFrom(self): message = unittest_pb2.TestEmptyMessage() @@ -250,7 +250,7 @@ def testClear(self): self.empty_message.Clear() # All cleared, even unknown fields. self.assertEqual(self.empty_message.SerializeToString(), b'') - self.assertEqual(len(unknown_field_set), 98) + self.assertEqual(len(unknown_field_set), 99) @unittest.skipIf((sys.version_info.major, sys.version_info.minor) < (3, 4), 'tracemalloc requires python 3.4+') @@ -309,7 +309,7 @@ def testUnknownField(self): def testUnknownExtensions(self): message = unittest_pb2.TestEmptyMessageWithExtensions() message.ParseFromString(self.all_fields_data) - self.assertEqual(len(unknown_fields.UnknownFieldSet(message)), 98) + self.assertEqual(len(unknown_fields.UnknownFieldSet(message)), 99) self.assertEqual(message.SerializeToString(), self.all_fields_data) diff --git a/src/google/protobuf/BUILD.bazel b/src/google/protobuf/BUILD.bazel index e845839f40f2..381e0e24ecde 100644 --- a/src/google/protobuf/BUILD.bazel +++ b/src/google/protobuf/BUILD.bazel @@ -1591,7 +1591,6 @@ cc_test( "message_unittest.inc", "message_unittest_legacy_apis.inc", ], - data = [":testdata"], deps = [ ":arena", ":cc_lite_test_protos", @@ -1624,7 +1623,6 @@ cc_test( "edition_message_unittest.cc", "message_unittest.inc", ], - data = [":testdata"], deps = [ ":arena", ":cc_test_protos", diff --git a/src/google/protobuf/arena_test_util.h b/src/google/protobuf/arena_test_util.h index db21949d084c..811c9dd8200a 100644 --- a/src/google/protobuf/arena_test_util.h +++ b/src/google/protobuf/arena_test_util.h @@ -21,46 +21,6 @@ namespace google { namespace protobuf { - -template -void TestParseCorruptedString(const T& message) { - int success_count = 0; - std::string s; - { - // Map order is not deterministic. To make the test deterministic we want - // to serialize the proto deterministically. - io::StringOutputStream output(&s); - io::CodedOutputStream out(&output); - out.SetSerializationDeterministic(true); - message.SerializePartialToCodedStream(&out); - } -#if defined(PROTOBUF_ASAN) || defined(PROTOBUF_TSAN) || defined(PROTOBUF_MSAN) - // Make the test smaller in sanitizer mode. - const int kMaxIters = 200; -#else - const int kMaxIters = 900; -#endif - const int stride = s.size() <= kMaxIters ? 1 : s.size() / kMaxIters; - const int start = stride == 1 || use_arena ? 0 : (stride + 1) / 2; - for (int i = start; i < s.size(); i += stride) { - for (int c = 1 + (i % 17); c < 256; c += 2 * c + (i & 3)) { - s[i] ^= c; - Arena arena; - T* message = Arena::Create(use_arena ? &arena : nullptr); - if (message->ParseFromString(s)) { - ++success_count; - } - if (!use_arena) { - delete message; - } - s[i] ^= c; // Restore s to its original state. - } - } - // This next line is a low bar. But getting through the test without crashing - // due to use-after-free or other bugs is a big part of what we're checking. - ABSL_CHECK_GT(success_count, 0); -} - namespace internal { struct ArenaTestPeer { diff --git a/src/google/protobuf/arena_unittest.cc b/src/google/protobuf/arena_unittest.cc index 75e1f83d53ce..857abf057d40 100644 --- a/src/google/protobuf/arena_unittest.cc +++ b/src/google/protobuf/arena_unittest.cc @@ -1607,13 +1607,6 @@ TEST(ArenaTest, NoHeapAllocationsTest) { arena.Reset(); } -TEST(ArenaTest, ParseCorruptedString) { - TestAllTypes message; - TestUtil::SetAllFields(&message); - TestParseCorruptedString(message); - TestParseCorruptedString(message); -} - #if PROTOBUF_RTTI // Test construction on an arena via generic MessageLite interface. We should be // able to successfully deserialize on the arena without incurring heap diff --git a/src/google/protobuf/compiler/BUILD.bazel b/src/google/protobuf/compiler/BUILD.bazel index 02729619fdd4..3f5624d5f8da 100644 --- a/src/google/protobuf/compiler/BUILD.bazel +++ b/src/google/protobuf/compiler/BUILD.bazel @@ -416,6 +416,7 @@ cc_test( "//src/google/protobuf:cc_test_protos", "//src/google/protobuf:port", "//src/google/protobuf:test_textproto", + "//src/google/protobuf:test_util", "//src/google/protobuf:test_util2", "//src/google/protobuf/compiler/cpp:names", "//src/google/protobuf/io", diff --git a/src/google/protobuf/compiler/command_line_interface_unittest.cc b/src/google/protobuf/compiler/command_line_interface_unittest.cc index 99fe927644af..92653caff80b 100644 --- a/src/google/protobuf/compiler/command_line_interface_unittest.cc +++ b/src/google/protobuf/compiler/command_line_interface_unittest.cc @@ -48,6 +48,7 @@ #include "google/protobuf/compiler/mock_code_generator.h" #include "google/protobuf/compiler/plugin.pb.h" #include "google/protobuf/test_textproto.h" +#include "google/protobuf/test_util.h" #include "google/protobuf/test_util2.h" #include "google/protobuf/unittest.pb.h" #include "google/protobuf/unittest_custom_options.pb.h" @@ -4153,7 +4154,16 @@ class EncodeDecodeTest : public testing::TestWithParam { std::string unittest_proto_descriptor_set_filename_; }; +static void WriteGoldenMessage(const std::string& filename) { + protobuf_unittest::TestAllTypes message; + TestUtil::SetAllFields(&message); + std::string golden = message.SerializeAsString(); + ABSL_CHECK_OK(File::SetContents(filename, golden, true)); +} + TEST_P(EncodeDecodeTest, Encode) { + std::string golden_path = absl::StrCat(TestTempDir(), "/golden_message"); + WriteGoldenMessage(golden_path); RedirectStdinFromFile(TestUtil::GetTestDataPath( "google/protobuf/" "testdata/text_format_unittest_data_oneof_implemented.txt")); @@ -4163,14 +4173,14 @@ TEST_P(EncodeDecodeTest, Encode) { } EXPECT_TRUE( Run(absl::StrCat(args, " --encode=protobuf_unittest.TestAllTypes"))); - ExpectStdoutMatchesBinaryFile(TestUtil::GetTestDataPath( - "google/protobuf/testdata/golden_message_oneof_implemented")); + ExpectStdoutMatchesBinaryFile(golden_path); ExpectStderrMatchesText(""); } TEST_P(EncodeDecodeTest, Decode) { - RedirectStdinFromFile(TestUtil::GetTestDataPath( - "google/protobuf/testdata/golden_message_oneof_implemented")); + std::string golden_path = absl::StrCat(TestTempDir(), "/golden_message"); + WriteGoldenMessage(golden_path); + RedirectStdinFromFile(golden_path); EXPECT_TRUE( Run("google/protobuf/unittest.proto" " --decode=protobuf_unittest.TestAllTypes")); @@ -4223,6 +4233,8 @@ TEST_P(EncodeDecodeTest, ProtoParseError) { } TEST_P(EncodeDecodeTest, EncodeDeterministicOutput) { + std::string golden_path = absl::StrCat(TestTempDir(), "/golden_message"); + WriteGoldenMessage(golden_path); RedirectStdinFromFile(TestUtil::GetTestDataPath( "google/protobuf/" "testdata/text_format_unittest_data_oneof_implemented.txt")); @@ -4232,14 +4244,14 @@ TEST_P(EncodeDecodeTest, EncodeDeterministicOutput) { } EXPECT_TRUE(Run(absl::StrCat( args, " --encode=protobuf_unittest.TestAllTypes --deterministic_output"))); - ExpectStdoutMatchesBinaryFile(TestUtil::GetTestDataPath( - "google/protobuf/testdata/golden_message_oneof_implemented")); + ExpectStdoutMatchesBinaryFile(golden_path); ExpectStderrMatchesText(""); } TEST_P(EncodeDecodeTest, DecodeDeterministicOutput) { - RedirectStdinFromFile(TestUtil::GetTestDataPath( - "google/protobuf/testdata/golden_message_oneof_implemented")); + std::string golden_path = absl::StrCat(TestTempDir(), "/golden_message"); + WriteGoldenMessage(golden_path); + RedirectStdinFromFile(golden_path); EXPECT_FALSE( Run("google/protobuf/unittest.proto" " --decode=protobuf_unittest.TestAllTypes --deterministic_output")); diff --git a/src/google/protobuf/compiler/cpp/field.cc b/src/google/protobuf/compiler/cpp/field.cc index cee1b665b8a4..b7a8d1f450fb 100644 --- a/src/google/protobuf/compiler/cpp/field.cc +++ b/src/google/protobuf/compiler/cpp/field.cc @@ -130,7 +130,6 @@ FieldGeneratorBase::FieldGeneratorBase(const FieldDescriptor* field, break; case FieldDescriptor::CPPTYPE_STRING: is_string_ = true; - string_type_ = field->options().ctype(); is_inlined_ = IsStringInlined(field, options); is_bytes_ = field->type() == FieldDescriptor::TYPE_BYTES; has_default_constexpr_constructor_ = is_repeated_or_map; @@ -229,40 +228,6 @@ void FieldGeneratorBase::GenerateCopyConstructorCode(io::Printer* p) const { } namespace { -// Use internal types instead of ctype or string_type. -enum class StringType { - kView, - kString, - kCord, - kStringPiece, -}; - -StringType GetStringType(const FieldDescriptor& field) { - ABSL_CHECK_EQ(field.cpp_type(), FieldDescriptor::CPPTYPE_STRING); - - if (field.options().has_ctype()) { - switch (field.options().ctype()) { - case FieldOptions::CORD: - return StringType::kCord; - case FieldOptions::STRING_PIECE: - return StringType::kStringPiece; - default: - return StringType::kString; - } - } - - const pb::CppFeatures& cpp_features = - CppGenerator::GetResolvedSourceFeatures(field).GetExtension(::pb::cpp); - switch (cpp_features.string_type()) { - case pb::CppFeatures::CORD: - return StringType::kCord; - case pb::CppFeatures::VIEW: - return StringType::kView; - default: - return StringType::kString; - } -} - std::unique_ptr MakeGenerator(const FieldDescriptor* field, const Options& options, MessageSCCAnalyzer* scc) { @@ -279,7 +244,7 @@ std::unique_ptr MakeGenerator(const FieldDescriptor* field, case FieldDescriptor::CPPTYPE_MESSAGE: return MakeRepeatedMessageGenerator(field, options, scc); case FieldDescriptor::CPPTYPE_STRING: { - if (GetStringType(*field) == StringType::kView) { + if (field->cpp_string_type() == FieldDescriptor::CppStringType::kView) { return MakeRepeatedStringViewGenerator(field, options, scc); } else { return MakeRepeatedStringGenerator(field, options, scc); @@ -303,10 +268,10 @@ std::unique_ptr MakeGenerator(const FieldDescriptor* field, case FieldDescriptor::CPPTYPE_ENUM: return MakeSinguarEnumGenerator(field, options, scc); case FieldDescriptor::CPPTYPE_STRING: { - switch (GetStringType(*field)) { - case StringType::kView: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kView: return MakeSingularStringViewGenerator(field, options, scc); - case StringType::kCord: + case FieldDescriptor::CppStringType::kCord: if (field->type() == FieldDescriptor::TYPE_BYTES) { if (field->real_containing_oneof()) { return MakeOneofCordGenerator(field, options, scc); diff --git a/src/google/protobuf/compiler/cpp/field.h b/src/google/protobuf/compiler/cpp/field.h index df1abb3a04b7..4722fc523625 100644 --- a/src/google/protobuf/compiler/cpp/field.h +++ b/src/google/protobuf/compiler/cpp/field.h @@ -95,9 +95,6 @@ class FieldGeneratorBase { // Returns true if the field API uses bytes (void) instead of chars. bool is_bytes() const { return is_bytes_; } - // Returns the public API string type for string fields. - FieldOptions::CType string_type() const { return string_type_; } - // Returns true if this field is part of a oneof field. bool is_oneof() const { return is_oneof_; } @@ -217,7 +214,6 @@ class FieldGeneratorBase { bool is_lazy_ = false; bool is_weak_ = false; bool is_oneof_ = false; - FieldOptions::CType string_type_ = FieldOptions::STRING; bool has_default_constexpr_constructor_ = false; }; @@ -269,7 +265,6 @@ class FieldGenerator { bool is_foreign() const { return impl_->is_foreign(); } bool is_string() const { return impl_->is_string(); } bool is_bytes() const { return impl_->is_bytes(); } - FieldOptions::CType string_type() const { return impl_->string_type(); } bool is_oneof() const { return impl_->is_oneof(); } bool is_inlined() const { return impl_->is_inlined(); } bool has_default_constexpr_constructor() const { diff --git a/src/google/protobuf/compiler/cpp/field_generators/string_view_field.cc b/src/google/protobuf/compiler/cpp/field_generators/string_view_field.cc index 204cabc6a9d3..6ac9de886aac 100644 --- a/src/google/protobuf/compiler/cpp/field_generators/string_view_field.cc +++ b/src/google/protobuf/compiler/cpp/field_generators/string_view_field.cc @@ -216,8 +216,6 @@ void SingularStringView::GenerateStaticMembers(io::Printer* p) const { } void SingularStringView::GenerateAccessorDeclarations(io::Printer* p) const { - ABSL_CHECK(!field_->options().has_ctype()); - auto vars = AnnotatedAccessors(field_, {"", "set_allocated_"}); vars.push_back(Sub{ "release_name", diff --git a/src/google/protobuf/compiler/cpp/helpers.cc b/src/google/protobuf/compiler/cpp/helpers.cc index c6a6c56f45d6..4a3a47e10641 100644 --- a/src/google/protobuf/compiler/cpp/helpers.cc +++ b/src/google/protobuf/compiler/cpp/helpers.cc @@ -1571,7 +1571,8 @@ MessageAnalysis MessageSCCAnalyzer::GetSCCAnalysis(const SCC* scc) { switch (field->type()) { case FieldDescriptor::TYPE_STRING: case FieldDescriptor::TYPE_BYTES: { - if (field->options().ctype() == FieldOptions::CORD) { + if (field->cpp_string_type() == + FieldDescriptor::CppStringType::kCord) { result.contains_cord = true; } break; diff --git a/src/google/protobuf/compiler/cpp/tracker.cc b/src/google/protobuf/compiler/cpp/tracker.cc index ae6edc013eda..9d7cbcf2e1d4 100644 --- a/src/google/protobuf/compiler/cpp/tracker.cc +++ b/src/google/protobuf/compiler/cpp/tracker.cc @@ -214,7 +214,8 @@ Getters RepeatedFieldGetters(const FieldDescriptor* field, Getters StringFieldGetters(const FieldDescriptor* field, const Options& opts) { std::string member = FieldMemberName(field, ShouldSplit(field, opts)); - bool is_std_string = field->options().ctype() == FieldOptions::STRING; + bool is_std_string = + field->cpp_string_type() == FieldDescriptor::CppStringType::kString; Getters getters; if (is_std_string && !field->default_value_string().empty()) { @@ -234,7 +235,8 @@ Getters StringOneofGetters(const FieldDescriptor* field, ABSL_CHECK(oneof != nullptr); std::string member = FieldMemberName(field, ShouldSplit(field, opts)); - bool is_std_string = field->options().ctype() == FieldOptions::STRING; + bool is_std_string = + field->cpp_string_type() == FieldDescriptor::CppStringType::kString; std::string field_ptr = member; if (is_std_string) { @@ -251,8 +253,8 @@ Getters StringOneofGetters(const FieldDescriptor* field, } Getters getters; - if (field->default_value_string().empty() || - field->options().ctype() == FieldOptions::STRING_PIECE) { + if (field->default_value_string().empty() + ) { getters.base = absl::Substitute("$0 ? $1 : nullptr", has, field_ptr); } else { getters.base = diff --git a/src/google/protobuf/compiler/cpp/unittest.inc b/src/google/protobuf/compiler/cpp/unittest.inc index a6552e1e1dc4..55878fa581a4 100644 --- a/src/google/protobuf/compiler/cpp/unittest.inc +++ b/src/google/protobuf/compiler/cpp/unittest.inc @@ -427,6 +427,7 @@ TEST(GENERATED_MESSAGE_TEST_NAME, SwapWithOther) { message1.set_optional_string("abc"); message1.mutable_optional_nested_message()->set_bb(1); message1.set_optional_nested_enum(UNITTEST::TestAllTypes::FOO); + message1.set_optional_bytes_cord("bytes cord"); message1.add_repeated_int32(1); message1.add_repeated_int32(2); message1.add_repeated_string("a"); @@ -440,6 +441,7 @@ TEST(GENERATED_MESSAGE_TEST_NAME, SwapWithOther) { message2.set_optional_string("def"); message2.mutable_optional_nested_message()->set_bb(2); message2.set_optional_nested_enum(UNITTEST::TestAllTypes::BAR); + message2.set_optional_bytes_cord("bytes cord"); message2.add_repeated_int32(3); message2.add_repeated_string("c"); message2.add_repeated_nested_message()->set_bb(9); @@ -451,6 +453,7 @@ TEST(GENERATED_MESSAGE_TEST_NAME, SwapWithOther) { EXPECT_EQ("def", message1.optional_string()); EXPECT_EQ(2, message1.optional_nested_message().bb()); EXPECT_EQ(UNITTEST::TestAllTypes::BAR, message1.optional_nested_enum()); + EXPECT_EQ(absl::Cord("bytes cord"), message1.optional_bytes_cord()); ASSERT_EQ(1, message1.repeated_int32_size()); EXPECT_EQ(3, message1.repeated_int32(0)); ASSERT_EQ(1, message1.repeated_string_size()); @@ -464,6 +467,7 @@ TEST(GENERATED_MESSAGE_TEST_NAME, SwapWithOther) { EXPECT_EQ("abc", message2.optional_string()); EXPECT_EQ(1, message2.optional_nested_message().bb()); EXPECT_EQ(UNITTEST::TestAllTypes::FOO, message2.optional_nested_enum()); + EXPECT_EQ(absl::Cord("bytes cord"), message2.optional_bytes_cord()); ASSERT_EQ(2, message2.repeated_int32_size()); EXPECT_EQ(1, message2.repeated_int32(0)); EXPECT_EQ(2, message2.repeated_int32(1)); diff --git a/src/google/protobuf/descriptor_unittest.cc b/src/google/protobuf/descriptor_unittest.cc index ac4e9fd43da9..e5bbfc7e6a63 100644 --- a/src/google/protobuf/descriptor_unittest.cc +++ b/src/google/protobuf/descriptor_unittest.cc @@ -7519,7 +7519,7 @@ TEST_F(FeaturesTest, Proto2Features) { name: "cord" number: 8 label: LABEL_OPTIONAL - type: TYPE_STRING + type: TYPE_BYTES options { ctype: CORD } } field { @@ -7595,6 +7595,8 @@ TEST_F(FeaturesTest, Proto2Features) { EXPECT_EQ(message->FindFieldByName("str")->cpp_string_type(), FieldDescriptor::CppStringType::kString); + EXPECT_EQ(message->FindFieldByName("cord")->cpp_string_type(), + FieldDescriptor::CppStringType::kCord); // Check round-trip consistency. FileDescriptorProto proto; diff --git a/src/google/protobuf/dynamic_message.cc b/src/google/protobuf/dynamic_message.cc index 00b55f60313c..54591cb321e8 100644 --- a/src/google/protobuf/dynamic_message.cc +++ b/src/google/protobuf/dynamic_message.cc @@ -117,9 +117,11 @@ int FieldSpaceUsed(const FieldDescriptor* field) { } case FD::CPPTYPE_STRING: - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + return sizeof(RepeatedField); + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: return sizeof(RepeatedPtrField); } break; @@ -147,9 +149,11 @@ int FieldSpaceUsed(const FieldDescriptor* field) { return sizeof(Message*); case FD::CPPTYPE_STRING: - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + return sizeof(absl::Cord); + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: return sizeof(ArenaStringPtr); } break; @@ -410,9 +414,31 @@ void DynamicMessage::SharedCtor(bool lock_factory) { break; case FieldDescriptor::CPPTYPE_STRING: - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + if (!field->is_repeated()) { + if (field->has_default_value()) { + new (field_ptr) absl::Cord(field->default_value_string()); + } else { + new (field_ptr) absl::Cord; + } + if (arena != nullptr) { + // Cord does not support arena so here we need to notify arena + // to remove the data it allocated on the heap by calling its + // destructor. + arena->OwnDestructor(static_cast(field_ptr)); + } + } else { + new (field_ptr) RepeatedField(arena); + if (arena != nullptr) { + // Needs to destroy Cord elements. + arena->OwnDestructor( + static_cast*>(field_ptr)); + } + } + break; + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: if (!field->is_repeated()) { ArenaStringPtr* asp = new (field_ptr) ArenaStringPtr(); asp->InitDefault(); @@ -502,9 +528,12 @@ DynamicMessage::~DynamicMessage() { if (*(reinterpret_cast(field_ptr)) == field->number()) { field_ptr = MutableOneofFieldRaw(field); if (field->cpp_type() == FieldDescriptor::CPPTYPE_STRING) { - switch (field->options().ctype()) { - default: - case FieldOptions::STRING: { + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + delete *reinterpret_cast(field_ptr); + break; + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: { reinterpret_cast(field_ptr)->Destroy(); break; } @@ -536,9 +565,13 @@ DynamicMessage::~DynamicMessage() { #undef HANDLE_TYPE case FieldDescriptor::CPPTYPE_STRING: - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + reinterpret_cast*>(field_ptr) + ->~RepeatedField(); + break; + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: reinterpret_cast*>(field_ptr) ->~RepeatedPtrField(); break; @@ -556,9 +589,12 @@ DynamicMessage::~DynamicMessage() { } } else if (field->cpp_type() == FieldDescriptor::CPPTYPE_STRING) { - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: { + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + reinterpret_cast(field_ptr)->~Cord(); + break; + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: { reinterpret_cast(field_ptr)->Destroy(); break; } diff --git a/src/google/protobuf/edition_unittest.proto b/src/google/protobuf/edition_unittest.proto index 81848b87e4fe..c0d0ebbbf768 100644 --- a/src/google/protobuf/edition_unittest.proto +++ b/src/google/protobuf/edition_unittest.proto @@ -91,6 +91,7 @@ message TestAllTypes { string optional_string_piece = 24 [ctype=STRING_PIECE]; string optional_cord = 25 [ctype=CORD]; + bytes optional_bytes_cord = 86 [ctype=CORD]; // Defined in unittest_import_public.proto protobuf_unittest_import.PublicImportMessage @@ -266,6 +267,7 @@ extend TestAllExtensions { // TODO: ctype=CORD is not supported for extension. Add // ctype=CORD option back after it is supported. string optional_cord_extension = 25; + bytes optional_bytes_cord_extension = 86; protobuf_unittest_import.PublicImportMessage optional_public_import_message_extension = 26; diff --git a/src/google/protobuf/generated_message_reflection.cc b/src/google/protobuf/generated_message_reflection.cc index 607addc73bdb..f0208ced9e71 100644 --- a/src/google/protobuf/generated_message_reflection.cc +++ b/src/google/protobuf/generated_message_reflection.cc @@ -402,9 +402,13 @@ size_t Reflection::SpaceUsedLong(const Message& message) const { #undef HANDLE_TYPE case FieldDescriptor::CPPTYPE_STRING: - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + total_size += GetRaw>(message, field) + .SpaceUsedExcludingSelfLong(); + break; + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: total_size += GetRaw >(message, field) .SpaceUsedExcludingSelfLong(); @@ -443,8 +447,8 @@ size_t Reflection::SpaceUsedLong(const Message& message) const { break; case FieldDescriptor::CPPTYPE_STRING: { - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: if (schema_.InRealOneof(field)) { total_size += GetField(message, field) ->EstimatedMemoryUsage(); @@ -456,8 +460,8 @@ size_t Reflection::SpaceUsedLong(const Message& message) const { sizeof(absl::Cord); } break; - default: - case FieldOptions::STRING: + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: if (IsInlined(field)) { const std::string* ptr = &GetField(message, field).GetNoArena(); @@ -553,15 +557,14 @@ struct OneofFieldMover { to->SetString(from->GetString()); break; } - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: to->SetCord(from->GetCord()); break; - default: - case FieldOptions::STRING: { + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: to->SetArenaStringPtr(from->GetArenaStringPtr()); break; - } } break; default: @@ -624,9 +627,19 @@ template void SwapFieldHelper::SwapRepeatedStringField(const Reflection* r, Message* lhs, Message* rhs, const FieldDescriptor* field) { - switch (field->options().ctype()) { - default: - case FieldOptions::STRING: { + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: { + auto* lhs_cord = r->MutableRaw>(lhs, field); + auto* rhs_cord = r->MutableRaw>(rhs, field); + if (unsafe_shallow_swap) { + lhs_cord->InternalSwap(rhs_cord); + } else { + lhs_cord->Swap(rhs_cord); + } + break; + } + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: { auto* lhs_string = r->MutableRaw(lhs, field); auto* rhs_string = r->MutableRaw(rhs, field); if (unsafe_shallow_swap) { @@ -690,14 +703,14 @@ template void SwapFieldHelper::SwapStringField(const Reflection* r, Message* lhs, Message* rhs, const FieldDescriptor* field) { - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: // Always shallow swap for Cord. std::swap(*r->MutableRaw(lhs, field), *r->MutableRaw(rhs, field)); break; - default: - case FieldOptions::STRING: { + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: { if (r->IsInlined(field)) { SwapFieldHelper::SwapInlinedStrings(r, lhs, rhs, field); @@ -1169,7 +1182,9 @@ void Reflection::SwapFieldsImpl( // may depend on the information in has bits. if (!field->is_repeated()) { SwapBit(message1, message2, field); - if (field->options().ctype() == FieldOptions::STRING && + if (field->cpp_type() == FieldDescriptor::CPPTYPE_STRING && + field->cpp_string_type() == + FieldDescriptor::CppStringType::kString && IsInlined(field)) { ABSL_DCHECK(!unsafe_shallow_swap || message1->GetArena() == message2->GetArena()); @@ -1283,9 +1298,10 @@ void Reflection::InternalSwap(Message* lhs, Message* rhs) const { int inlined_string_count = 0; for (int i = 0; i < descriptor_->field_count(); i++) { const FieldDescriptor* field = descriptor_->field(i); + if (field->cpp_type() != FieldDescriptor::CPPTYPE_STRING) continue; if (field->is_extension() || field->is_repeated() || schema_.InRealOneof(field) || - field->options().ctype() != FieldOptions::STRING || + field->cpp_string_type() != FieldDescriptor::CppStringType::kString || !IsInlined(field)) { continue; } @@ -1337,6 +1353,10 @@ int Reflection::FieldSize(const Message& message, #undef HANDLE_TYPE case FieldDescriptor::CPPTYPE_STRING: + if (field->cpp_string_type() == FieldDescriptor::CppStringType::kCord) { + return GetRaw >(message, field).size(); + } + ABSL_FALLTHROUGH_INTENDED; case FieldDescriptor::CPPTYPE_MESSAGE: if (IsMapFieldInApi(field)) { const internal::MapFieldBase& map = @@ -1395,8 +1415,8 @@ void Reflection::ClearField(Message* message, break; case FieldDescriptor::CPPTYPE_STRING: { - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: if (field->has_default_value()) { *MutableRaw(message, field) = field->default_value_string(); @@ -1404,8 +1424,8 @@ void Reflection::ClearField(Message* message, MutableRaw(message, field)->Clear(); } break; - default: - case FieldOptions::STRING: + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: if (IsInlined(field)) { // Currently, string with default value can't be inlined. So we // don't have to handle default value here. @@ -1452,9 +1472,12 @@ void Reflection::ClearField(Message* message, #undef HANDLE_TYPE case FieldDescriptor::CPPTYPE_STRING: { - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + MutableRaw>(message, field)->Clear(); + break; + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: MutableRaw >(message, field)->Clear(); break; } @@ -1502,9 +1525,12 @@ void Reflection::RemoveLast(Message* message, #undef HANDLE_TYPE case FieldDescriptor::CPPTYPE_STRING: - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + MutableRaw>(message, field)->RemoveLast(); + break; + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: MutableRaw >(message, field) ->RemoveLast(); break; @@ -1596,6 +1622,12 @@ void Reflection::SwapElements(Message* message, const FieldDescriptor* field, #undef HANDLE_TYPE case FieldDescriptor::CPPTYPE_STRING: + if (field->cpp_string_type() == FieldDescriptor::CppStringType::kCord) { + MutableRaw >(message, field) + ->SwapElements(index1, index2); + break; + } + ABSL_FALLTHROUGH_INTENDED; case FieldDescriptor::CPPTYPE_MESSAGE: if (IsMapFieldInApi(field)) { MutableRaw(message, field) @@ -1802,15 +1834,15 @@ std::string Reflection::GetString(const Message& message, if (schema_.InRealOneof(field) && !HasOneofField(message, field)) { return field->default_value_string(); } - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: if (schema_.InRealOneof(field)) { return std::string(*GetField(message, field)); } else { return std::string(GetField(message, field)); } - default: - case FieldOptions::STRING: + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: if (IsInlined(field)) { return GetField(message, field).GetNoArena(); } else { @@ -1818,6 +1850,7 @@ std::string Reflection::GetString(const Message& message, return str.IsDefault() ? field->default_value_string() : str.Get(); } } + internal::Unreachable(); } } @@ -1833,8 +1866,8 @@ const std::string& Reflection::GetStringReference(const Message& message, if (schema_.InRealOneof(field) && !HasOneofField(message, field)) { return field->default_value_string(); } - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: if (schema_.InRealOneof(field)) { absl::CopyCordToString(*GetField(message, field), scratch); @@ -1842,8 +1875,8 @@ const std::string& Reflection::GetStringReference(const Message& message, absl::CopyCordToString(GetField(message, field), scratch); } return *scratch; - default: - case FieldOptions::STRING: + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: if (IsInlined(field)) { return GetField(message, field).GetNoArena(); } else { @@ -1851,6 +1884,7 @@ const std::string& Reflection::GetStringReference(const Message& message, return str.IsDefault() ? field->default_value_string() : str.Get(); } } + internal::Unreachable(); } } @@ -1864,15 +1898,15 @@ absl::Cord Reflection::GetCord(const Message& message, if (schema_.InRealOneof(field) && !HasOneofField(message, field)) { return absl::Cord(field->default_value_string()); } - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: if (schema_.InRealOneof(field)) { return *GetField(message, field); } else { return GetField(message, field); } - default: - case FieldOptions::STRING: + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: if (IsInlined(field)) { return absl::Cord( GetField(message, field).GetNoArena()); @@ -1882,9 +1916,7 @@ absl::Cord Reflection::GetCord(const Message& message, : str.Get()); } } - - ABSL_LOG(FATAL) << "Can't get here."; - return absl::Cord(); // Make compiler happy. + internal::Unreachable(); } } @@ -1901,8 +1933,8 @@ absl::string_view Reflection::GetStringView(const Message& message, return field->default_value_string(); } - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: { + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: { const auto& cord = schema_.InRealOneof(field) ? *GetField(message, field) : GetField(message, field); @@ -1922,8 +1954,8 @@ void Reflection::SetString(Message* message, const FieldDescriptor* field, return MutableExtensionSet(message)->SetString( field->number(), field->type(), std::move(value), field); } else { - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: if (schema_.InRealOneof(field)) { if (!HasOneofField(*message, field)) { ClearOneof(message, field->containing_oneof()); @@ -1935,8 +1967,8 @@ void Reflection::SetString(Message* message, const FieldDescriptor* field, } *MutableField(message, field) = value; break; - default: - case FieldOptions::STRING: { + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: { if (IsInlined(field)) { const uint32_t index = schema_.InlinedStringIndex(field); ABSL_DCHECK_GT(index, 0u); @@ -1974,8 +2006,8 @@ void Reflection::SetString(Message* message, const FieldDescriptor* field, MutableExtensionSet(message)->MutableString( field->number(), field->type(), field)); } else { - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: if (schema_.InRealOneof(field)) { if (!HasOneofField(*message, field)) { ClearOneof(message, field->containing_oneof()); @@ -1987,8 +2019,8 @@ void Reflection::SetString(Message* message, const FieldDescriptor* field, *MutableField(message, field) = value; } break; - default: - case FieldOptions::STRING: { + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: { // Oneof string fields are never set as a default instance. // We just need to pass some arbitrary default string to make it work. // This allows us to not have the real default accessible from @@ -2024,11 +2056,14 @@ std::string Reflection::GetRepeatedString(const Message& message, if (field->is_extension()) { return GetExtensionSet(message).GetRepeatedString(field->number(), index); } else { - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + return std::string(GetRepeatedField(message, field, index)); + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: return GetRepeatedPtrField(message, field, index); } + internal::Unreachable(); } } @@ -2040,11 +2075,16 @@ const std::string& Reflection::GetRepeatedStringReference( if (field->is_extension()) { return GetExtensionSet(message).GetRepeatedString(field->number(), index); } else { - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + absl::CopyCordToString( + GetRepeatedField(message, field, index), scratch); + return *scratch; + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: return GetRepeatedPtrField(message, field, index); } + internal::Unreachable(); } } @@ -2059,11 +2099,16 @@ absl::string_view Reflection::GetRepeatedStringView( return GetExtensionSet(message).GetRepeatedString(field->number(), index); } - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::STRING: - default: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: { + auto& cord = GetRepeatedField(message, field, index); + return scratch.CopyFromCord(cord); + } + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: return GetRepeatedPtrField(message, field, index); } + internal::Unreachable(); } @@ -2075,9 +2120,12 @@ void Reflection::SetRepeatedString(Message* message, MutableExtensionSet(message)->SetRepeatedString(field->number(), index, std::move(value)); } else { - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + SetRepeatedField(message, field, index, absl::Cord(value)); + break; + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: MutableRepeatedField(message, field, index) ->assign(std::move(value)); break; @@ -2093,9 +2141,12 @@ void Reflection::AddString(Message* message, const FieldDescriptor* field, MutableExtensionSet(message)->AddString(field->number(), field->type(), std::move(value), field); } else { - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + AddField(message, field, absl::Cord(value)); + break; + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: AddField(message, field)->assign(std::move(value)); break; } @@ -2747,7 +2798,7 @@ static Type* AllocIfDefault(const FieldDescriptor* field, Type*& ptr, // be e.g. char). if (field->cpp_type() < FieldDescriptor::CPPTYPE_STRING || (field->cpp_type() == FieldDescriptor::CPPTYPE_STRING && - internal::cpp::EffectiveStringCType(field) == FieldOptions::CORD)) { + field->cpp_string_type() == FieldDescriptor::CppStringType::kCord)) { ptr = reinterpret_cast(Arena::Create>(arena)); } else { @@ -2926,11 +2977,11 @@ bool Reflection::HasBit(const Message& message, // (which uses HasField()) needs to be consistent with this. switch (field->cpp_type()) { case FieldDescriptor::CPPTYPE_STRING: - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: return !GetField(message, field).empty(); - default: - case FieldOptions::STRING: { + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: { if (IsInlined(field)) { return !GetField(message, field) .GetNoArena() @@ -2940,7 +2991,7 @@ bool Reflection::HasBit(const Message& message, return GetField(message, field).Get().size() > 0; } } - return false; + internal::Unreachable(); case FieldDescriptor::CPPTYPE_BOOL: return GetRaw(message, field) != false; case FieldDescriptor::CPPTYPE_INT32: @@ -3041,12 +3092,12 @@ void Reflection::ClearOneof(Message* message, if (message->GetArena() == nullptr) { switch (field->cpp_type()) { case FieldDescriptor::CPPTYPE_STRING: { - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: delete *MutableRaw(message, field); break; - default: - case FieldOptions::STRING: { + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: { // Oneof string fields are never set as a default instance. // We just need to pass some arbitrary default string to make it // work. This allows us to not have the real default accessible diff --git a/src/google/protobuf/generated_message_tctable_gen.cc b/src/google/protobuf/generated_message_tctable_gen.cc index 577c6f50cb0f..0b6a1c38a4cf 100644 --- a/src/google/protobuf/generated_message_tctable_gen.cc +++ b/src/google/protobuf/generated_message_tctable_gen.cc @@ -168,10 +168,10 @@ TailCallTableInfo::FastFieldInfo::Field MakeFastFieldEntry( : field->is_repeated() ? PROTOBUF_PICK_FUNCTION(fn##R) \ : PROTOBUF_PICK_FUNCTION(fn##S)) -#define PROTOBUF_PICK_STRING_FUNCTION(fn) \ - (field->options().ctype() == FieldOptions::CORD \ - ? PROTOBUF_PICK_FUNCTION(fn##cS) \ - : options.is_string_inlined ? PROTOBUF_PICK_FUNCTION(fn##iS) \ +#define PROTOBUF_PICK_STRING_FUNCTION(fn) \ + (field->cpp_string_type() == FieldDescriptor::CppStringType::kCord \ + ? PROTOBUF_PICK_FUNCTION(fn##cS) \ + : options.is_string_inlined ? PROTOBUF_PICK_FUNCTION(fn##iS) \ : PROTOBUF_PICK_REPEATABLE_FUNCTION(fn)) const FieldDescriptor* field = entry.field; @@ -304,10 +304,12 @@ bool IsFieldEligibleForFastParsing( switch (field->type()) { // Some bytes fields can be handled on fast path. case FieldDescriptor::TYPE_STRING: - case FieldDescriptor::TYPE_BYTES: - if (field->options().ctype() == FieldOptions::STRING) { + case FieldDescriptor::TYPE_BYTES: { + auto ctype = field->cpp_string_type(); + if (ctype == FieldDescriptor::CppStringType::kString || + ctype == FieldDescriptor::CppStringType::kView) { // strings are fine... - } else if (field->options().ctype() == FieldOptions::CORD) { + } else if (ctype == FieldDescriptor::CppStringType::kCord) { // Cords are worth putting into the fast table, if they're not repeated if (field->is_repeated()) return false; } else { @@ -321,6 +323,7 @@ bool IsFieldEligibleForFastParsing( aux_idx = entry.inlined_string_idx; } break; + } case FieldDescriptor::TYPE_ENUM: { uint8_t rmax_value; @@ -760,12 +763,13 @@ uint16_t MakeTypeCardForField( // Fill in extra information about string and bytes field representations. if (field->type() == FieldDescriptor::TYPE_BYTES || field->type() == FieldDescriptor::TYPE_STRING) { - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: // `Cord` is always used, even for repeated fields. type_card |= fl::kRepCord; break; - case FieldOptions::STRING: + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: if (field->is_repeated()) { // A repeated string field uses RepeatedPtrField // (unless it has a ctype option; see above). @@ -775,8 +779,6 @@ uint16_t MakeTypeCardForField( type_card |= fl::kRepAString; } break; - default: - Unreachable(); } } diff --git a/src/google/protobuf/generated_message_tctable_lite.cc b/src/google/protobuf/generated_message_tctable_lite.cc index ef63b3dfd2b5..155efbbbcccf 100644 --- a/src/google/protobuf/generated_message_tctable_lite.cc +++ b/src/google/protobuf/generated_message_tctable_lite.cc @@ -1640,6 +1640,12 @@ bool TcParser::ChangeOneof(const TcParseTableBase* table, field.Destroy(); break; } + case field_layout::kRepCord: { + if (msg->GetArena() == nullptr) { + delete RefAt(msg, current_entry->offset); + } + break; + } case field_layout::kRepSString: case field_layout::kRepIString: default: diff --git a/src/google/protobuf/io/BUILD.bazel b/src/google/protobuf/io/BUILD.bazel index 096ebf71b863..66819966b954 100644 --- a/src/google/protobuf/io/BUILD.bazel +++ b/src/google/protobuf/io/BUILD.bazel @@ -191,9 +191,6 @@ cc_test( "zero_copy_stream_unittest.cc", ], copts = COPTS, - data = [ - "//src/google/protobuf:testdata", - ], deps = [ ":gzip_stream", ":io", @@ -203,6 +200,7 @@ cc_test( "//:protobuf", "//src/google/protobuf", "//src/google/protobuf:port", + "//src/google/protobuf:test_util", "//src/google/protobuf:test_util2", "//src/google/protobuf/stubs", "//src/google/protobuf/testing", diff --git a/src/google/protobuf/io/zero_copy_stream_unittest.cc b/src/google/protobuf/io/zero_copy_stream_unittest.cc index d4a39fd07424..abbc40cd0511 100644 --- a/src/google/protobuf/io/zero_copy_stream_unittest.cc +++ b/src/google/protobuf/io/zero_copy_stream_unittest.cc @@ -56,12 +56,14 @@ #include "google/protobuf/io/coded_stream.h" #include "google/protobuf/io/io_win32.h" #include "google/protobuf/io/zero_copy_stream_impl.h" +#include "google/protobuf/test_util.h" #include "google/protobuf/test_util2.h" #if HAVE_ZLIB #include "google/protobuf/io/gzip_stream.h" #endif +#include "google/protobuf/test_util.h" // Must be included last. #include "google/protobuf/port_def.inc" @@ -557,10 +559,9 @@ std::string IoTest::Uncompress(const std::string& data) { TEST_F(IoTest, CompressionOptions) { // Some ad-hoc testing of compression options. - std::string golden_filename = - TestUtil::GetTestDataPath("google/protobuf/testdata/golden_message"); - std::string golden; - ABSL_CHECK_OK(File::GetContents(golden_filename, &golden, true)); + protobuf_unittest::TestAllTypes message; + TestUtil::SetAllFields(&message); + std::string golden = message.SerializeAsString(); GzipOutputStream::Options options; std::string gzip_compressed = Compress(golden, options); diff --git a/src/google/protobuf/lite_unittest.cc b/src/google/protobuf/lite_unittest.cc index bd5f5e7926a0..fc6b05211744 100644 --- a/src/google/protobuf/lite_unittest.cc +++ b/src/google/protobuf/lite_unittest.cc @@ -958,6 +958,18 @@ TYPED_TEST(LiteTest, AllLite43) { EXPECT_TRUE(message2.MergeFromCodedStream(&input_stream)); EXPECT_EQ(17, message2.oneof_int32()); } + + // Bytes [ctype = CORD] + { + protobuf_unittest::TestOneofParsingLite message2; + message2.set_oneof_bytes_cord("bytes cord"); + io::CodedInputStream input_stream( + reinterpret_cast(serialized.data()), + serialized.size()); + EXPECT_TRUE(message2.MergeFromCodedStream(&input_stream)); + EXPECT_EQ(17, message2.oneof_int32()); + } + } // Verify that we can successfully parse fields of various types within oneof @@ -1044,6 +1056,18 @@ TYPED_TEST(LiteTest, AllLite44) { } } + // Bytes [ctype = CORD] + { + protobuf_unittest::TestOneofParsingLite original; + original.set_oneof_bytes_cord("bytes cord"); + std::string serialized; + EXPECT_TRUE(original.SerializeToString(&serialized)); + protobuf_unittest::TestOneofParsingLite parsed; + EXPECT_TRUE(parsed.MergeFromString(serialized)); + EXPECT_EQ("bytes cord", std::string(parsed.oneof_bytes_cord())); + EXPECT_TRUE(parsed.MergeFromString(serialized)); + } + std::cout << "PASS" << std::endl; } diff --git a/src/google/protobuf/map_test.inc b/src/google/protobuf/map_test.inc index 14cd1eb8f30c..4b752343460a 100644 --- a/src/google/protobuf/map_test.inc +++ b/src/google/protobuf/map_test.inc @@ -3938,16 +3938,35 @@ TEST(MapSerializationTest, Deterministic) { EXPECT_TRUE(util::MessageDifferencer::Equals(u, t)); } +static std::string GetGoldenMessageTextProto() { + static std::string* golden_message_textproto = [] { + std::string* textproto = new std::string; + ABSL_CHECK_OK(File::GetContents( + TestUtil::GetTestDataPath("google/protobuf/" + "testdata/map_test_data.txt"), + textproto, true)); + return textproto; + }(); + return *golden_message_textproto; +} + +static std::string GetGoldenMessageBinary() { + static std::string* golden_message_binary = [] { + UNITTEST::TestMaps t; + TextFormat::Parser parser; + parser.ParseFromString(GetGoldenMessageTextProto(), &t); + + std::string* result = new std::string; + t.SerializeToString(result); + return result; + }(); + return *golden_message_binary; +} + TEST(MapSerializationTest, DeterministicSubmessage) { UNITTEST::TestSubmessageMaps p; UNITTEST::TestMaps t; - const std::string filename = "golden_message_maps"; - std::string golden; - ABSL_CHECK_OK( - File::GetContents(TestUtil::GetTestDataPath(absl::StrCat( - "google/protobuf/testdata/", filename)), - &golden, true)); - t.ParseFromString(golden); + t.ParseFromString(GetGoldenMessageBinary()); *(p.mutable_m()) = t; std::vector v; // Use multiple attempts to increase the chance of a failure if something is @@ -3985,15 +4004,9 @@ TEST(TextFormatMapTest, DynamicMessage) { MapReflectionTester tester(message->GetDescriptor()); tester.SetMapFieldsViaReflection(message.get()); - std::string expected_text; - ABSL_CHECK_OK( - File::GetContents(TestUtil::GetTestDataPath("google/protobuf/" - "testdata/map_test_data.txt"), - &expected_text, true)); - std::string actual_text; TextFormat::PrintToString(*message, &actual_text); - EXPECT_EQ(actual_text, expected_text); + EXPECT_EQ(actual_text, GetGoldenMessageTextProto()); } TEST(TextFormatMapTest, Sorted) { @@ -4001,35 +4014,17 @@ TEST(TextFormatMapTest, Sorted) { MapReflectionTester tester(message.GetDescriptor()); tester.SetMapFieldsViaReflection(&message); - std::string expected_text; - ABSL_CHECK_OK( - File::GetContents(TestUtil::GetTestDataPath("google/protobuf/" - "testdata/map_test_data.txt"), - &expected_text, true)); - TextFormat::Printer printer; std::string actual_text; printer.PrintToString(message, &actual_text); - EXPECT_EQ(actual_text, expected_text); + EXPECT_EQ(actual_text, GetGoldenMessageTextProto()); // Test again on the reverse order. UNITTEST::TestMap message2; tester.SetMapFieldsViaReflection(&message2); tester.SwapMapsViaReflection(&message2); printer.PrintToString(message2, &actual_text); - EXPECT_EQ(actual_text, expected_text); -} - -TEST(TextFormatMapTest, ParseCorruptedString) { - std::string serialized_message; - ABSL_CHECK_OK(File::GetContents( - TestUtil::GetTestDataPath( - "google/protobuf/testdata/golden_message_maps"), - &serialized_message, true)); - UNITTEST::TestMaps message; - ABSL_CHECK(message.ParseFromString(serialized_message)); - TestParseCorruptedString(message); - TestParseCorruptedString(message); + EXPECT_EQ(actual_text, GetGoldenMessageTextProto()); } // Previously, serializing to text format will disable iterator from generated diff --git a/src/google/protobuf/message.cc b/src/google/protobuf/message.cc index 27c82f786941..089a77922cba 100644 --- a/src/google/protobuf/message.cc +++ b/src/google/protobuf/message.cc @@ -477,9 +477,11 @@ const internal::RepeatedFieldAccessor* Reflection::RepeatedFieldAccessor( HANDLE_PRIMITIVE_TYPE(ENUM, int32_t) #undef HANDLE_PRIMITIVE_TYPE case FieldDescriptor::CPPTYPE_STRING: - switch (field->options().ctype()) { - default: - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + ABSL_LOG(FATAL) << "Repeated cords are not supported."; + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: return GetSingleton(); } break; diff --git a/src/google/protobuf/message_unittest.inc b/src/google/protobuf/message_unittest.inc index 42c06ed82027..dfd6fe5c9f33 100644 --- a/src/google/protobuf/message_unittest.inc +++ b/src/google/protobuf/message_unittest.inc @@ -29,6 +29,8 @@ #include #include +#include "google/protobuf/testing/file.h" +#include "google/protobuf/testing/file.h" #include "google/protobuf/descriptor.pb.h" #include #include "google/protobuf/testing/googletest.h" @@ -143,8 +145,12 @@ TEST(MESSAGE_TEST_NAME, SerializeToBrokenOstream) { } TEST(MESSAGE_TEST_NAME, ParseFromFileDescriptor) { - std::string filename = - TestUtil::GetTestDataPath("google/protobuf/testdata/golden_message"); + std::string filename = absl::StrCat(TestTempDir(), "/golden_message"); + UNITTEST::TestAllTypes expected_message; + TestUtil::SetAllFields(&expected_message); + ABSL_CHECK_OK(File::SetContents( + filename, expected_message.SerializeAsString(), true)); + int file = open(filename.c_str(), O_RDONLY | O_BINARY); ASSERT_GE(file, 0); @@ -156,8 +162,12 @@ TEST(MESSAGE_TEST_NAME, ParseFromFileDescriptor) { } TEST(MESSAGE_TEST_NAME, ParsePackedFromFileDescriptor) { - std::string filename = TestUtil::GetTestDataPath( - "google/protobuf/testdata/golden_packed_fields_message"); + std::string filename = absl::StrCat(TestTempDir(), "/golden_message"); + UNITTEST::TestPackedTypes expected_message; + TestUtil::SetPackedFields(&expected_message); + ABSL_CHECK_OK(File::SetContents( + filename, expected_message.SerializeAsString(), true)); + int file = open(filename.c_str(), O_RDONLY | O_BINARY); ASSERT_GE(file, 0); diff --git a/src/google/protobuf/test_util.h b/src/google/protobuf/test_util.h index c52c38fc4589..4f6e07ed3493 100644 --- a/src/google/protobuf/test_util.h +++ b/src/google/protobuf/test_util.h @@ -223,6 +223,8 @@ inline void TestUtil::ReflectionTester::SetAllFieldsViaReflection( reflection->SetString(message, F("optional_string_piece"), "124"); reflection->SetString(message, F("optional_cord"), "125"); + reflection->SetString(message, F("optional_bytes_cord"), + "optional bytes cord"); sub_message = reflection->MutableMessage(message, F("optional_public_import_message")); @@ -491,6 +493,7 @@ inline void TestUtil::ReflectionTester::ExpectAllFieldsSetViaReflection1( EXPECT_TRUE(reflection->HasField(message, F("optional_string_piece"))); EXPECT_TRUE(reflection->HasField(message, F("optional_cord"))); + EXPECT_TRUE(reflection->HasField(message, F("optional_bytes_cord"))); EXPECT_EQ(101, reflection->GetInt32(message, F("optional_int32"))); EXPECT_EQ(102, reflection->GetInt64(message, F("optional_int64"))); @@ -552,6 +555,14 @@ inline void TestUtil::ReflectionTester::ExpectAllFieldsSetViaReflection1( EXPECT_EQ("125", reflection->GetStringReference(message, F("optional_cord"), &scratch)); + EXPECT_EQ("optional bytes cord", + reflection->GetString(message, F("optional_bytes_cord"))); + EXPECT_EQ("optional bytes cord", + reflection->GetStringReference(message, F("optional_bytes_cord"), + &scratch)); + EXPECT_EQ("optional bytes cord", + reflection->GetCord(message, F("optional_bytes_cord"))); + EXPECT_TRUE(reflection->HasField(message, F("oneof_bytes"))); EXPECT_EQ("604", reflection->GetString(message, F("oneof_bytes"))); @@ -912,6 +923,7 @@ inline void TestUtil::ReflectionTester::ExpectClearViaReflection( EXPECT_FALSE(reflection->HasField(message, F("optional_string_piece"))); EXPECT_FALSE(reflection->HasField(message, F("optional_cord"))); + EXPECT_FALSE(reflection->HasField(message, F("optional_bytes_cord"))); // Optional fields without defaults are set to zero or something like it. EXPECT_EQ(0, reflection->GetInt32(message, F("optional_int32"))); @@ -978,6 +990,11 @@ inline void TestUtil::ReflectionTester::ExpectClearViaReflection( EXPECT_EQ("", reflection->GetStringReference(message, F("optional_cord"), &scratch)); + EXPECT_EQ("", reflection->GetString(message, F("optional_bytes_cord"))); + EXPECT_EQ("", reflection->GetStringReference( + message, F("optional_bytes_cord"), &scratch)); + EXPECT_EQ("", reflection->GetCord(message, F("optional_bytes_cord"))); + // Repeated fields are empty. EXPECT_EQ(0, reflection->FieldSize(message, F("repeated_int32"))); EXPECT_EQ(0, reflection->FieldSize(message, F("repeated_int64"))); diff --git a/src/google/protobuf/test_util.inc b/src/google/protobuf/test_util.inc index 77ea70ba2e90..f58f951d324a 100644 --- a/src/google/protobuf/test_util.inc +++ b/src/google/protobuf/test_util.inc @@ -137,6 +137,7 @@ inline void TestUtil::SetOptionalFields(UNITTEST::TestAllTypes* message) { message, message->GetDescriptor()->FindFieldByName("optional_cord"), "125"); #endif // !PROTOBUF_TEST_NO_DESCRIPTORS + message->set_optional_bytes_cord("optional bytes cord"); } // ------------------------------------------------------------------- @@ -352,6 +353,7 @@ inline void TestUtil::ExpectAllFieldsSet( EXPECT_TRUE(message.has_optional_string_piece()); EXPECT_TRUE(message.has_optional_cord()); #endif + EXPECT_TRUE(message.has_optional_bytes_cord()); EXPECT_EQ(101, message.optional_int32()); EXPECT_EQ(102, message.optional_int64()); @@ -381,6 +383,7 @@ inline void TestUtil::ExpectAllFieldsSet( EXPECT_EQ(UNITTEST::FOREIGN_BAZ, message.optional_foreign_enum()); EXPECT_EQ(UNITTEST_IMPORT::IMPORT_BAZ, message.optional_import_enum()); + EXPECT_EQ("optional bytes cord", message.optional_bytes_cord()); // ----------------------------------------------------------------- @@ -554,6 +557,7 @@ inline void TestUtil::ExpectClear(const UNITTEST::TestAllTypes& message) { EXPECT_FALSE(message.has_optional_string_piece()); EXPECT_FALSE(message.has_optional_cord()); + EXPECT_FALSE(message.has_optional_bytes_cord()); // Optional fields without defaults are set to zero or something like it. EXPECT_EQ(0, message.optional_int32()); @@ -594,6 +598,7 @@ inline void TestUtil::ExpectClear(const UNITTEST::TestAllTypes& message) { EXPECT_EQ(UNITTEST::FOREIGN_FOO, message.optional_foreign_enum()); EXPECT_EQ(UNITTEST_IMPORT::IMPORT_FOO, message.optional_import_enum()); + EXPECT_EQ("", message.optional_bytes_cord()); // Repeated fields are empty. EXPECT_EQ(0, message.repeated_int32_size()); @@ -975,6 +980,8 @@ inline void TestUtil::SetAllExtensions(UNITTEST::TestAllExtensions* message) { message->SetExtension(UNITTEST::optional_string_piece_extension, "124"); message->SetExtension(UNITTEST::optional_cord_extension, "125"); + message->SetExtension(UNITTEST::optional_bytes_cord_extension, + "optional bytes cord"); message->MutableExtension(UNITTEST::optional_public_import_message_extension) ->set_e(126); @@ -1204,6 +1211,7 @@ inline void TestUtil::ExpectAllExtensionsSet( EXPECT_TRUE(message.HasExtension(UNITTEST::optional_string_piece_extension)); EXPECT_TRUE(message.HasExtension(UNITTEST::optional_cord_extension)); + EXPECT_TRUE(message.HasExtension(UNITTEST::optional_bytes_cord_extension)); EXPECT_EQ(101, message.GetExtension(UNITTEST::optional_int32_extension)); EXPECT_EQ(102, message.GetExtension(UNITTEST::optional_int64_extension)); @@ -1242,6 +1250,8 @@ inline void TestUtil::ExpectAllExtensionsSet( EXPECT_EQ("124", message.GetExtension(UNITTEST::optional_string_piece_extension)); EXPECT_EQ("125", message.GetExtension(UNITTEST::optional_cord_extension)); + EXPECT_EQ("optional bytes cord", + message.GetExtension(UNITTEST::optional_bytes_cord_extension)); EXPECT_EQ( 126, message.GetExtension(UNITTEST::optional_public_import_message_extension) @@ -1490,6 +1500,7 @@ inline void TestUtil::ExpectExtensionsClear( EXPECT_FALSE(message.HasExtension(UNITTEST::optional_string_piece_extension)); EXPECT_FALSE(message.HasExtension(UNITTEST::optional_cord_extension)); + EXPECT_FALSE(message.HasExtension(UNITTEST::optional_bytes_cord_extension)); // Optional fields without defaults are set to zero or something like it. EXPECT_EQ(0, message.GetExtension(UNITTEST::optional_int32_extension)); @@ -1557,6 +1568,7 @@ inline void TestUtil::ExpectExtensionsClear( EXPECT_EQ("", message.GetExtension(UNITTEST::optional_string_piece_extension)); EXPECT_EQ("", message.GetExtension(UNITTEST::optional_cord_extension)); + EXPECT_EQ("", message.GetExtension(UNITTEST::optional_bytes_cord_extension)); // Repeated fields are empty. EXPECT_EQ(0, message.ExtensionSize(UNITTEST::repeated_int32_extension)); diff --git a/src/google/protobuf/testdata/golden_message b/src/google/protobuf/testdata/golden_message deleted file mode 100644 index 5825975ce07b..000000000000 Binary files a/src/google/protobuf/testdata/golden_message and /dev/null differ diff --git a/src/google/protobuf/testdata/golden_message_maps b/src/google/protobuf/testdata/golden_message_maps deleted file mode 100644 index c70a4d7cde5b..000000000000 Binary files a/src/google/protobuf/testdata/golden_message_maps and /dev/null differ diff --git a/src/google/protobuf/testdata/golden_message_oneof_implemented b/src/google/protobuf/testdata/golden_message_oneof_implemented deleted file mode 100644 index 794ca5e0d15e..000000000000 Binary files a/src/google/protobuf/testdata/golden_message_oneof_implemented and /dev/null differ diff --git a/src/google/protobuf/testdata/golden_message_proto3 b/src/google/protobuf/testdata/golden_message_proto3 deleted file mode 100644 index bd646a0dc85e..000000000000 Binary files a/src/google/protobuf/testdata/golden_message_proto3 and /dev/null differ diff --git a/src/google/protobuf/testdata/golden_packed_fields_message b/src/google/protobuf/testdata/golden_packed_fields_message deleted file mode 100644 index ee28d3883055..000000000000 Binary files a/src/google/protobuf/testdata/golden_packed_fields_message and /dev/null differ diff --git a/src/google/protobuf/testdata/text_format_unittest_data.txt b/src/google/protobuf/testdata/text_format_unittest_data.txt index 7a874b54cdba..b218f291b067 100644 --- a/src/google/protobuf/testdata/text_format_unittest_data.txt +++ b/src/google/protobuf/testdata/text_format_unittest_data.txt @@ -126,6 +126,7 @@ default_foreign_enum: FOREIGN_FOO default_import_enum: IMPORT_FOO default_string_piece: "424" default_cord: "425" +optional_bytes_cord: "optional bytes cord" oneof_uint32: 601 oneof_nested_message { bb: 602 diff --git a/src/google/protobuf/testdata/text_format_unittest_data_oneof_implemented.txt b/src/google/protobuf/testdata/text_format_unittest_data_oneof_implemented.txt index 86389c93cbce..c477bbef1b91 100644 --- a/src/google/protobuf/testdata/text_format_unittest_data_oneof_implemented.txt +++ b/src/google/protobuf/testdata/text_format_unittest_data_oneof_implemented.txt @@ -129,4 +129,5 @@ default_foreign_enum: FOREIGN_FOO default_import_enum: IMPORT_FOO default_string_piece: "424" default_cord: "425" +optional_bytes_cord: "optional bytes cord" oneof_bytes: "604" diff --git a/src/google/protobuf/testdata/text_format_unittest_data_pointy.txt b/src/google/protobuf/testdata/text_format_unittest_data_pointy.txt index 788025c51530..d46ec7bb3a4f 100644 --- a/src/google/protobuf/testdata/text_format_unittest_data_pointy.txt +++ b/src/google/protobuf/testdata/text_format_unittest_data_pointy.txt @@ -129,6 +129,7 @@ default_foreign_enum: FOREIGN_FOO default_import_enum: IMPORT_FOO default_string_piece: "424" default_cord: "425" +optional_bytes_cord: "optional bytes cord" oneof_uint32: 601 oneof_nested_message < bb: 602 diff --git a/src/google/protobuf/testdata/text_format_unittest_data_pointy_oneof.txt b/src/google/protobuf/testdata/text_format_unittest_data_pointy_oneof.txt index b2d3367098ce..139e3df348e7 100644 --- a/src/google/protobuf/testdata/text_format_unittest_data_pointy_oneof.txt +++ b/src/google/protobuf/testdata/text_format_unittest_data_pointy_oneof.txt @@ -129,4 +129,5 @@ default_foreign_enum: FOREIGN_FOO default_import_enum: IMPORT_FOO default_string_piece: "424" default_cord: "425" +optional_bytes_cord: "optional bytes cord" oneof_bytes: "604" diff --git a/src/google/protobuf/testdata/text_format_unittest_extensions_data.txt b/src/google/protobuf/testdata/text_format_unittest_extensions_data.txt index 5c3a03ac3708..8ab4d2e85b0c 100644 --- a/src/google/protobuf/testdata/text_format_unittest_extensions_data.txt +++ b/src/google/protobuf/testdata/text_format_unittest_extensions_data.txt @@ -129,6 +129,7 @@ [protobuf_unittest.default_import_enum_extension]: IMPORT_FOO [protobuf_unittest.default_string_piece_extension]: "424" [protobuf_unittest.default_cord_extension]: "425" +[protobuf_unittest.optional_bytes_cord_extension]: "optional bytes cord" [protobuf_unittest.oneof_uint32_extension]: 601 [protobuf_unittest.oneof_nested_message_extension] { bb: 602 diff --git a/src/google/protobuf/testdata/text_format_unittest_extensions_data_pointy.txt b/src/google/protobuf/testdata/text_format_unittest_extensions_data_pointy.txt index 4233ca78f301..20b5ea116292 100644 --- a/src/google/protobuf/testdata/text_format_unittest_extensions_data_pointy.txt +++ b/src/google/protobuf/testdata/text_format_unittest_extensions_data_pointy.txt @@ -129,6 +129,7 @@ [protobuf_unittest.default_import_enum_extension]: IMPORT_FOO [protobuf_unittest.default_string_piece_extension]: "424" [protobuf_unittest.default_cord_extension]: "425" +[protobuf_unittest.optional_bytes_cord_extension]: "optional bytes cord" [protobuf_unittest.oneof_uint32_extension]: 601 [protobuf_unittest.oneof_nested_message_extension] < bb: 602 diff --git a/src/google/protobuf/unittest.proto b/src/google/protobuf/unittest.proto index cb46956d31f1..89e4dbeb5806 100644 --- a/src/google/protobuf/unittest.proto +++ b/src/google/protobuf/unittest.proto @@ -84,6 +84,7 @@ message TestAllTypes { optional string optional_string_piece = 24 [ctype=STRING_PIECE]; optional string optional_cord = 25 [ctype=CORD]; + optional bytes optional_bytes_cord = 86 [ctype=CORD]; // Defined in unittest_import_public.proto optional protobuf_unittest_import.PublicImportMessage @@ -252,6 +253,7 @@ extend TestAllExtensions { // TODO: ctype=CORD is not supported for extension. Add // ctype=CORD option back after it is supported. optional string optional_cord_extension = 25; + optional bytes optional_bytes_cord_extension = 86; optional protobuf_unittest_import.PublicImportMessage optional_public_import_message_extension = 26; diff --git a/src/google/protobuf/unittest_lite.proto b/src/google/protobuf/unittest_lite.proto index b3fcfa431c00..cdafcf7933ba 100644 --- a/src/google/protobuf/unittest_lite.proto +++ b/src/google/protobuf/unittest_lite.proto @@ -72,6 +72,7 @@ message TestAllTypesLite { string optional_string_piece = 24 [ctype = STRING_PIECE]; string optional_cord = 25 [ctype = CORD]; + bytes optional_bytes_cord = 86 [ctype = CORD]; // Defined in unittest_import_public.proto protobuf_unittest_import.PublicImportMessageLite @@ -261,6 +262,7 @@ extend TestAllExtensionsLite { // TODO: ctype=CORD is not supported for extension. Add // ctype=CORD option back after it is supported. string optional_cord_extension_lite = 25; + bytes optional_bytes_cord_extension_lite = 86; protobuf_unittest_import.PublicImportMessageLite optional_public_import_message_extension_lite = 26; TestAllTypesLite.NestedMessage optional_lazy_message_extension_lite = 27 diff --git a/src/google/protobuf/unittest_proto3_arena.proto b/src/google/protobuf/unittest_proto3_arena.proto index 96a9ea85e295..ea801cef647e 100644 --- a/src/google/protobuf/unittest_proto3_arena.proto +++ b/src/google/protobuf/unittest_proto3_arena.proto @@ -67,6 +67,7 @@ message TestAllTypes { string optional_string_piece = 24 [ctype=STRING_PIECE]; string optional_cord = 25 [ctype=CORD]; + bytes optional_bytes_cord = 86 [ctype=CORD]; // Defined in unittest_import_public.proto protobuf_unittest_import.PublicImportMessage diff --git a/src/google/protobuf/util/field_mask_util_test.cc b/src/google/protobuf/util/field_mask_util_test.cc index 79fe32c9dfda..624d6ac613ce 100644 --- a/src/google/protobuf/util/field_mask_util_test.cc +++ b/src/google/protobuf/util/field_mask_util_test.cc @@ -202,7 +202,7 @@ TEST(FieldMaskUtilTest, TestGetFieldMaskForAllFields) { EXPECT_TRUE(FieldMaskUtil::IsPathInFieldMask("bb", mask)); mask = FieldMaskUtil::GetFieldMaskForAllFields(); - EXPECT_EQ(79, mask.paths_size()); + EXPECT_EQ(80, mask.paths_size()); EXPECT_TRUE(FieldMaskUtil::IsPathInFieldMask("optional_int32", mask)); EXPECT_TRUE(FieldMaskUtil::IsPathInFieldMask("optional_int64", mask)); EXPECT_TRUE(FieldMaskUtil::IsPathInFieldMask("optional_uint32", mask)); diff --git a/src/google/protobuf/wire_format.cc b/src/google/protobuf/wire_format.cc index 99db63f76e75..cc32a79b2cee 100644 --- a/src/google/protobuf/wire_format.cc +++ b/src/google/protobuf/wire_format.cc @@ -560,7 +560,7 @@ bool WireFormat::ParseAndMergeField( } case FieldDescriptor::TYPE_BYTES: { - if (internal::cpp::EffectiveStringCType(field) == FieldOptions::CORD) { + if (field->cpp_string_type() == FieldDescriptor::CppStringType::kCord) { absl::Cord value; if (!WireFormatLite::ReadBytes(input, &value)) return false; message_reflection->SetString(message, field, value); @@ -1003,7 +1003,7 @@ const char* WireFormat::_InternalParseAndMergeField( case FieldDescriptor::TYPE_BYTES: { int size = ReadSize(&ptr); if (ptr == nullptr) return nullptr; - if (internal::cpp::EffectiveStringCType(field) == FieldOptions::CORD) { + if (field->cpp_string_type() == FieldDescriptor::CppStringType::kCord) { absl::Cord value; ptr = ctx->ReadCord(ptr, size, &value); if (ptr == nullptr) return nullptr; @@ -1425,7 +1425,7 @@ uint8_t* WireFormat::InternalSerializeField(const FieldDescriptor* field, } case FieldDescriptor::TYPE_BYTES: { - if (internal::cpp::EffectiveStringCType(field) == FieldOptions::CORD) { + if (field->cpp_string_type() == FieldDescriptor::CppStringType::kCord) { absl::Cord value = message_reflection->GetCord(message, field); target = stream->WriteString(field->number(), value, target); break; @@ -1726,7 +1726,7 @@ size_t WireFormat::FieldDataOnlyByteSize(const FieldDescriptor* field, // instead of copying. case FieldDescriptor::TYPE_STRING: case FieldDescriptor::TYPE_BYTES: { - if (internal::cpp::EffectiveStringCType(field) == FieldOptions::CORD) { + if (field->cpp_string_type() == FieldDescriptor::CppStringType::kCord) { for (size_t j = 0; j < count; j++) { absl::Cord value = message_reflection->GetCord(message, field); data_size += WireFormatLite::StringSize(value);