-
Notifications
You must be signed in to change notification settings - Fork 28
allow memo on methods #538
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
base: main
Are you sure you want to change the base?
Conversation
10f31ea to
957ef06
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks great! Just some minor nits.
| } | ||
|
|
||
| impl TestDatabase { | ||
| #[memo] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
YES
|
|
||
| impl TestStruct { | ||
| #[memo(db = test_db)] | ||
| fn first_letter(&self, test_db: &TestDatabase, input_id: SourceId<Input>) -> char { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nifty
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what if we access self here? That would break memoization, right?
I suppose that's true of methods on structs that implements Database, but maybe then it's clear that you are only supposed to store sources on that struct...
|
|
||
| let (db_arg, closure_db_arg) = match &sig.inputs[db_pos] { | ||
| FnArg::Receiver(rcv) => { | ||
| if rcv.reference.is_none() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we disallow &mut self in the macro? It looks like we rewrite it to be &self regardless, which makes for confusing errors:
#[memo]
fn first_letter_2(&mut self, input_id: SourceId<Input>) -> char {
self.storage.set(Input {
key: todo!(),
value: todo!(),
});
#[memo]
| ^^^^^^^ `__db` is a `&` reference, so the data it refers to cannot be borrowed as mutable
957ef06 to
e44300c
Compare
A new version of
memomacro that allows us to apply it on methods as follows:dbis provided, e.g.#[memo(db = my_db)then we try to use this argument as a database. Unfortunately we don't know a concreteDatabaseimplementation so we can find it only by namedbmacro argument and the first argument is&selfwe try to use it as databaseone can use
#[memo(db)]if name of the argument is db,#[memo(db = test_db)]or#[memo(db = "test_db")]