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
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
211 changes: 56 additions & 155 deletions crates/build/re_types_builder/src/codegen/cpp/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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,
Expand Down Expand Up @@ -605,21 +578,16 @@ 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);

methods.push(Method {
methods.push(Method {
docs: obj_field.docs.clone().into(),
declaration: MethodDeclaration {
is_static: false,
Expand All @@ -631,43 +599,10 @@ impl QuotedObject {
definition_body: quote! {
#field_ident = ComponentBatch::from_loggable(#parameter_ident, #descriptor).value_or_throw();
#NEWLINE_TOKEN
#gcc_ignore_comment
RR_WITH_MAYBE_UNINITIALIZED_DISABLED(return std::move(*this);)
},
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);)
return std::move(*this);
},
inline: true,
});
}
}

let quoted_namespace = if let Some(scope) = obj.scope() {
Expand Down Expand Up @@ -1760,47 +1695,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());
}
}
});
Expand Down
1 change: 0 additions & 1 deletion crates/build/re_types_builder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
6 changes: 0 additions & 6 deletions crates/store/re_types/definitions/attributes/cpp.fbs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Up @@ -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"
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading
Loading