[swift] mutating function for bool field has Byte argument, but not Bool

[mr-swifter]

I have a simple table

table Event
{
    handled:bool=null;
}

when I compile with flatc protocol.fbs --swift --gen-mutable

  public var handled: Bool? { let o = _accessor.offset(VTOFFSET.handled.v); return o == 0 ? nil : 0 != _accessor.readBuffer(of: Byte.self, at: o) }
  @discardableResult public func mutate(handled: Byte) -> Bool {let o = _accessor.offset(VTOFFSET.handled.v);  return _accessor.mutate(handled, index: o) }

It's strange mutating function argument type is Byte, but not a Bool. Is there any special reason for this?

I manually modified autogenerated code and changed Byte to Bool and see no issue (at least it is not visible)

If it's an issue I would happy to fix it.

[CasperN]

Feel free to contribute! @mustiikhalil is the swift owner

[mustiikhalil]

Oh weird, That means that the GenMutate is called with a {{VALUETYPE}} that's not a Bool. Or to be precise resetting the type to Byte so we can compare it in the function that generates the values.

@mr-swifter feel free to open a PR to fix this.

I will need to refactor how the code generation works.

[github-actions[bot]]

This issue is stale because it has been open 6 months with no activity. Please comment or label not-stale, or this will be closed in 14 days.

[mr-swifter]

not-stale

[blindspotbounty]

@mustiikhalil as I see the bug is around this code

code_.SetValue("CONSTANT", default_value);
code_.SetValue("VALUETYPE", "Bool");
code_ += GenReaderMainBody(optional) + "\\";
code_.SetValue("VALUETYPE", "Byte"); //      <---  Bug is here
code_ += GenOffset() + "return o == 0 ? {{CONSTANT}} : 0 != " + GenReader("VALUETYPE", "o") + " }";

I played a bit and looks like a workaround because if I remain it as bool, there are compilation errors for swift tests:

flatbuffers/tests/swift/tests/Tests/FlatBuffers.Test.SwiftTests/monster_test_generated.swift:1224:117: error: cannot convert value of type 'Bool' to expected argument type 'Int'
  public var testbool: Bool { let o = _accessor.offset(VTOFFSET.testbool.v); return o == 0 ? false : 0 != _accessor.readBuffer(of: Bool.self, at: o) }

I tried with the following code and seems both are working:

diff --git a/src/idl_gen_swift.cpp b/src/idl_gen_swift.cpp
index 6afa069e..60181890 100644
--- a/src/idl_gen_swift.cpp
+++ b/src/idl_gen_swift.cpp
@@ -723,11 +723,12 @@ class SwiftGenerator : public BaseGenerator {
           field.IsOptional() ? "nil"
                              : SwiftConstant(field);
       code_.SetValue("CONSTANT", default_value);
-      code_.SetValue("VALUETYPE", "Bool");
       code_ += GenReaderMainBody(optional) + "\\";
       code_.SetValue("VALUETYPE", "Byte");
       code_ += GenOffset() + "return o == 0 ? {{CONSTANT}} : 0 != " +
                GenReader("VALUETYPE", "o") + " }";
+      code_.SetValue("VALUETYPE", "Bool");
       if (parser_.opts.mutable_buffer) code_ += GenMutate("o", GenOffset());
       return;
     }

However, generated code looks a little bit ugly since we decoding Byte but encoding Bool in this case:

  public var handled: Bool? { let o = _accessor.offset(VTOFFSET.handled.v); return o == 0 ? nil : 0 != _accessor.readBuffer(of: Byte.self, at: o) } 
  @discardableResult public func mutate(handled: Bool) -> Bool {let o = _accessor.offset(VTOFFSET.handled.v);  return _accessor.mutate(handled, index: o) } 

Though these type have the same size, it would look weired.

[mustiikhalil]

I think this would be the proper solution here:

@@ -723,11 +723,12 @@ class SwiftGenerator : public BaseGenerator {
           field.IsOptional() ? "nil"
                              : SwiftConstant(field);
       code_.SetValue("CONSTANT", default_value);
       code_.SetValue("VALUETYPE", "Bool");
       code_ += GenReaderMainBody(optional) + "\\";
       code_ += GenOffset() + "return o == 0 ? {{CONSTANT}} : 0 != " +
                GenReader("VALUETYPE", "o") + " }";
       if (parser_.opts.mutable_buffer) code_ += GenMutate("o", GenOffset());
       return;
     }

and then we add proper readers for bool values within the mutators, tables, structs and ByteBuffer. so instead of this

public var handled: Bool? { let o = _accessor.offset(VTOFFSET.handled.v); return o == 0 ? nil : 0 != _accessor.readBuffer(of: Byte.self, at: o) } 

we will have something similar to this

public var handled: Bool? { let o = _accessor.offset(VTOFFSET.handled.v); return o == 0 ? nil : _accessor.readBuffer(of: Bool.self, at: o) } 

[blindspotbounty]

It seems that in ByteBuffer/Table/Structs we have a generic method for read/write:

  /// Reads an object from the buffer
  /// - Parameters:
  ///   - def: Type of the object
  ///   - position: the index of the object in the buffer
  @inline(__always)
  public func read<T>(def: T.Type, position: Int) -> T {
    _storage.memory.advanced(by: position).load(as: T.self)
  }

Do you think we should add something there?