Implemented proto3 presence for Ruby. #7406
Conversation
| continue; | ||
| } | ||
|
|
||
| // Prepend '_' until we are no longer conflicting. |
There was a problem hiding this comment.
Will this renaming suddenly change when users reorder fields in their proto definition?
There was a problem hiding this comment.
If there are conflicts it could. But the synthetic oneof name was not created by the user, so the user cannot assume what its name is.
| if (accessor_type == METHOD_PRESENCE && test_f != NULL) { | ||
| if (!upb_fielddef_haspresence(test_f)) return METHOD_UNKNOWN; | ||
|
|
||
| // TODO(haberman): remove this case, allow for proto3 oneofs. |
There was a problem hiding this comment.
Do we previously support this?
There was a problem hiding this comment.
No, we have never supported this. The unit tests were failing until I added this check, see:
Lines 45 to 50 in 1895045
| size_t hasbit = layout->fields[upb_fielddef_index(field)].hasbit; | ||
| assert(field_contains_hasbit(layout, field)); |
There was a problem hiding this comment.
Do you need to move this assert?
There was a problem hiding this comment.
Yes, because it is a statement and cannot go before the declaration of hasbit.
This worked previously because we had not been testing with asserts enabled.
|
Re: synthetic oneof (and for more background information in general), see this doc which is currently in review: https://github.com/protocolbuffers/protobuf/blob/9ae5203712eb80f89261d6df8d5674efa5a0edb8/docs/implementing_proto3_presence.md |
|
Did ruby 2.6 fail on mac? |
|
@TeBoring fixed the crash that was happening on mac. |
No description provided.