From 8427b80b25a604ba001598335460c0e64f1a9361 Mon Sep 17 00:00:00 2001 From: hs3366677 Date: Wed, 14 Sep 2022 16:43:29 +0800 Subject: [PATCH 1/2] Fix conform; Make sure the command fails when tail fields are removed; --- src/idl_parser.cpp | 25 +++++++++++++++++++++---- tests/evolution_test.cpp | 2 ++ 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/idl_parser.cpp b/src/idl_parser.cpp index fabe9acb568..c7089ef0c38 100644 --- a/src/idl_parser.cpp +++ b/src/idl_parser.cpp @@ -4131,17 +4131,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::vector 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); + 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 @@ -4150,14 +4152,29 @@ 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.push_back(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 foot 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 (std::find(renamed_fields.begin(), renamed_fields.end(), &field_base) == renamed_fields.end()) { + auto field = struct_def.fields.Lookup(field_base.name); + auto qualified_field_name = qualified_name + "." + field_base.name; + if (!field) { + return "field deleted: " + qualified_field_name; + } + } + } } + for (auto eit = enums_.vec.begin(); eit != enums_.vec.end(); ++eit) { auto &enum_def = **eit; auto qualified_name = diff --git a/tests/evolution_test.cpp b/tests/evolution_test.cpp index da0fd743cf9..e7404d4218d 100644 --- a/tests/evolution_test.cpp +++ b/tests/evolution_test.cpp @@ -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) { From 1e36dba05866aa48395f302e5c53f88796abfa6c Mon Sep 17 00:00:00 2001 From: hs3366677 Date: Thu, 15 Sep 2022 11:15:35 +0800 Subject: [PATCH 2/2] Fix last commit according to dbaileychess' comment --- src/idl_parser.cpp | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/idl_parser.cpp b/src/idl_parser.cpp index c7089ef0c38..02d847e93a2 100644 --- a/src/idl_parser.cpp +++ b/src/idl_parser.cpp @@ -4131,12 +4131,12 @@ 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::vector renamed_fields; + std::set 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); - auto qualified_field_name = qualified_name + "." + 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: " + qualified_field_name; @@ -4152,7 +4152,7 @@ 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.push_back(field_base); + renamed_fields.insert(field_base); if (!EqualByName(field.value.type, field_base->value.type)) return "field renamed to different type: " + qualified_field_name; break; @@ -4160,16 +4160,15 @@ std::string Parser::ConformTo(const Parser &base) { } } } - //deletion of foot fields are not allowed + //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 (std::find(renamed_fields.begin(), renamed_fields.end(), &field_base) == renamed_fields.end()) { + if (renamed_fields.find(&field_base) == renamed_fields.end()) { auto field = struct_def.fields.Lookup(field_base.name); - auto qualified_field_name = qualified_name + "." + field_base.name; - if (!field) { - return "field deleted: " + qualified_field_name; + if (!field) { + return "field deleted: " + qualified_name + "." + field_base.name; } } }