Skip to content

Default-empty vectors of enums #6505

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
CasperN merged 14 commits into from
Apr 6, 2021
Merged

Default-empty vectors of enums #6505

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
c140c71
disable clippy
Mar 1, 2021
b03e913
Merge branch 'master' of https://github.com/google/flatbuffers into c…
Mar 2, 2021
f05f2d8
Vector of enum default
Mar 2, 2021
e9d142f
swift and tests
Mar 7, 2021
27f0191
Merge branch 'master' of https://github.com/google/flatbuffers into tst
Mar 7, 2021
6801ab0
git clang format
Mar 7, 2021
b9c8dd6
Rewrite enum parser checks
Mar 8, 2021
4c27a59
Merge branch 'master' of https://github.com/google/flatbuffers into tst
Mar 15, 2021
6b8380f
Remove Voids from more_defaults
Mar 15, 2021
5d3f849
vector enum swift
Mar 15, 2021
372aac4
remove vector accessor from swift
Mar 17, 2021
6b5ae47
Merge branch 'master' of https://github.com/google/flatbuffers into tst
Apr 2, 2021
ec5b670
clang format
Apr 2, 2021
88cf2e9
Merge branch 'master' of https://github.com/google/flatbuffers into tst
Apr 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/idl_gen_swift.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -847,7 +847,7 @@ class SwiftGenerator : public BaseGenerator {
code_.SetValue("ENUM_NAME", NameWrappedInNameSpace(enum_def));
code_.SetValue("BASE_TYPE", GenTypeBasic(enum_def.underlying_type, false));
GenComment(enum_def.doc_comment);
code_ += "{{ACCESS_TYPE}} enum {{ENUM_NAME}}: {{BASE_TYPE}}, Enum { ";
code_ += "{{ACCESS_TYPE}} enum {{ENUM_NAME}}: {{BASE_TYPE}}, Enum {";
Indent();
code_ += "{{ACCESS_TYPE}} typealias T = {{BASE_TYPE}}";
code_ +=
Expand Down Expand Up @@ -1471,7 +1471,9 @@ class SwiftGenerator : public BaseGenerator {
auto &value = field.value;
FLATBUFFERS_ASSERT(value.type.enum_def);
auto &enum_def = *value.type.enum_def;
auto enum_val = enum_def.FindByValue(value.constant);
// Vector of enum defaults are always "[]" which never works.
const std::string constant = IsVector(value.type) ? "0" : value.constant;
auto enum_val = enum_def.FindByValue(constant);
std::string name;
if (enum_val) {
name = Name(*enum_val);
Expand Down
20 changes: 8 additions & 12 deletions src/idl_parser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -912,18 +912,14 @@ CheckedError Parser::ParseField(StructDef &struct_def) {
FLATBUFFERS_ASSERT(IsInteger(type.base_type) ||
(type.base_type == BASE_TYPE_UNION) || IsVector(type) ||
IsArray(type));
if (IsVector(type)) {
// Vector can't use initialization list.
FLATBUFFERS_ASSERT(field->value.constant == "0");
} else {
// All unions should have the NONE ("0") enum value.
auto in_enum = field->IsOptional() ||
type.enum_def->attributes.Lookup("bit_flags") ||
type.enum_def->FindByValue(field->value.constant);
if (false == in_enum)
return Error("default value of " + field->value.constant +
" for field " + name + " is not part of enum " +
type.enum_def->name);

@vglavnyy vglavnyy Mar 7, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current if(IsVector(type))-else is more readable than proposed solution. The else branch covers only scalars or enums.
With the new solution, any high-level error (like = [1,2,3]) will be ignored.

Instead of FLATBUFFERS_ASSERT just before if(IsVector(type)) it might be better to write full decision tree including IsArray, for example:

if (type.base_type == BASE_TYPE_UNION) {
  if(field->value.constant != "0") return Error("");
} else if (IsInteger(type.base_type)) {
  ...
} else if (IsVector(type)) {
  FLATBUFFERS_ASSERT(field->value.constant == "0" || field->value.constant == "[]");
} else if (IsArray(type)) {
  FLATBUFFERS_ASSERT(field->value.constant == "0");
} else {
  FLATBUFFERS_ASSERT(false && "unexpected type");
}

@CasperN CasperN Mar 8, 2021

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I refactored away the asserts and used a chain of if/else. PTAL

@vglavnyy vglavnyy Mar 9, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good.

// Optional and bitflags enums may have default constants that are not
// their specified variants.

Optional fields might have only null initializer to indicate optionality, so it never can be an enum's variant.

// Ensure defautl values are present in the enum if needed.
// All unions should have the NONE ("0") enum value.
const bool needs_default = !(IsVector(type) || field->IsOptional() ||
type.enum_def->attributes.Lookup("bit_flags"));
if (needs_default && !type.enum_def->FindByValue(field->value.constant)) {
return Error("default value of " + field->value.constant + " for field " +
name + " is not part of enum " + type.enum_def->name);
}
}

Expand Down
12 changes: 12 additions & 0 deletions tests/FlatBuffers.Test.Swift/Tests/FlatBuffers.Test.SwiftTests/FlatbuffersMoreDefaults.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,14 @@ class FlatBuffersMoreDefaults: XCTestCase {
XCTAssertEqual(defaults.someString, "some")
XCTAssertEqual(defaults.ints, [])
XCTAssertEqual(defaults.floats, [])
XCTAssertEqual(defaults.abcs, [])
XCTAssertEqual(defaults.voids, [])
XCTAssertEqual(defaults.bools, [])
XCTAssertEqual(defaults.intsCount, 0)
XCTAssertEqual(defaults.floatsCount, 0)
XCTAssertEqual(defaults.abcsCount, 0)
XCTAssertEqual(defaults.voidsCount, 0)
XCTAssertEqual(defaults.boolsCount, 0)
}

func testFlatbuffersObjectAPI() {
Expand All @@ -40,6 +46,9 @@ class FlatBuffersMoreDefaults: XCTestCase {
XCTAssertEqual(defaults.someString, "some")
XCTAssertEqual(defaults.ints, [])
XCTAssertEqual(defaults.floats, [])
XCTAssertEqual(defaults.abcs, [])
XCTAssertEqual(defaults.voids, [])
XCTAssertEqual(defaults.bools, [])

let buffer = defaults.serialize(builder: &fbb, type: MoreDefaults.self)
let fDefaults = MoreDefaults.getRootAsMoreDefaults(bb: buffer)
Expand All @@ -49,5 +58,8 @@ class FlatBuffersMoreDefaults: XCTestCase {
XCTAssertEqual(fDefaults.floats, [])
XCTAssertEqual(fDefaults.intsCount, 0)
XCTAssertEqual(fDefaults.floatsCount, 0)
XCTAssertEqual(fDefaults.abcsCount, 0)
XCTAssertEqual(fDefaults.voidsCount, 0)
XCTAssertEqual(fDefaults.boolsCount, 0)
}
}
71 changes: 69 additions & 2 deletions tests/FlatBuffers.Test.Swift/Tests/FlatBuffers.Test.SwiftTests/more_defaults_generated.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,30 @@

import FlatBuffers

public enum ABC: Int32, Enum {
public typealias T = Int32
public static var byteSize: Int { return MemoryLayout<Int32>.size }
public var value: Int32 { return self.rawValue }
case a = 0
case b = 1
case c = 2


public static var max: ABC { return .c }
public static var min: ABC { return .a }
}

public enum Void: Int32, Enum {
public typealias T = Int32
public static var byteSize: Int { return MemoryLayout<Int32>.size }
public var value: Int32 { return self.rawValue }
case none_ = 0


public static var max: Void { return .none_ }
public static var min: Void { return .none_ }
}

public struct MoreDefaults: FlatBufferObject, ObjectAPIPacker {

static func validateVersion() { FlatBuffersVersion_1_12_0() }
Expand All @@ -20,6 +44,9 @@ public struct MoreDefaults: FlatBufferObject, ObjectAPIPacker {
case floats = 6
case emptyString = 8
case someString = 10
case abcs = 12
case voids = 14
case bools = 16
var v: Int32 { Int32(self.rawValue) }
var p: VOffset { self.rawValue }
}
Expand All @@ -34,24 +61,40 @@ public struct MoreDefaults: FlatBufferObject, ObjectAPIPacker {
public var emptyStringSegmentArray: [UInt8]? { return _accessor.getVector(at: VTOFFSET.emptyString.v) }
public var someString: String? { let o = _accessor.offset(VTOFFSET.someString.v); return o == 0 ? "some" : _accessor.string(at: o) }
public var someStringSegmentArray: [UInt8]? { return _accessor.getVector(at: VTOFFSET.someString.v) }
public static func startMoreDefaults(_ fbb: inout FlatBufferBuilder) -> UOffset { fbb.startTable(with: 4) }
public var abcsCount: Int32 { let o = _accessor.offset(VTOFFSET.abcs.v); return o == 0 ? 0 : _accessor.vector(count: o) }
public func abcs(at index: Int32) -> ABC? { let o = _accessor.offset(VTOFFSET.abcs.v); return o == 0 ? ABC.a : ABC(rawValue: _accessor.directRead(of: Int32.self, offset: _accessor.vector(at: o) + index * 4)) }
public var voidsCount: Int32 { let o = _accessor.offset(VTOFFSET.voids.v); return o == 0 ? 0 : _accessor.vector(count: o) }
public func voids(at index: Int32) -> Void? { let o = _accessor.offset(VTOFFSET.voids.v); return o == 0 ? Void.none_ : Void(rawValue: _accessor.directRead(of: Int32.self, offset: _accessor.vector(at: o) + index * 4)) }
public var boolsCount: Int32 { let o = _accessor.offset(VTOFFSET.bools.v); return o == 0 ? 0 : _accessor.vector(count: o) }
public func bools(at index: Int32) -> Bool { let o = _accessor.offset(VTOFFSET.bools.v); return o == 0 ? true : _accessor.directRead(of: Bool.self, offset: _accessor.vector(at: o) + index * 1) }
public var bools: [Bool] { return _accessor.getVector(at: VTOFFSET.bools.v) ?? [] }
public static func startMoreDefaults(_ fbb: inout FlatBufferBuilder) -> UOffset { fbb.startTable(with: 7) }
public static func addVectorOf(ints: Offset<UOffset>, _ fbb: inout FlatBufferBuilder) { fbb.add(offset: ints, at: VTOFFSET.ints.p) }
public static func addVectorOf(floats: Offset<UOffset>, _ fbb: inout FlatBufferBuilder) { fbb.add(offset: floats, at: VTOFFSET.floats.p) }
public static func add(emptyString: Offset<String>, _ fbb: inout FlatBufferBuilder) { fbb.add(offset: emptyString, at: VTOFFSET.emptyString.p) }
public static func add(someString: Offset<String>, _ fbb: inout FlatBufferBuilder) { fbb.add(offset: someString, at: VTOFFSET.someString.p) }
public static func addVectorOf(abcs: Offset<UOffset>, _ fbb: inout FlatBufferBuilder) { fbb.add(offset: abcs, at: VTOFFSET.abcs.p) }
public static func addVectorOf(voids: Offset<UOffset>, _ fbb: inout FlatBufferBuilder) { fbb.add(offset: voids, at: VTOFFSET.voids.p) }
public static func addVectorOf(bools: Offset<UOffset>, _ fbb: inout FlatBufferBuilder) { fbb.add(offset: bools, at: VTOFFSET.bools.p) }
public static func endMoreDefaults(_ fbb: inout FlatBufferBuilder, start: UOffset) -> Offset<UOffset> { let end = Offset<UOffset>(offset: fbb.endTable(at: start)); return end }
public static func createMoreDefaults(
_ fbb: inout FlatBufferBuilder,
intsVectorOffset ints: Offset<UOffset> = Offset(),
floatsVectorOffset floats: Offset<UOffset> = Offset(),
emptyStringOffset emptyString: Offset<String> = Offset(),
someStringOffset someString: Offset<String> = Offset()
someStringOffset someString: Offset<String> = Offset(),
abcsVectorOffset abcs: Offset<UOffset> = Offset(),
voidsVectorOffset voids: Offset<UOffset> = Offset(),
boolsVectorOffset bools: Offset<UOffset> = Offset()
) -> Offset<UOffset> {
let __start = MoreDefaults.startMoreDefaults(&fbb)
MoreDefaults.addVectorOf(ints: ints, &fbb)
MoreDefaults.addVectorOf(floats: floats, &fbb)
MoreDefaults.add(emptyString: emptyString, &fbb)
MoreDefaults.add(someString: someString, &fbb)
MoreDefaults.addVectorOf(abcs: abcs, &fbb)
MoreDefaults.addVectorOf(voids: voids, &fbb)
MoreDefaults.addVectorOf(bools: bools, &fbb)
return MoreDefaults.endMoreDefaults(&fbb, start: __start)
}

Expand Down Expand Up @@ -81,11 +124,17 @@ public struct MoreDefaults: FlatBufferObject, ObjectAPIPacker {
__someString = Offset<String>()
}

let __abcs = builder.createVector(obj.abcs)
let __voids = builder.createVector(obj.voids)
let __bools = builder.createVector(obj.bools)
let __root = MoreDefaults.startMoreDefaults(&builder)
MoreDefaults.addVectorOf(ints: __ints, &builder)
MoreDefaults.addVectorOf(floats: __floats, &builder)
MoreDefaults.add(emptyString: __emptyString, &builder)
MoreDefaults.add(someString: __someString, &builder)
MoreDefaults.addVectorOf(abcs: __abcs, &builder)
MoreDefaults.addVectorOf(voids: __voids, &builder)
MoreDefaults.addVectorOf(bools: __bools, &builder)
return MoreDefaults.endMoreDefaults(&builder, start: __root)
}
}
Expand All @@ -96,6 +145,9 @@ public class MoreDefaultsT: NativeObject {
public var floats: [Float32]
public var emptyString: String?
public var someString: String?
public var abcs: [ABC]
public var voids: [Void]
public var bools: [Bool]

public init(_ _t: inout MoreDefaults) {
ints = []
Expand All @@ -108,13 +160,28 @@ public class MoreDefaultsT: NativeObject {
}
emptyString = _t.emptyString
someString = _t.someString
abcs = []
for index in 0..<_t.abcsCount {
abcs.append(_t.abcs(at: index)!)
}
voids = []
for index in 0..<_t.voidsCount {
voids.append(_t.voids(at: index)!)
}
bools = []
for index in 0..<_t.boolsCount {
bools.append(_t.bools(at: index))
}
}

public init() {
ints = []
floats = []
emptyString = ""
someString = "some"
abcs = []
voids = []
bools = []
}

public func serialize() -> ByteBuffer { return serialize(type: MoreDefaults.self) }
Expand Down
7 changes: 7 additions & 0 deletions tests/more_defaults.fbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@

enum ABC: int { A, B, C }
enum Void: int {}


table MoreDefaults {
ints: [int] = [];
floats: [float] = [ ];
empty_string: string = "";
some_string: string = "some";
abcs: [ABC] = [];
voids: [Void] = [];
bools: [bool] = [];
}
Loading