From e810635eaac4cad6e026522843152e2b501c5889 Mon Sep 17 00:00:00 2001 From: Liu Liu Date: Sun, 28 Jun 2020 02:36:55 -0700 Subject: [PATCH] [Swift] FlatBuffers createMonster method doesn't treat struct properly (#5992) * [Swift] FlatBuffers createMonster method doesn't treat struct properly This PR fixed a issue where a struct is not treated properly when use create inside. A example would be the pos inside Monster. The createMonster method takes an Offset for pos. However, FlatBuffersBuilder.add(struct:) doesn't really take Offset argument. That means we don't really add a struct at all for Monster. It will show up as the pos never set. This doesn't show up in FlatBuffersMonsterWriterTests.swift because it implements its own createMonster method, which happens do the dance properly (i.e. first call create(struct) and then immediately call add:). This PR modified the `add(pos:)` interface such that it takes the UnsafeMutableRawPointer directly, calling `create(struct:)` under the hood. I can add unit tests once the direction of this PR approved. * Fix object api pack method codegen. * Add unit tests that uses Monster.createMonster method to serialize. * Updated sample_binary.swift --- samples/sample_binary.swift | 35 +++++++++---------- src/idl_gen_swift.cpp | 31 ++++++++++------ .../FlatBuffersMonsterWriterTests.swift | 27 ++++++++++++-- .../XCTestManifests.swift | 2 ++ .../monster_test_generated.swift | 7 ++-- 5 files changed, 68 insertions(+), 34 deletions(-) diff --git a/samples/sample_binary.swift b/samples/sample_binary.swift index 3a6dc747c..832eb3d55 100644 --- a/samples/sample_binary.swift +++ b/samples/sample_binary.swift @@ -1,5 +1,5 @@ // THIS IS JUST TO SHOW THE CODE, PLEASE DO IMPORT FLATBUFFERS WITH SPM.. -import Flatbuffers +import FlatBuffers typealias Monster = MyGame.Sample.Monster typealias Weapon = MyGame.Sample.Weapon @@ -10,29 +10,28 @@ func main() { let expectedDMG: [Int16] = [3, 5] let expectedNames = ["Sword", "Axe"] - let builder = FlatBufferBuilder(initialSize: 1024) + var builder = FlatBufferBuilder(initialSize: 1024) let weapon1Name = builder.create(string: expectedNames[0]) let weapon2Name = builder.create(string: expectedNames[1]) - let weapon1Start = Weapon.startWeapon(builder) - Weapon.add(name: weapon1Name, builder) - Weapon.add(damage: expectedDMG[0], builder) - let sword = Weapon.endWeapon(builder, start: weapon1Start) - let weapon2Start = Weapon.startWeapon(builder) - Weapon.add(name: weapon2Name, builder) - Weapon.add(damage: expectedDMG[1], builder) - let axe = Weapon.endWeapon(builder, start: weapon2Start) + let weapon1Start = Weapon.startWeapon(&builder) + Weapon.add(name: weapon1Name, &builder) + Weapon.add(damage: expectedDMG[0], &builder) + let sword = Weapon.endWeapon(&builder, start: weapon1Start) + let weapon2Start = Weapon.startWeapon(&builder) + Weapon.add(name: weapon2Name, &builder) + Weapon.add(damage: expectedDMG[1], &builder) + let axe = Weapon.endWeapon(&builder, start: weapon2Start) let name = builder.create(string: "Orc") let inventory: [Byte] = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9] let inventoryOffset = builder.createVector(inventory) let weaponsOffset = builder.createVector(ofOffsets: [sword, axe]) - let pos = builder.create(struct: MyGame.Sample.createVec3(x: 1, y: 2, z: 3), type: Vec3.self) - - - let orc = Monster.createMonster(builder, - offsetOfPos: pos, + let pos = MyGame.Sample.createVec3(x: 1, y: 2, z: 3) + + let orc = Monster.createMonster(&builder, + structOfPos: pos, hp: 300, offsetOfName: name, vectorOfInventory: inventoryOffset, @@ -42,8 +41,8 @@ func main() { offsetOfEquipped: axe) builder.finish(offset: orc) - var buf = builder.sizedByteArray - var monster = Monster.getRootAsMonster(bb: ByteBuffer(bytes: buf)) + let buf = builder.sizedByteArray + let monster = Monster.getRootAsMonster(bb: ByteBuffer(bytes: buf)) assert(monster.mana == 150) assert(monster.hp == 300) @@ -65,4 +64,4 @@ func main() { assert(equipped?.name == "Axe") assert(equipped?.damage == 5) print("Monster Object is Verified") -} \ No newline at end of file +} diff --git a/src/idl_gen_swift.cpp b/src/idl_gen_swift.cpp index 6245b8e25..d5ff4447e 100644 --- a/src/idl_gen_swift.cpp +++ b/src/idl_gen_swift.cpp @@ -485,7 +485,7 @@ class SwiftGenerator : public BaseGenerator { void GenTableWriterFields(const FieldDef &field, std::vector *create_body, std::vector *create_header) { - std::string builder_string = ", _ fbb: inout FlatBufferBuilder) { fbb.add("; + std::string builder_string = ", _ fbb: inout FlatBufferBuilder) { "; auto &create_func_body = *create_body; auto &create_func_header = *create_header; auto name = Name(field); @@ -509,7 +509,7 @@ class SwiftGenerator : public BaseGenerator { auto default_value = IsEnum(field.value.type) ? GenEnumDefaultValue(field) : field.value.constant; auto is_enum = IsEnum(field.value.type) ? ".rawValue" : ""; - code_ += "{{VALUETYPE}}" + builder_string + "element: {{VALUENAME}}" + + code_ += "{{VALUETYPE}}" + builder_string + "fbb.add(element: {{VALUENAME}}" + is_enum + ", def: {{CONSTANT}}, at: {{TABLEOFFSET}}.{{OFFSET}}.p) }"; create_func_header.push_back("" + name + ": " + type + " = " + @@ -523,12 +523,22 @@ class SwiftGenerator : public BaseGenerator { code_.SetValue("VALUETYPE", "Bool"); code_.SetValue("CONSTANT", default_value); code_ += "{{VALUETYPE}}" + builder_string + - "element: {{VALUENAME}}, def: {{CONSTANT}}, at: " + "fbb.add(element: {{VALUENAME}}, def: {{CONSTANT}}, at: " "{{TABLEOFFSET}}.{{OFFSET}}.p) }"; create_func_header.push_back(name + ": " + type + " = " + default_value); return; } + if (IsStruct(field.value.type)) { + auto struct_type = "UnsafeMutableRawPointer?"; + auto camel_case_name = "structOf" + MakeCamel(name, true); + create_func_header.push_back(camel_case_name + " " + name + ": " + struct_type + " = nil"); + auto create_struct = "guard let {{VALUENAME}} = {{VALUENAME}} else { return }; fbb.create(struct: {{VALUENAME}}, type: {{VALUETYPE}}.self); "; + auto reader_type = "fbb.add(structOffset: {{TABLEOFFSET}}.{{OFFSET}}.p) }"; + code_ += struct_type + builder_string + create_struct + reader_type; + return; + } + auto offset_type = field.value.type.base_type == BASE_TYPE_STRING ? "Offset" : "Offset"; @@ -544,7 +554,7 @@ class SwiftGenerator : public BaseGenerator { IsStruct(field.value.type) && field.value.type.struct_def->fixed ? "structOffset: {{TABLEOFFSET}}.{{OFFSET}}.p) }" : "offset: {{VALUENAME}}, at: {{TABLEOFFSET}}.{{OFFSET}}.p) }"; - code_ += offset_type + builder_string + reader_type; + code_ += offset_type + builder_string + "fbb.add(" + reader_type; } void GenTableReaderFields(const FieldDef &field) { @@ -869,16 +879,17 @@ class SwiftGenerator : public BaseGenerator { case BASE_TYPE_STRUCT: { if (field.value.type.struct_def && field.value.type.struct_def->fixed) { - std::string struct_builder = - type + ".pack(&builder, obj: &obj." + name + ")"; - unpack_body.push_back("{{STRUCTNAME}}." + body + struct_builder + - builder); + // This is a Struct (IsStruct), not a table. We create UnsafeMutableRawPointer in this case. + std::string code; + GenerateStructArgs(*field.value.type.struct_def, &code, "", "", "$0", true); + code = code.substr(0, code.size() - 2); + code_ += "let __" + name + " = obj." + name + ".map { create" + field.value.type.struct_def->name + "(" + code + ") }"; } else { - unpack_body.push_back("{{STRUCTNAME}}." + body + "__" + name + - builder); code_ += "let __" + name + " = " + type + ".pack(&builder, obj: &obj." + name + ")"; } + unpack_body.push_back("{{STRUCTNAME}}." + body + "__" + name + + builder); break; } case BASE_TYPE_STRING: { diff --git a/tests/FlatBuffers.Test.Swift/Tests/FlatBuffers.Test.SwiftTests/FlatBuffersMonsterWriterTests.swift b/tests/FlatBuffers.Test.Swift/Tests/FlatBuffers.Test.SwiftTests/FlatBuffersMonsterWriterTests.swift index 0d1f9fd82..b25a6760e 100644 --- a/tests/FlatBuffers.Test.Swift/Tests/FlatBuffers.Test.SwiftTests/FlatBuffersMonsterWriterTests.swift +++ b/tests/FlatBuffers.Test.Swift/Tests/FlatBuffers.Test.SwiftTests/FlatBuffersMonsterWriterTests.swift @@ -45,6 +45,27 @@ class FlatBuffersMonsterWriterTests: XCTestCase { readMonster(fb: newBuf) } + func testCreateMonsterUsingCreateMonsterMethodWithNilPos() { + var fbb = FlatBufferBuilder(initialSize: 1) + let name = fbb.create(string: "Frodo") + let monster = Monster.createMonster(&fbb, offsetOfName: name) + fbb.finish(offset: monster) + let newMonster = Monster.getRootAsMonster(bb: fbb.sizedBuffer) + XCTAssertNil(newMonster.pos) + XCTAssertEqual(newMonster.name, "Frodo") + } + + func testCreateMonsterUsingCreateMonsterMethodWithPosX() { + var fbb = FlatBufferBuilder(initialSize: 1) + let pos = MyGame.Example.createVec3(x: 10, test2: .blue) + let name = fbb.create(string: "Barney") + let monster = Monster.createMonster(&fbb, structOfPos: pos, offsetOfName: name) + fbb.finish(offset: monster) + let newMonster = Monster.getRootAsMonster(bb: fbb.sizedBuffer) + XCTAssertEqual(newMonster.pos!.x, 10) + XCTAssertEqual(newMonster.name, "Barney") + } + func testReadMonsterFromUnsafePointerWithoutCopying() { var array: [UInt8] = [48, 0, 0, 0, 77, 79, 78, 83, 0, 0, 0, 0, 36, 0, 72, 0, 40, 0, 0, 0, 38, 0, 32, 0, 0, 0, 28, 0, 0, 0, 27, 0, 20, 0, 16, 0, 12, 0, 4, 0, 0, 0, 0, 0, 0, 0, 11, 0, 36, 0, 0, 0, 164, 0, 0, 0, 0, 0, 0, 1, 60, 0, 0, 0, 68, 0, 0, 0, 76, 0, 0, 0, 0, 0, 0, 1, 88, 0, 0, 0, 120, 0, 0, 0, 0, 0, 80, 0, 0, 0, 128, 63, 0, 0, 0, 64, 0, 0, 64, 64, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 8, 64, 2, 0, 5, 0, 6, 0, 0, 0, 2, 0, 0, 0, 64, 0, 0, 0, 48, 0, 0, 0, 2, 0, 0, 0, 30, 0, 40, 0, 10, 0, 20, 0, 152, 255, 255, 255, 4, 0, 0, 0, 4, 0, 0, 0, 70, 114, 101, 100, 0, 0, 0, 0, 5, 0, 0, 0, 0, 1, 2, 3, 4, 0, 0, 0, 5, 0, 0, 0, 116, 101, 115, 116, 50, 0, 0, 0, 5, 0, 0, 0, 116, 101, 115, 116, 49, 0, 0, 0, 9, 0, 0, 0, 77, 121, 77, 111, 110, 115, 116, 101, 114, 0, 0, 0, 3, 0, 0, 0, 20, 0, 0, 0, 36, 0, 0, 0, 4, 0, 0, 0, 240, 255, 255, 255, 32, 0, 0, 0, 248, 255, 255, 255, 36, 0, 0, 0, 12, 0, 8, 0, 0, 0, 0, 0, 0, 0, 4, 0, 12, 0, 0, 0, 28, 0, 0, 0, 5, 0, 0, 0, 87, 105, 108, 109, 97, 0, 0, 0, 6, 0, 0, 0, 66, 97, 114, 110, 101, 121, 0, 0, 5, 0, 0, 0, 70, 114, 111, 100, 111, 0, 0, 0] let unpacked = array.withUnsafeMutableBytes { (memory) -> MyGame.Example.MonsterT in @@ -100,10 +121,10 @@ class FlatBuffersMonsterWriterTests: XCTestCase { type: Test.self) let stringTestVector = fbb.createVector(ofOffsets: [test1, test2]) - + + let posStruct = MyGame.Example.createVec3(x: 1, y: 2, z: 3, test1: 3, test2: .green, test3a: 5, test3b: 6) let mStart = Monster.startMonster(&fbb) - let posOffset = fbb.create(struct: MyGame.Example.createVec3(x: 1, y: 2, z: 3, test1: 3, test2: .green, test3a: 5, test3b: 6), type: Vec3.self) - Monster.add(pos: posOffset, &fbb) + Monster.add(pos: posStruct, &fbb) Monster.add(hp: 80, &fbb) Monster.add(name: str, &fbb) Monster.addVectorOf(inventory: inv, &fbb) diff --git a/tests/FlatBuffers.Test.Swift/Tests/FlatBuffers.Test.SwiftTests/XCTestManifests.swift b/tests/FlatBuffers.Test.Swift/Tests/FlatBuffers.Test.SwiftTests/XCTestManifests.swift index f2adc4f52..b840c0bdb 100644 --- a/tests/FlatBuffers.Test.Swift/Tests/FlatBuffers.Test.SwiftTests/XCTestManifests.swift +++ b/tests/FlatBuffers.Test.Swift/Tests/FlatBuffers.Test.SwiftTests/XCTestManifests.swift @@ -20,6 +20,8 @@ extension FlatBuffersMonsterWriterTests { ("testCreateMonster", testCreateMonster), ("testCreateMonsterPrefixed", testCreateMonsterPrefixed), ("testCreateMonsterResizedBuffer", testCreateMonsterResizedBuffer), + ("testCreateMonsterUsingCreateMonsterMethodWithNilPos", testCreateMonsterUsingCreateMonsterMethodWithNilPos), + ("testCreateMonsterUsingCreateMonsterMethodWithPosX", testCreateMonsterUsingCreateMonsterMethodWithPosX), ("testData", testData), ("testReadFromOtherLanguages", testReadFromOtherLanguages), ("testReadMonsterFromUnsafePointerWithoutCopying", testReadMonsterFromUnsafePointerWithoutCopying), diff --git a/tests/FlatBuffers.Test.Swift/Tests/FlatBuffers.Test.SwiftTests/monster_test_generated.swift b/tests/FlatBuffers.Test.Swift/Tests/FlatBuffers.Test.SwiftTests/monster_test_generated.swift index 7f3a8c4dd..f30689f2d 100644 --- a/tests/FlatBuffers.Test.Swift/Tests/FlatBuffers.Test.SwiftTests/monster_test_generated.swift +++ b/tests/FlatBuffers.Test.Swift/Tests/FlatBuffers.Test.SwiftTests/monster_test_generated.swift @@ -825,7 +825,7 @@ public struct Monster: FlatBufferObject { public var signedEnum: MyGame.Example.Race { let o = _accessor.offset(VTOFFSET.signedEnum.v); return o == 0 ? .none_ : MyGame.Example.Race(rawValue: _accessor.readBuffer(of: Int8.self, at: o)) ?? .none_ } @discardableResult public func mutate(signedEnum: MyGame.Example.Race) -> Bool {let o = _accessor.offset(VTOFFSET.signedEnum.v); return _accessor.mutate(signedEnum.rawValue, index: o) } public static func startMonster(_ fbb: inout FlatBufferBuilder) -> UOffset { fbb.startTable(with: 49) } - public static func add(pos: Offset, _ fbb: inout FlatBufferBuilder) { fbb.add(structOffset: VTOFFSET.pos.p) } + public static func add(pos: UnsafeMutableRawPointer?, _ fbb: inout FlatBufferBuilder) { guard let pos = pos else { return }; fbb.create(struct: pos, type: MyGame.Example.Vec3.self); fbb.add(structOffset: VTOFFSET.pos.p) } public static func add(mana: Int16, _ fbb: inout FlatBufferBuilder) { fbb.add(element: mana, def: 150, at: VTOFFSET.mana.p) } public static func add(hp: Int16, _ fbb: inout FlatBufferBuilder) { fbb.add(element: hp, def: 100, at: VTOFFSET.hp.p) } public static func add(name: Offset, _ fbb: inout FlatBufferBuilder) { fbb.add(offset: name, at: VTOFFSET.name.p) } @@ -875,7 +875,7 @@ public struct Monster: FlatBufferObject { public static func add(signedEnum: MyGame.Example.Race, _ fbb: inout FlatBufferBuilder) { fbb.add(element: signedEnum.rawValue, def: -1, at: VTOFFSET.signedEnum.p) } public static func endMonster(_ fbb: inout FlatBufferBuilder, start: UOffset) -> Offset { let end = Offset(offset: fbb.endTable(at: start)); fbb.require(table: end, fields: [10]); return end } public static func createMonster(_ fbb: inout FlatBufferBuilder, - offsetOfPos pos: Offset = Offset(), + structOfPos pos: UnsafeMutableRawPointer? = nil, mana: Int16 = 150, hp: Int16 = 100, offsetOfName name: Offset = Offset(), @@ -1010,6 +1010,7 @@ public struct Monster: FlatBufferObject { } public static func pack(_ builder: inout FlatBufferBuilder, obj: inout MonsterT) -> Offset { + let __pos = obj.pos.map { createVec3(x: $0.x, y: $0.y, z: $0.z, test1: $0.test1, test2: $0.test2, test3a: $0.test3.a, test3b: $0.test3.b) } let __name = builder.create(string: obj.name) let __inventory = builder.createVector(obj.inventory) let __test = obj.test?.pack(builder: &builder) ?? Offset() @@ -1063,7 +1064,7 @@ public struct Monster: FlatBufferObject { let __anyAmbiguous = obj.anyAmbiguous?.pack(builder: &builder) ?? Offset() let __vectorOfEnums = builder.createVector(obj.vectorOfEnums) let __root = Monster.startMonster(&builder) - Monster.add(pos: MyGame.Example.Vec3.pack(&builder, obj: &obj.pos), &builder) + Monster.add(pos: __pos, &builder) Monster.add(mana: obj.mana, &builder) Monster.add(hp: obj.hp, &builder) Monster.add(name: __name, &builder)