Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions include/flatbuffers/idl.h
Original file line number Diff line number Diff line change
Expand Up @@ -227,9 +227,12 @@ struct Type {
struct Value {
Value()
: constant("0"),
has_non_scalar_constant(false),
offset(static_cast<voffset_t>(~(static_cast<voffset_t>(0U)))) {}
Type type;
std::string constant;
// not the most elegant solution, but much better than a full refactor.
bool has_non_scalar_constant;
voffset_t offset;
};

Expand Down
22 changes: 18 additions & 4 deletions include/flatbuffers/reflection_generated.h
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,8 @@ struct Field FLATBUFFERS_FINAL_CLASS : private ::flatbuffers::Table {
VT_DOCUMENTATION = 24,
VT_OPTIONAL = 26,
VT_PADDING = 28,
VT_OFFSET64 = 30
VT_OFFSET64 = 30,
VT_DEFAULT_NON_SCALAR = 32
};
const ::flatbuffers::String *name() const {
return GetPointer<const ::flatbuffers::String *>(VT_NAME);
Expand Down Expand Up @@ -679,6 +680,9 @@ struct Field FLATBUFFERS_FINAL_CLASS : private ::flatbuffers::Table {
bool offset64() const {
return GetField<uint8_t>(VT_OFFSET64, 0) != 0;
}
const ::flatbuffers::String *default_non_scalar() const {
return GetPointer<const ::flatbuffers::String *>(VT_DEFAULT_NON_SCALAR);
}
template <bool B = false>
bool Verify(::flatbuffers::VerifierTemplate<B> &verifier) const {
return VerifyTableStart(verifier) &&
Expand All @@ -702,6 +706,8 @@ struct Field FLATBUFFERS_FINAL_CLASS : private ::flatbuffers::Table {
VerifyField<uint8_t>(verifier, VT_OPTIONAL, 1) &&
VerifyField<uint16_t>(verifier, VT_PADDING, 2) &&
VerifyField<uint8_t>(verifier, VT_OFFSET64, 1) &&
VerifyOffset(verifier, VT_DEFAULT_NON_SCALAR) &&
verifier.VerifyString(default_non_scalar()) &&
verifier.EndTable();
}
};
Expand Down Expand Up @@ -752,6 +758,9 @@ struct FieldBuilder {
void add_offset64(bool offset64) {
fbb_.AddElement<uint8_t>(Field::VT_OFFSET64, static_cast<uint8_t>(offset64), 0);
}
void add_default_non_scalar(::flatbuffers::Offset<::flatbuffers::String> default_non_scalar) {
fbb_.AddOffset(Field::VT_DEFAULT_NON_SCALAR, default_non_scalar);
}
explicit FieldBuilder(::flatbuffers::FlatBufferBuilder &_fbb)
: fbb_(_fbb) {
start_ = fbb_.StartTable();
Expand Down Expand Up @@ -780,10 +789,12 @@ inline ::flatbuffers::Offset<Field> CreateField(
::flatbuffers::Offset<::flatbuffers::Vector<::flatbuffers::Offset<::flatbuffers::String>>> documentation = 0,
bool optional = false,
uint16_t padding = 0,
bool offset64 = false) {
bool offset64 = false,
::flatbuffers::Offset<::flatbuffers::String> default_non_scalar = 0) {
FieldBuilder builder_(_fbb);
builder_.add_default_real(default_real);
builder_.add_default_integer(default_integer);
builder_.add_default_non_scalar(default_non_scalar);
builder_.add_documentation(documentation);
builder_.add_attributes(attributes);
builder_.add_type(type);
Expand Down Expand Up @@ -814,10 +825,12 @@ inline ::flatbuffers::Offset<Field> CreateFieldDirect(
const std::vector<::flatbuffers::Offset<::flatbuffers::String>> *documentation = nullptr,
bool optional = false,
uint16_t padding = 0,
bool offset64 = false) {
bool offset64 = false,
const char *default_non_scalar = nullptr) {
auto name__ = name ? _fbb.CreateString(name) : 0;
auto attributes__ = attributes ? _fbb.CreateVectorOfSortedTables<reflection::KeyValue>(attributes) : 0;
auto documentation__ = documentation ? _fbb.CreateVector<::flatbuffers::Offset<::flatbuffers::String>>(*documentation) : 0;
auto default_non_scalar__ = default_non_scalar ? _fbb.CreateString(default_non_scalar) : 0;
return reflection::CreateField(
_fbb,
name__,
Expand All @@ -833,7 +846,8 @@ inline ::flatbuffers::Offset<Field> CreateFieldDirect(
documentation__,
optional,
padding,
offset64);
offset64,
default_non_scalar__);
}

struct Object FLATBUFFERS_FINAL_CLASS : private ::flatbuffers::Table {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ public final class Field extends com.google.flatbuffers.Table {
* If the field uses 64-bit offsets.
*/
public boolean offset64() { int o = __offset(30); return o != 0 ? 0!=bb.get(o + bb_pos) : false; }
public String defaultNonScalar() { int o = __offset(32); return o != 0 ? __string(o + bb_pos) : null; }
public ByteBuffer defaultNonScalarAsByteBuffer() { return __vector_as_bytebuffer(32, 1); }
public ByteBuffer defaultNonScalarInByteBuffer(ByteBuffer _bb) { return __vector_in_bytebuffer(_bb, 32, 1); }

public static int createField(FlatBufferBuilder builder,
int nameOffset,
Expand All @@ -73,10 +76,12 @@ public static int createField(FlatBufferBuilder builder,
int documentationOffset,
boolean optional,
int padding,
boolean offset64) {
builder.startTable(14);
boolean offset64,
int defaultNonScalarOffset) {
builder.startTable(15);
Field.addDefaultReal(builder, defaultReal);
Field.addDefaultInteger(builder, defaultInteger);
Field.addDefaultNonScalar(builder, defaultNonScalarOffset);
Field.addDocumentation(builder, documentationOffset);
Field.addAttributes(builder, attributesOffset);
Field.addType(builder, typeOffset);
Expand All @@ -92,7 +97,7 @@ public static int createField(FlatBufferBuilder builder,
return Field.endField(builder);
}

public static void startField(FlatBufferBuilder builder) { builder.startTable(14); }
public static void startField(FlatBufferBuilder builder) { builder.startTable(15); }
public static void addName(FlatBufferBuilder builder, int nameOffset) { builder.addOffset(nameOffset); builder.slot(0); }
public static void addType(FlatBufferBuilder builder, int typeOffset) { builder.addOffset(1, typeOffset, 0); }
public static void addId(FlatBufferBuilder builder, int id) { builder.addShort(2, (short) id, (short) 0); }
Expand All @@ -111,6 +116,7 @@ public static int createField(FlatBufferBuilder builder,
public static void addOptional(FlatBufferBuilder builder, boolean optional) { builder.addBoolean(11, optional, false); }
public static void addPadding(FlatBufferBuilder builder, int padding) { builder.addShort(12, (short) padding, (short) 0); }
public static void addOffset64(FlatBufferBuilder builder, boolean offset64) { builder.addBoolean(13, offset64, false); }
public static void addDefaultNonScalar(FlatBufferBuilder builder, int defaultNonScalarOffset) { builder.addOffset(14, defaultNonScalarOffset, 0); }
public static int endField(FlatBufferBuilder builder) {
int o = builder.endTable();
builder.required(o, 4); // name
Expand Down
15 changes: 14 additions & 1 deletion python/flatbuffers/reflection/Field.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,15 @@ def Offset64(self):
return bool(self._tab.Get(flatbuffers.number_types.BoolFlags, o + self._tab.Pos))
return False

# Field
def DefaultNonScalar(self):
o = flatbuffers.number_types.UOffsetTFlags.py_type(self._tab.Offset(32))
if o != 0:
return self._tab.String(o + self._tab.Pos)
return None

def FieldStart(builder):
builder.StartObject(14)
builder.StartObject(15)

def Start(builder):
FieldStart(builder)
Expand Down Expand Up @@ -265,6 +272,12 @@ def FieldAddOffset64(builder, offset64):
def AddOffset64(builder, offset64):
FieldAddOffset64(builder, offset64)

def FieldAddDefaultNonScalar(builder, defaultNonScalar):
builder.PrependUOffsetTRelativeSlot(14, flatbuffers.number_types.UOffsetTFlags.py_type(defaultNonScalar), 0)

def AddDefaultNonScalar(builder, defaultNonScalar):
FieldAddDefaultNonScalar(builder, defaultNonScalar)

def FieldEnd(builder):
return builder.EndObject()

Expand Down
1 change: 1 addition & 0 deletions reflection/reflection.fbs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ table Field {
padding:uint16 = 0;
/// If the field uses 64-bit offsets.
offset64:bool = false;
default_non_scalar:string;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like a better long term solution is to have a default_value field that is of type Union (long, string, short, etc..)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you want me to deprecate the old constant field to do so?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would also have to be a union of struct of primitive, since unions of primitives aren't a thing

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not great to remove the existing fields.. so we may as well continue with the current pattern?

Just not great to merge strings and vectors into 1 field. Better probably to have default:string (which at least in FlatBuffers can be detected as not-set vs empty) and default_vector:bool` ?

In terms of the variable constant in the parser.. I actually like your idea of making it contain the string including extra "": this would make a lot of sense as way to represent different types of values inside a string.

The long term solution would be to replace constant with a variant<int64_t,double,string,bool> or similar? But that will likely cause a LOT of code churn.

}

table Object { // Used for both tables and structs.
Expand Down
6 changes: 6 additions & 0 deletions src/binary_annotator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,12 @@ BinaryAnnotator::VTable* BinaryAnnotator::GetOrBuildVTable(
? std::to_string(field->default_real())
: std::to_string(field->default_integer());
default_label += "> (";
} else if ((field->type()->base_type() == reflection::String ||
field->type()->base_type() == reflection::Vector) &&
field->default_non_scalar() != nullptr) {
default_label += "<defaults to \"";
default_label += field->default_non_scalar()->str();
default_label += "\"> (";
} else {
default_label += "<null> (";
}
Expand Down
10 changes: 9 additions & 1 deletion src/idl_parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1016,6 +1016,7 @@ CheckedError Parser::ParseField(StructDef& struct_def) {
"Default values for strings and vectors are not supported in one "
"of the specified programming languages");
}
field->value.has_non_scalar_constant = true;
}

if (IsVector(type) && field->value.constant != "0" &&
Expand Down Expand Up @@ -4136,6 +4137,8 @@ Offset<reflection::Field> FieldDef::Serialize(FlatBufferBuilder* builder,
auto docs__ = parser.opts.binary_schema_comments && !doc_comment.empty()
? builder->CreateVectorOfStrings(doc_comment)
: 0;
auto default_non_scalar__ =
value.has_non_scalar_constant ? builder->CreateString(value.constant) : 0;
double d;
StringToNumber(value.constant.c_str(), &d);
return reflection::CreateField(
Expand All @@ -4144,7 +4147,8 @@ Offset<reflection::Field> FieldDef::Serialize(FlatBufferBuilder* builder,
IsInteger(value.type.base_type) ? StringToInt(value.constant.c_str()) : 0,
// result may be platform-dependent if underlying is float (not double)
IsFloat(value.type.base_type) ? d : 0.0, deprecated, IsRequired(), key,
attr__, docs__, IsOptional(), static_cast<uint16_t>(padding), offset64);
attr__, docs__, IsOptional(), static_cast<uint16_t>(padding), offset64,
default_non_scalar__);
// TODO: value.constant is almost always "0", we could save quite a bit of
// space by sharing it. Same for common values of value.type.
}
Expand All @@ -4158,6 +4162,10 @@ bool FieldDef::Deserialize(Parser& parser, const reflection::Field* field) {
value.constant = NumToString(field->default_integer());
} else if (IsFloat(value.type.base_type)) {
value.constant = FloatToString(field->default_real(), 17);
} else if (IsString(value.type) || IsVector(value.type)) {
value.has_non_scalar_constant = field->default_non_scalar();
value.constant =
value.has_non_scalar_constant ? field->default_non_scalar()->str() : "";
}
presence = FieldDef::MakeFieldPresence(field->optional(), field->required());
padding = field->padding();
Expand Down
Loading