Default-empty vectors of enums

[CasperN]

This PR implements default-empty vectors of enums for #6053, which was omitted in #6421 and #6461.

Unlike the last time I did this, I didn't make more_defaults2.fbs, I just patched Swift and Rust together :)

[vglavnyy]

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]

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

[vglavnyy]

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.

[CasperN]

@mustiikhalil, I might need some guidance on how to fix swift.

Why were enums excluded in this line?

flatbuffers/src/idl_gen_swift.cpp

Line 769 in b240ab7

if (IsScalar(vectortype.base_type) && !IsEnum(vectortype) &&

[mustiikhalil]

So there are two issues there;
1- the use of Void void is a reserved word in swift, and it's not being checked for that (#6507 already fixing that here).
2- warning: the Swift runtime was unable to demangle the type of field ''. the mangled type name is ''. this field will show up as an empty tuple in Mirrors swift doesn't like it when we cast values into enums since enums in swift work in a weird way. So my suggestion would be that we disable this, until a proper solution is found

/// [...., 1, 2, 8, 0] output byte buffer of an encoded array of enums
/// when we try to read it with the enum below swift will return the error mentioned above,
/// and will give us the following array [FlatBuffers_Test_SwiftTests.ABC.b, FlatBuffers_Test_SwiftTests.ABC.c, 
/// FlatBuffers_Test_SwiftTests.ABC.()]
///  Composite components of Monster color.
public enum abc: UInt8, Enum {
  case a = 1
  case b = 2
  case c = 8
}

3- Sometimes casting enums would actually cause a fatalError

[mustiikhalil]

@CasperN what is the status on this? i am not sure if you saw the comment i made

[CasperN]

Yea I saw your comment. I just didn't find time to work on this briefly. It sounds like this problem with enums is much bigger than default-empty-vector-of-enums. Perhaps Swift should model enums C style? I made a change like that for Rust #6098 since the old way caused UB.

[mustiikhalil]

Enums in swift isn't completely a problem, since when we actually fetch the values we fetch them as Integers and then use the rawValue initializer to map the integers to actual swift enums. That's why we disabled the return of entire vectors, but if we really want it as a part of the API, then they start becoming an issue.
I see two ways of moving forward here,
1- Implementing it C style. By using a swift struct, that should definitely be possible.

struct Color: Enum {
 public static var red = Color(0)
 ...
 var value: Uint8
 
 var named: String {
   switch { ... }
 }
}

2- We create return _accessor.getVectorOfEnums(at: VTOFFSET.vectorOfEnums.v) which returns the numeric values of enums and then we can use map to convert the values to enums. This would affect the performance for sure.

Depending on what we want moving forward, the swift implementation would change to adapt to it

[CasperN]

Ok, so for the purposes of this PR, I'll just remove "returning the entire vector" from the API. Its up to you if you want to add that in the future or change how enums are modelled.

[mustiikhalil]

Sounds good

[mustiikhalil]

This should still not allow enums since enums have their own way of returning their object:

    if (IsEnum(vectortype)) {
      code_.SetValue("BASEVALUE", GenTypeBasic(vectortype, false));
      code_ += "return o == 0 ? {{VALUETYPE}}" + GenEnumDefaultValue(field) +
               " : {{VALUETYPE}}(rawValue: {{ACCESS}}.directRead(of: "
               "{{BASEVALUE}}.self, offset: {{ACCESS}}.vector(at: o) + "
               "index * {{SIZE}})) }";
      return;
      }

This will eventually depend on what we go with in my comment ofc

[CasperN]

hi all, sorry this one is taking so long, I've been busy recently

[CasperN]

I don't know what the buildkite failure is, seems transient

[mustiikhalil]

@CasperN fixed it

[CasperN]

Can I get LGTMs before I merge, @mustiikhalil @vglavnyy?

[mustiikhalil]

LGTM

[vglavnyy]

LGTM