Skip to content

Commit 7b6e56c

Browse files
peter-soospeter-soos
committed
[Python] Fix invalid generated code for packing structs with array of struct fields
1 parent dc92173 commit 7b6e56c

13 files changed

Lines changed: 235 additions & 63 deletions

File tree

‎goldens/py/flatbuffers/goldens/Universe.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ def UniverseCreateGalaxiesVector(builder, data):
8484
return builder.CreateVectorOfTables(data)
8585

8686
def CreateGalaxiesVector(builder, data):
87-
UniverseCreateGalaxiesVector(builder, data)
87+
return UniverseCreateGalaxiesVector(builder, data)
8888

8989
def UniverseEnd(builder):
9090
return builder.EndObject()

‎goldens/swift/basic_generated.swift‎

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,13 @@ public struct flatbuffers_goldens_Galaxy: FlatBufferTable, FlatbuffersVectorInit
1717
private init(_ t: Table) { _accessor = t }
1818
public init(_ bb: ByteBuffer, o: Int32) { _accessor = Table(bb: bb, position: o) }
1919

20-
private enum VTOFFSET: VOffset {
21-
case numStars = 4
22-
var v: Int32 { Int32(self.rawValue) }
23-
var p: VOffset { self.rawValue }
20+
private struct VT {
21+
static let numStars: VOffset = 4
2422
}
2523

26-
public var numStars: Int64 { let o = _accessor.offset(VTOFFSET.numStars.v); return o == 0 ? 0 : _accessor.readBuffer(of: Int64.self, at: o) }
24+
public var numStars: Int64 { let o = _accessor.offset(VT.numStars); return o == 0 ? 0 : _accessor.readBuffer(of: Int64.self, at: o) }
2725
public static func startGalaxy(_ fbb: inout FlatBufferBuilder) -> UOffset { fbb.startTable(with: 1) }
28-
public static func add(numStars: Int64, _ fbb: inout FlatBufferBuilder) { fbb.add(element: numStars, def: 0, at: VTOFFSET.numStars.p) }
26+
public static func add(numStars: Int64, _ fbb: inout FlatBufferBuilder) { fbb.add(element: numStars, def: 0, at: VT.numStars) }
2927
public static func endGalaxy(_ fbb: inout FlatBufferBuilder, start: UOffset) -> Offset { let end = Offset(offset: fbb.endTable(at: start)); return end }
3028
public static func createGalaxy(
3129
_ fbb: inout FlatBufferBuilder,
@@ -38,7 +36,7 @@ public struct flatbuffers_goldens_Galaxy: FlatBufferTable, FlatbuffersVectorInit
3836

3937
public static func verify<T>(_ verifier: inout Verifier, at position: Int, of type: T.Type) throws where T: Verifiable {
4038
var _v = try verifier.visitTable(at: position)
41-
try _v.visit(field: VTOFFSET.numStars.p, fieldName: "numStars", required: false, type: Int64.self)
39+
try _v.visit(field: VT.numStars, fieldName: "numStars", required: false, type: Int64.self)
4240
_v.finish()
4341
}
4442
}
@@ -52,18 +50,16 @@ public struct flatbuffers_goldens_Universe: FlatBufferTable, FlatbuffersVectorIn
5250
private init(_ t: Table) { _accessor = t }
5351
public init(_ bb: ByteBuffer, o: Int32) { _accessor = Table(bb: bb, position: o) }
5452

55-
private enum VTOFFSET: VOffset {
56-
case age = 4
57-
case galaxies = 6
58-
var v: Int32 { Int32(self.rawValue) }
59-
var p: VOffset { self.rawValue }
53+
private struct VT {
54+
static let age: VOffset = 4
55+
static let galaxies: VOffset = 6
6056
}
6157

62-
public var age: Double { let o = _accessor.offset(VTOFFSET.age.v); return o == 0 ? 0.0 : _accessor.readBuffer(of: Double.self, at: o) }
63-
public var galaxies: FlatbufferVector<flatbuffers_goldens_Galaxy> { return _accessor.vector(at: VTOFFSET.galaxies.v, byteSize: 4) }
58+
public var age: Double { let o = _accessor.offset(VT.age); return o == 0 ? 0.0 : _accessor.readBuffer(of: Double.self, at: o) }
59+
public var galaxies: FlatbufferVector<flatbuffers_goldens_Galaxy> { return _accessor.vector(at: VT.galaxies, byteSize: 4) }
6460
public static func startUniverse(_ fbb: inout FlatBufferBuilder) -> UOffset { fbb.startTable(with: 2) }
65-
public static func add(age: Double, _ fbb: inout FlatBufferBuilder) { fbb.add(element: age, def: 0.0, at: VTOFFSET.age.p) }
66-
public static func addVectorOf(galaxies: Offset, _ fbb: inout FlatBufferBuilder) { fbb.add(offset: galaxies, at: VTOFFSET.galaxies.p) }
61+
public static func add(age: Double, _ fbb: inout FlatBufferBuilder) { fbb.add(element: age, def: 0.0, at: VT.age) }
62+
public static func addVectorOf(galaxies: Offset, _ fbb: inout FlatBufferBuilder) { fbb.add(offset: galaxies, at: VT.galaxies) }
6763
public static func endUniverse(_ fbb: inout FlatBufferBuilder, start: UOffset) -> Offset { let end = Offset(offset: fbb.endTable(at: start)); return end }
6864
public static func createUniverse(
6965
_ fbb: inout FlatBufferBuilder,
@@ -78,8 +74,8 @@ public struct flatbuffers_goldens_Universe: FlatBufferTable, FlatbuffersVectorIn
7874

7975
public static func verify<T>(_ verifier: inout Verifier, at position: Int, of type: T.Type) throws where T: Verifiable {
8076
var _v = try verifier.visitTable(at: position)
81-
try _v.visit(field: VTOFFSET.age.p, fieldName: "age", required: false, type: Double.self)
82-
try _v.visit(field: VTOFFSET.galaxies.p, fieldName: "galaxies", required: false, type: ForwardOffset<Vector<ForwardOffset<flatbuffers_goldens_Galaxy>, flatbuffers_goldens_Galaxy>>.self)
77+
try _v.visit(field: VT.age, fieldName: "age", required: false, type: Double.self)
78+
try _v.visit(field: VT.galaxies, fieldName: "galaxies", required: false, type: ForwardOffset<Vector<ForwardOffset<flatbuffers_goldens_Galaxy>, flatbuffers_goldens_Galaxy>>.self)
8379
_v.finish()
8480
}
8581
}

‎src/idl_gen_python.cpp‎

Lines changed: 51 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1298,7 +1298,7 @@ class PythonGenerator : public BaseGenerator {
12981298
code += "):\n";
12991299
}
13001300

1301-
// Recursively generate struct construction statements and instert manual
1301+
// Recursively generate struct construction statements and insert manual
13021302
// padding.
13031303
void StructBuilderBody(const StructDef& struct_def, const char* nameprefix,
13041304
std::string* code_ptr, size_t index = 0,
@@ -1326,7 +1326,7 @@ class PythonGenerator : public BaseGenerator {
13261326
if (IsArray(field_type)) {
13271327
code += indent + " for " + index_var + " in range(";
13281328
code += NumToString(field_type.fixed_length);
1329-
code += " , 0, -1):\n";
1329+
code += ", 0, -1):\n";
13301330
in_array = true;
13311331
}
13321332
if (IsStruct(type)) {
@@ -2303,21 +2303,59 @@ class PythonGenerator : public BaseGenerator {
23032303
code_base += "\n";
23042304
}
23052305

2306+
void GenPackForStructBody(const StructDef &struct_def, const char *nameprefix,
2307+
std::string *code_ptr, const size_t depth = 0) const {
2308+
auto &code = *code_ptr;
2309+
const std::string indent((depth + 2) * 4, ' ');
2310+
code +=
2311+
indent + "builder.Prep(" + NumToString(struct_def.minalign) + ", ";
2312+
code += NumToString(struct_def.bytesize) + ")\n";
2313+
for (auto it = struct_def.fields.vec.rbegin();
2314+
it != struct_def.fields.vec.rend(); ++it) {
2315+
auto& field = **it;
2316+
const auto& field_type = field.value.type;
2317+
if (field.padding) {
2318+
code +=
2319+
indent + "builder.Pad(" + NumToString(field.padding) + ")\n";
2320+
}
2321+
if (IsStruct(field_type)) {
2322+
GenPackForStructBody(
2323+
*field_type.struct_def,
2324+
(nameprefix + (namer_.Field(field) + ".")).c_str(),
2325+
code_ptr, depth);
2326+
} else {
2327+
std::string namesuffix;
2328+
if (IsArray(field_type)) {
2329+
const auto index_var = "_idx" + NumToString(depth);
2330+
code += indent + "for " + index_var + " in range(";
2331+
code += NumToString(field_type.fixed_length);
2332+
code += ", 0, -1):\n";
2333+
namesuffix = "[" + index_var + "-1]";
2334+
}
2335+
const auto& type =
2336+
IsArray(field_type) ? field_type.VectorType() : field_type;
2337+
if (IsStruct(type)) {
2338+
GenPackForStructBody(
2339+
*field_type.struct_def,
2340+
(nameprefix + namer_.Field(field) + namesuffix + ".").c_str(),
2341+
code_ptr, depth + 1);
2342+
} else {
2343+
code += IsArray(field_type) ? " " : "";
2344+
code += indent + "builder.Prepend" + GenMethod(field) + "(";
2345+
code += nameprefix + namer_.Variable(field) + namesuffix;
2346+
code += ")\n";
2347+
}
2348+
}
2349+
}
2350+
}
2351+
23062352
void GenPackForStruct(const StructDef& struct_def,
23072353
std::string* code_ptr) const {
23082354
auto& code = *code_ptr;
2309-
const auto struct_fn = namer_.Function(struct_def);
2310-
23112355
GenReceiverForObjectAPI(struct_def, code_ptr);
2312-
code += "Pack(self, builder):";
2313-
code += GenIndents(2) + "return Create" + struct_fn + "(builder";
2314-
2315-
StructBuilderArgs(struct_def,
2316-
/* nameprefix = */ "self.",
2317-
/* namesuffix = */ "",
2318-
/* has_field_name = */ true,
2319-
/* fieldname_suffix = */ ".", code_ptr);
2320-
code += ")\n";
2356+
code += "Pack(self, builder):\n";
2357+
GenPackForStructBody(struct_def, "self.", code_ptr);
2358+
code += " return builder.Offset()\n";
23212359
}
23222360

23232361
void GenPackForStructVectorField(const StructDef& struct_def,

‎tests/MyGame/Example/Ability.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,4 +66,7 @@ def _UnPack(self, ability):
6666

6767
# AbilityT
6868
def Pack(self, builder):
69-
return CreateAbility(builder, self.id, self.distance)
69+
builder.Prep(4, 8)
70+
builder.PrependUint32(self.distance)
71+
builder.PrependUint32(self.id)
72+
return builder.Offset()

‎tests/MyGame/Example/ArrayStruct.py‎

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -84,23 +84,23 @@ def FIsNone(self) -> bool:
8484

8585
def CreateArrayStruct(builder, a, b, c, d_a, d_b, d_c, d_d, e, f):
8686
builder.Prep(8, 160)
87-
for _idx0 in range(2 , 0, -1):
87+
for _idx0 in range(2, 0, -1):
8888
builder.PrependInt64(f[_idx0-1])
8989
builder.Pad(4)
9090
builder.PrependInt32(e)
91-
for _idx0 in range(2 , 0, -1):
91+
for _idx0 in range(2, 0, -1):
9292
builder.Prep(8, 32)
93-
for _idx1 in range(2 , 0, -1):
93+
for _idx1 in range(2, 0, -1):
9494
builder.PrependInt64(d_d[_idx0-1][_idx1-1])
9595
builder.Pad(5)
96-
for _idx1 in range(2 , 0, -1):
96+
for _idx1 in range(2, 0, -1):
9797
builder.PrependInt8(d_c[_idx0-1][_idx1-1])
9898
builder.PrependInt8(d_b[_idx0-1])
99-
for _idx1 in range(2 , 0, -1):
99+
for _idx1 in range(2, 0, -1):
100100
builder.PrependInt32(d_a[_idx0-1][_idx1-1])
101101
builder.Pad(7)
102102
builder.PrependInt8(c)
103-
for _idx0 in range(15 , 0, -1):
103+
for _idx0 in range(15, 0, -1):
104104
builder.PrependInt32(b[_idx0-1])
105105
builder.PrependFloat32(a)
106106
return builder.Offset()
@@ -179,4 +179,24 @@ def _UnPack(self, arrayStruct):
179179

180180
# ArrayStructT
181181
def Pack(self, builder):
182-
return CreateArrayStruct(builder, self.a, self.b, self.c, self.d.a, self.d.b, self.d.c, self.d.d, self.e, self.f)
182+
builder.Prep(8, 160)
183+
for _idx0 in range(2, 0, -1):
184+
builder.PrependInt64(self.f[_idx0-1])
185+
builder.Pad(4)
186+
builder.PrependInt32(self.e)
187+
for _idx0 in range(2, 0, -1):
188+
builder.Prep(8, 32)
189+
for _idx1 in range(2, 0, -1):
190+
builder.PrependInt64(self.d[_idx0-1].d[_idx1-1])
191+
builder.Pad(5)
192+
for _idx1 in range(2, 0, -1):
193+
builder.PrependInt8(self.d[_idx0-1].c[_idx1-1])
194+
builder.PrependInt8(self.d[_idx0-1].b)
195+
for _idx1 in range(2, 0, -1):
196+
builder.PrependInt32(self.d[_idx0-1].a[_idx1-1])
197+
builder.Pad(7)
198+
builder.PrependInt8(self.c)
199+
for _idx0 in range(15, 0, -1):
200+
builder.PrependInt32(self.b[_idx0-1])
201+
builder.PrependFloat32(self.a)
202+
return builder.Offset()

‎tests/MyGame/Example/LargeArrayStruct.py‎

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -120,23 +120,23 @@ def HIsNone(self) -> bool:
120120

121121
def CreateLargeArrayStruct(builder, d, e, f, g_a, g_b, g_c, g_d, h):
122122
builder.Prep(8, 2496)
123-
for _idx0 in range(64 , 0, -1):
123+
for _idx0 in range(64, 0, -1):
124124
builder.PrependInt8(h[_idx0-1])
125-
for _idx0 in range(64 , 0, -1):
125+
for _idx0 in range(64, 0, -1):
126126
builder.Prep(8, 32)
127-
for _idx1 in range(2 , 0, -1):
127+
for _idx1 in range(2, 0, -1):
128128
builder.PrependInt64(g_d[_idx0-1][_idx1-1])
129129
builder.Pad(5)
130-
for _idx1 in range(2 , 0, -1):
130+
for _idx1 in range(2, 0, -1):
131131
builder.PrependInt8(g_c[_idx0-1][_idx1-1])
132132
builder.PrependInt8(g_b[_idx0-1])
133-
for _idx1 in range(2 , 0, -1):
133+
for _idx1 in range(2, 0, -1):
134134
builder.PrependInt32(g_a[_idx0-1][_idx1-1])
135-
for _idx0 in range(64 , 0, -1):
135+
for _idx0 in range(64, 0, -1):
136136
builder.PrependBool(f[_idx0-1])
137-
for _idx0 in range(64 , 0, -1):
137+
for _idx0 in range(64, 0, -1):
138138
builder.PrependFloat32(e[_idx0-1])
139-
for _idx0 in range(64 , 0, -1):
139+
for _idx0 in range(64, 0, -1):
140140
builder.PrependUint8(d[_idx0-1])
141141
return builder.Offset()
142142

@@ -223,4 +223,23 @@ def _UnPack(self, largeArrayStruct):
223223

224224
# LargeArrayStructT
225225
def Pack(self, builder):
226-
return CreateLargeArrayStruct(builder, self.d, self.e, self.f, self.g.a, self.g.b, self.g.c, self.g.d, self.h)
226+
builder.Prep(8, 2496)
227+
for _idx0 in range(64, 0, -1):
228+
builder.PrependInt8(self.h[_idx0-1])
229+
for _idx0 in range(64, 0, -1):
230+
builder.Prep(8, 32)
231+
for _idx1 in range(2, 0, -1):
232+
builder.PrependInt64(self.g[_idx0-1].d[_idx1-1])
233+
builder.Pad(5)
234+
for _idx1 in range(2, 0, -1):
235+
builder.PrependInt8(self.g[_idx0-1].c[_idx1-1])
236+
builder.PrependInt8(self.g[_idx0-1].b)
237+
for _idx1 in range(2, 0, -1):
238+
builder.PrependInt32(self.g[_idx0-1].a[_idx1-1])
239+
for _idx0 in range(64, 0, -1):
240+
builder.PrependBool(self.f[_idx0-1])
241+
for _idx0 in range(64, 0, -1):
242+
builder.PrependFloat32(self.e[_idx0-1])
243+
for _idx0 in range(64, 0, -1):
244+
builder.PrependUint8(self.d[_idx0-1])
245+
return builder.Offset()

‎tests/MyGame/Example/NestedStruct.py‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,13 +86,13 @@ def DIsNone(self) -> bool:
8686

8787
def CreateNestedStruct(builder, a, b, c, d):
8888
builder.Prep(8, 32)
89-
for _idx0 in range(2 , 0, -1):
89+
for _idx0 in range(2, 0, -1):
9090
builder.PrependInt64(d[_idx0-1])
9191
builder.Pad(5)
92-
for _idx0 in range(2 , 0, -1):
92+
for _idx0 in range(2, 0, -1):
9393
builder.PrependInt8(c[_idx0-1])
9494
builder.PrependInt8(b)
95-
for _idx0 in range(2 , 0, -1):
95+
for _idx0 in range(2, 0, -1):
9696
builder.PrependInt32(a[_idx0-1])
9797
return builder.Offset()
9898

@@ -162,4 +162,13 @@ def _UnPack(self, nestedStruct):
162162

163163
# NestedStructT
164164
def Pack(self, builder):
165-
return CreateNestedStruct(builder, self.a, self.b, self.c, self.d)
165+
builder.Prep(8, 32)
166+
for _idx0 in range(2, 0, -1):
167+
builder.PrependInt64(self.d[_idx0-1])
168+
builder.Pad(5)
169+
for _idx0 in range(2, 0, -1):
170+
builder.PrependInt8(self.c[_idx0-1])
171+
builder.PrependInt8(self.b)
172+
for _idx0 in range(2, 0, -1):
173+
builder.PrependInt32(self.a[_idx0-1])
174+
return builder.Offset()

‎tests/MyGame/Example/NestedUnion/Test.py‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,4 +68,8 @@ def _UnPack(self, test):
6868

6969
# TestT
7070
def Pack(self, builder):
71-
return CreateTest(builder, self.a, self.b)
71+
builder.Prep(2, 4)
72+
builder.Pad(1)
73+
builder.PrependInt8(self.b)
74+
builder.PrependInt16(self.a)
75+
return builder.Offset()

‎tests/MyGame/Example/StructOfStructs.py‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,4 +97,15 @@ def _UnPack(self, structOfStructs):
9797

9898
# StructOfStructsT
9999
def Pack(self, builder):
100-
return CreateStructOfStructs(builder, self.a.id, self.a.distance, self.b.a, self.b.b, self.c.id, self.c.distance)
100+
builder.Prep(4, 20)
101+
builder.Prep(4, 8)
102+
builder.PrependUint32(self.c.distance)
103+
builder.PrependUint32(self.c.id)
104+
builder.Prep(2, 4)
105+
builder.Pad(1)
106+
builder.PrependInt8(self.b.b)
107+
builder.PrependInt16(self.b.a)
108+
builder.Prep(4, 8)
109+
builder.PrependUint32(self.a.distance)
110+
builder.PrependUint32(self.a.id)
111+
return builder.Offset()

‎tests/MyGame/Example/StructOfStructsOfStructs.py‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,4 +79,16 @@ def _UnPack(self, structOfStructsOfStructs):
7979

8080
# StructOfStructsOfStructsT
8181
def Pack(self, builder):
82-
return CreateStructOfStructsOfStructs(builder, self.a.a.id, self.a.a.distance, self.a.b.a, self.a.b.b, self.a.c.id, self.a.c.distance)
82+
builder.Prep(4, 20)
83+
builder.Prep(4, 20)
84+
builder.Prep(4, 8)
85+
builder.PrependUint32(self.a.c.distance)
86+
builder.PrependUint32(self.a.c.id)
87+
builder.Prep(2, 4)
88+
builder.Pad(1)
89+
builder.PrependInt8(self.a.b.b)
90+
builder.PrependInt16(self.a.b.a)
91+
builder.Prep(4, 8)
92+
builder.PrependUint32(self.a.a.distance)
93+
builder.PrependUint32(self.a.a.id)
94+
return builder.Offset()

0 commit comments

Comments
 (0)