Skip to content

Conversation

ch1ffa
Copy link
Collaborator

@ch1ffa ch1ffa commented Aug 4, 2025

A new version of memo macro that allows us to apply it on methods as follows:

  • if argument 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 concrete Database implementation so we can find it only by name
  • if there is no db macro argument and the first argument is &self we try to use it as database
  • if memoized function is a free function, then we use its first argument as database

one can use #[memo(db)] if name of the argument is db, #[memo(db = test_db)] or #[memo(db = "test_db")]

@ch1ffa ch1ffa force-pushed the allow-memo-on-methods branch 3 times, most recently from 10f31ea to 957ef06 Compare August 9, 2025 15:07
Copy link
Collaborator

@rbalicki2 rbalicki2 left a 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]
Copy link
Collaborator

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 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nifty

Copy link
Collaborator

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() {
Copy link
Collaborator

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

@ch1ffa ch1ffa force-pushed the allow-memo-on-methods branch from 957ef06 to e44300c Compare August 11, 2025 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants