Skip to content

Commit

Permalink
Fix conform (#7532)
Browse files Browse the repository at this point in the history
* Fix conform;
Make sure the command fails when tail fields are removed;

* Fix last commit according to dbaileychess' comment
  • Loading branch information
hs3366677 committed Sep 15, 2022
1 parent bc44fad commit bfceebb
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 4 deletions.
24 changes: 20 additions & 4 deletions src/idl_parser.cpp
Expand Up @@ -4120,17 +4120,19 @@ std::string Parser::ConformTo(const Parser &base) {
struct_def.defined_namespace->GetFullyQualifiedName(struct_def.name);
auto struct_def_base = base.LookupStruct(qualified_name);
if (!struct_def_base) continue;
std::set<FieldDef*> renamed_fields;
for (auto fit = struct_def.fields.vec.begin();
fit != struct_def.fields.vec.end(); ++fit) {
auto &field = **fit;
auto field_base = struct_def_base->fields.Lookup(field.name);
const auto qualified_field_name = qualified_name + "." + field.name;
if (field_base) {
if (field.value.offset != field_base->value.offset)
return "offsets differ for field: " + field.name;
return "offsets differ for field: " + qualified_field_name;
if (field.value.constant != field_base->value.constant)
return "defaults differ for field: " + field.name;
return "defaults differ for field: " + qualified_field_name;
if (!EqualByName(field.value.type, field_base->value.type))
return "types differ for field: " + field.name;
return "types differ for field: " + qualified_field_name;
} else {
// Doesn't have to exist, deleting fields is fine.
// But we should check if there is a field that has the same offset
Expand All @@ -4139,14 +4141,28 @@ std::string Parser::ConformTo(const Parser &base) {
fbit != struct_def_base->fields.vec.end(); ++fbit) {
field_base = *fbit;
if (field.value.offset == field_base->value.offset) {
renamed_fields.insert(field_base);
if (!EqualByName(field.value.type, field_base->value.type))
return "field renamed to different type: " + field.name;
return "field renamed to different type: " + qualified_field_name;
break;
}
}
}
}
//deletion of trailing fields are not allowed
for (auto fit = struct_def_base->fields.vec.begin();
fit != struct_def_base->fields.vec.end(); ++fit ) {
auto &field_base = **fit;
//not a renamed field
if (renamed_fields.find(&field_base) == renamed_fields.end()) {
auto field = struct_def.fields.Lookup(field_base.name);
if (!field) {
return "field deleted: " + qualified_name + "." + field_base.name;
}
}
}
}

for (auto eit = enums_.vec.begin(); eit != enums_.vec.end(); ++eit) {
auto &enum_def = **eit;
auto qualified_name =
Expand Down
2 changes: 2 additions & 0 deletions tests/evolution_test.cpp
Expand Up @@ -98,6 +98,8 @@ void ConformTest() {
test_conform(parser, "table T { B:float; }",
"field renamed to different type");
test_conform(parser, "enum E:byte { B, A }", "values differ for enum");
test_conform(parser, "table T { }", "field deleted");
test_conform(parser, "table T { B:int; }", ""); //renaming a field is allowed
}

void UnionDeprecationTest(const std::string& tests_data_path) {
Expand Down

0 comments on commit bfceebb

Please sign in to comment.