Skip to content

Commit

Permalink
[ObjC] Add the concept of a closed enum.
Browse files Browse the repository at this point in the history 
- FieldDescriptor:
  - Add a new flag to mark when the enum on the field is closed (vs open).
  - Support computing the state for when generated sources predate the support.
- EnumDescriptor:
  - Support passing flags to the descriptor creation, currently closed is the
    only new flag.
  - Add an isClosed property to expose the state of the enum.

This does NOT update generation yet, allows things to be tested before the
generation support is added.

PiperOrigin-RevId: 488671606
  • Loading branch information
@protobuf-github-bot @copybara-github
protobuf-github-bot authored and copybara-github committed Nov 15, 2022
1 parent 3a74266 commit 7bb699b
Show file tree
Hide file tree
Showing 6 changed files with 143 additions and 32 deletions.
12 changes: 12 additions & 0 deletions objectivec/GPBDescriptor.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,18 @@ typedef NS_ENUM(uint8_t, GPBFieldType) {
@property(nonatomic, readonly, copy) NSString *name;
/** Function that validates that raw values are valid enum values. */
@property(nonatomic, readonly) GPBEnumValidationFunc enumVerifier;
/**
* Is this a closed enum, meaning that it:
* - Has a fixed set of named values.
* - Encountering values not in this set causes them to be treated as unknown
* fields.
* - The first value (i.e., the default) may be nonzero.
*
* NOTE: This is only accurate if the generate sources for a proto file were
* generated with a protobuf release after the v21.9 version, as the ObjC
* generator wasn't capturing this information.
*/
@property(nonatomic, readonly) BOOL isClosed;

/**
* Returns the enum value name for the given raw enum.
Expand Down
83 changes: 74 additions & 9 deletions objectivec/GPBDescriptor.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ - (instancetype)initWithFieldDescription:(void *)description
file:(GPBFileDescriptor *)file
includesDefault:(BOOL)includesDefault
usesClassRefs:(BOOL)usesClassRefs
proto3OptionalKnown:(BOOL)proto3OptionalKnown;
proto3OptionalKnown:(BOOL)proto3OptionalKnown
closedEnumSupportKnown:(BOOL)closedEnumSupportKnown;

@end

Expand All @@ -60,7 +61,8 @@ - (instancetype)initWithName:(NSString *)name
valueNames:(const char *)valueNames
values:(const int32_t *)values
count:(uint32_t)valueCount
enumVerifier:(GPBEnumValidationFunc)enumVerifier;
enumVerifier:(GPBEnumValidationFunc)enumVerifier
flags:(GPBEnumDescriptorInitializationFlags)flags;
@end

// Direct access is use for speed, to avoid even internally declaring things
Expand Down Expand Up @@ -150,6 +152,8 @@ + (instancetype)allocDescriptorForClass:(Class)messageClass
BOOL fieldsIncludeDefault = (flags & GPBDescriptorInitializationFlag_FieldsWithDefault) != 0;
BOOL usesClassRefs = (flags & GPBDescriptorInitializationFlag_UsesClassRefs) != 0;
BOOL proto3OptionalKnown = (flags & GPBDescriptorInitializationFlag_Proto3OptionalKnown) != 0;
BOOL closedEnumSupportKnown =
(flags & GPBDescriptorInitializationFlag_ClosedEnumSupportKnown) != 0;

void *desc;
for (uint32_t i = 0; i < fieldCount; ++i) {
Expand All @@ -164,7 +168,8 @@ + (instancetype)allocDescriptorForClass:(Class)messageClass
file:file
includesDefault:fieldsIncludeDefault
usesClassRefs:usesClassRefs
proto3OptionalKnown:proto3OptionalKnown];
proto3OptionalKnown:proto3OptionalKnown
closedEnumSupportKnown:closedEnumSupportKnown];
[fields addObject:fieldDescriptor];
[fieldDescriptor release];
}
Expand Down Expand Up @@ -475,7 +480,8 @@ - (instancetype)initWithFieldDescription:(void *)description
file:(GPBFileDescriptor *)file
includesDefault:(BOOL)includesDefault
usesClassRefs:(BOOL)usesClassRefs
proto3OptionalKnown:(BOOL)proto3OptionalKnown {
proto3OptionalKnown:(BOOL)proto3OptionalKnown
closedEnumSupportKnown:(BOOL)closedEnumSupportKnown {
if ((self = [super init])) {
GPBMessageFieldDescription *coreDesc;
if (includesDefault) {
Expand Down Expand Up @@ -506,6 +512,21 @@ - (instancetype)initWithFieldDescription:(void *)description
}
}

// If the ClosedEnum flag wasn't known (i.e. generated code from an older
// version), compute the flag for the rest of the runtime.
if (!closedEnumSupportKnown) {
// NOTE: This isn't correct, it is using the syntax of the file that
// declared the field, not the syntax of the file that declared the
// enum; but for older generated code, that's all we have and that happens
// to be what the runtime was doing (even though it was wrong). This is
// only wrong in the rare cases an enum is declared in a proto3 syntax
// file but used for a field in the proto2 syntax file.
BOOL isClosedEnum = (dataType == GPBDataTypeEnum && file.syntax != GPBFileSyntaxProto3);
if (isClosedEnum) {
coreDesc->flags |= GPBFieldClosedEnum;
}
}

if (isMapOrArray) {
// map<>/repeated fields get a *Count property (inplace of a has*) to
// support checking if there are any entries without triggering
Expand Down Expand Up @@ -535,9 +556,14 @@ - (instancetype)initWithFieldDescription:(void *)description
NSAssert(msgClass_, @"Class %s not defined", className);
}
} else if (dataType == GPBDataTypeEnum) {
enumDescriptor_ = coreDesc->dataTypeSpecific.enumDescFunc();
#if defined(DEBUG) && DEBUG
NSAssert((coreDesc->flags & GPBFieldHasEnumDescriptor) != 0,
@"Field must have GPBFieldHasEnumDescriptor set");
enumDescriptor_ = coreDesc->dataTypeSpecific.enumDescFunc();
NSAssert(!closedEnumSupportKnown ||
(((coreDesc->flags & GPBFieldClosedEnum) != 0) == enumDescriptor_.isClosed),
@"Internal error, ClosedEnum flag doesn't agree with EnumDescriptor");
#endif // DEBUG
}

// Non map<>/repeated fields can have defaults in proto2 syntax.
Expand Down Expand Up @@ -740,6 +766,7 @@ @implementation GPBEnumDescriptor {
const uint8_t *extraTextFormatInfo_;
uint32_t *nameOffsets_;
uint32_t valueCount_;
uint32_t flags_;
}

@synthesize name = name_;
Expand All @@ -749,12 +776,14 @@ + (instancetype)allocDescriptorForName:(NSString *)name
valueNames:(const char *)valueNames
values:(const int32_t *)values
count:(uint32_t)valueCount
enumVerifier:(GPBEnumValidationFunc)enumVerifier {
enumVerifier:(GPBEnumValidationFunc)enumVerifier
flags:(GPBEnumDescriptorInitializationFlags)flags {
GPBEnumDescriptor *descriptor = [[self alloc] initWithName:name
valueNames:valueNames
values:values
count:valueCount
enumVerifier:enumVerifier];
enumVerifier:enumVerifier
flags:flags];
return descriptor;
}

Expand All @@ -763,29 +792,61 @@ + (instancetype)allocDescriptorForName:(NSString *)name
values:(const int32_t *)values
count:(uint32_t)valueCount
enumVerifier:(GPBEnumValidationFunc)enumVerifier
flags:(GPBEnumDescriptorInitializationFlags)flags
extraTextFormatInfo:(const char *)extraTextFormatInfo {
// Call the common case.
GPBEnumDescriptor *descriptor = [self allocDescriptorForName:name
valueNames:valueNames
values:values
count:valueCount
enumVerifier:enumVerifier];
enumVerifier:enumVerifier
flags:flags];
// Set the extra info.
descriptor->extraTextFormatInfo_ = (const uint8_t *)extraTextFormatInfo;
return descriptor;
}

+ (instancetype)allocDescriptorForName:(NSString *)name
valueNames:(const char *)valueNames
values:(const int32_t *)values
count:(uint32_t)valueCount
enumVerifier:(GPBEnumValidationFunc)enumVerifier {
return [self allocDescriptorForName:name
valueNames:valueNames
values:values
count:valueCount
enumVerifier:enumVerifier
flags:GPBEnumDescriptorInitializationFlag_None];
}

+ (instancetype)allocDescriptorForName:(NSString *)name
valueNames:(const char *)valueNames
values:(const int32_t *)values
count:(uint32_t)valueCount
enumVerifier:(GPBEnumValidationFunc)enumVerifier
extraTextFormatInfo:(const char *)extraTextFormatInfo {
return [self allocDescriptorForName:name
valueNames:valueNames
values:values
count:valueCount
enumVerifier:enumVerifier
flags:GPBEnumDescriptorInitializationFlag_None
extraTextFormatInfo:extraTextFormatInfo];
}

- (instancetype)initWithName:(NSString *)name
valueNames:(const char *)valueNames
values:(const int32_t *)values
count:(uint32_t)valueCount
enumVerifier:(GPBEnumValidationFunc)enumVerifier {
enumVerifier:(GPBEnumValidationFunc)enumVerifier
flags:(GPBEnumDescriptorInitializationFlags)flags {
if ((self = [super init])) {
name_ = [name copy];
valueNames_ = valueNames;
values_ = values;
valueCount_ = valueCount;
enumVerifier_ = enumVerifier;
flags_ = flags;
}
return self;
}
Expand All @@ -796,6 +857,10 @@ - (void)dealloc {
[super dealloc];
}

- (BOOL)isClosed {
return (flags_ & GPBEnumDescriptorInitializationFlag_IsClosed) != 0;
}

- (void)calcValueNameOffsets {
@synchronized(self) {
if (nameOffsets_ != NULL) {
Expand Down
48 changes: 44 additions & 4 deletions objectivec/GPBDescriptor_PackagePrivate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,18 @@ typedef NS_OPTIONS(uint16_t, GPBFieldFlags) {
GPBFieldMapKeySFixed64 = 10 << 8,
GPBFieldMapKeyBool = 11 << 8,
GPBFieldMapKeyString = 12 << 8,

// If the enum for this field is "closed", meaning that it:
// - Has a fixed set of named values.
// - Encountering values not in this set causes them to be treated as unknown
// fields.
// - The first value (i.e., the default) may be nonzero.
// NOTE: This could be tracked just on the GPBEnumDescriptor, but to support
// previously generated code, there would be not data to get the behavior
// correct, so instead it is tracked on the field. If old source compatibility
// is removed, this could be removed and the GPBEnumDescription fetched from
// the GPBFieldDescriptor instead.
GPBFieldClosedEnum = 1 << 12,
};

// NOTE: The structures defined here have their members ordered to minimize
Expand Down Expand Up @@ -173,6 +185,12 @@ typedef NS_OPTIONS(uint32_t, GPBDescriptorInitializationFlags) {
// at startup. This allows older generated code to still work with the
// current runtime library.
GPBDescriptorInitializationFlag_Proto3OptionalKnown = 1 << 3,

// This flag is used to indicate that the generated sources already contain
// the `GPBFieldCloseEnum` flag and it doesn't have to be computed at startup.
// This allows the older generated code to still work with the current runtime
// library.
GPBDescriptorInitializationFlag_ClosedEnumSupportKnown = 1 << 4,
};

@interface GPBDescriptor () {
Expand Down Expand Up @@ -236,14 +254,36 @@ typedef NS_OPTIONS(uint32_t, GPBDescriptorInitializationFlags) {
}
@end

typedef NS_OPTIONS(uint32_t, GPBEnumDescriptorInitializationFlags) {
GPBEnumDescriptorInitializationFlag_None = 0,

// Marks this enum as a closed enum.
GPBEnumDescriptorInitializationFlag_IsClosed = 1 << 1,
};

@interface GPBEnumDescriptor ()
// valueNames, values and extraTextFormatInfo have to be long lived, they are
// held as raw pointers.
+ (instancetype)allocDescriptorForName:(NSString *)name
valueNames:(const char *)valueNames
values:(const int32_t *)values
count:(uint32_t)valueCount
enumVerifier:(GPBEnumValidationFunc)enumVerifier
flags:(GPBEnumDescriptorInitializationFlags)flags;
+ (instancetype)allocDescriptorForName:(NSString *)name
valueNames:(const char *)valueNames
values:(const int32_t *)values
count:(uint32_t)valueCount
enumVerifier:(GPBEnumValidationFunc)enumVerifier
flags:(GPBEnumDescriptorInitializationFlags)flags
extraTextFormatInfo:(const char *)extraTextFormatInfo;
// Deprecated. Calls above with `flags = 0`
+ (instancetype)allocDescriptorForName:(NSString *)name
valueNames:(const char *)valueNames
values:(const int32_t *)values
count:(uint32_t)valueCount
enumVerifier:(GPBEnumValidationFunc)enumVerifier;
// Deprecated. Calls above with `flags = 0`
+ (instancetype)allocDescriptorForName:(NSString *)name
valueNames:(const char *)valueNames
values:(const int32_t *)values
Expand Down Expand Up @@ -297,6 +337,10 @@ GPB_INLINE uint32_t GPBFieldNumber(GPBFieldDescriptor *field) {
return field->description_->number;
}

GPB_INLINE BOOL GPBFieldIsClosedEnum(GPBFieldDescriptor *field) {
return (field->description_->flags & GPBFieldClosedEnum) != 0;
}

#pragma clang diagnostic pop

uint32_t GPBFieldTag(GPBFieldDescriptor *self);
Expand All @@ -307,10 +351,6 @@ uint32_t GPBFieldTag(GPBFieldDescriptor *self);
// would be the wire type for packed.
uint32_t GPBFieldAlternateTag(GPBFieldDescriptor *self);

GPB_INLINE BOOL GPBHasPreservingUnknownEnumSemantics(GPBFileSyntax syntax) {
return syntax == GPBFileSyntaxProto3;
}

GPB_INLINE BOOL GPBExtensionIsRepeated(GPBExtensionDescription *description) {
return (description->options & GPBExtensionRepeated) != 0;
}
Expand Down
3 changes: 1 addition & 2 deletions objectivec/GPBDictionary.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -501,8 +501,7 @@ void GPBDictionaryReadEntry(id mapDictionary, GPBCodedInputStream *stream,
[(NSMutableDictionary *)mapDictionary setObject:value.valueString forKey:key.valueString];
} else {
if (valueDataType == GPBDataTypeEnum) {
if (GPBHasPreservingUnknownEnumSemantics([parentMessage descriptor].file.syntax) ||
[field isValidEnumValue:value.valueEnum]) {
if (!GPBFieldIsClosedEnum(field) || [field isValidEnumValue:value.valueEnum]) {
[mapDictionary setGPBGenericValue:&value forGPBGenericValueKey:&key];
} else {
NSData *data = [mapDictionary serializedDataForUnknownValue:value.valueEnum
Expand Down
Loading

0 comments on commit 7bb699b

Please sign in to comment.