Skip to content

Commit

Permalink
[ObjC] Validate MessageSet expectations.
Browse files Browse the repository at this point in the history
- Make the generator error if things are violated.
- Have the runtime assert in debug about the things also.

PiperOrigin-RevId: 651810474
  • Loading branch information
thomasvl authored and copybara-github committed Jul 12, 2024
1 parent 8e3eca4 commit 339261f
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 3 deletions.
14 changes: 13 additions & 1 deletion objectivec/GPBDescriptor.m
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,10 @@ - (instancetype)initWithClass:(Class)messageClass
fields:(NSArray *)fields
storageSize:(uint32_t)storageSize
wireFormat:(BOOL)wireFormat {
#if defined(DEBUG) && DEBUG && !defined(NS_BLOCK_ASSERTIONS)
// This is also checked by the generator.
NSAssert(!wireFormat || fields.count == 0, @"Internal error: MessageSets should not have fields");
#endif
if ((self = [super init])) {
messageClass_ = messageClass;
messageName_ = [messageName copy];
Expand Down Expand Up @@ -1139,8 +1143,16 @@ - (instancetype)initWithExtensionDescription:(GPBExtensionDescription *)desc
GPBRuntimeMatchFailure();
}

#if defined(DEBUG) && DEBUG
#if defined(DEBUG) && DEBUG && !defined(NS_BLOCK_ASSERTIONS)
NSAssert(usesClassRefs, @"Internal error: all extensions should have class refs");

// This is also checked by the generator.
// If the extension is a MessageSet extension, then it must be a message field.
NSAssert(
((desc->options & GPBExtensionSetWireFormat) == 0) || desc->dataType == GPBDataTypeMessage,
@"Internal error: If a MessageSet extension is set, the data type must be a message.");
// NOTE: Could also check that the exteneded class is a MessageSet, but that would force the ObjC
// runtime to start up that class and that isn't desirable here.
#endif

if ((self = [super init])) {
Expand Down
5 changes: 5 additions & 0 deletions src/google/protobuf/compiler/objectivec/extension.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ ExtensionGenerator::ExtensionGenerator(
ABSL_CHECK(!descriptor->is_map())
<< "error: Extension is a map<>!"
<< " That used to be blocked by the compiler.";
ABSL_CHECK(
!descriptor->containing_type()->options().message_set_wire_format() ||
descriptor->type() == FieldDescriptor::TYPE_MESSAGE)
<< "error: Extension to a message_set_wire_format message and the type "
"wasn't a message!";
}

void ExtensionGenerator::GenerateMembersHeader(io::Printer* printer) const {
Expand Down
8 changes: 6 additions & 2 deletions src/google/protobuf/compiler/objectivec/message.cc
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ struct ExtensionRangeOrdering {

// This is a reduced case of Descriptor::ExtensionRange with just start and end.
struct SimpleExtensionRange {
SimpleExtensionRange(int start, int end) : start(start), end(end){};
SimpleExtensionRange(int start, int end) : start(start), end(end) {};
int start; // inclusive
int end; // exclusive

Expand Down Expand Up @@ -202,8 +202,12 @@ MessageGenerator::MessageGenerator(const std::string& file_description_name,
class_name_(ClassName(descriptor_)),
deprecated_attribute_(
GetOptionalDeprecatedAttribute(descriptor, descriptor->file())) {
ABSL_DCHECK(!descriptor->options().map_entry())
ABSL_CHECK(!descriptor->options().map_entry())
<< "error: MessageGenerator create of a map<>!";
ABSL_CHECK(!descriptor->options().message_set_wire_format() ||
descriptor->field_count() == 0)
<< "error: MessageGenerator message_set_wire_format should never have "
"fields!";
for (int i = 0; i < descriptor_->real_oneof_decl_count(); i++) {
oneof_generators_.push_back(std::make_unique<OneofGenerator>(
descriptor_->real_oneof_decl(i), generation_options));
Expand Down

0 comments on commit 339261f

Please sign in to comment.