Skip to content

Commit

Permalink
Started implementation for private flags in rust (#7269)
Browse files Browse the repository at this point in the history
Adds flag to the cpp to fix bad annotations in all the languages

Addresses comments to the PR, and fixes logic for leaking annotations
  • Loading branch information
mustiikhalil authored Jun 5, 2022
1 parent 967df08 commit 11a1988
Show file tree
Hide file tree
Showing 13 changed files with 924 additions and 11 deletions.
5 changes: 5 additions & 0 deletions include/flatbuffers/idl.h
Original file line number Diff line number Diff line change
Expand Up @@ -603,6 +603,7 @@ struct IDLOptions {
bool json_nested_flexbuffers;
bool json_nested_legacy_flatbuffers;
bool ts_flat_file;
bool no_leak_private_annotations;

// Possible options for the more general generator below.
enum Language {
Expand Down Expand Up @@ -702,6 +703,7 @@ struct IDLOptions {
json_nested_flexbuffers(true),
json_nested_legacy_flatbuffers(false),
ts_flat_file(false),
no_leak_private_annotations(false),
mini_reflect(IDLOptions::kNone),
require_explicit_ids(false),
rust_serialize(false),
Expand Down Expand Up @@ -993,6 +995,9 @@ class Parser : public ParserState {
FLATBUFFERS_CHECKED_ERROR ParseRoot(const char *_source,
const char **include_paths,
const char *source_filename);
FLATBUFFERS_CHECKED_ERROR CheckPrivateLeak();
FLATBUFFERS_CHECKED_ERROR CheckPrivatelyLeakedFields(
const Definition &def, const Definition &value_type);
FLATBUFFERS_CHECKED_ERROR DoParse(const char *_source,
const char **include_paths,
const char *source_filename,
Expand Down
5 changes: 5 additions & 0 deletions scripts/generate_code.py
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,11 @@ def glob(path, pattern):
prefix=cpp_17_prefix,
)

# Private annotations
annotations_test_schema = "private_annotation_test.fbs"

flatc(RUST_OPTS + ["--no-leak-private-annotation", "--gen-object-api"], prefix="private_annotation_test", schema=annotations_test_schema)

# Sample files
samples_schema = "monster.fbs"
flatc(BASE_OPTS + CPP_OPTS + LOBSTER_OPTS, schema=samples_schema, cwd=samples_path)
Expand Down
5 changes: 5 additions & 0 deletions src/flatc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,9 @@ const static FlatCOption options[] = {
"Only generated one typescript file per .fbs file." },
{ "", "annotate", "SCHEMA",
"Annotate the provided BINARY_FILE with the specified SCHEMA file." },
{ "", "no-leak-private-annotation", "",
"Prevents multiple type of annotations within a Fbs SCHEMA file."
"Currently this is required to generate private types in Rust" },
};

static void AppendTextWrappedString(std::stringstream &ss, std::string &text,
Expand Down Expand Up @@ -596,6 +599,8 @@ int FlatCompiler::Compile(int argc, const char **argv) {
opts.json_nested_legacy_flatbuffers = true;
} else if (arg == "--ts-flat-files") {
opts.ts_flat_file = true;
} else if (arg == "--no-leak-private-annotation") {
opts.no_leak_private_annotations = true;
} else if (arg == "--annotate") {
if (++argi >= argc) Error("missing path following: " + arg, true);
annotate_schema = flatbuffers::PosixPath(argv[argi]);
Expand Down
32 changes: 21 additions & 11 deletions src/idl_gen_rust.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -698,6 +698,9 @@ class RustGenerator : public BaseGenerator {
// an enum match function,
// and an enum array of values
void GenEnum(const EnumDef &enum_def) {
const bool is_private = parser_.opts.no_leak_private_annotations &&
(enum_def.attributes.Lookup("private") != nullptr);
code_.SetValue("ACCESS_TYPE", is_private ? "pub(crate)" : "pub");
code_.SetValue("ENUM_TY", namer_.Type(enum_def));
code_.SetValue("BASE_TYPE", GetEnumTypeForDecl(enum_def.underlying_type));
code_.SetValue("ENUM_NAMESPACE", namer_.Namespace(enum_def.name));
Expand All @@ -718,7 +721,7 @@ class RustGenerator : public BaseGenerator {
code_ += " flatbuffers::bitflags::bitflags! {";
GenComment(enum_def.doc_comment, " ");
code_ += " #[derive(Default)]";
code_ += " pub struct {{ENUM_TY}}: {{BASE_TYPE}} {";
code_ += " {{ACCESS_TYPE}} struct {{ENUM_TY}}: {{BASE_TYPE}} {";
ForAllEnumValues1(enum_def, [&](const EnumVal &ev) {
this->GenComment(ev.doc_comment, " ");
code_ += " const {{VARIANT}} = {{VALUE}};";
Expand Down Expand Up @@ -764,7 +767,7 @@ class RustGenerator : public BaseGenerator {
"#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, "
"Default)]";
code_ += "#[repr(transparent)]";
code_ += "pub struct {{ENUM_TY}}(pub {{BASE_TYPE}});";
code_ += "{{ACCESS_TYPE}} struct {{ENUM_TY}}(pub {{BASE_TYPE}});";
code_ += "#[allow(non_upper_case_globals)]";
code_ += "impl {{ENUM_TY}} {";
ForAllEnumValues1(enum_def, [&](const EnumVal &ev) {
Expand Down Expand Up @@ -881,7 +884,7 @@ class RustGenerator : public BaseGenerator {
if (enum_def.is_union) {
// Generate typesafe offset(s) for unions
code_.SetValue("UNION_TYPE", namer_.Type(enum_def));
code_ += "pub struct {{UNION_TYPE}}UnionTableOffset {}";
code_ += "{{ACCESS_TYPE}} struct {{UNION_TYPE}}UnionTableOffset {}";
code_ += "";
if (parser_.opts.generate_object_based_api) { GenUnionObject(enum_def); }
}
Expand Down Expand Up @@ -916,7 +919,7 @@ class RustGenerator : public BaseGenerator {
// intended.
code_ += "#[non_exhaustive]";
code_ += "#[derive(Debug, Clone, PartialEq)]";
code_ += "pub enum {{ENUM_OTY}} {";
code_ += "{{ACCESS_TYPE}} enum {{ENUM_OTY}} {";
code_ += " NONE,";
ForAllUnionObjectVariantsBesidesNone(enum_def, [&] {
code_ += "{{NATIVE_VARIANT}}(Box<{{U_ELEMENT_TABLE_TYPE}}>),";
Expand Down Expand Up @@ -1620,18 +1623,22 @@ class RustGenerator : public BaseGenerator {
// Generate an accessor struct, builder struct, and create function for a
// table.
void GenTable(const StructDef &struct_def) {

const bool is_private = parser_.opts.no_leak_private_annotations &&
(struct_def.attributes.Lookup("private") != nullptr);
code_.SetValue("ACCESS_TYPE", is_private ? "pub(crate)" : "pub");
code_.SetValue("STRUCT_TY", namer_.Type(struct_def));
code_.SetValue("STRUCT_FN", namer_.Function(struct_def));

// Generate an offset type, the base type, the Follow impl, and the
// init_from_table impl.
code_ += "pub enum {{STRUCT_TY}}Offset {}";
code_ += "{{ACCESS_TYPE}} enum {{STRUCT_TY}}Offset {}";
code_ += "#[derive(Copy, Clone, PartialEq)]";
code_ += "";

GenComment(struct_def.doc_comment);

code_ += "pub struct {{STRUCT_TY}}<'a> {";
code_ += "{{ACCESS_TYPE}} struct {{STRUCT_TY}}<'a> {";
code_ += " pub _tab: flatbuffers::Table<'a>,";
code_ += "}";
code_ += "";
Expand Down Expand Up @@ -1965,7 +1972,7 @@ class RustGenerator : public BaseGenerator {
// Generate an args struct:
code_.SetValue("MAYBE_LT",
TableBuilderArgsNeedsLifetime(struct_def) ? "<'a>" : "");
code_ += "pub struct {{STRUCT_TY}}Args{{MAYBE_LT}} {";
code_ += "{{ACCESS_TYPE}} struct {{STRUCT_TY}}Args{{MAYBE_LT}} {";
ForAllTableFields(struct_def, [&](const FieldDef &field) {
code_.SetValue("PARAM_TYPE", TableBuilderArgsDefnType(field, "'a"));
code_ += " pub {{FIELD}}: {{PARAM_TYPE}},";
Expand Down Expand Up @@ -2054,7 +2061,7 @@ class RustGenerator : public BaseGenerator {
}

// Generate a builder struct:
code_ += "pub struct {{STRUCT_TY}}Builder<'a: 'b, 'b> {";
code_ += "{{ACCESS_TYPE}} struct {{STRUCT_TY}}Builder<'a: 'b, 'b> {";
code_ += " fbb_: &'b mut flatbuffers::FlatBufferBuilder<'a>,";
code_ +=
" start_: flatbuffers::WIPOffset<"
Expand Down Expand Up @@ -2175,7 +2182,7 @@ class RustGenerator : public BaseGenerator {
// Generate the native object.
code_ += "#[non_exhaustive]";
code_ += "#[derive(Debug, Clone, PartialEq)]";
code_ += "pub struct {{STRUCT_OTY}} {";
code_ += "{{ACCESS_TYPE}} struct {{STRUCT_OTY}} {";
ForAllObjectTableFields(table, [&](const FieldDef &field) {
// Union objects combine both the union discriminant and value, so we
// skip making a field for the discriminant.
Expand Down Expand Up @@ -2583,6 +2590,9 @@ class RustGenerator : public BaseGenerator {
}
// Generate an accessor struct with constructor for a flatbuffers struct.
void GenStruct(const StructDef &struct_def) {
const bool is_private = parser_.opts.no_leak_private_annotations &&
(struct_def.attributes.Lookup("private") != nullptr);
code_.SetValue("ACCESS_TYPE", is_private ? "pub(crate)" : "pub");
// Generates manual padding and alignment.
// Variables are private because they contain little endian data on all
// platforms.
Expand All @@ -2600,7 +2610,7 @@ class RustGenerator : public BaseGenerator {
code_ += "// struct {{STRUCT_TY}}, aligned to {{ALIGN}}";
code_ += "#[repr(transparent)]";
code_ += "#[derive(Clone, Copy, PartialEq)]";
code_ += "pub struct {{STRUCT_TY}}(pub [u8; {{STRUCT_SIZE}}]);";
code_ += "{{ACCESS_TYPE}} struct {{STRUCT_TY}}(pub [u8; {{STRUCT_SIZE}}]);";
code_ += "impl Default for {{STRUCT_TY}} { ";
code_ += " fn default() -> Self { ";
code_ += " Self([0; {{STRUCT_SIZE}}])";
Expand Down Expand Up @@ -2847,7 +2857,7 @@ class RustGenerator : public BaseGenerator {
if (parser_.opts.generate_object_based_api) {
// Struct declaration
code_ += "#[derive(Debug, Clone, PartialEq, Default)]";
code_ += "pub struct {{STRUCT_OTY}} {";
code_ += "{{ACCESS_TYPE}} struct {{STRUCT_OTY}} {";
ForAllStructFields(struct_def, [&](const FieldDef &field) {
(void)field; // unused.
code_ += "pub {{FIELD}}: {{FIELD_OTY}},";
Expand Down
59 changes: 59 additions & 0 deletions src/idl_parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3266,19 +3266,78 @@ CheckedError Parser::ParseRoot(const char *source, const char **include_paths,
for (auto val_it = enum_def.Vals().begin();
val_it != enum_def.Vals().end(); ++val_it) {
auto &val = **val_it;

if (!(opts.lang_to_generate != 0 && SupportsAdvancedUnionFeatures()) &&
(IsStruct(val.union_type) || IsString(val.union_type)))

return Error(
"only tables can be union elements in the generated language: " +
val.name);
}
}
}

auto err = CheckPrivateLeak();
if (err.Check()) return err;

// Parse JSON object only if the scheme has been parsed.
if (token_ == '{') { ECHECK(DoParseJson()); }
return NoError();
}

CheckedError Parser::CheckPrivateLeak() {
if (!opts.no_leak_private_annotations) return NoError();
// Iterate over all structs/tables to validate we arent leaking
// any private (structs/tables/enums)
for (auto it = structs_.vec.begin(); it != structs_.vec.end(); it++) {
auto &struct_def = **it;
for (auto fld_it = struct_def.fields.vec.begin();
fld_it != struct_def.fields.vec.end(); ++fld_it) {
auto &field = **fld_it;

if (field.value.type.enum_def) {
auto err =
CheckPrivatelyLeakedFields(struct_def, *field.value.type.enum_def);
if (err.Check()) { return err; }
} else if (field.value.type.struct_def) {
auto err = CheckPrivatelyLeakedFields(struct_def,
*field.value.type.struct_def);
if (err.Check()) { return err; }
}
}
}
// Iterate over all enums to validate we arent leaking
// any private (structs/tables)
for (auto it = enums_.vec.begin(); it != enums_.vec.end(); ++it) {
auto &enum_def = **it;
if (enum_def.is_union) {
for (auto val_it = enum_def.Vals().begin();
val_it != enum_def.Vals().end(); ++val_it) {
auto &val = **val_it;
if (val.union_type.struct_def) {
auto err =
CheckPrivatelyLeakedFields(enum_def, *val.union_type.struct_def);
if (err.Check()) { return err; }
}
}
}
}
return NoError();
}

CheckedError Parser::CheckPrivatelyLeakedFields(const Definition &def,
const Definition &value_type) {
if (!opts.no_leak_private_annotations) return NoError();
const auto is_private = def.attributes.Lookup("private");
const auto is_field_private = value_type.attributes.Lookup("private");
if (!is_private && is_field_private) {
return Error(
"Leaking private implementation, verify all objects have similar "
"annotations");
}
return NoError();
}

// Generate a unique hash for a file based on its name and contents (if any).
static uint64_t HashFile(const char *source_filename, const char *source) {
uint64_t hash = 0;
Expand Down
18 changes: 18 additions & 0 deletions tests/private_annotation_test.fbs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
enum AB:byte (private) {
A,
B,
}

struct Object (private) {
value: int;
}

union Any (private) { Game, Annotations }

table Game (private) {
value: int;
}

table Annotations (private) {
value: int;
}
97 changes: 97 additions & 0 deletions tests/private_annotation_test/ab_generated.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
// automatically generated by the FlatBuffers compiler, do not modify
extern crate alloc;
extern crate flatbuffers;
use alloc::boxed::Box;
use alloc::string::{String, ToString};
use alloc::vec::Vec;
use core::mem;
use core::cmp::Ordering;
use self::flatbuffers::{EndianScalar, Follow};
use super::*;
#[deprecated(since = "2.0.0", note = "Use associated constants instead. This will no longer be generated in 2021.")]
pub const ENUM_MIN_AB: i8 = 0;
#[deprecated(since = "2.0.0", note = "Use associated constants instead. This will no longer be generated in 2021.")]
pub const ENUM_MAX_AB: i8 = 1;
#[deprecated(since = "2.0.0", note = "Use associated constants instead. This will no longer be generated in 2021.")]
#[allow(non_camel_case_types)]
pub const ENUM_VALUES_AB: [AB; 2] = [
AB::A,
AB::B,
];

#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Default)]
#[repr(transparent)]
pub(crate) struct AB(pub i8);
#[allow(non_upper_case_globals)]
impl AB {
pub const A: Self = Self(0);
pub const B: Self = Self(1);

pub const ENUM_MIN: i8 = 0;
pub const ENUM_MAX: i8 = 1;
pub const ENUM_VALUES: &'static [Self] = &[
Self::A,
Self::B,
];
/// Returns the variant's name or "" if unknown.
pub fn variant_name(self) -> Option<&'static str> {
match self {
Self::A => Some("A"),
Self::B => Some("B"),
_ => None,
}
}
}
impl core::fmt::Debug for AB {
fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
if let Some(name) = self.variant_name() {
f.write_str(name)
} else {
f.write_fmt(format_args!("<UNKNOWN {:?}>", self.0))
}
}
}
impl<'a> flatbuffers::Follow<'a> for AB {
type Inner = Self;
#[inline]
fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
let b = unsafe {
flatbuffers::read_scalar_at::<i8>(buf, loc)
};
Self(b)
}
}

impl flatbuffers::Push for AB {
type Output = AB;
#[inline]
fn push(&self, dst: &mut [u8], _rest: &[u8]) {
unsafe { flatbuffers::emplace_scalar::<i8>(dst, self.0); }
}
}

impl flatbuffers::EndianScalar for AB {
#[inline]
fn to_little_endian(self) -> Self {
let b = i8::to_le(self.0);
Self(b)
}
#[inline]
#[allow(clippy::wrong_self_convention)]
fn from_little_endian(self) -> Self {
let b = i8::from_le(self.0);
Self(b)
}
}

impl<'a> flatbuffers::Verifiable for AB {
#[inline]
fn run_verifier(
v: &mut flatbuffers::Verifier, pos: usize
) -> Result<(), flatbuffers::InvalidFlatbuffer> {
use self::flatbuffers::Verifiable;
i8::run_verifier(v, pos)
}
}

impl flatbuffers::SimpleToVerifyInSlice for AB {}
Loading

0 comments on commit 11a1988

Please sign in to comment.