Skip to content

Default strings and vectors: Parser + Rust support #6421

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 12 commits into from
Feb 12, 2021
Merged

Default strings and vectors: Parser + Rust support #6421

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
ec58b0f
Fix tests.cpp
Jan 24, 2021
cd9a9d7
Parser support for vector/string defaults
Jan 24, 2021
0113860
tests and default empty vectors
Feb 2, 2021
3671891
addressed comments
Feb 3, 2021
7ff69af
Default strings and vectors for Rust
Feb 4, 2021
5787a6a
Merge branch 'master' of https://github.com/google/flatbuffers into def
Feb 7, 2021
997af69
Tested Rust more_defaults
Feb 8, 2021
d6464c7
git-clang-format
Feb 8, 2021
d3e0d84
add more_defaults_test
Feb 8, 2021
7f83ce4
fixed vector default
Feb 8, 2021
57e3b18
removed commented out code
Feb 8, 2021
a582755
more unreachable
Feb 11, 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
2 changes: 2 additions & 0 deletions include/flatbuffers/idl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand Down
45 changes: 37 additions & 8 deletions src/idl_parser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -794,10 +794,19 @@ 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).
Expand Down Expand Up @@ -856,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.");
}
Expand Down Expand Up @@ -1297,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); \
Expand Down Expand Up @@ -1483,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); \
Expand Down Expand Up @@ -1944,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++;
Comment thread
CasperN marked this conversation as resolved.
Outdated
if (*c == ']') {
e.constant = "[]";
NEXT();
NEXT();
match = true;
}
}

#undef FORCE_ECHECK
#undef TRY_ECHECK
#undef IF_ECHECK_
Expand Down Expand Up @@ -2388,12 +2413,16 @@ 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;
return !(opts.lang_to_generate & ~supported_langs);
}

bool Parser::SupportsAdvancedUnionFeatures() const {
return opts.lang_to_generate != 0 &&
(opts.lang_to_generate &
Expand Down
44 changes: 32 additions & 12 deletions tests/test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment thread
CasperN marked this conversation as resolved.
Outdated
#ifndef FLATBUFFERS_CPP98_STL
// Create a vector of structures from a lambda.
auto testv2 = builder.CreateVectorOfStructs<Test>(
Expand Down Expand Up @@ -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
Expand All @@ -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();
Expand All @@ -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<uint8_t> test_buff;
test_buff.resize(length * 2);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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<int>(schema.length() / 1024),
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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<typename T>
Expand Down Expand Up @@ -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:
Comment thread
CasperN marked this conversation as resolved.
Outdated
// { 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([&]() {
Expand Down Expand Up @@ -3620,6 +3623,22 @@ void TestEmbeddedBinarySchema() {
0);
}

void MoreDefaultsTest() {
Comment thread
CasperN marked this conversation as resolved.
Outdated
std::vector<std::string> 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<std::string> schemas;
Expand Down Expand Up @@ -3852,6 +3871,7 @@ int FlatBufferTests() {
FlatbuffersSpanTest();
FixedLengthArrayConstructorTest();
FieldIdentifierTest();
MoreDefaultsTest();
return 0;
}

Expand Down