Skip to content

Commit

Permalink
Migrate away from examining the syntax of a proto file via reflection.
Browse files Browse the repository at this point in the history 
PiperOrigin-RevId: 511796130
  • Loading branch information
@zhangskz @copybara-github
zhangskz authored and copybara-github committed Feb 23, 2023
1 parent 684a3de commit 2a55706
Showing 1 changed file with 1 addition and 4 deletions.
5 changes: 1 addition & 4 deletions java/core/src/main/java/com/google/protobuf/GeneratedMessageV3.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2407,10 +2407,7 @@ public void clear(final GeneratedMessageV3.Builder<?> builder) {
isOneofField =
descriptor.getContainingOneof() != null
&& !descriptor.getContainingOneof().isSynthetic();
hasHasMethod =
descriptor.getFile().getSyntax() == FileDescriptor.Syntax.PROTO2
|| descriptor.hasOptionalKeyword()
|| (!isOneofField && descriptor.getJavaType() == FieldDescriptor.JavaType.MESSAGE);
hasHasMethod = descriptor.hasPresence();

This comment has been minimized.

Sign in to view

Copy link
@zzsdsgdtc

zzsdsgdtc Sep 25, 2023 •
edited
Loading

@zhangskz this seems to be a breaking change for oneof: before this change, hasHasMethod for oneof will be false, but after this it will be true. When hasHasMethod is true, the generated codes for oneof need to have a hasXXX() method, which was introduced after 3.15 protoc. So for any generated codes from old protoc version, they will break when using this runtime, however it states here that for minor version older protoc should work on newer runtime

This comment has been minimized.

Sign in to view

Copy link
@zhangskz

zhangskz Sep 28, 2023

Author Member

Thanks for raising this -- we knew that this changed gencode to consider hazzers for oneofs but missed that those hazzers weren't added until 3.15. Per https://protobuf.dev/support/cross-version-runtime-guarantee/, we will roll this back for now and introduce this in our next Java major bump instead (slated for 26.x Q12024 release).

This fix should be in the upcoming 25.x release and will be backported to 24.x branch for future 24.x patches.

This comment has been minimized.

Sign in to view

Copy link
@zhangskz

zhangskz Sep 28, 2023

Author Member

Tracked in #14245
ReflectionInvoker reflectionInvoker =
new ReflectionInvoker(
descriptor,
Expand Down

0 comments on commit 2a55706

Please sign in to comment.