salsa-rs / salsa

A generic framework for on-demand, incrementalized computation. Inspired by adapton, glimmer, and rustc's query system.
https://salsa-rs.netlify.app/
Apache License 2.0
2.09k stars 142 forks source link

DebugWithDb doesn't print all fields #478

Closed gavrilikhin-d closed 5 months ago

gavrilikhin-d commented 5 months ago

I have the following AST:

#[salsa::tracked]
pub struct Function {
    #[id]
    pub name: FunctionId,
    #[return_ref]
    pub name_parts: Vec<FunctionNamePart>,
}

#[salsa::interned]
pub struct FunctionId {
    #[return_ref]
    pub text: String,
}

#[derive(Debug, PartialEq, Eq, From)]
pub enum FunctionNamePart {
    Text(Text),
    Parameter(Parameter),
}

Note that FunctionNamePart and Parameter aren't salsa objects.

DebugWithDb generates me the following string:

Function {
  [salsa id]: 0,
  name: FunctionId {
    [salsa id]: 0,
    text: "say hello world",
  },
},

There are no traces of name_parts. Is it possible to dump all fields? Salsa doesn't support enums as salsa objects

gavrilikhin-d commented 5 months ago

Oh, I see, you should implement it yourself. That's understandable, but in the book example is empty, that's why I missed it https://salsa-rs.github.io/salsa/tutorial/debug.html#forwarding-to-the-ordinary-debug-trait

gavrilikhin-d commented 5 months ago

And there is this debug_all function, which isn't explicitly mentioned but is in the code

gavrilikhin-d commented 5 months ago

I tried to implement DebugWithDb manually, but faced issues with lifetimes. Any advices?

src/ppl_ast/src/declarations/mod.rs:12:5
   |
9  |   pub struct Function {
   |              -------- has type `&<db::Jar as salsa_2022::jar::Jar<'1>>::DynDb`
...
12 | /     #[return_ref]
13 | |     pub name_parts: Vec<FunctionNamePart>,
   | |_________________________________________^ cast requires that `'1` must outlive `'static`

error: lifetime may not live long enough
  --> src/ppl_ast/src/declarations/mod.rs:54:44
   |
48 |         db: &dyn Db,
   |             - let's call the lifetime of this reference `'1`
...
54 |                 DebugWithDb::fmt(param, f, db, include_all_fields)
   |                                            ^^ cast requires that `'1` must outlive `'static`
gavrilikhin-d commented 5 months ago

It's actually quite frustrating to impl DebugWithDb yourself. There is no example in book. Here's how I did it:

#[derive(Debug, PartialEq, Eq, Clone, From)]
pub enum Declaration {
    Function(Function),
    Type(Type),
}

impl<DB: Sized + Db> DebugWithDb<DB> for Declaration {
    fn fmt(
        &self,
        f: &mut std::fmt::Formatter<'_>,
        db: &DB,
        include_all_fields: bool,
    ) -> std::fmt::Result {
        match self {
            Declaration::Function(fun) => fun.fmt(f, db, include_all_fields),
            Declaration::Type(t) => t.fmt(f, db, include_all_fields),
        }
    }
}

(Db is your crate::Db trait)

xiyuzhai commented 5 months ago

I wrote salsa::derive_debug_with_db proc macro for my customized salsa. It's pretty handy for me.


#[salsa::derive_debug_with_db]
#[derive(Debug, PartialEq, Eq)]
pub enum AstData {
    Err {
        token_verse_idx: TokenVerseIdx,
        error: AstError,
    },
    Use {
        token_verse_idx: TokenVerseIdx,
        visibility_expr: VisibilityExpr,
        state_after_visibility_expr: Option<TokenStreamState>,
    },
    /// specify comptime sorceries
    /// doesn't need to be processed until comptime
    Sorc { token_verse_idx: TokenVerseIdx },
    /// decoration, used for deriving trait implementations, etc.
    /// needs to be processed before inference
    Attr {
        token_verse_idx: TokenVerseIdx,
        ident: Ident,
    },
...

The code for the proc macro is here. It's worth the trouble. https://github.com/xiyuzhai-husky-lang/husky/blob/main/crates/abstractions/salsa-macros/src/debug_with_db.rs

gavrilikhin-d commented 5 months ago

@xiyuzhai Do you have your version of salsa as a fork? It looks like development of the library isn't owner's first priority right now. I'm developing a programming language myself and could invest some time in improving salsa

xiyuzhai commented 5 months ago

I don't have a fork. I only have a customized version inside my own programming language https://github.com/xiyuzhai-husky-lang/husky/tree/main/crates/abstractions/salsa

I did many modifications but mostly on macros and traits. I have yet to find time to understand the internal structure of the database.

nikomatsakis commented 5 months ago

I've modified this logic in #482 -- in short, DebugWithDb should now print all fields, and the impl is somewhat less annoying to write.

nikomatsakis commented 5 months ago

Also, I'm investing somewhat more time in salsa at the moment, though I've definitely been inconsistent and may continue to be so! I'd like to boot up a set of maintainers to help keep things steady when I have to drop off, so if you're interested, reach out on the salsa zulip and we can discuss. At the moment I am trying to put in a little bit of time in my spare time as it's not a work priority.

nikomatsakis commented 5 months ago

Closing as I think the original issue is resolved.