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

[breaking-salsa-2022] Debug with db: ignore deps, support customization #482

Merged

Conversation

nikomatsakis
Copy link
Member

This changes how DebugWithDb works:

  • All field values are always included (useful) and thus the include_all_fields flag is removed (it was annoying).
  • Dependencies from those field values are ignored by salsa (useful, but potentially dangerous).
  • You can use #[customize(DebugWithDb)] to skip generating the impl for a given type.

I think overall this is an improvement, though it is a breaking change.

In a previous PR we added the `include_all_fields`
parameter to `DebugWithDb` to allow it to
not create spurious dependencies.

This PR takes a different approach: we simply
ignore the dependencies created during debug
operations. This is risky as it can create
incorrect dependencies, but it is way more
convenient and seems like what users probably
want.

It also means that `DebugWithDb` has a simpler
signature that matches the `Debug` trait again,
which seems good to me.
Copy link

netlify bot commented Apr 3, 2024

Deploy Preview for salsa-rs canceled.

Name Link
🔨 Latest commit ce2f782
🔍 Latest deploy log https://app.netlify.com/sites/salsa-rs/deploys/660d30a2c44336000817b339

@nikomatsakis nikomatsakis added this pull request to the merge queue Apr 4, 2024
Merged via the queue into salsa-rs:master with commit bdf7d9c Apr 4, 2024
8 checks passed
@nikomatsakis nikomatsakis deleted the debug-with-db-improvements branch April 4, 2024 10:06
Comment on lines +96 to +105
impl DebugWithDb<dyn Db + '_> for DerivedCustom {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>, db: &dyn Db) -> std::fmt::Result {
write!(
f,
"{:?} / {:?}",
self.my_input(db).debug(db),
self.value(db)
)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice: That's something I've come across too where some tracked structs have fields that are salsa ingredients and the "default" generated DebugWithDb implementation didn't emit them with debug. Thanks for making this change.

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