-
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
memo
macro that allows us to apply it on methods as follows:db
is provided, e.g.#[memo(db = my_db)
then we try to use this argument as a database. Unfortunately we don't know a concreteDatabase
implementation so we can find it only by namedb
macro argument and the first argument is&self
we 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")]