From ec58b0fb5eaf482a674dd831bd9190fd6c2d2822 Mon Sep 17 00:00:00 2001 From: Casper Neo Date: Sat, 23 Jan 2021 22:04:52 -0500 Subject: [PATCH 01/11] Fix tests.cpp --- tests/test.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test.cpp b/tests/test.cpp index d070b9d04a3..e711fdfa342 100644 --- a/tests/test.cpp +++ b/tests/test.cpp @@ -935,8 +935,8 @@ void ReflectionTest(uint8_t *flatbuf, size_t length) { // Test nullability of fields: hp is a 0-default scalar, pos is a struct => // optional, and name is a required string => not optional. - TEST_EQ(hp_field.optional(), false); - TEST_EQ(pos_field_ptr->optional(), true); + TEST_EQ(hp_field.IsOptional(), false); + TEST_EQ(pos_field_ptr->IsOptional(), true); TEST_EQ(fields->LookupByKey("name")->optional(), false); // Now use it to dynamically access a buffer. From cd9a9d7468d1eb1283d91c7d86556da9e4b6e092 Mon Sep 17 00:00:00 2001 From: Casper Neo Date: Sun, 24 Jan 2021 12:09:56 -0500 Subject: [PATCH 02/11] Parser support for vector/string defaults --- include/flatbuffers/idl.h | 2 ++ src/idl_parser.cpp | 23 +++++++++++++++++++---- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/include/flatbuffers/idl.h b/include/flatbuffers/idl.h index 1f549f93554..e418801d444 100644 --- a/include/flatbuffers/idl.h +++ b/include/flatbuffers/idl.h @@ -337,6 +337,7 @@ struct FieldDef : public Definition { kDefault, }; Presence static MakeFieldPresence(bool optional, bool required) { + FLATBUFFERS_ASSERT(!(required && optional)); // clang-format off return required ? FieldDef::kRequired : optional ? FieldDef::kOptional @@ -971,6 +972,7 @@ class Parser : public ParserState { bool SupportsAdvancedUnionFeatures() const; bool SupportsAdvancedArrayFeatures() const; bool SupportsOptionalScalars() const; + bool SupportsDefaultVectorsAndStrings() const; Namespace *UniqueNamespace(Namespace *ns); FLATBUFFERS_CHECKED_ERROR RecurseError(); diff --git a/src/idl_parser.cpp b/src/idl_parser.cpp index 7cdddaffd7e..b691dd46400 100644 --- a/src/idl_parser.cpp +++ b/src/idl_parser.cpp @@ -794,10 +794,20 @@ CheckedError Parser::ParseField(StructDef &struct_def) { if (token_ == '=') { NEXT(); ECHECK(ParseSingleValue(&field->name, field->value, true)); - if (!IsScalar(type.base_type) || - (struct_def.fixed && field->value.constant != "0")) + if (IsStruct(type) || (struct_def.fixed && field->value.constant != "0")) return Error( - "default values currently only supported for scalars in tables"); + "default values are not supported for struct fields, table fields, " + "or in structs."); + if ((IsString(type) || IsVector(type)) && field->value.constant != "0" && + field->value.constant != "null" && !SupportsDefaultVectorsAndStrings()) + return Error( + "Default values for strings and vectors are not supported in one of " + "the " + "specified programming languages"); + if (IsVector(type) && + (field->value.constant != "0" || field->value.constant != "[]")) { + return Error("The only supported default for vectors is `[]`"); + } } // Append .0 if the value has not it (skip hex and scientific floats). @@ -2388,12 +2398,17 @@ bool Parser::SupportsOptionalScalars(const flatbuffers::IDLOptions &opts) { unsigned long langs = opts.lang_to_generate; return (langs > 0 && langs < IDLOptions::kMAX) && !(langs & ~supported_langs); } - bool Parser::SupportsOptionalScalars() const { // Check in general if a language isn't specified. return opts.lang_to_generate == 0 || SupportsOptionalScalars(opts); } +bool Parser::SupportsDefaultVectorsAndStrings() const { + static FLATBUFFERS_CONSTEXPR unsigned long supported_langs = 0; + unsigned long langs = opts.lang_to_generate; + return (langs > 0 && langs < IDLOptions::kMAX) && !(langs & ~supported_langs); +} + bool Parser::SupportsAdvancedUnionFeatures() const { return opts.lang_to_generate != 0 && (opts.lang_to_generate & From 0113860f92c5bfe283ab26bab457b4e96a00974c Mon Sep 17 00:00:00 2001 From: Casper Neo Date: Mon, 1 Feb 2021 19:08:44 -0500 Subject: [PATCH 03/11] tests and default empty vectors --- src/idl_parser.cpp | 36 +++++++++++++++++++++++----------- tests/test.cpp | 48 ++++++++++++++++++++++++++++++++-------------- 2 files changed, 59 insertions(+), 25 deletions(-) diff --git a/src/idl_parser.cpp b/src/idl_parser.cpp index b691dd46400..dc9b5b484db 100644 --- a/src/idl_parser.cpp +++ b/src/idl_parser.cpp @@ -802,11 +802,10 @@ CheckedError Parser::ParseField(StructDef &struct_def) { field->value.constant != "null" && !SupportsDefaultVectorsAndStrings()) return Error( "Default values for strings and vectors are not supported in one of " - "the " - "specified programming languages"); - if (IsVector(type) && - (field->value.constant != "0" || field->value.constant != "[]")) { - return Error("The only supported default for vectors is `[]`"); + "the specified programming languages"); + if (IsVector(type) && field->value.constant != "0" && + field->value.constant != "[]") { + return Error("The only supported default for vectors is `[]`."); } } @@ -866,8 +865,11 @@ CheckedError Parser::ParseField(StructDef &struct_def) { field->key = field->attributes.Lookup("key") != nullptr; const bool required = field->attributes.Lookup("required") != nullptr || (IsString(type) && field->key); - const bool optional = - IsScalar(type.base_type) ? (field->value.constant == "null") : !required; + const bool default_str_or_vec = + ((IsString(type) || IsVector(type)) && field->value.constant != "0"); + const bool optional = IsScalar(type.base_type) + ? (field->value.constant == "null") + : !(required || default_str_or_vec); if (required && optional) { return Error("Fields cannot be both optional and required."); } @@ -1307,7 +1309,7 @@ CheckedError Parser::ParseTable(const StructDef &struct_def, std::string *value, if (!struct_def.sortbysize || size == SizeOf(field_value.type.base_type)) { switch (field_value.type.base_type) { - // clang-format off +// clang-format off #define FLATBUFFERS_TD(ENUM, IDLTYPE, CTYPE, ...) \ case BASE_TYPE_ ## ENUM: \ builder_.Pad(field->padding); \ @@ -1493,7 +1495,7 @@ CheckedError Parser::ParseVector(const Type &type, uoffset_t *ovalue, // start at the back, since we're building the data backwards. auto &val = field_stack_.back().first; switch (val.type.base_type) { - // clang-format off +// clang-format off #define FLATBUFFERS_TD(ENUM, IDLTYPE, CTYPE,...) \ case BASE_TYPE_ ## ENUM: \ if (IsStruct(val.type)) SerializeStruct(*val.type.struct_def, val); \ @@ -1954,6 +1956,19 @@ CheckedError Parser::ParseSingleValue(const std::string *name, Value &e, // Integer token can init any scalar (integer of float). FORCE_ECHECK(kTokenIntegerConstant, IsScalar(in_type), BASE_TYPE_INT); } + // Match empty vectors for default-empty-vectors. + if (!match && IsVector(e.type) && token_ == '[') { + const char *c = cursor_; + // Peek next token. + while (*c == ' ' || *c == '\t' || *c == '\n') c++; + if (*c == ']') { + e.constant = "[]"; + NEXT(); + NEXT(); + match = true; + } + } + #undef FORCE_ECHECK #undef TRY_ECHECK #undef IF_ECHECK_ @@ -2405,8 +2420,7 @@ bool Parser::SupportsOptionalScalars() const { bool Parser::SupportsDefaultVectorsAndStrings() const { static FLATBUFFERS_CONSTEXPR unsigned long supported_langs = 0; - unsigned long langs = opts.lang_to_generate; - return (langs > 0 && langs < IDLOptions::kMAX) && !(langs & ~supported_langs); + return !(opts.lang_to_generate & ~supported_langs); } bool Parser::SupportsAdvancedUnionFeatures() const { diff --git a/tests/test.cpp b/tests/test.cpp index e711fdfa342..fa22ac118be 100644 --- a/tests/test.cpp +++ b/tests/test.cpp @@ -107,7 +107,7 @@ flatbuffers::DetachedBuffer CreateFlatBufferTest(std::string &buffer) { Test tests[] = { Test(10, 20), Test(30, 40) }; auto testv = builder.CreateVectorOfStructs(tests, 2); - // clang-format off +// clang-format off #ifndef FLATBUFFERS_CPP98_STL // Create a vector of structures from a lambda. auto testv2 = builder.CreateVectorOfStructs( @@ -204,7 +204,6 @@ flatbuffers::DetachedBuffer CreateFlatBufferTest(std::string &buffer) { flexbuild.Int(1234); flexbuild.Finish(); auto flex = builder.CreateVector(flexbuild.GetBuffer()); - // Test vector of enums. Color colors[] = { Color_Blue, Color_Green }; // We use this special creation function because we have an array of @@ -224,7 +223,7 @@ flatbuffers::DetachedBuffer CreateFlatBufferTest(std::string &buffer) { FinishMonsterBuffer(builder, mloc); - // clang-format off +// clang-format off #ifdef FLATBUFFERS_TEST_VERBOSE // print byte data for debugging: auto p = builder.GetBufferPointer(); @@ -248,7 +247,7 @@ void AccessFlatBufferTest(const uint8_t *flatbuf, size_t length, flatbuffers::Verifier verifier(flatbuf, length); TEST_EQ(VerifyMonsterBuffer(verifier), true); - // clang-format off +// clang-format off #ifdef FLATBUFFERS_TRACK_VERIFIER_BUFFER_SIZE std::vector test_buff; test_buff.resize(length * 2); @@ -603,7 +602,7 @@ void SizePrefixedTest() { } void TriviallyCopyableTest() { - // clang-format off +// clang-format off #if __GNUG__ && __GNUC__ < 5 TEST_EQ(__has_trivial_copy(Vec3), true); #else @@ -935,8 +934,8 @@ void ReflectionTest(uint8_t *flatbuf, size_t length) { // Test nullability of fields: hp is a 0-default scalar, pos is a struct => // optional, and name is a required string => not optional. - TEST_EQ(hp_field.IsOptional(), false); - TEST_EQ(pos_field_ptr->IsOptional(), true); + TEST_EQ(hp_field.optional(), false); + TEST_EQ(pos_field_ptr->optional(), true); TEST_EQ(fields->LookupByKey("name")->optional(), false); // Now use it to dynamically access a buffer. @@ -1439,7 +1438,7 @@ void FuzzTest2() { } }; - // clang-format off +// clang-format off #define AddToSchemaAndInstances(schema_add, instance_add) \ RndDef::Add(definitions, schema, instances_per_definition, \ schema_add, instance_add, definition) @@ -1591,7 +1590,7 @@ void FuzzTest2() { TEST_NOTNULL(nullptr); //-V501 (this comment supresses CWE-570 warning) } - // clang-format off +// clang-format off #ifdef FLATBUFFERS_TEST_VERBOSE TEST_OUTPUT_LINE("%dk schema tested with %dk of json\n", static_cast(schema.length() / 1024), @@ -1645,7 +1644,6 @@ void ErrorTest() { TestError("table X { Y:int; Y:int; }", "field already"); TestError("table Y {} table X { Y:int; }", "same as table"); TestError("struct X { Y:string; }", "only scalar"); - TestError("table X { Y:string = \"\"; }", "default values"); TestError("struct X { a:uint = 42; }", "default values"); TestError("enum Y:byte { Z = 1 } table X { y:Y; }", "not part of enum"); TestError("struct X { Y:int (deprecated); }", "deprecate"); @@ -1690,6 +1688,11 @@ void ErrorTest() { "may contain only scalar or struct fields"); // Non-snake case field names TestError("table X { Y: int; } root_type Y: {Y:1.0}", "snake_case"); + // Complex defaults + TestError("table X { y: string = 1; }", "expecting: string"); + TestError("table X { y: string = []; }", " Cannot assign token"); + TestError("table X { y: [int] = [1]; }", " Cannot assign token"); + TestError("table X { y: [int] = \"\"; }", "type mismatch"); } template @@ -2865,10 +2868,10 @@ void FlexBuffersTest() { flexbuffers::Builder slb(512, flexbuffers::BUILDER_FLAG_SHARE_KEYS_AND_STRINGS); - // Write the equivalent of: - // { vec: [ -100, "Fred", 4.0, false ], bar: [ 1, 2, 3 ], bar3: [ 1, 2, 3 ], - // foo: 100, bool: true, mymap: { foo: "Fred" } } - // clang-format off +// Write the equivalent of: +// { vec: [ -100, "Fred", 4.0, false ], bar: [ 1, 2, 3 ], bar3: [ 1, 2, 3 ], +// foo: 100, bool: true, mymap: { foo: "Fred" } } +// clang-format off #ifndef FLATBUFFERS_CPP98_STL // It's possible to do this without std::function support as well. slb.Map([&]() { @@ -3620,6 +3623,22 @@ void TestEmbeddedBinarySchema() { 0); } +void MoreDefaultsTest() { + std::vector schemas; + schemas.push_back("table Monster { mana: string = \"\"; }"); + schemas.push_back("table Monster { mana: string = \"mystr\"; }"); + schemas.push_back("table Monster { mana: string = \" \"; }"); + schemas.push_back("table Monster { mana: [int] = []; }"); + schemas.push_back("table Monster { mana: [uint] = [ ]; }"); + schemas.push_back("table Monster { mana: [byte] = [\t\t\n]; }"); + for (auto s = schemas.begin(); s < schemas.end(); s++) { + flatbuffers::Parser parser; + TEST_ASSERT(parser.Parse(s->c_str())); + const auto *mana = parser.structs_.Lookup("Monster")->fields.Lookup("mana"); + TEST_EQ(mana->IsDefault(), true); + } +} + void OptionalScalarsTest() { // Simple schemas and a "has optional scalar" sentinal. std::vector schemas; @@ -3852,6 +3871,7 @@ int FlatBufferTests() { FlatbuffersSpanTest(); FixedLengthArrayConstructorTest(); FieldIdentifierTest(); + MoreDefaultsTest(); return 0; } From 3671891ee6ff1a1380f080e1bba52d6d2d2ca79a Mon Sep 17 00:00:00 2001 From: Casper Neo Date: Wed, 3 Feb 2021 18:11:28 -0500 Subject: [PATCH 04/11] addressed comments --- src/idl_parser.cpp | 14 ++++++-------- tests/test.cpp | 7 ++++--- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/idl_parser.cpp b/src/idl_parser.cpp index dc9b5b484db..8ef4d8db2b9 100644 --- a/src/idl_parser.cpp +++ b/src/idl_parser.cpp @@ -1958,15 +1958,13 @@ CheckedError Parser::ParseSingleValue(const std::string *name, Value &e, } // Match empty vectors for default-empty-vectors. if (!match && IsVector(e.type) && token_ == '[') { - const char *c = cursor_; - // Peek next token. - while (*c == ' ' || *c == '\t' || *c == '\n') c++; - if (*c == ']') { - e.constant = "[]"; - NEXT(); - NEXT(); - match = true; + NEXT(); + if (token_ != ']') { + return Error("Expected `]` in vector default"); } + NEXT(); + match = true; + e.constant = "[]"; } #undef FORCE_ECHECK diff --git a/tests/test.cpp b/tests/test.cpp index fa22ac118be..a60bcfaf1d5 100644 --- a/tests/test.cpp +++ b/tests/test.cpp @@ -1691,7 +1691,8 @@ void ErrorTest() { // Complex defaults TestError("table X { y: string = 1; }", "expecting: string"); TestError("table X { y: string = []; }", " Cannot assign token"); - TestError("table X { y: [int] = [1]; }", " Cannot assign token"); + TestError("table X { y: [int] = [1]; }", "Expected `]`"); + TestError("table X { y: [int] = [; }", "Expected `]`"); TestError("table X { y: [int] = \"\"; }", "type mismatch"); } @@ -3623,7 +3624,7 @@ void TestEmbeddedBinarySchema() { 0); } -void MoreDefaultsTest() { +void StringVectorDefaultsTest() { std::vector schemas; schemas.push_back("table Monster { mana: string = \"\"; }"); schemas.push_back("table Monster { mana: string = \"mystr\"; }"); @@ -3871,7 +3872,7 @@ int FlatBufferTests() { FlatbuffersSpanTest(); FixedLengthArrayConstructorTest(); FieldIdentifierTest(); - MoreDefaultsTest(); + StringVectorDefaultsTest(); return 0; } From 7ff69af482610c44224d545c69e01c05f2181b19 Mon Sep 17 00:00:00 2001 From: Casper Neo Date: Wed, 3 Feb 2021 19:48:49 -0500 Subject: [PATCH 05/11] Default strings and vectors for Rust --- src/idl_gen_rust.cpp | 83 ++++++------- src/idl_parser.cpp | 3 +- tests/generate_code.sh | 3 + tests/more_defaults.fbs | 7 ++ tests/more_defaults_generated.rs | 201 +++++++++++++++++++++++++++++++ 5 files changed, 247 insertions(+), 50 deletions(-) create mode 100644 tests/more_defaults.fbs create mode 100644 tests/more_defaults_generated.rs diff --git a/src/idl_gen_rust.cpp b/src/idl_gen_rust.cpp index 1677ce82549..18b8443b1d8 100644 --- a/src/idl_gen_rust.cpp +++ b/src/idl_gen_rust.cpp @@ -183,6 +183,12 @@ bool IsBitFlagsEnum(const FieldDef &field) { return ed && IsBitFlagsEnum(*ed); } +// TableArgs make required non-scalars "Option<_>". +// TODO(cneo): Rework how we do defaults and stuff. +bool IsOptionalOrRequiredNonScalar(const FieldDef &field) { + return field.IsOptional() || (field.IsRequired() && !IsScalar(field.value.type.base_type)); +} + namespace rust { class RustGenerator : public BaseGenerator { @@ -884,28 +890,22 @@ class RustGenerator : public BaseGenerator { } std::string GetDefaultScalarValue(const FieldDef &field) { + if (IsOptionalOrRequiredNonScalar(field)) return "None"; + switch (GetFullType(field.value.type)) { - case ftInteger: - case ftFloat: { - return field.IsOptional() ? "None" : field.value.constant; - } case ftBool: { return field.IsOptional() ? "None" : field.value.constant == "0" ? "false" : "true"; } case ftUnionKey: case ftEnumKey: { - if (field.IsOptional()) { return "None"; } auto ev = field.value.type.enum_def->FindByValue(field.value.constant); if (!ev) return "Default::default()"; // Bitflags enum. return WrapInNameSpace(field.value.type.enum_def->defined_namespace, GetEnumValue(*field.value.type.enum_def, *ev)); } - - // All pointer-ish types have a default value of None, because they are - // wrapped in Option. default: { - return "None"; + return field.value.constant; } } } @@ -924,30 +924,39 @@ class RustGenerator : public BaseGenerator { std::string TableBuilderArgsDefnType(const FieldDef &field, const std::string &lifetime) { const Type &type = field.value.type; + auto WrapOption = [&](std::string s) { + return IsOptionalOrRequiredNonScalar(field) ? "Option<" + s + ">" : s; + }; + auto WrapVector = [&](std::string ty) { + return WrapOption( + "flatbuffers::WIPOffset>" + ); + }; + auto WrapUOffsetsVector = [&](std::string ty) { + return WrapVector("flatbuffers::ForwardsUOffset<"+ty+">"); + }; switch (GetFullType(type)) { case ftInteger: case ftFloat: case ftBool: { - const auto typname = GetTypeBasic(type); - return field.IsOptional() ? "Option<" + typname + ">" : typname; + return WrapOption(GetTypeBasic(type)); } case ftStruct: { const auto typname = WrapInNameSpace(*type.struct_def); - return "Option<&" + lifetime + " " + typname + ">"; + return WrapOption("&" + lifetime + " " + typname); } case ftTable: { const auto typname = WrapInNameSpace(*type.struct_def); - return "Option>>"; + return WrapOption("flatbuffers::WIPOffset<" + typname + "<" + lifetime + + ">>"); } case ftString: { - return "Option>"; + return WrapOption("flatbuffers::WIPOffset<&" + lifetime + " str>"); } case ftEnumKey: case ftUnionKey: { - const auto typname = WrapInNameSpace(*type.enum_def); - return field.IsOptional() ? "Option<" + typname + ">" : typname; + return WrapOption(WrapInNameSpace(*type.enum_def)); } case ftUnionValue: { return "Option>"; @@ -957,36 +966,25 @@ class RustGenerator : public BaseGenerator { case ftVectorOfBool: case ftVectorOfFloat: { const auto typname = GetTypeBasic(type.VectorType()); - return "Option>>"; + return WrapVector(typname); } case ftVectorOfEnumKey: { const auto typname = WrapInNameSpace(*type.enum_def); - return "Option>>"; + return WrapVector(typname); } case ftVectorOfStruct: { const auto typname = WrapInNameSpace(*type.struct_def); - return "Option>>"; + return WrapVector(typname); } case ftVectorOfTable: { const auto typname = WrapInNameSpace(*type.struct_def); - return "Option>>>>"; + return WrapUOffsetsVector(typname + "<" + lifetime + ">"); } case ftVectorOfString: { - return "Option>>>"; + return WrapUOffsetsVector("&" + lifetime + " str"); } case ftVectorOfUnionValue: { - const auto typname = - WrapInNameSpace(*type.enum_def) + "UnionTableOffset"; - return "Option>>>"; + return WrapUOffsetsVector("flatbuffers::Table<"+lifetime+">"); } } return "INVALID_CODE_GENERATION"; // for return analysis @@ -1055,8 +1053,7 @@ class RustGenerator : public BaseGenerator { return "INVALID_CODE_GENERATION"; // OH NO! } } - if (in_a_table && !IsUnion(type) && - (IsScalar(type.base_type) ? field.IsOptional() : !field.IsRequired())) { + if (in_a_table && !IsUnion(type) && field.IsOptional()) { return "Option<" + ty + ">"; } else { return ty; @@ -1344,18 +1341,6 @@ class RustGenerator : public BaseGenerator { ", " + default_value + ")" + safe_slice + unwrap; } - bool TableFieldReturnsOption(const FieldDef &field) { - if (field.IsOptional()) return true; - switch (GetFullType(field.value.type)) { - case ftInteger: - case ftFloat: - case ftBool: - case ftEnumKey: - case ftUnionKey: return false; - default: return true; - } - } - // Generates a fully-qualified name getter for use with --gen-name-strings void GenFullyQualifiedNameGetter(const StructDef &struct_def, const std::string &name) { @@ -1469,7 +1454,7 @@ class RustGenerator : public BaseGenerator { if (struct_def.sortbysize && size != SizeOf(field.value.type.base_type)) return; - if (TableFieldReturnsOption(field)) { + if (IsOptionalOrRequiredNonScalar(field)) { code_ += " if let Some(x) = args.{{FIELD_NAME}} " "{ builder.add_{{FIELD_NAME}}(x); }"; diff --git a/src/idl_parser.cpp b/src/idl_parser.cpp index 8ef4d8db2b9..92f6b9f2985 100644 --- a/src/idl_parser.cpp +++ b/src/idl_parser.cpp @@ -2417,7 +2417,8 @@ bool Parser::SupportsOptionalScalars() const { } bool Parser::SupportsDefaultVectorsAndStrings() const { - static FLATBUFFERS_CONSTEXPR unsigned long supported_langs = 0; + static FLATBUFFERS_CONSTEXPR unsigned long supported_langs = + IDLOptions::kRust; return !(opts.lang_to_generate & ~supported_langs); } diff --git a/tests/generate_code.sh b/tests/generate_code.sh index 1531281e377..0f374f6f938 100755 --- a/tests/generate_code.sh +++ b/tests/generate_code.sh @@ -61,6 +61,9 @@ $TEST_NOINCL_FLAGS $TEST_CPP_FLAGS $TEST_CS_FLAGS $TEST_JS_TS_FLAGS -o namespace ../flatc --csharp --rust --gen-object-api optional_scalars.fbs ../flatc $TEST_NOINCL_FLAGS $TEST_CPP_FLAGS --cpp optional_scalars.fbs +# Generate string/vector default code for tests +../flatc --rust --gen-object-api more_defaults.fbs + # Generate the schema evolution tests ../flatc --cpp --scoped-enums $TEST_CPP_FLAGS -o evolution_test ./evolution_test/evolution_v*.fbs diff --git a/tests/more_defaults.fbs b/tests/more_defaults.fbs new file mode 100644 index 00000000000..913f2bda18c --- /dev/null +++ b/tests/more_defaults.fbs @@ -0,0 +1,7 @@ + +table MoreDefaults { + ints: [int] = []; + floats: [float] = [ ]; + empty_string: string = ""; + some_string: string = "some"; +} diff --git a/tests/more_defaults_generated.rs b/tests/more_defaults_generated.rs new file mode 100644 index 00000000000..f1453a4ca60 --- /dev/null +++ b/tests/more_defaults_generated.rs @@ -0,0 +1,201 @@ +// automatically generated by the FlatBuffers compiler, do not modify + + +#![allow(unused_imports, dead_code)] + +use std::mem; +use std::cmp::Ordering; + +extern crate flatbuffers; +use self::flatbuffers::EndianScalar; + +pub enum MoreDefaultsOffset {} +#[derive(Copy, Clone, PartialEq)] + +pub struct MoreDefaults<'a> { + pub _tab: flatbuffers::Table<'a>, +} + +impl<'a> flatbuffers::Follow<'a> for MoreDefaults<'a> { + type Inner = MoreDefaults<'a>; + #[inline] + fn follow(buf: &'a [u8], loc: usize) -> Self::Inner { + Self { _tab: flatbuffers::Table { buf, loc } } + } +} + +impl<'a> MoreDefaults<'a> { + #[inline] + pub fn init_from_table(table: flatbuffers::Table<'a>) -> Self { + MoreDefaults { _tab: table } + } + #[allow(unused_mut)] + pub fn create<'bldr: 'args, 'args: 'mut_bldr, 'mut_bldr>( + _fbb: &'mut_bldr mut flatbuffers::FlatBufferBuilder<'bldr>, + args: &'args MoreDefaultsArgs<'args>) -> flatbuffers::WIPOffset> { + let mut builder = MoreDefaultsBuilder::new(_fbb); + builder.add_some_string(args.some_string); + builder.add_empty_string(args.empty_string); + builder.add_floats(args.floats); + builder.add_ints(args.ints); + builder.finish() + } + + pub fn unpack(&self) -> MoreDefaultsT { + let ints = { + let x = self.ints(); + x.into_iter().collect() + }; + let floats = { + let x = self.floats(); + x.into_iter().collect() + }; + let empty_string = { + let x = self.empty_string(); + x.to_string() + }; + let some_string = { + let x = self.some_string(); + x.to_string() + }; + MoreDefaultsT { + ints, + floats, + empty_string, + some_string, + } + } + pub const VT_INTS: flatbuffers::VOffsetT = 4; + pub const VT_FLOATS: flatbuffers::VOffsetT = 6; + pub const VT_EMPTY_STRING: flatbuffers::VOffsetT = 8; + pub const VT_SOME_STRING: flatbuffers::VOffsetT = 10; + + #[inline] + pub fn ints(&self) -> Option> { + self._tab.get::>>(MoreDefaults::VT_INTS, Some([])).unwrap() + } + #[inline] + pub fn floats(&self) -> Option> { + self._tab.get::>>(MoreDefaults::VT_FLOATS, Some([])).unwrap() + } + #[inline] + pub fn empty_string(&self) -> Option<&'a str> { + self._tab.get::>(MoreDefaults::VT_EMPTY_STRING, Some()).unwrap() + } + #[inline] + pub fn some_string(&self) -> Option<&'a str> { + self._tab.get::>(MoreDefaults::VT_SOME_STRING, Some(some)).unwrap() + } +} + +impl flatbuffers::Verifiable for MoreDefaults<'_> { + #[inline] + fn run_verifier( + v: &mut flatbuffers::Verifier, pos: usize + ) -> Result<(), flatbuffers::InvalidFlatbuffer> { + use self::flatbuffers::Verifiable; + v.visit_table(pos)? + .visit_field::>>(&"ints", Self::VT_INTS, false)? + .visit_field::>>(&"floats", Self::VT_FLOATS, false)? + .visit_field::>(&"empty_string", Self::VT_EMPTY_STRING, false)? + .visit_field::>(&"some_string", Self::VT_SOME_STRING, false)? + .finish(); + Ok(()) + } +} +pub struct MoreDefaultsArgs<'a> { + pub ints: flatbuffers::WIPOffset>, + pub floats: flatbuffers::WIPOffset>, + pub empty_string: flatbuffers::WIPOffset<&'a str>, + pub some_string: flatbuffers::WIPOffset<&'a str>, +} +impl<'a> Default for MoreDefaultsArgs<'a> { + #[inline] + fn default() -> Self { + MoreDefaultsArgs { + ints: [], + floats: [], + empty_string: , + some_string: some, + } + } +} +pub struct MoreDefaultsBuilder<'a: 'b, 'b> { + fbb_: &'b mut flatbuffers::FlatBufferBuilder<'a>, + start_: flatbuffers::WIPOffset, +} +impl<'a: 'b, 'b> MoreDefaultsBuilder<'a, 'b> { + #[inline] + pub fn add_ints(&mut self, ints: flatbuffers::WIPOffset>) { + self.fbb_.push_slot_always::>(MoreDefaults::VT_INTS, ints); + } + #[inline] + pub fn add_floats(&mut self, floats: flatbuffers::WIPOffset>) { + self.fbb_.push_slot_always::>(MoreDefaults::VT_FLOATS, floats); + } + #[inline] + pub fn add_empty_string(&mut self, empty_string: flatbuffers::WIPOffset<&'b str>) { + self.fbb_.push_slot_always::>(MoreDefaults::VT_EMPTY_STRING, empty_string); + } + #[inline] + pub fn add_some_string(&mut self, some_string: flatbuffers::WIPOffset<&'b str>) { + self.fbb_.push_slot_always::>(MoreDefaults::VT_SOME_STRING, some_string); + } + #[inline] + pub fn new(_fbb: &'b mut flatbuffers::FlatBufferBuilder<'a>) -> MoreDefaultsBuilder<'a, 'b> { + let start = _fbb.start_table(); + MoreDefaultsBuilder { + fbb_: _fbb, + start_: start, + } + } + #[inline] + pub fn finish(self) -> flatbuffers::WIPOffset> { + let o = self.fbb_.end_table(self.start_); + flatbuffers::WIPOffset::new(o.value()) + } +} + +impl std::fmt::Debug for MoreDefaults<'_> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let mut ds = f.debug_struct("MoreDefaults"); + ds.field("ints", &self.ints()); + ds.field("floats", &self.floats()); + ds.field("empty_string", &self.empty_string()); + ds.field("some_string", &self.some_string()); + ds.finish() + } +} +#[non_exhaustive] +#[derive(Debug, Clone, PartialEq, Default)] +pub struct MoreDefaultsT { + pub ints: Vec, + pub floats: Vec, + pub empty_string: String, + pub some_string: String, +} +impl MoreDefaultsT { + pub fn pack<'b>( + &self, + _fbb: &mut flatbuffers::FlatBufferBuilder<'b> + ) -> flatbuffers::WIPOffset> { + let ints = self.ints.as_ref().map(|x|{ + _fbb.create_vector(x) + }); + let floats = self.floats.as_ref().map(|x|{ + _fbb.create_vector(x) + }); + let empty_string = self.empty_string.as_ref().map(|x|{ + _fbb.create_string(x) + }); + let some_string = self.some_string.as_ref().map(|x|{ + _fbb.create_string(x) + }); + MoreDefaults::create(_fbb, &MoreDefaultsArgs{ + ints, + floats, + empty_string, + some_string, + }) + } +} From 997af69e23467fae9a67df016c491c77f24e0c64 Mon Sep 17 00:00:00 2001 From: Casper Neo Date: Mon, 8 Feb 2021 17:08:11 -0500 Subject: [PATCH 06/11] Tested Rust more_defaults --- rust/flatbuffers/Cargo.toml | 2 +- rust/flatbuffers/src/vector.rs | 1 + src/idl_gen_rust.cpp | 102 +++++++++--------- tests/monster_test_generated.rs | 2 +- tests/more_defaults_generated.rs | 65 ++++++----- .../rust_usage_test/tests/integration_test.rs | 1 + 6 files changed, 97 insertions(+), 76 deletions(-) diff --git a/rust/flatbuffers/Cargo.toml b/rust/flatbuffers/Cargo.toml index 13d534f41a9..d2bd785ff4e 100644 --- a/rust/flatbuffers/Cargo.toml +++ b/rust/flatbuffers/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "flatbuffers" -version = "0.8.2" +version = "0.8.3" edition = "2018" authors = ["Robert Winslow ", "FlatBuffers Maintainers"] license = "Apache-2.0" diff --git a/rust/flatbuffers/src/vector.rs b/rust/flatbuffers/src/vector.rs index c53a878cf8b..adb8532e2e6 100644 --- a/rust/flatbuffers/src/vector.rs +++ b/rust/flatbuffers/src/vector.rs @@ -27,6 +27,7 @@ use crate::endian_scalar::EndianScalar; use crate::follow::Follow; use crate::primitives::*; +#[derive(Default)] pub struct Vector<'a, T: 'a>(&'a [u8], usize, PhantomData); impl<'a, T> Debug for Vector<'a, T> diff --git a/src/idl_gen_rust.cpp b/src/idl_gen_rust.cpp index 305e645b51d..3628ea1e54a 100644 --- a/src/idl_gen_rust.cpp +++ b/src/idl_gen_rust.cpp @@ -185,8 +185,8 @@ bool IsBitFlagsEnum(const FieldDef &field) { // TableArgs make required non-scalars "Option<_>". // TODO(cneo): Rework how we do defaults and stuff. -bool IsOptionalOrRequiredNonScalar(const FieldDef &field) { - return field.IsOptional() || (field.IsRequired() && !IsScalar(field.value.type.base_type)); +bool IsOptionalToBuilder(const FieldDef &field) { + return field.IsOptional() || !IsScalar(field.value.type.base_type); } namespace rust { @@ -889,8 +889,9 @@ class RustGenerator : public BaseGenerator { return "VT_" + MakeUpper(Name(field)); } - std::string GetDefaultValue(const FieldDef &field, bool for_builder) { - if (for_builder) { + enum DefaultContext { kBuilder, kAccessor, kObject }; + std::string GetDefaultValue(const FieldDef &field, const DefaultContext context) { + if (context == kBuilder) { // Builders and Args structs model nonscalars "optional" even if they're // required or have defaults according to the schema. I guess its because // WIPOffset is not nullable. @@ -921,8 +922,16 @@ class RustGenerator : public BaseGenerator { return ObjectFieldType(field, true) + "::NONE"; } case ftString: { - // Required strings. - return "String::new()"; // No default strings yet. + // Required fields do not have defaults defined by the schema, but we + // need one for Rust's Default trait so we use empty string. The usual + // value of field.value.constant is `0`, which is non-sensical except + // maybe to c++ (nullptr == 0). + // TODO: Escape strings? + const std::string defval = + field.IsRequired() ? "\"\"" : "\"" + field.value.constant + "\""; + if (context == kObject) return defval + ".to_string()"; + if (context == kAccessor) return "&" + defval; + FLATBUFFERS_ASSERT("Unreachable."); } case ftVectorOfBool: case ftVectorOfFloat: @@ -933,7 +942,11 @@ class RustGenerator : public BaseGenerator { case ftVectorOfEnumKey: case ftVectorOfUnionValue: { // Required vectors. - return "Vec::new()"; // No default strings yet. + return "Default::default()"; + // if (context == kObject) return "Vec::new()"; + // if (context == kAccessor) return "&[]"; + // FLATBUFFERS_ASSERT("Unreachable."); + } case ftStruct: case ftTable: { @@ -960,7 +973,7 @@ class RustGenerator : public BaseGenerator { const std::string &lifetime) { const Type &type = field.value.type; auto WrapOption = [&](std::string s) { - return IsOptionalOrRequiredNonScalar(field) ? "Option<" + s + ">" : s; + return IsOptionalToBuilder(field) ? "Option<" + s + ">" : s; }; auto WrapVector = [&](std::string ty) { return WrapOption( @@ -1210,73 +1223,66 @@ class RustGenerator : public BaseGenerator { std::string GenTableAccessorFuncReturnType(const FieldDef &field, const std::string &lifetime) { const Type &type = field.value.type; + const auto WrapOption = [&](std::string s) { + return field.IsOptional() ? "Option<" + s + ">" : s; + }; switch (GetFullType(field.value.type)) { case ftInteger: case ftFloat: case ftBool: { - const auto typname = GetTypeBasic(type); - return field.IsOptional() ? "Option<" + typname + ">" : typname; + return WrapOption(GetTypeBasic(type)); } case ftStruct: { const auto typname = WrapInNameSpace(*type.struct_def); - return WrapInOptionIfNotRequired("&" + lifetime + " " + typname, - field.IsRequired()); + return WrapOption("&" + lifetime + " " + typname); } case ftTable: { const auto typname = WrapInNameSpace(*type.struct_def); - return WrapInOptionIfNotRequired(typname + "<" + lifetime + ">", - field.IsRequired()); + return WrapOption(typname + "<" + lifetime + ">"); } case ftEnumKey: case ftUnionKey: { - const auto typname = WrapInNameSpace(*type.enum_def); - return field.IsOptional() ? "Option<" + typname + ">" : typname; + return WrapOption(WrapInNameSpace(*type.enum_def)); } case ftUnionValue: { - return WrapInOptionIfNotRequired("flatbuffers::Table<" + lifetime + ">", - field.IsRequired()); + return WrapOption("flatbuffers::Table<" + lifetime + ">"); } case ftString: { - return WrapInOptionIfNotRequired("&" + lifetime + " str", - field.IsRequired()); + return WrapOption("&" + lifetime + " str"); } case ftVectorOfInteger: case ftVectorOfBool: case ftVectorOfFloat: { const auto typname = GetTypeBasic(type.VectorType()); - if (IsOneByte(type.VectorType().base_type)) { - return WrapInOptionIfNotRequired( - "&" + lifetime + " [" + typname + "]", field.IsRequired()); - } - return WrapInOptionIfNotRequired( - "flatbuffers::Vector<" + lifetime + ", " + typname + ">", - field.IsRequired()); + const auto vector_type = + IsOneByte(type.VectorType().base_type) ? + "&" + lifetime + " [" + typname + "]" : + "flatbuffers::Vector<" + lifetime + ", " + typname + ">"; + return WrapOption(vector_type); } case ftVectorOfEnumKey: { const auto typname = WrapInNameSpace(*type.enum_def); - return WrapInOptionIfNotRequired( - "flatbuffers::Vector<" + lifetime + ", " + typname + ">", - field.IsRequired()); + return WrapOption("flatbuffers::Vector<" + lifetime + ", " + + typname + ">"); } case ftVectorOfStruct: { const auto typname = WrapInNameSpace(*type.struct_def); - return WrapInOptionIfNotRequired("&" + lifetime + " [" + typname + "]", - field.IsRequired()); + return WrapOption("&" + lifetime + " [" + typname + "]"); } case ftVectorOfTable: { const auto typname = WrapInNameSpace(*type.struct_def); - return WrapInOptionIfNotRequired("flatbuffers::Vector<" + lifetime + - ", flatbuffers::ForwardsUOffset<" + - typname + "<" + lifetime + ">>>", - field.IsRequired()); + return WrapOption( + "flatbuffers::Vector<" + lifetime + + ", flatbuffers::ForwardsUOffset<" + + typname + "<" + lifetime + ">>>" + ); } case ftVectorOfString: { - return WrapInOptionIfNotRequired( + return WrapOption( "flatbuffers::Vector<" + lifetime + - ", flatbuffers::ForwardsUOffset<&" + lifetime + " str>>", - field.IsRequired()); + ", flatbuffers::ForwardsUOffset<&" + lifetime + " str>>"); } case ftVectorOfUnionValue: { FLATBUFFERS_ASSERT(false && "vectors of unions are not yet supported"); @@ -1357,7 +1363,7 @@ class RustGenerator : public BaseGenerator { // Default-y fields (scalars so far) are neither optional nor required. const std::string default_value = !(field.IsOptional() || field.IsRequired()) - ? "Some(" + GetDefaultValue(field, /*builder=*/true) + ")" + ? "Some(" + GetDefaultValue(field, kAccessor) + ")" : "None"; const std::string unwrap = field.IsOptional() ? "" : ".unwrap()"; @@ -1416,7 +1422,7 @@ class RustGenerator : public BaseGenerator { code_.SetValue("OFFSET_NAME", GetFieldOffsetName(field)); code_.SetValue("OFFSET_VALUE", NumToString(field.value.offset)); code_.SetValue("FIELD_NAME", Name(field)); - code_.SetValue("BLDR_DEF_VAL", GetDefaultValue(field, /*builder=*/true)); + code_.SetValue("BLDR_DEF_VAL", GetDefaultValue(field, kBuilder)); cb(field); }; const auto &fields = struct_def.fields.vec; @@ -1489,7 +1495,7 @@ class RustGenerator : public BaseGenerator { if (struct_def.sortbysize && size != SizeOf(field.value.type.base_type)) return; - if (IsOptionalOrRequiredNonScalar(field)) { + if (IsOptionalToBuilder(field)) { code_ += " if let Some(x) = args.{{FIELD_NAME}} " "{ builder.add_{{FIELD_NAME}}(x); }"; @@ -1934,7 +1940,7 @@ class RustGenerator : public BaseGenerator { code_ += " Self {"; ForAllObjectTableFields(table, [&](const FieldDef &field) { if (field.value.type.base_type == BASE_TYPE_UTYPE) return; - std::string default_value = GetDefaultValue(field, /*builder=*/false); + std::string default_value = GetDefaultValue(field, kObject); code_ += " {{FIELD_NAME}}: " + default_value + ","; }); code_ += " }"; @@ -2055,17 +2061,17 @@ class RustGenerator : public BaseGenerator { } } void MapNativeTableField(const FieldDef &field, const std::string &expr) { - if (field.IsRequired()) { + if (field.IsOptional()) { + code_ += " let {{FIELD_NAME}} = self.{{FIELD_NAME}}.as_ref().map(|x|{"; + code_ += " " + expr; + code_ += " });"; + } else { // For some reason Args has optional types for required fields. // TODO(cneo): Fix this... but its a breaking change? code_ += " let {{FIELD_NAME}} = Some({"; code_ += " let x = &self.{{FIELD_NAME}};"; code_ += " " + expr; code_ += " });"; - } else { - code_ += " let {{FIELD_NAME}} = self.{{FIELD_NAME}}.as_ref().map(|x|{"; - code_ += " " + expr; - code_ += " });"; } } diff --git a/tests/monster_test_generated.rs b/tests/monster_test_generated.rs index 2e4b840c98c..0102c74e0db 100644 --- a/tests/monster_test_generated.rs +++ b/tests/monster_test_generated.rs @@ -3197,7 +3197,7 @@ impl Default for MonsterT { pos: None, mana: 150, hp: 100, - name: String::new(), + name: "".to_string(), inventory: None, color: Color::Blue, test: AnyT::NONE, diff --git a/tests/more_defaults_generated.rs b/tests/more_defaults_generated.rs index f1453a4ca60..800677e4a10 100644 --- a/tests/more_defaults_generated.rs +++ b/tests/more_defaults_generated.rs @@ -1,7 +1,6 @@ // automatically generated by the FlatBuffers compiler, do not modify -#![allow(unused_imports, dead_code)] use std::mem; use std::cmp::Ordering; @@ -34,10 +33,10 @@ impl<'a> MoreDefaults<'a> { _fbb: &'mut_bldr mut flatbuffers::FlatBufferBuilder<'bldr>, args: &'args MoreDefaultsArgs<'args>) -> flatbuffers::WIPOffset> { let mut builder = MoreDefaultsBuilder::new(_fbb); - builder.add_some_string(args.some_string); - builder.add_empty_string(args.empty_string); - builder.add_floats(args.floats); - builder.add_ints(args.ints); + if let Some(x) = args.some_string { builder.add_some_string(x); } + if let Some(x) = args.empty_string { builder.add_empty_string(x); } + if let Some(x) = args.floats { builder.add_floats(x); } + if let Some(x) = args.ints { builder.add_ints(x); } builder.finish() } @@ -71,20 +70,20 @@ impl<'a> MoreDefaults<'a> { pub const VT_SOME_STRING: flatbuffers::VOffsetT = 10; #[inline] - pub fn ints(&self) -> Option> { - self._tab.get::>>(MoreDefaults::VT_INTS, Some([])).unwrap() + pub fn ints(&self) -> flatbuffers::Vector<'a, i32> { + self._tab.get::>>(MoreDefaults::VT_INTS, Some(Default::default())).unwrap() } #[inline] - pub fn floats(&self) -> Option> { - self._tab.get::>>(MoreDefaults::VT_FLOATS, Some([])).unwrap() + pub fn floats(&self) -> flatbuffers::Vector<'a, f32> { + self._tab.get::>>(MoreDefaults::VT_FLOATS, Some(Default::default())).unwrap() } #[inline] - pub fn empty_string(&self) -> Option<&'a str> { - self._tab.get::>(MoreDefaults::VT_EMPTY_STRING, Some()).unwrap() + pub fn empty_string(&self) -> &'a str { + self._tab.get::>(MoreDefaults::VT_EMPTY_STRING, Some(&"")).unwrap() } #[inline] - pub fn some_string(&self) -> Option<&'a str> { - self._tab.get::>(MoreDefaults::VT_SOME_STRING, Some(some)).unwrap() + pub fn some_string(&self) -> &'a str { + self._tab.get::>(MoreDefaults::VT_SOME_STRING, Some(&"some")).unwrap() } } @@ -104,19 +103,19 @@ impl flatbuffers::Verifiable for MoreDefaults<'_> { } } pub struct MoreDefaultsArgs<'a> { - pub ints: flatbuffers::WIPOffset>, - pub floats: flatbuffers::WIPOffset>, - pub empty_string: flatbuffers::WIPOffset<&'a str>, - pub some_string: flatbuffers::WIPOffset<&'a str>, + pub ints: Option>>, + pub floats: Option>>, + pub empty_string: Option>, + pub some_string: Option>, } impl<'a> Default for MoreDefaultsArgs<'a> { #[inline] fn default() -> Self { MoreDefaultsArgs { - ints: [], - floats: [], - empty_string: , - some_string: some, + ints: None, + floats: None, + empty_string: None, + some_string: None, } } } @@ -167,28 +166,42 @@ impl std::fmt::Debug for MoreDefaults<'_> { } } #[non_exhaustive] -#[derive(Debug, Clone, PartialEq, Default)] +#[derive(Debug, Clone, PartialEq)] pub struct MoreDefaultsT { pub ints: Vec, pub floats: Vec, pub empty_string: String, pub some_string: String, } +impl Default for MoreDefaultsT { + fn default() -> Self { + Self { + ints: Default::default(), + floats: Default::default(), + empty_string: "".to_string(), + some_string: "some".to_string(), + } + } +} impl MoreDefaultsT { pub fn pack<'b>( &self, _fbb: &mut flatbuffers::FlatBufferBuilder<'b> ) -> flatbuffers::WIPOffset> { - let ints = self.ints.as_ref().map(|x|{ + let ints = Some({ + let x = &self.ints; _fbb.create_vector(x) }); - let floats = self.floats.as_ref().map(|x|{ + let floats = Some({ + let x = &self.floats; _fbb.create_vector(x) }); - let empty_string = self.empty_string.as_ref().map(|x|{ + let empty_string = Some({ + let x = &self.empty_string; _fbb.create_string(x) }); - let some_string = self.some_string.as_ref().map(|x|{ + let some_string = Some({ + let x = &self.some_string; _fbb.create_string(x) }); MoreDefaults::create(_fbb, &MoreDefaultsArgs{ diff --git a/tests/rust_usage_test/tests/integration_test.rs b/tests/rust_usage_test/tests/integration_test.rs index 9bbbe88a7f0..ccc69d2b1bb 100644 --- a/tests/rust_usage_test/tests/integration_test.rs +++ b/tests/rust_usage_test/tests/integration_test.rs @@ -30,6 +30,7 @@ extern crate quickcheck_derive; mod flexbuffers_tests; mod optional_scalars_test; +mod more_defaults_test; #[allow(dead_code, unused_imports)] #[path = "../../include_test/include_test1_generated.rs"] From d6464c7b4ef9d3e2ec78018538a8097a7d559280 Mon Sep 17 00:00:00 2001 From: Casper Neo Date: Mon, 8 Feb 2021 17:11:09 -0500 Subject: [PATCH 07/11] git-clang-format --- src/idl_gen_rust.cpp | 43 ++++++++++++++++++++----------------------- src/idl_parser.cpp | 8 +++----- tests/test.cpp | 20 ++++++++++---------- 3 files changed, 33 insertions(+), 38 deletions(-) diff --git a/src/idl_gen_rust.cpp b/src/idl_gen_rust.cpp index 3628ea1e54a..6d9a141e833 100644 --- a/src/idl_gen_rust.cpp +++ b/src/idl_gen_rust.cpp @@ -186,7 +186,7 @@ bool IsBitFlagsEnum(const FieldDef &field) { // TableArgs make required non-scalars "Option<_>". // TODO(cneo): Rework how we do defaults and stuff. bool IsOptionalToBuilder(const FieldDef &field) { - return field.IsOptional() || !IsScalar(field.value.type.base_type); + return field.IsOptional() || !IsScalar(field.value.type.base_type); } namespace rust { @@ -890,7 +890,8 @@ class RustGenerator : public BaseGenerator { } enum DefaultContext { kBuilder, kAccessor, kObject }; - std::string GetDefaultValue(const FieldDef &field, const DefaultContext context) { + std::string GetDefaultValue(const FieldDef &field, + const DefaultContext context) { if (context == kBuilder) { // Builders and Args structs model nonscalars "optional" even if they're // required or have defaults according to the schema. I guess its because @@ -946,7 +947,6 @@ class RustGenerator : public BaseGenerator { // if (context == kObject) return "Vec::new()"; // if (context == kAccessor) return "&[]"; // FLATBUFFERS_ASSERT("Unreachable."); - } case ftStruct: case ftTable: { @@ -973,15 +973,14 @@ class RustGenerator : public BaseGenerator { const std::string &lifetime) { const Type &type = field.value.type; auto WrapOption = [&](std::string s) { - return IsOptionalToBuilder(field) ? "Option<" + s + ">" : s; + return IsOptionalToBuilder(field) ? "Option<" + s + ">" : s; }; auto WrapVector = [&](std::string ty) { - return WrapOption( - "flatbuffers::WIPOffset>" - ); + return WrapOption("flatbuffers::WIPOffset>"); }; auto WrapUOffsetsVector = [&](std::string ty) { - return WrapVector("flatbuffers::ForwardsUOffset<"+ty+">"); + return WrapVector("flatbuffers::ForwardsUOffset<" + ty + ">"); }; switch (GetFullType(type)) { @@ -997,7 +996,7 @@ class RustGenerator : public BaseGenerator { case ftTable: { const auto typname = WrapInNameSpace(*type.struct_def); return WrapOption("flatbuffers::WIPOffset<" + typname + "<" + lifetime + - ">>"); + ">>"); } case ftString: { return WrapOption("flatbuffers::WIPOffset<&" + lifetime + " str>"); @@ -1032,7 +1031,7 @@ class RustGenerator : public BaseGenerator { return WrapUOffsetsVector("&" + lifetime + " str"); } case ftVectorOfUnionValue: { - return WrapUOffsetsVector("flatbuffers::Table<"+lifetime+">"); + return WrapUOffsetsVector("flatbuffers::Table<" + lifetime + ">"); } } return "INVALID_CODE_GENERATION"; // for return analysis @@ -1257,15 +1256,15 @@ class RustGenerator : public BaseGenerator { case ftVectorOfFloat: { const auto typname = GetTypeBasic(type.VectorType()); const auto vector_type = - IsOneByte(type.VectorType().base_type) ? - "&" + lifetime + " [" + typname + "]" : - "flatbuffers::Vector<" + lifetime + ", " + typname + ">"; + IsOneByte(type.VectorType().base_type) + ? "&" + lifetime + " [" + typname + "]" + : "flatbuffers::Vector<" + lifetime + ", " + typname + ">"; return WrapOption(vector_type); } case ftVectorOfEnumKey: { const auto typname = WrapInNameSpace(*type.enum_def); - return WrapOption("flatbuffers::Vector<" + lifetime + ", " + - typname + ">"); + return WrapOption("flatbuffers::Vector<" + lifetime + ", " + typname + + ">"); } case ftVectorOfStruct: { const auto typname = WrapInNameSpace(*type.struct_def); @@ -1273,16 +1272,14 @@ class RustGenerator : public BaseGenerator { } case ftVectorOfTable: { const auto typname = WrapInNameSpace(*type.struct_def); - return WrapOption( - "flatbuffers::Vector<" + lifetime + - ", flatbuffers::ForwardsUOffset<" + - typname + "<" + lifetime + ">>>" - ); + return WrapOption("flatbuffers::Vector<" + lifetime + + ", flatbuffers::ForwardsUOffset<" + typname + "<" + + lifetime + ">>>"); } case ftVectorOfString: { - return WrapOption( - "flatbuffers::Vector<" + lifetime + - ", flatbuffers::ForwardsUOffset<&" + lifetime + " str>>"); + return WrapOption("flatbuffers::Vector<" + lifetime + + ", flatbuffers::ForwardsUOffset<&" + lifetime + + " str>>"); } case ftVectorOfUnionValue: { FLATBUFFERS_ASSERT(false && "vectors of unions are not yet supported"); diff --git a/src/idl_parser.cpp b/src/idl_parser.cpp index 92f6b9f2985..65feeebe334 100644 --- a/src/idl_parser.cpp +++ b/src/idl_parser.cpp @@ -1309,7 +1309,7 @@ CheckedError Parser::ParseTable(const StructDef &struct_def, std::string *value, if (!struct_def.sortbysize || size == SizeOf(field_value.type.base_type)) { switch (field_value.type.base_type) { -// clang-format off + // clang-format off #define FLATBUFFERS_TD(ENUM, IDLTYPE, CTYPE, ...) \ case BASE_TYPE_ ## ENUM: \ builder_.Pad(field->padding); \ @@ -1495,7 +1495,7 @@ CheckedError Parser::ParseVector(const Type &type, uoffset_t *ovalue, // start at the back, since we're building the data backwards. auto &val = field_stack_.back().first; switch (val.type.base_type) { -// clang-format off + // clang-format off #define FLATBUFFERS_TD(ENUM, IDLTYPE, CTYPE,...) \ case BASE_TYPE_ ## ENUM: \ if (IsStruct(val.type)) SerializeStruct(*val.type.struct_def, val); \ @@ -1959,9 +1959,7 @@ CheckedError Parser::ParseSingleValue(const std::string *name, Value &e, // Match empty vectors for default-empty-vectors. if (!match && IsVector(e.type) && token_ == '[') { NEXT(); - if (token_ != ']') { - return Error("Expected `]` in vector default"); - } + if (token_ != ']') { return Error("Expected `]` in vector default"); } NEXT(); match = true; e.constant = "[]"; diff --git a/tests/test.cpp b/tests/test.cpp index a60bcfaf1d5..f745bc7c384 100644 --- a/tests/test.cpp +++ b/tests/test.cpp @@ -107,7 +107,7 @@ flatbuffers::DetachedBuffer CreateFlatBufferTest(std::string &buffer) { Test tests[] = { Test(10, 20), Test(30, 40) }; auto testv = builder.CreateVectorOfStructs(tests, 2); -// clang-format off + // clang-format off #ifndef FLATBUFFERS_CPP98_STL // Create a vector of structures from a lambda. auto testv2 = builder.CreateVectorOfStructs( @@ -223,7 +223,7 @@ flatbuffers::DetachedBuffer CreateFlatBufferTest(std::string &buffer) { FinishMonsterBuffer(builder, mloc); -// clang-format off + // clang-format off #ifdef FLATBUFFERS_TEST_VERBOSE // print byte data for debugging: auto p = builder.GetBufferPointer(); @@ -247,7 +247,7 @@ void AccessFlatBufferTest(const uint8_t *flatbuf, size_t length, flatbuffers::Verifier verifier(flatbuf, length); TEST_EQ(VerifyMonsterBuffer(verifier), true); -// clang-format off + // clang-format off #ifdef FLATBUFFERS_TRACK_VERIFIER_BUFFER_SIZE std::vector test_buff; test_buff.resize(length * 2); @@ -602,7 +602,7 @@ void SizePrefixedTest() { } void TriviallyCopyableTest() { -// clang-format off + // clang-format off #if __GNUG__ && __GNUC__ < 5 TEST_EQ(__has_trivial_copy(Vec3), true); #else @@ -1438,7 +1438,7 @@ void FuzzTest2() { } }; -// clang-format off + // clang-format off #define AddToSchemaAndInstances(schema_add, instance_add) \ RndDef::Add(definitions, schema, instances_per_definition, \ schema_add, instance_add, definition) @@ -1590,7 +1590,7 @@ void FuzzTest2() { TEST_NOTNULL(nullptr); //-V501 (this comment supresses CWE-570 warning) } -// clang-format off + // clang-format off #ifdef FLATBUFFERS_TEST_VERBOSE TEST_OUTPUT_LINE("%dk schema tested with %dk of json\n", static_cast(schema.length() / 1024), @@ -2869,10 +2869,10 @@ void FlexBuffersTest() { flexbuffers::Builder slb(512, flexbuffers::BUILDER_FLAG_SHARE_KEYS_AND_STRINGS); -// Write the equivalent of: -// { vec: [ -100, "Fred", 4.0, false ], bar: [ 1, 2, 3 ], bar3: [ 1, 2, 3 ], -// foo: 100, bool: true, mymap: { foo: "Fred" } } -// clang-format off + // Write the equivalent of: + // { vec: [ -100, "Fred", 4.0, false ], bar: [ 1, 2, 3 ], bar3: [ 1, 2, 3 ], + // foo: 100, bool: true, mymap: { foo: "Fred" } } + // clang-format off #ifndef FLATBUFFERS_CPP98_STL // It's possible to do this without std::function support as well. slb.Map([&]() { From d3e0d84326a11f77fc48076ec6c583bf3680e363 Mon Sep 17 00:00:00 2001 From: Casper Neo Date: Mon, 8 Feb 2021 17:41:35 -0500 Subject: [PATCH 08/11] add more_defaults_test --- .../tests/more_defaults_test.rs | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 tests/rust_usage_test/tests/more_defaults_test.rs diff --git a/tests/rust_usage_test/tests/more_defaults_test.rs b/tests/rust_usage_test/tests/more_defaults_test.rs new file mode 100644 index 00000000000..5127bb9641d --- /dev/null +++ b/tests/rust_usage_test/tests/more_defaults_test.rs @@ -0,0 +1,26 @@ +#[allow(dead_code, unused_imports)] +#[path = "../../more_defaults_generated.rs"] +mod more_defaults_generated; +use self::more_defaults_generated::*; + +#[test] +fn object_defaults() { + assert_eq!( + MoreDefaultsT::default(), + MoreDefaultsT { + ints: Vec::new(), + floats: Vec::new(), + empty_string: "".to_string(), + some_string: "some".to_string(), + }, + ) +} + +#[test] +fn nonpresent_values() { + let m = flatbuffers::root::(&[0; 4]).unwrap(); + assert_eq!(m.ints().len(), 0); + assert_eq!(m.floats().len(), 0); + assert_eq!(m.empty_string(), ""); + assert_eq!(m.some_string(), "some"); +} From 7f83ce438e4339e3bcc83206541456ca5e915fa9 Mon Sep 17 00:00:00 2001 From: Casper Neo Date: Mon, 8 Feb 2021 18:20:45 -0500 Subject: [PATCH 09/11] fixed vector default --- rust/flatbuffers/src/vector.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/rust/flatbuffers/src/vector.rs b/rust/flatbuffers/src/vector.rs index adb8532e2e6..b54d3ec89a6 100644 --- a/rust/flatbuffers/src/vector.rs +++ b/rust/flatbuffers/src/vector.rs @@ -27,9 +27,16 @@ use crate::endian_scalar::EndianScalar; use crate::follow::Follow; use crate::primitives::*; -#[derive(Default)] pub struct Vector<'a, T: 'a>(&'a [u8], usize, PhantomData); +impl<'a, T:'a> Default for Vector<'a, T> { + fn default() -> Self { + // Static, length 0 vector. + // Note that derived default causes UB due to issues in read_scalar_at /facepalm. + Self(&[0; core::mem::size_of::()], 0, Default::default()) + } +} + impl<'a, T> Debug for Vector<'a, T> where T: 'a + Follow<'a>, From 57e3b18faf41aa9053ed04a97da43f731fbe821b Mon Sep 17 00:00:00 2001 From: Casper Neo Date: Mon, 8 Feb 2021 18:28:32 -0500 Subject: [PATCH 10/11] removed commented out code --- src/idl_gen_rust.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/idl_gen_rust.cpp b/src/idl_gen_rust.cpp index 6d9a141e833..34735735df8 100644 --- a/src/idl_gen_rust.cpp +++ b/src/idl_gen_rust.cpp @@ -934,6 +934,7 @@ class RustGenerator : public BaseGenerator { if (context == kAccessor) return "&" + defval; FLATBUFFERS_ASSERT("Unreachable."); } + case ftVectorOfBool: case ftVectorOfFloat: case ftVectorOfInteger: @@ -941,17 +942,16 @@ class RustGenerator : public BaseGenerator { case ftVectorOfStruct: case ftVectorOfTable: case ftVectorOfEnumKey: - case ftVectorOfUnionValue: { - // Required vectors. - return "Default::default()"; - // if (context == kObject) return "Vec::new()"; - // if (context == kAccessor) return "&[]"; - // FLATBUFFERS_ASSERT("Unreachable."); - } + case ftVectorOfUnionValue: case ftStruct: case ftTable: { - // Required struct/tables. - return "Default::default()"; // punt. + // We only support empty vectors which matches the defaults for + // &[T] and Vec anyway. + // + // For required structs and tables fields, we defer to their object API + // defaults. This works so long as there's nothing recursive happening, + // but `table Infinity { i: Infinity (required); }` does compile. + return "Default::default()"; } } FLATBUFFERS_ASSERT("Unreachable."); From a5827551158b0d242c863d36426ea3ebb30c84d1 Mon Sep 17 00:00:00 2001 From: Casper Neo Date: Thu, 11 Feb 2021 08:33:02 -0500 Subject: [PATCH 11/11] more unreachable --- src/idl_gen_rust.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/idl_gen_rust.cpp b/src/idl_gen_rust.cpp index 34735735df8..dc26fa32ad7 100644 --- a/src/idl_gen_rust.cpp +++ b/src/idl_gen_rust.cpp @@ -933,6 +933,7 @@ class RustGenerator : public BaseGenerator { if (context == kObject) return defval + ".to_string()"; if (context == kAccessor) return "&" + defval; FLATBUFFERS_ASSERT("Unreachable."); + return "INVALID_CODE_GENERATION"; } case ftVectorOfBool: