Skip to content

fix: Improve hygiene of trait impl macros #839

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
RobertJacobsonCDC merged 1 commit into from
Apr 7, 2026
Merged

fix: Improve hygiene of trait impl macros #839

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions src/entity/property_list.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ macro_rules! impl_property_list {
impl<E: Entity, #( P~N: Property<E>,)*> PropertyList<E> for (#(P~N, )*){
fn validate() -> Result<(), IxaError> {
// For `Property` distinctness check
let property_type_ids: [TypeId; $ct] = [#(P~N::type_id(),)*];
let property_type_ids: [TypeId; $ct] = [#(<P~N as $crate::entity::property::Property<E>>::type_id(),)*];

for i in 0..$ct - 1 {
for j in (i + 1)..$ct {
Expand All @@ -159,7 +159,7 @@ macro_rules! impl_property_list {
}

fn contains_properties(property_type_ids: &[TypeId]) -> bool {
let self_property_type_ids: [TypeId; $ct] = [#(P~N::type_id(),)*];
let self_property_type_ids: [TypeId; $ct] = [#(<P~N as $crate::entity::property::Property<E>>::type_id(),)*];

property_type_ids.len() <= $ct && property_type_ids.iter().all(|id| self_property_type_ids.contains(id))
}
Expand Down
29 changes: 19 additions & 10 deletions src/entity/query/query_impls.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,12 @@ macro_rules! impl_query {
fn get_query(&self) -> Vec<(usize, HashValueType)> {
let mut ordered_items = vec![
#(
(T~N::id(), T~N::hash_property_value(&T~N::make_canonical(self.N))),
(
<T~N as $crate::entity::property::Property<E>>::id(),
<T~N as $crate::entity::property::Property<E>>::hash_property_value(
&<T~N as $crate::entity::property::Property<E>>::make_canonical(self.N)
),
),
)*
];
ordered_items.sort_unstable_by(|a, b| a.0.cmp(&b.0));
Expand All @@ -210,7 +215,7 @@ macro_rules! impl_query {
fn get_type_ids(&self) -> Vec<TypeId> {
vec![
#(
T~N::type_id(),
<T~N as $crate::entity::property::Property<E>>::type_id(),
)*
]
}
Expand All @@ -230,7 +235,7 @@ macro_rules! impl_query {
// ];
let keys: [&str; $ct] = [
#(
T~N::name(),
<T~N as $crate::entity::property::Property<E>>::name(),
)*
];
// It is convenient to have the elements of the array to be `Copy` in the `static_apply_reordering`
Expand All @@ -239,7 +244,11 @@ macro_rules! impl_query {
// keep the referenced value in scope.)
let mut values: [&Vec<u8>; $ct] = [
#(
&$crate::bincode::serde::encode_to_vec(self.N, bincode::config::standard()).unwrap(),
&$crate::bincode::serde::encode_to_vec(
self.N,
$crate::bincode::config::standard(),
)
.unwrap(),
)*
];
static_reorder_by_keys(&keys, &mut values);
Expand All @@ -253,11 +262,11 @@ macro_rules! impl_query {
// This mirrors the indexed case in `SourceSet<'a, E>::new()`. The difference is, if the
// multi-property is unindexed, we fall through to create `SourceSet`s for the components
// rather than wrapping a `DerivedPropertySource`.
if let Some(multi_property_id) = self.multi_property_id() {
if let Some(multi_property_id) = <Self as $crate::entity::Query<E>>::multi_property_id(self) {
let property_store = context.entity_store.get_property_store::<E>();
match property_store.get_index_set_with_hash_for_property_id(
multi_property_id,
self.multi_property_value_hash(),
<Self as $crate::entity::Query<E>>::multi_property_value_hash(self),
) {
$crate::entity::index::IndexSetResult::Set(entity_set) => {
return EntitySet::from_source(SourceSet::IndexSet(entity_set));
Expand Down Expand Up @@ -288,11 +297,11 @@ macro_rules! impl_query {
fn new_query_result_iterator<'c>(&self, context: &'c Context) -> EntitySetIterator<'c, E> {
// Constructing the `EntitySetIterator` directly instead of constructing an `EntitySet`
// first is a micro-optimization improving tight-loop benchmark performance.
if let Some(multi_property_id) = self.multi_property_id() {
if let Some(multi_property_id) = <Self as $crate::entity::Query<E>>::multi_property_id(self) {
let property_store = context.entity_store.get_property_store::<E>();
match property_store.get_index_set_with_hash_for_property_id(
multi_property_id,
self.multi_property_value_hash(),
<Self as $crate::entity::Query<E>>::multi_property_value_hash(self),
) {
$crate::entity::index::IndexSetResult::Set(entity_set) => {
return EntitySetIterator::from_index_set(entity_set);
Expand Down Expand Up @@ -331,11 +340,11 @@ macro_rules! impl_query {

fn filter_entities(&self, entities: &mut Vec<EntityId<E>>, context: &Context) {
// The fast path: If this query is indexed, we only have to do one pass over the entities.
if let Some(multi_property_id) = self.multi_property_id() {
if let Some(multi_property_id) = <Self as $crate::entity::Query<E>>::multi_property_id(self) {
let property_store = context.entity_store.get_property_store::<E>();
match property_store.get_index_set_with_hash_for_property_id(
multi_property_id,
self.multi_property_value_hash(),
<Self as $crate::entity::Query<E>>::multi_property_value_hash(self),
) {
$crate::entity::index::IndexSetResult::Set(entity_set) => {
entities.retain(|entity_id| entity_set.contains(entity_id));
Expand Down
2 changes: 1 addition & 1 deletion src/macros/define_global_property.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,6 @@ macro_rules! define_global_property {
};

($global_property: ident, $value: ty) => {
define_global_property!($global_property, $value, |_| { Ok(()) });
$crate::define_global_property!($global_property, $value, |_| { Ok(()) });
};
}
2 changes: 1 addition & 1 deletion src/macros/entity_impl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ macro_rules! define_entity {
impl $entity_name {
#[allow(unused)]
pub fn new() -> Self {
Self::default()
<Self as std::default::Default>::default()
}
}

Expand Down
19 changes: 10 additions & 9 deletions src/macros/property_impl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,8 @@ macro_rules! define_property {
/// * `make_uncanonical = <expr>` — Function converting from `CanonicalValue` to `Self`; defaults to `std::convert::identity`.
/// * Optional parameters that should generally be left alone, used internally to implement derived properties and
/// multi-properties:
/// * `index_id_fn = <expr>` — Function used to initialize the property index id; defaults to `Self::id()`.
/// * `index_id_fn = <expr>` — Function used to initialize the property index id; defaults to
/// `<Self as $crate::entity::property::Property<$entity>>::id()`.
/// * `collect_deps_fn = <expr>` — Function used to collect property dependencies; defaults to an empty implementation.
/// * `ctor_registration = <expr>` — Code run in the `ctor` for property registration.
///
Expand Down Expand Up @@ -683,10 +684,10 @@ macro_rules! impl_derived_property {
),
collect_deps_fn = | deps: &mut $crate::HashSet<usize> | {
$(
if $dependency::is_derived() {
$dependency::collect_non_derived_dependencies(deps);
if <$dependency as $crate::entity::property::Property<$entity>>::is_derived() {
<$dependency as $crate::entity::property::Property<$entity>>::collect_non_derived_dependencies(deps);
} else {
deps.insert($dependency::id());
deps.insert(<$dependency as $crate::entity::property::Property<$entity>>::id());
}
)*
}
Expand Down Expand Up @@ -800,17 +801,17 @@ macro_rules! define_multi_property {

collect_deps_fn = | deps: &mut $crate::HashSet<usize> | {
$(
if $dependency::is_derived() {
$dependency::collect_non_derived_dependencies(deps);
if <$dependency as $crate::entity::property::Property<$entity>>::is_derived() {
<$dependency as $crate::entity::property::Property<$entity>>::collect_non_derived_dependencies(deps);
} else {
deps.insert($dependency::id());
deps.insert(<$dependency as $crate::entity::property::Property<$entity>>::id());
}
)*
},

ctor_registration = {
// Ensure `Self::index_id()` is initialized at startup.
let _ = [<$($dependency)*>]::index_id();
// Ensure the property's `index_id()` is initialized at startup.
let _ = < [<$($dependency)*>] as $crate::entity::property::Property::<$entity> >::index_id();
$crate::entity::property_store::add_to_property_registry::<$entity, [<$($dependency)*>]>();
}
);
Expand Down
Loading