diff --git a/java/core/src/main/java/com/google/protobuf/GeneratedMessageV3.java b/java/core/src/main/java/com/google/protobuf/GeneratedMessageV3.java index acc0e06141..fa5a070e74 100644 --- a/java/core/src/main/java/com/google/protobuf/GeneratedMessageV3.java +++ b/java/core/src/main/java/com/google/protobuf/GeneratedMessageV3.java @@ -2096,8 +2096,10 @@ public abstract class GeneratedMessageV3 extends AbstractMessage implements Seri FieldDescriptor field = descriptor.getFields().get(i); String containingOneofCamelCaseName = null; if (field.getContainingOneof() != null) { - containingOneofCamelCaseName = - camelCaseNames[fieldsSize + field.getContainingOneof().getIndex()]; + int index = fieldsSize + field.getContainingOneof().getIndex(); + if (index < camelCaseNames.length) { + containingOneofCamelCaseName = camelCaseNames[index]; + } } if (field.isRepeated()) { if (field.getJavaType() == FieldDescriptor.JavaType.MESSAGE) { @@ -2153,12 +2155,16 @@ public abstract class GeneratedMessageV3 extends AbstractMessage implements Seri } } - int oneofsSize = oneofs.length; - for (int i = 0; i < oneofsSize; i++) { - oneofs[i] = - new OneofAccessor( - descriptor, i, camelCaseNames[i + fieldsSize], messageClass, builderClass); + for (int i = 0; i < descriptor.getOneofs().size(); i++) { + if (i < descriptor.getRealOneofs().size()) { + oneofs[i] = + new RealOneofAccessor( + descriptor, i, camelCaseNames[i + fieldsSize], messageClass, builderClass); + } else { + oneofs[i] = new SyntheticOneofAccessor(descriptor, i); + } } + initialized = true; camelCaseNames = null; return this; @@ -2230,24 +2236,29 @@ public abstract class GeneratedMessageV3 extends AbstractMessage implements Seri } /** OneofAccessor provides access to a single oneof. */ - private static class OneofAccessor { - OneofAccessor( + private static interface OneofAccessor { + public boolean has(final GeneratedMessageV3 message); + + public boolean has(GeneratedMessageV3.Builder builder); + + public FieldDescriptor get(final GeneratedMessageV3 message); + + public FieldDescriptor get(GeneratedMessageV3.Builder builder); + + public void clear(final Builder builder); + } + + /** RealOneofAccessor provides access to a single real oneof. */ + private static class RealOneofAccessor implements OneofAccessor { + RealOneofAccessor( final Descriptor descriptor, final int oneofIndex, final String camelCaseName, final Class messageClass, final Class> builderClass) { this.descriptor = descriptor; - OneofDescriptor oneofDescriptor = descriptor.getOneofs().get(oneofIndex); - if (oneofDescriptor.isSynthetic()) { - caseMethod = null; - caseMethodBuilder = null; - fieldDescriptor = oneofDescriptor.getFields().get(0); - } else { - caseMethod = getMethodOrDie(messageClass, "get" + camelCaseName + "Case"); - caseMethodBuilder = getMethodOrDie(builderClass, "get" + camelCaseName + "Case"); - fieldDescriptor = null; - } + caseMethod = getMethodOrDie(messageClass, "get" + camelCaseName + "Case"); + caseMethodBuilder = getMethodOrDie(builderClass, "get" + camelCaseName + "Case"); clearMethod = getMethodOrDie(builderClass, "clear" + camelCaseName); } @@ -2255,55 +2266,76 @@ public abstract class GeneratedMessageV3 extends AbstractMessage implements Seri private final Method caseMethod; private final Method caseMethodBuilder; private final Method clearMethod; - private final FieldDescriptor fieldDescriptor; + @Override public boolean has(final GeneratedMessageV3 message) { - if (fieldDescriptor != null) { - return message.hasField(fieldDescriptor); - } else { - return ((Internal.EnumLite) invokeOrDie(caseMethod, message)).getNumber() != 0; - } + return ((Internal.EnumLite) invokeOrDie(caseMethod, message)).getNumber() != 0; } + @Override public boolean has(GeneratedMessageV3.Builder builder) { - if (fieldDescriptor != null) { - return builder.hasField(fieldDescriptor); - } else { - return ((Internal.EnumLite) invokeOrDie(caseMethodBuilder, builder)).getNumber() != 0; - } + return ((Internal.EnumLite) invokeOrDie(caseMethodBuilder, builder)).getNumber() != 0; } + @Override public FieldDescriptor get(final GeneratedMessageV3 message) { - if (fieldDescriptor != null) { - return message.hasField(fieldDescriptor) ? fieldDescriptor : null; - } else { - int fieldNumber = ((Internal.EnumLite) invokeOrDie(caseMethod, message)).getNumber(); - if (fieldNumber > 0) { - return descriptor.findFieldByNumber(fieldNumber); - } + int fieldNumber = ((Internal.EnumLite) invokeOrDie(caseMethod, message)).getNumber(); + if (fieldNumber > 0) { + return descriptor.findFieldByNumber(fieldNumber); } return null; } + @Override public FieldDescriptor get(GeneratedMessageV3.Builder builder) { - if (fieldDescriptor != null) { - return builder.hasField(fieldDescriptor) ? fieldDescriptor : null; - } else { - int fieldNumber = - ((Internal.EnumLite) invokeOrDie(caseMethodBuilder, builder)).getNumber(); - if (fieldNumber > 0) { - return descriptor.findFieldByNumber(fieldNumber); - } + int fieldNumber = ((Internal.EnumLite) invokeOrDie(caseMethodBuilder, builder)).getNumber(); + if (fieldNumber > 0) { + return descriptor.findFieldByNumber(fieldNumber); } return null; } + @Override public void clear(final Builder builder) { // TODO(b/230609037): remove the unused variable Object unused = invokeOrDie(clearMethod, builder); } } + /** SyntheticOneofAccessor provides access to a single synthetic oneof. */ + private static class SyntheticOneofAccessor implements OneofAccessor { + SyntheticOneofAccessor(final Descriptor descriptor, final int oneofIndex) { + OneofDescriptor oneofDescriptor = descriptor.getOneofs().get(oneofIndex); + fieldDescriptor = oneofDescriptor.getFields().get(0); + } + + private final FieldDescriptor fieldDescriptor; + + @Override + public boolean has(final GeneratedMessageV3 message) { + return message.hasField(fieldDescriptor); + } + + @Override + public boolean has(GeneratedMessageV3.Builder builder) { + return builder.hasField(fieldDescriptor); + } + + @Override + public FieldDescriptor get(final GeneratedMessageV3 message) { + return message.hasField(fieldDescriptor) ? fieldDescriptor : null; + } + + public FieldDescriptor get(GeneratedMessageV3.Builder builder) { + return builder.hasField(fieldDescriptor) ? fieldDescriptor : null; + } + + @Override + public void clear(final Builder builder) { + builder.clearField(fieldDescriptor); + } + } + // --------------------------------------------------------------- @SuppressWarnings("SameNameButDifferent") diff --git a/src/google/protobuf/compiler/java/message.cc b/src/google/protobuf/compiler/java/message.cc index 3ef5045268..5520f023ac 100644 --- a/src/google/protobuf/compiler/java/message.cc +++ b/src/google/protobuf/compiler/java/message.cc @@ -247,7 +247,8 @@ int ImmutableMessageGenerator::GenerateFieldAccessorTableInitializer( bytecode_estimate += 6; printer->Print("\"$field_name$\", ", "field_name", info->capitalized_name); } - // We reproduce synthetic oneofs here since proto reflection needs these. + // TODO(b/286434301): Once cl/534906231 propagates, only consider real oneofs + // since proto reflection does not use this for synthetic oneofs. for (int i = 0; i < descriptor_->oneof_decl_count(); i++) { const OneofDescriptor* oneof = descriptor_->oneof_decl(i); const OneofGeneratorInfo* info = context_->GetOneofGeneratorInfo(oneof);