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

Make cqrs::TypedEvent static #21

Merged
merged 6 commits into from
Sep 28, 2022
Merged

Make cqrs::TypedEvent static #21

merged 6 commits into from
Sep 28, 2022

Conversation

50U10FCA7
Copy link

For optimization purposes result of cqrs::TypedEvent::event_types() should be generates once.

There's two ways to do implement this:

Add additional trait cqrs::TypedEventStatic which provides static version of event_types;
Replace implementation of cqrs::TypedEvent to make it static.

@50U10FCA7
Copy link
Author

@tyranron Reimpl way has problem with generic params. once_cell::sync::Lazy can't pass generics from outer function. I think we can use something like this to solve this problem.

@tyranron
Copy link
Member

@50U10FCA7 please, remind me, why we don't use arrays and slices and const?

@50U10FCA7
Copy link
Author

50U10FCA7 commented Sep 23, 2022

@tyranron Most traits implemented on Vec<T> but not for &[T]. In our case its postgres_types::ToSql.

Also checked the variant with constant slices and it doesn't work with generics too. At now it's impossible to pass generic to constant. Compiler throws these errors on attempt to pass generic/associated item to constant:

  • can't use generic parameters from outer function;
  • generic Self types are currently not permitted in anonymous constants.

Here's a MRE: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=d206aef139f79bfc27c94e18c4bd0274

@50U10FCA7
Copy link
Author

@ilslv ping

@ilslv
Copy link

ilslv commented Sep 26, 2022

@50U10FCA7 I believe you can hack around it by generating separate non-generic struct and implement Event on it: playground

In juniper we also do similar things: link

@50U10FCA7
Copy link
Author

@ilslv There still problem with const evaluation: playground. Const block is required because of nested event types concatenation. Concatenation uses this solution.

@ilslv
Copy link

ilslv commented Sep 26, 2022

@50U10FCA7 please provide more exact MRE, because I don't understand why are you using inline const in Len implementation and why using <Self as Len>::LEN isn't possible.

@50U10FCA7
Copy link
Author

50U10FCA7 commented Sep 26, 2022

@ilslv

Full MRE looks like this. I tried to avoid the problem with:

@ilslv
Copy link

ilslv commented Sep 27, 2022

@50U10FCA7 I've got pretty close to make it work, but compiler warned about this issue

As we only allowed generic constants where the actual type did not matter, it should always be possible to replace the generic parameters with dummy types. For example [0; std::mem::size_of::<*mut T>()] can be changed to [0; std::mem::size_of::<*mut ()>()].

So I think we can simply replace every generic parameter with (), because thats what compiler will eventually try to do.

@50U10FCA7
Copy link
Author

FCM

Replace `TypedEvent::event_types` with associated constant (#21)

@50U10FCA7 50U10FCA7 self-assigned this Sep 27, 2022
@50U10FCA7 50U10FCA7 added the enhancement New feature or request label Sep 27, 2022
@50U10FCA7 50U10FCA7 marked this pull request as ready for review September 27, 2022 12:54
@tyranron tyranron merged commit 0849a04 into master Sep 28, 2022
@tyranron tyranron deleted the static-typed-event branch September 28, 2022 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants