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.13k stars 152 forks source link

Rename `Update` to `StoreInSalsaDb` or something like that #592

Open nikomatsakis opened 1 month ago

nikomatsakis commented 1 month ago

The name Update is highly unintuitive. What this value really means is "safe to store in the salsa database".

puuuuh commented 3 weeks ago

SalsaIngredient?

MichaReiser commented 3 weeks ago

I would prefer a term other than Ingredient because it's already a widely used term and I fear we would start overloading it.

I also think that Update is implemented by types that aren't ingredients, but are just part of Ingredients.

nikomatsakis commented 3 weeks ago

Yeah, it is not an ingredient. It is for data structures that can be included in tracked/interned structs but which are not, themselves, tracked/interned structs.

nikomatsakis commented 2 weeks ago

What about SalsaStructMember ?

MichaReiser commented 2 weeks ago

My understanding of the Update trait is still very limited but I like the name if it is the trait that must be implemented by every salsa-struct field (maybe SalsaStructField?)

nikomatsakis commented 2 weeks ago

I like the name SalsaStructField. Also, as discussed in our sync meeting today, I wonder if we should remove the 'static fallback (or, more likely, make it "opt-in" with an annotation on the field). The intent would be that deriving SalsaStructField is more efficient, and it also would make it possible for us to discourage people from using HashMap and HashSet (as opposed to FxHashSet etc) since those can easily lead to non-deterministic results.

MichaReiser commented 2 weeks ago

The intent would be that deriving SalsaStructField is more efficient

I'm open to trying this on Ruff if we have a PR that adds this requirement to get some data on how painful it is for users.