Volatile functions were helpful to avoid `Eq` bounds for return values

[orium]

Hi,

I was looking into updating chalk to the latest version of salsa. Given that the salsa::volatile attribute was removed in #174 in favor of calling Runtime::report_untracked_read() this means that chalk_integration::LoweringDatabase::solver() (https://github.com/rust-lang/chalk/blob/73a74be3bc1d0cdef3f76fa529a112a0d8367ddb/chalk-integration/src/query.rs#L54) needs to return something that implements Eq, which is not the case.

Obviously we can create a wrapper struct NeverEqual<T>(T) that will always say things are different, but that is just a weird thing to have to do.

I couldn't find the reason for volatile to be removed, but offering something like that makes sense IMO. Was there any fundamental buggy behavior involving it or is it something that can be re-implemented in one way or another? @matklad @nikomatsakis

Edit: CC #56

[bjorn3]

Obviously we can create a wrapper struct NeverEqual(T) that will always say things are different, but that is just a weird thing to have to do.

https://doc.rust-lang.org/stable/std/cmp/trait.Eq.html

Trait for equality comparisons which are equivalence relations.

This means, that in addition to a == b and a != b being strict inverses, the equality must be (for all a, b and c):

• reflexive: a == a;
• symmetric: a == b implies b == a; and
• transitive: a == b and b == c implies a == c.

Emphasis mine. For PartialEq it would be allowed. The fact that NaN floats compare unequal to themselves is the whole reason for the PartialEq/Eq split.

[orium]

Yes, it would be a wrong implementation of Eq, which makes it an even worse hack than it superficially looks.

[nikomatsakis]

I can't remember if we ever addressed this problem? I agree that it would be good to not require Eq

[orium]

@nikomatsakis This is still a problem. For example, it complicates updating salsa in chalk, which is still using salsa 0.10 where the salsa::volatile attribute still exists.

This attribute was removed in #174 by @matklad, but I don't know the motivation for that. It can be the case that something was broken about salsa::volatile, but if there wasn't anything wrong with it we could simply revert that PR.

[nikomatsakis]

I remember removing volatile -- you can now emulate it via helper calls. But I don't remember what solution we had in mind for not requiring Eq. From skimming the source, it looks like you could declare the query as #[dependencies] and we would not record any value at all, just dependency information.

It'd be nice if we had a way to let you define how "eq" works, though.