Skip to content

Commit

Permalink
[Kotlin] Support for optional scalars. (google#6115)
Browse files Browse the repository at this point in the history 
More information on google#6014
  • Loading branch information
@paulovap @ivannp
paulovap authored and ivannp committed Oct 2, 2020
1 parent 3892084 commit deb5931
Show file tree
Hide file tree
Showing 7 changed files with 464 additions and 32 deletions.
4 changes: 4 additions & 0 deletions include/flatbuffers/idl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,10 @@ struct FieldDef : public Definition {

bool Deserialize(Parser &parser, const reflection::Field *field);

bool IsScalarOptional() const {
return IsScalar(value.type.base_type) && optional;
}

Value value;
bool deprecated; // Field is allowed to be present in old data, but can't be.
// written in new data nor accessed in new code.
Expand Down
95 changes: 66 additions & 29 deletions src/idl_gen_kotlin.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,24 @@ class KotlinGenerator : public BaseGenerator {
}
}

// with the addition of optional scalar types,
// we are adding the nullable '?' operator to return type of a field.
std::string GetterReturnType(const FieldDef &field) const {
auto base_type = field.value.type.base_type;

auto r_type = GenTypeGet(field.value.type);
if (field.IsScalarOptional() ||
// string, structs and unions
(base_type == BASE_TYPE_STRING || base_type == BASE_TYPE_STRUCT ||
base_type == BASE_TYPE_UNION) ||
// vector of anything not scalar
(base_type == BASE_TYPE_VECTOR &&
!IsScalar(field.value.type.VectorType().base_type))) {
r_type += "?";
}
return r_type;
}

std::string GenTypeGet(const Type &type) const {
return IsScalar(type.base_type) ? GenTypeBasic(type.base_type)
: GenTypePointer(type);
Expand All @@ -179,6 +197,18 @@ class KotlinGenerator : public BaseGenerator {
// - Floats are upcasted to doubles
// - Unsigned are casted to signed
std::string GenFBBDefaultValue(const FieldDef &field) const {
if (field.IsScalarOptional()) {
// although default value is null, java API forces us to present a real
// default value for scalars, while adding a field to the buffer. This is
// not a problem because the default can be representing just by not
// calling builder.addMyField()
switch (field.value.type.base_type) {
case BASE_TYPE_DOUBLE:
case BASE_TYPE_FLOAT: return "0.0";
case BASE_TYPE_BOOL: return "false";
default: return "0";
}
}
auto out = GenDefaultValue(field, true);
// All FlatBufferBuilder default floating point values are doubles
if (field.value.type.base_type == BASE_TYPE_FLOAT) {
Expand All @@ -204,6 +234,8 @@ class KotlinGenerator : public BaseGenerator {
bool force_signed = false) const {
auto &value = field.value;
auto base_type = field.value.type.base_type;

if (field.IsScalarOptional()) { return "null"; }
if (IsFloat(base_type)) {
auto val = KotlinFloatGen.GenFloatConstant(field);
if (base_type == BASE_TYPE_DOUBLE && val.back() == 'f') {
Expand Down Expand Up @@ -655,19 +687,22 @@ class KotlinGenerator : public BaseGenerator {
CodeWriter &writer, const IDLOptions options) const {
auto field_type = GenTypeBasic(field.value.type.base_type);
auto secondArg = MakeCamel(Esc(field.name), false) + ": " + field_type;
GenerateFunOneLine(writer, "add" + MakeCamel(Esc(field.name), true),
"builder: FlatBufferBuilder, " + secondArg, "", [&]() {
auto method = GenMethod(field.value.type);
writer.SetValue("field_name",
MakeCamel(Esc(field.name), false));
writer.SetValue("method_name", method);
writer.SetValue("pos", field_pos);
writer.SetValue("default", GenFBBDefaultValue(field));
writer.SetValue("cast", GenFBBValueCast(field));

writer += "builder.add{{method_name}}({{pos}}, \\";
writer += "{{field_name}}{{cast}}, {{default}})";
}, options.gen_jvmstatic);

GenerateFunOneLine(
writer, "add" + MakeCamel(Esc(field.name), true),
"builder: FlatBufferBuilder, " + secondArg, "",
[&]() {
auto method = GenMethod(field.value.type);
writer.SetValue("field_name", MakeCamel(Esc(field.name), false));
writer.SetValue("method_name", method);
writer.SetValue("pos", field_pos);
writer.SetValue("default", GenFBBDefaultValue(field));
writer.SetValue("cast", GenFBBValueCast(field));

writer += "builder.add{{method_name}}({{pos}}, \\";
writer += "{{field_name}}{{cast}}, {{default}})";
},
options.gen_jvmstatic);
}

static std::string ToSignedType(const Type &type) {
Expand Down Expand Up @@ -754,7 +789,8 @@ class KotlinGenerator : public BaseGenerator {
} else {
params << ": ";
}
params << GenTypeBasic(field.value.type.base_type);
auto optional = field.IsScalarOptional() ? "?" : "";
params << GenTypeBasic(field.value.type.base_type) << optional;
}

GenerateFun(writer, name, params.str(), "Int", [&]() {
Expand All @@ -773,11 +809,16 @@ class KotlinGenerator : public BaseGenerator {
MakeCamel(Esc(field.name), true));
writer.SetValue("field_name", MakeCamel(Esc(field.name), false));

// we wrap on null check for scalar optionals
writer +=
field.IsScalarOptional() ? "{{field_name}}?.run { \\" : "\\";

writer += "add{{camel_field_name}}(builder, {{field_name}}\\";
if (!IsScalar(field.value.type.base_type)) {
writer += "Offset\\";
}
writer += ")";
// we wrap on null check for scalar optionals
writer += field.IsScalarOptional() ? ") }" : ")";
}
}
}
Expand Down Expand Up @@ -812,7 +853,7 @@ class KotlinGenerator : public BaseGenerator {
auto field_name = MakeCamel(Esc(field.name), false);
auto field_type = GenTypeGet(field.value.type);
auto field_default_value = GenDefaultValue(field);
auto return_type = GenTypeGet(field.value.type);
auto return_type = GetterReturnType(field);
auto bbgetter = ByteBufferGetter(field.value.type, "bb");
auto ucast = CastToUsigned(field);
auto offset_val = NumToString(field.value.offset);
Expand All @@ -829,23 +870,22 @@ class KotlinGenerator : public BaseGenerator {
writer.SetValue("bbgetter", bbgetter);
writer.SetValue("ucast", ucast);

auto opt_ret_type = return_type + "?";
// Generate the accessors that don't do object reuse.
if (value_base_type == BASE_TYPE_STRUCT) {
// Calls the accessor that takes an accessor object with a
// new object.
// val pos
// get() = pos(Vec3())
GenerateGetterOneLine(writer, field_name, opt_ret_type, [&]() {
GenerateGetterOneLine(writer, field_name, return_type, [&]() {
writer += "{{field_name}}({{field_type}}())";
});
} else if (value_base_type == BASE_TYPE_VECTOR &&
field.value.type.element == BASE_TYPE_STRUCT) {
// Accessors for vectors of structs also take accessor objects,
// this generates a variant without that argument.
// ex: fun weapons(j: Int) = weapons(Weapon(), j)
GenerateFunOneLine(writer, field_name, "j: Int", opt_ret_type, [&]() {
writer += "{{field_name}}({{return_type}}(), j)";
GenerateFunOneLine(writer, field_name, "j: Int", return_type, [&]() {
writer += "{{field_name}}({{field_type}}(), j)";
});
}

Expand All @@ -872,7 +912,7 @@ class KotlinGenerator : public BaseGenerator {
// fun pos(obj: Vec3) : Vec3? = obj.__assign(bb_pos + 4, bb)
// ? adds nullability annotation
GenerateFunOneLine(
writer, field_name, "obj: " + field_type, return_type + "?",
writer, field_name, "obj: " + field_type, return_type,
[&]() { writer += "obj.__assign(bb_pos + {{offset}}, bb)"; });
} else {
// create getter with object reuse
Expand All @@ -887,8 +927,7 @@ class KotlinGenerator : public BaseGenerator {
// }
// ? adds nullability annotation
GenerateFun(
writer, field_name, "obj: " + field_type, return_type + "?",
[&]() {
writer, field_name, "obj: " + field_type, return_type, [&]() {
auto fixed = field.value.type.struct_def->fixed;

writer.SetValue("seek", Indirect("o + bb_pos", fixed));
Expand All @@ -908,7 +947,7 @@ class KotlinGenerator : public BaseGenerator {
// return if (o != 0) __string(o + bb_pos) else null
// }
// ? adds nullability annotation
GenerateGetter(writer, field_name, return_type + "?", [&]() {
GenerateGetter(writer, field_name, return_type, [&]() {
writer += "val o = __offset({{offset}})";
writer += "return if (o != 0) __string(o + bb_pos) else null";
});
Expand All @@ -926,15 +965,13 @@ class KotlinGenerator : public BaseGenerator {

auto vectortype = field.value.type.VectorType();
std::string params = "j: Int";
std::string nullable = IsScalar(vectortype.base_type) ? "" : "?";

if (vectortype.base_type == BASE_TYPE_STRUCT ||
vectortype.base_type == BASE_TYPE_UNION) {
params = "obj: " + field_type + ", j: Int";
}

auto ret_type = return_type + nullable;
GenerateFun(writer, field_name, params, ret_type, [&]() {
GenerateFun(writer, field_name, params, return_type, [&]() {
auto inline_size = NumToString(InlineSize(vectortype));
auto index = "__vector(o) + j * " + inline_size;
auto not_found = NotFoundReturn(field.value.type.element);
Expand All @@ -959,8 +996,8 @@ class KotlinGenerator : public BaseGenerator {
break;
}
case BASE_TYPE_UNION:
GenerateFun(writer, field_name, "obj: " + field_type,
return_type + "?", [&]() {
GenerateFun(writer, field_name, "obj: " + field_type, return_type,
[&]() {
writer += OffsetWrapperOneLine(
offset_val, bbgetter + "(obj, o + bb_pos)",
"null");
Expand Down
3 changes: 2 additions & 1 deletion src/idl_parser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2263,7 +2263,8 @@ CheckedError Parser::CheckClash(std::vector<FieldDef *> &fields,

bool Parser::SupportsOptionalScalars() const {
return !(opts.lang_to_generate &
~(IDLOptions::kRust | IDLOptions::kSwift | IDLOptions::kLobster));
~(IDLOptions::kRust | IDLOptions::kSwift | IDLOptions::kLobster |
IDLOptions::kKotlin));
}

bool Parser::SupportsAdvancedUnionFeatures() const {
Expand Down
123 changes: 122 additions & 1 deletion tests/KotlinTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

import MyGame.Example.*
import optional_scalars.*
import com.google.flatbuffers.ByteBufferUtil
import com.google.flatbuffers.FlatBufferBuilder
import NamespaceA.*
Expand Down Expand Up @@ -77,7 +78,7 @@ class KotlinTest {
TestVectorOfUnions()

TestSharedStringPool()

TestScalarOptional()
println("FlatBuffers test: completed successfully")
}

Expand Down Expand Up @@ -467,5 +468,125 @@ class KotlinTest {
assert(offset == fb.createSharedString(testString));
}
}

fun TestScalarOptional() {
val fbb = FlatBufferBuilder(1)
ScalarStuff.startScalarStuff(fbb)
var pos = ScalarStuff.endScalarStuff(fbb)
fbb.finish(pos)

var scalarStuff = ScalarStuff.getRootAsScalarStuff(fbb.dataBuffer())

assert(scalarStuff.justI8 == 0.toByte())
assert(scalarStuff.maybeI8 == null)
assert(scalarStuff.defaultI8 == 42.toByte())
assert(scalarStuff.justU8 == 0.toUByte())
assert(scalarStuff.maybeU8 == null)
assert(scalarStuff.defaultU8 == 42.toUByte())
assert(scalarStuff.justI16 == 0.toShort())
assert(scalarStuff.maybeI16 == null)
assert(scalarStuff.defaultI16 == 42.toShort())
assert(scalarStuff.justU16 == 0.toUShort())
assert(scalarStuff.maybeU16 == null)
assert(scalarStuff.defaultU16 == 42.toUShort())
assert(scalarStuff.justI32 == 0)
assert(scalarStuff.maybeI32 == null)
assert(scalarStuff.defaultI32 == 42)
assert(scalarStuff.justU32 == 0.toUInt())
assert(scalarStuff.maybeU32 == null)
assert(scalarStuff.defaultU32 == 42U)
assert(scalarStuff.justI64 == 0L)
assert(scalarStuff.maybeI64 == null)
assert(scalarStuff.defaultI64 == 42L)
assert(scalarStuff.justU64 == 0UL)
assert(scalarStuff.maybeU64 == null)
assert(scalarStuff.defaultU64 == 42UL)
assert(scalarStuff.justF32 == 0.0f)
assert(scalarStuff.maybeF32 == null)
assert(scalarStuff.defaultF32 == 42.0f)
assert(scalarStuff.justF64 == 0.0)
assert(scalarStuff.maybeF64 == null)
assert(scalarStuff.defaultF64 == 42.0)
assert(scalarStuff.justBool == false)
assert(scalarStuff.maybeBool == null)
assert(scalarStuff.defaultBool == true)

fbb.clear()

ScalarStuff.startScalarStuff(fbb)
ScalarStuff.addJustI8(fbb, 5.toByte())
ScalarStuff.addMaybeI8(fbb, 5.toByte())
ScalarStuff.addDefaultI8(fbb, 5.toByte())
ScalarStuff.addJustU8(fbb, 6.toUByte())
ScalarStuff.addMaybeU8(fbb, 6.toUByte())
ScalarStuff.addDefaultU8(fbb, 6.toUByte())
ScalarStuff.addJustI16(fbb, 7.toShort())
ScalarStuff.addMaybeI16(fbb, 7.toShort())
ScalarStuff.addDefaultI16(fbb, 7.toShort())
ScalarStuff.addJustU16(fbb, 8.toUShort())
ScalarStuff.addMaybeU16(fbb, 8.toUShort())
ScalarStuff.addDefaultU16(fbb, 8.toUShort())
ScalarStuff.addJustI32(fbb, 9)
ScalarStuff.addMaybeI32(fbb, 9)
ScalarStuff.addDefaultI32(fbb, 9)
ScalarStuff.addJustU32(fbb, 10.toUInt())
ScalarStuff.addMaybeU32(fbb, 10.toUInt())
ScalarStuff.addDefaultU32(fbb, 10.toUInt())
ScalarStuff.addJustI64(fbb, 11L)
ScalarStuff.addMaybeI64(fbb, 11L)
ScalarStuff.addDefaultI64(fbb, 11L)
ScalarStuff.addJustU64(fbb, 12UL)
ScalarStuff.addMaybeU64(fbb, 12UL)
ScalarStuff.addDefaultU64(fbb, 12UL)
ScalarStuff.addJustF32(fbb, 13.0f)
ScalarStuff.addMaybeF32(fbb, 13.0f)
ScalarStuff.addDefaultF32(fbb, 13.0f)
ScalarStuff.addJustF64(fbb, 14.0)
ScalarStuff.addMaybeF64(fbb, 14.0)
ScalarStuff.addDefaultF64(fbb, 14.0)
ScalarStuff.addJustBool(fbb, true)
ScalarStuff.addMaybeBool(fbb, true)
ScalarStuff.addDefaultBool(fbb, true)

pos = ScalarStuff.endScalarStuff(fbb)

fbb.finish(pos)

scalarStuff = ScalarStuff.getRootAsScalarStuff(fbb.dataBuffer())

assert(scalarStuff.justI8 == 5.toByte())
assert(scalarStuff.maybeI8 == 5.toByte())
assert(scalarStuff.defaultI8 == 5.toByte())
assert(scalarStuff.justU8 == 6.toUByte())
assert(scalarStuff.maybeU8 == 6.toUByte())
assert(scalarStuff.defaultU8 == 6.toUByte())
assert(scalarStuff.justI16 == 7.toShort())
assert(scalarStuff.maybeI16 == 7.toShort())
assert(scalarStuff.defaultI16 == 7.toShort())
assert(scalarStuff.justU16 == 8.toUShort())
assert(scalarStuff.maybeU16 == 8.toUShort())
assert(scalarStuff.defaultU16 == 8.toUShort())
assert(scalarStuff.justI32 == 9)
assert(scalarStuff.maybeI32 == 9)
assert(scalarStuff.defaultI32 == 9)
assert(scalarStuff.justU32 == 10u)
assert(scalarStuff.maybeU32 == 10u)
assert(scalarStuff.defaultU32 == 10u)
assert(scalarStuff.justI64 == 11L)
assert(scalarStuff.maybeI64 == 11L)
assert(scalarStuff.defaultI64 == 11L)
assert(scalarStuff.justU64 == 12UL)
assert(scalarStuff.maybeU64 == 12UL)
assert(scalarStuff.defaultU64 == 12UL)
assert(scalarStuff.justF32 == 13.0f)
assert(scalarStuff.maybeF32 == 13.0f)
assert(scalarStuff.defaultF32 == 13.0f)
assert(scalarStuff.justF64 == 14.0)
assert(scalarStuff.maybeF64 == 14.0)
assert(scalarStuff.defaultF64 == 14.0)
assert(scalarStuff.justBool == true)
assert(scalarStuff.maybeBool == true)
assert(scalarStuff.defaultBool == true)
}
}
}
2 changes: 1 addition & 1 deletion tests/generate_code.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ $TEST_NOINCL_FLAGS $TEST_CPP_FLAGS $TEST_CS_FLAGS $TEST_JS_TS_FLAGS -o namespace
../flatc --dart monster_extra.fbs

# Generate optional scalar code for tests.
../flatc --rust --lobster optional_scalars.fbs
../flatc --kotlin --rust --lobster optional_scalars.fbs

# Generate the schema evolution tests
../flatc --cpp --scoped-enums $TEST_CPP_FLAGS -o evolution_test ./evolution_test/evolution_v*.fbs
Expand Down
Loading

0 comments on commit deb5931

Please sign in to comment.