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

Missing #[salsa::db] on impl block should fail compilation #613

Open
MichaReiser opened this issue Nov 11, 2024 · 1 comment
Open

Missing #[salsa::db] on impl block should fail compilation #613

MichaReiser opened this issue Nov 11, 2024 · 1 comment

Comments

@MichaReiser
Copy link
Contributor

The following example misses a #[salsa::db] on the impl Db for Pimpl but it only fails at runtime with a panic in as_view.

Ideally, we would catch this error at compile time. If this isn't possible, improve the error message to guide users towards the proper fix.

use salsa::Accumulator;

#[salsa::db]
trait Db: salsa::Database {}

#[salsa::db]
pub(crate) struct Pimpl {
    storage: salsa::Storage<Self>,
}

#[salsa::db]
impl salsa::Database for Pimpl {
    fn salsa_event(&self, _event: &dyn Fn() -> salsa::Event) {}
}

// #[salsa::db] <-----MISSING
impl Db for Pimpl {
}

#[salsa::input]
struct File {
    #[return_ref]
    contents: String,
}

#[salsa::tracked]
struct Element<'db> {}

#[salsa::accumulator]
struct Diagnostic(&'static str);

#[salsa::tracked]
fn parse_single_file(db: &dyn Db, file: File) -> Option<Element<'_>> {
    Diagnostic("argh").accumulate(db);
    None
}

impl Pimpl {
    pub(crate) fn new() -> Self {
        Self {
            storage: Default::default(),
        }
    }

}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn it_works() {
        let mut pimpl = Pimpl::new();
        let this = &mut pimpl;
        let file = File::new(this, "Hello World!".to_string());
        let node = parse_single_file(this, file);
        // FIXME: This panics with 'Unwrap on None value'
        parse_single_file::accumulated::<Diagnostic>(this, file);
    }
}
@NickNick
Copy link

Without the #[salsa::db], a function zalsa_db() is required. Perhaps it would be possible to add another function fn you_forgot_salsa_db()?

Coming from someone who hasn't spent a lot of time with macros, but I thought that could be a quick first step towards making this a little more clear.

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

No branches or pull requests

2 participants