Skip to content
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.

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

Remove attr.cpp.archetype_eager codegen attribute #8780

Merged
merged 2 commits into from
Jan 23, 2025
Merged
Changes from 1 commit
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
Next Next commit
remove attr.cpp.archetype_eager
Wumpf committed Jan 23, 2025
commit aa015099ab6ec482fcde6d939ce644f1c8855b19
213 changes: 60 additions & 153 deletions crates/build/re_types_builder/src/codegen/cpp/mod.rs
Original file line number Diff line number Diff line change
@@ -16,8 +16,7 @@ use crate::{
format_path,
objects::ObjectClass,
ArrowRegistry, Docs, ElementType, GeneratedFiles, Object, ObjectField, ObjectKind, Objects,
Reporter, Type, ATTR_CPP_ARCHETYPE_EAGER, ATTR_CPP_NO_DEFAULT_CTOR, ATTR_CPP_NO_FIELD_CTORS,
ATTR_CPP_RENAME_FIELD, ATTR_RERUN_LOG_MISSING_AS_EMPTY,
Reporter, Type, ATTR_CPP_NO_DEFAULT_CTOR, ATTR_CPP_NO_FIELD_CTORS, ATTR_CPP_RENAME_FIELD,
};

use self::array_builder::{arrow_array_builder_type, arrow_array_builder_type_object};
@@ -454,8 +453,6 @@ impl QuotedObject {
let archetype_name = &obj.fqname;
let quoted_docs = quote_obj_docs(reporter, objects, obj);

let eager_serialization = obj.is_attr_set(ATTR_CPP_ARCHETYPE_EAGER);

let mut cpp_includes = Includes::new(obj.fqname.clone(), obj.scope());
cpp_includes.insert_rerun("collection_adapter_builtins.hpp");
hpp_includes.insert_system("utility"); // std::move
@@ -467,29 +464,11 @@ impl QuotedObject {
.map(|obj_field| {
let docstring = quote_field_docs(reporter, objects, obj_field);
let field_name = field_name_ident(obj_field);

if eager_serialization {
hpp_includes.insert_system("optional");
quote! {
#NEWLINE_TOKEN
#docstring
std::optional<ComponentBatch> #field_name
}
} else {
let field_type =
quote_archetype_unserialized_type(&mut hpp_includes, obj_field);
let field_type = if obj_field.is_nullable {
hpp_includes.insert_system("optional");
quote! { std::optional<#field_type> }
} else {
field_type
};

quote! {
hpp_includes.insert_system("optional");
quote! {
#NEWLINE_TOKEN
#docstring
#field_type #field_name
}
std::optional<ComponentBatch> #field_name
}
})
.collect_vec();
@@ -512,14 +491,10 @@ impl QuotedObject {
let field_ident = field_name_ident(obj_field);
// C++ compilers give warnings for re-using the same name as the member variable.
let parameter_ident = format_ident!("_{}", obj_field.name);
let descriptor = archetype_component_descriptor_constant_ident(obj_field);
(
quote! { #field_type #parameter_ident },
if eager_serialization {
let descriptor = archetype_component_descriptor_constant_ident(obj_field);
quote! { #field_ident(ComponentBatch::from_loggable(std::move(#parameter_ident), #descriptor).value_or_throw()) }
} else {
quote! { #field_ident(std::move(#parameter_ident)) }
},
quote! { #field_ident(ComponentBatch::from_loggable(std::move(#parameter_ident), #descriptor).value_or_throw()) }
)
})
.unzip();
@@ -534,48 +509,46 @@ impl QuotedObject {
});
}

let descriptor_constants = if eager_serialization {
obj.fields
.iter()
.map(|obj_field| {
let field_name = field_name(obj_field);
let comment = quote_doc_comment(&format!("`ComponentDescriptor` for the `{field_name}` field."));
let constant_name = archetype_component_descriptor_constant_ident(obj_field);
let field_type = obj_field.typ.fqname();
let field_type = quote_fqname_as_type_path(
&mut hpp_includes,
field_type.expect("Component field must have a non trivial type"),
let descriptor_constants = obj
.fields
.iter()
.map(|obj_field| {
let field_name = field_name(obj_field);
let comment = quote_doc_comment(&format!(
"`ComponentDescriptor` for the `{field_name}` field."
));
let constant_name = archetype_component_descriptor_constant_ident(obj_field);
let field_type = obj_field.typ.fqname();
let field_type = quote_fqname_as_type_path(
&mut hpp_includes,
field_type.expect("Component field must have a non trivial type"),
);
quote! {
#NEWLINE_TOKEN
#comment
static constexpr auto #constant_name = ComponentDescriptor(
ArchetypeName, #field_name, Loggable<#field_type>::Descriptor.component_name
);
quote! {
#NEWLINE_TOKEN
#comment
static constexpr auto #constant_name = ComponentDescriptor(
ArchetypeName, #field_name, Loggable<#field_type>::Descriptor.component_name
);
}
})
.collect_vec()
} else {
Vec::new()
};
}
})
.collect_vec();

if eager_serialization {
// update_fields method - this is equivalent to the default constructor.
methods.push(Method {
docs: format!("Update only some specific fields of a `{type_ident}`.").into(),
declaration: MethodDeclaration {
is_static: true,
return_type: quote!(#type_ident),
name_and_parameters: quote! { update_fields() },
},
definition_body: quote! {
return #type_ident();
},
inline: true,
});
// update_fields method - this is equivalent to the default constructor.
methods.push(Method {
docs: format!("Update only some specific fields of a `{type_ident}`.").into(),
declaration: MethodDeclaration {
is_static: true,
return_type: quote!(#type_ident),
name_and_parameters: quote! { update_fields() },
},
definition_body: quote! {
return #type_ident();
},
inline: true,
});

// clear_fields method.
methods.push(Method {
// clear_fields method.
methods.push(Method {
docs: format!("Clear all the fields of a `{type_ident}`.").into(),
declaration: MethodDeclaration {
is_static: true,
@@ -605,21 +578,21 @@ impl QuotedObject {
inline: false,
});

// Builder methods for all components.
for obj_field in &obj.fields {
let field_ident = field_name_ident(obj_field);
// C++ compilers give warnings for re-using the same name as the member variable.
let parameter_ident = format_ident!("_{}", obj_field.name);
let method_ident = format_ident!("with_{}", obj_field.name);
let field_type = quote_archetype_unserialized_type(&mut hpp_includes, obj_field);
let descriptor = archetype_component_descriptor_constant_ident(obj_field);

// TODO(andreas): Haven't tested this again since introducing eager serialization.
hpp_includes.insert_rerun("compiler_utils.hpp");
let gcc_ignore_comment =
quote_comment("See: https://github.com/rerun-io/rerun/issues/4027");
// Builder methods for all components.
for obj_field in &obj.fields {
let field_ident = field_name_ident(obj_field);
// C++ compilers give warnings for re-using the same name as the member variable.
let parameter_ident = format_ident!("_{}", obj_field.name);
let method_ident = format_ident!("with_{}", obj_field.name);
let field_type = quote_archetype_unserialized_type(&mut hpp_includes, obj_field);
let descriptor = archetype_component_descriptor_constant_ident(obj_field);

// TODO: Haven't tested this again since introducing eager serialization.
hpp_includes.insert_rerun("compiler_utils.hpp");
let gcc_ignore_comment =
quote_comment("See: https://github.com/rerun-io/rerun/issues/4027");

methods.push(Method {
methods.push(Method {
docs: obj_field.docs.clone().into(),
declaration: MethodDeclaration {
is_static: false,
@@ -636,38 +609,6 @@ impl QuotedObject {
},
inline: true,
});
}
} else {
// Builder methods for all optional components.
for obj_field in obj.fields.iter().filter(|field| field.is_nullable) {
let field_ident = field_name_ident(obj_field);
// C++ compilers give warnings for re-using the same name as the member variable.
let parameter_ident = format_ident!("_{}", obj_field.name);
let method_ident = format_ident!("with_{}", obj_field.name);
let field_type = quote_archetype_unserialized_type(&mut hpp_includes, obj_field);

hpp_includes.insert_rerun("compiler_utils.hpp");
let gcc_ignore_comment =
quote_comment("See: https://github.com/rerun-io/rerun/issues/4027");

methods.push(Method {
docs: obj_field.docs.clone().into(),
declaration: MethodDeclaration {
is_static: false,
return_type: quote!(#type_ident),
name_and_parameters: quote! {
#method_ident(#field_type #parameter_ident) &&
},
},
definition_body: quote! {
#field_ident = std::move(#parameter_ident);
#NEWLINE_TOKEN
#gcc_ignore_comment
RR_WITH_MAYBE_UNINITIALIZED_DISABLED(return std::move(*this);)
},
inline: true,
});
}
}

let quoted_namespace = if let Some(scope) = obj.scope() {
@@ -1760,47 +1701,13 @@ fn archetype_serialize(type_ident: &Ident, obj: &Object, hpp_includes: &mut Incl
quote!(archetypes)
};

let archetype_name = &obj.fqname;
let eager_serialization = obj.is_attr_set(ATTR_CPP_ARCHETYPE_EAGER);

let num_fields = quote_integer(obj.fields.len() + 1); // Plus one for the indicator.
let push_batches = obj.fields.iter().map(|field| {
let archetype_field_name = field_name(field);
let field_name_ident = field_name_ident(field);

if eager_serialization {
quote! {
if (archetype.#field_name_ident.has_value()) {
cells.push_back(archetype.#field_name_ident.value());
}
}
} else {
let field_fqname = &field.typ.fqname();
let push_back = quote! {
RR_RETURN_NOT_OK(result.error);
cells.push_back(std::move(result.value));
};

if field.is_nullable && !obj.attrs.has(ATTR_RERUN_LOG_MISSING_AS_EMPTY) {
quote! {
if (archetype.#field_name_ident.has_value()) {
auto result = ComponentBatch::from_loggable(
archetype.#field_name_ident.value(),
ComponentDescriptor(#archetype_name, #archetype_field_name, #field_fqname)
);
#push_back
}
}
} else {
quote! {
{
auto result = ComponentBatch::from_loggable(
archetype.#field_name_ident,
ComponentDescriptor(#archetype_name, #archetype_field_name, #field_fqname)
);
#push_back
}
}
quote! {
if (archetype.#field_name_ident.has_value()) {
cells.push_back(archetype.#field_name_ident.value());
}
}
});
1 change: 0 additions & 1 deletion crates/build/re_types_builder/src/lib.rs
Original file line number Diff line number Diff line change
@@ -194,7 +194,6 @@ pub const ATTR_RERUN_EXPERIMENTAL: &str = "attr.rerun.experimental";
pub const ATTR_PYTHON_ALIASES: &str = "attr.python.aliases";
pub const ATTR_PYTHON_ARRAY_ALIASES: &str = "attr.python.array_aliases";

pub const ATTR_CPP_ARCHETYPE_EAGER: &str = "attr.cpp.archetype_eager";
pub const ATTR_CPP_NO_DEFAULT_CTOR: &str = "attr.cpp.no_default_ctor";
pub const ATTR_CPP_NO_FIELD_CTORS: &str = "attr.cpp.no_field_ctors";
pub const ATTR_CPP_RENAME_FIELD: &str = "attr.cpp.rename_field";
6 changes: 0 additions & 6 deletions crates/store/re_types/definitions/attributes/cpp.fbs
Original file line number Diff line number Diff line change
@@ -11,9 +11,3 @@ attribute "attr.cpp.no_default_ctor";
/// Changes the name of a field in the generated C++ code.
/// This can be used to avoid name clashes with C++ keywords or methods.
attribute "attr.cpp.rename_field";

/// The generated C++ object should be eagerly serialized, i.e. only comprised of Arrow arrays.
///
/// Applies only to archetypes. No-op otherwise.
// TODO(#7245): This should be always enabled.
attribute "attr.cpp.archetype_eager";
Original file line number Diff line number Diff line change
@@ -16,7 +16,6 @@ namespace rerun.archetypes;
/// \example archetypes/annotation_context_connections !api title="Connections" image="https://static.rerun.io/annotation_context_connections/4a8422bc154699c5334f574ff01b55c5cd1748e3/1200w.png"
table AnnotationContext (
// TODO(#7245): "attr.rust.archetype_eager",
"attr.cpp.archetype_eager",
"attr.rust.derive": "Eq, PartialEq",
"attr.docs.view_types": "Spatial2DView, Spatial3DView"
) {
Original file line number Diff line number Diff line change
@@ -8,7 +8,6 @@ namespace rerun.archetypes;
/// \example archetypes/arrows2d_simple title="Simple batch of 2D arrows" image="https://static.rerun.io/arrow2d_simple/59f044ccc03f7bc66ee802288f75706618b29a6e/1200w.png"
table Arrows2D (
"attr.rust.archetype_eager",
"attr.cpp.archetype_eager",
"attr.rust.derive": "PartialEq",
"attr.rust.new_pub_crate",
"attr.cpp.no_field_ctors",
Original file line number Diff line number Diff line change
@@ -7,7 +7,6 @@ namespace rerun.archetypes;
/// \example archetypes/arrows3d_simple title="Simple batch of 3D arrows" image="https://static.rerun.io/arrow3d_simple/55e2f794a520bbf7527d7b828b0264732146c5d0/1200w.png"
table Arrows3D (
"attr.rust.archetype_eager",
"attr.cpp.archetype_eager",
"attr.rust.derive": "PartialEq",
"attr.rust.new_pub_crate",
"attr.cpp.no_field_ctors",
Original file line number Diff line number Diff line change
@@ -11,7 +11,6 @@ namespace rerun.archetypes;
///
/// \example archetypes/asset3d_simple title="Simple 3D asset" image="https://static.rerun.io/asset3d_simple/af238578188d3fd0de3e330212120e2842a8ddb2/1200w.png"
table Asset3D (
"attr.cpp.archetype_eager",
// TODO(#7245): "attr.rust.archetype_eager",
"attr.rust.derive": "PartialEq, Eq",
"attr.docs.category": "Spatial 3D",
Original file line number Diff line number Diff line change
@@ -12,7 +12,6 @@ namespace rerun.archetypes;
/// \example archetypes/video_auto_frames title="Video with automatically determined frames" image="https://static.rerun.io/video_manual_frames/320a44e1e06b8b3a3161ecbbeae3e04d1ccb9589/1200w.png"
/// \example archetypes/video_manual_frames title="Demonstrates manual use of video frame references" image="https://static.rerun.io/video_manual_frames/9f41c00f84a98cc3f26875fba7c1d2fa2bad7151/1200w.png"
table AssetVideo (
"attr.cpp.archetype_eager",
// TODO(#7245): "attr.rust.archetype_eager",
"attr.docs.category": "Video",
"attr.docs.view_types": "Spatial2DView, Spatial3DView: if logged under a projection"
Original file line number Diff line number Diff line change
@@ -8,7 +8,6 @@ namespace rerun.archetypes;
///
/// \example archetypes/bar_chart title="Simple bar chart" image="https://static.rerun.io/barchart_simple/cf6014b18265edfcaa562c06526c0716b296b193/1200w.png"
table BarChart (
"attr.cpp.archetype_eager",
"attr.rust.archetype_eager",
"attr.rust.derive": "PartialEq",
"attr.docs.category": "Plotting",
Original file line number Diff line number Diff line change
@@ -6,7 +6,6 @@ namespace rerun.archetypes;
///
/// \example archetypes/boxes2d_simple title="Simple 2D boxes" image="https://static.rerun.io/box2d_simple/ac4424f3cf747382867649610cbd749c45b2020b/1200w.png"
table Boxes2D (
"attr.cpp.archetype_eager",
// TODO(#7245): "attr.rust.archetype_eager",
"attr.rust.derive": "PartialEq",
"attr.rust.new_pub_crate",
Original file line number Diff line number Diff line change
@@ -11,7 +11,6 @@ namespace rerun.archetypes;
/// \example archetypes/boxes3d_simple !api title="Simple 3D boxes" image="https://static.rerun.io/box3d_simple/d6a3f38d2e3360fbacac52bb43e44762635be9c8/1200w.png"
/// \example archetypes/boxes3d_batch title="Batch of 3D boxes" image="https://static.rerun.io/box3d_batch/5aac5b5d29c9f2ecd572c93f6970fcec17f4984b/1200w.png"
table Boxes3D (
"attr.cpp.archetype_eager",
// TODO(#7245): "attr.rust.archetype_eager",
"attr.rust.derive": "PartialEq",
"attr.rust.new_pub_crate",
Original file line number Diff line number Diff line change
@@ -14,7 +14,6 @@ namespace rerun.archetypes;
// TODO(#1361): This archetype should eventually generalize to cylinders without caps, truncated
// cones, and tapered capsules -- all common shapes based on expanding a line segment circularly.
table Capsules3D (
"attr.cpp.archetype_eager",
"attr.rust.archetype_eager",
"attr.rust.derive": "PartialEq",
"attr.rust.new_pub_crate",
Original file line number Diff line number Diff line change
@@ -18,7 +18,6 @@ namespace rerun.archetypes;
/// \example archetypes/clear_recursive !api "Recursive"
table Clear (
"attr.cpp.no_default_ctor",
"attr.cpp.archetype_eager",
"attr.rust.archetype_eager",
"attr.rust.derive": "PartialEq",
"attr.rust.override_crate": "re_types_core",
Original file line number Diff line number Diff line change
@@ -12,7 +12,6 @@ namespace rerun.archetypes;
/// \example archetypes/depth_image_simple !api title="Simple example" image="https://static.rerun.io/depth_image_simple/77a6fa4f938a742bdc7c5350f668c4f31eed4d01/1200w.png"
/// \example archetypes/depth_image_3d title="Depth to 3D example" image="https://static.rerun.io/depth_image_3d/924e9d4d6a39d63d4fdece82582855fdaa62d15e/1200w.png"
table DepthImage (
"attr.cpp.archetype_eager",
"attr.rust.archetype_eager",
"attr.rust.derive": "PartialEq",
"attr.cpp.no_field_ctors",
Original file line number Diff line number Diff line change
@@ -14,7 +14,6 @@ namespace rerun.archetypes;
///
/// \example archetypes/ellipsoids3d_simple title="Covariance ellipsoid" image="https://static.rerun.io/elliopsoid3d_simple/bd5d46e61b80ae44792b52ee07d750a7137002ea/1200w.png"
table Ellipsoids3D (
"attr.cpp.archetype_eager",
"attr.rust.archetype_eager",
"attr.rust.derive": "PartialEq",
"attr.rust.new_pub_crate",
Loading