Skip to content

feature: reintroduce ID-based based equality/hashing to #[salsa::Supertype] #694

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

Open
davidbarsky opened this issue Feb 18, 2025 · 1 comment

Comments

@davidbarsky
Copy link
Contributor

After #677 lands, we should re-add the following code to #[salsa:Supertype]:

let std_traits = quote! {
    impl #impl_generics ::core::cmp::PartialEq for #enum_name #type_generics
    #where_clause {
        #[inline]
        fn eq(&self, __other: &Self) -> bool {
            zalsa::AsId::as_id(self) == zalsa::AsId::as_id(__other)
        }
    }

    impl #impl_generics ::core::hash::Hash for #enum_name #type_generics
    #where_clause {
        #[inline]
        fn hash<__H: ::core::hash::Hasher>(&self, __state: &mut __H) {
            ::core::hash::Hash::hash(&zalsa::AsId::as_id(self), __state);
        }
    }
};

It was removed in this commit because ID-based hashing and equality conflicted with with rust-analyzer's interner, which outlives the Salsa database in tests. Once rust-analyzer gets rid of its native interner in favor of Salsa's interner, then I think it'd safe to re-add this functionality. Alternatively, we can reimplement #[salsa::Supertype] as an attribute macro (instead of a derive) and add options to disable deriving Hash/Equality, but I'm too lazy to do that right now.

@Veykril
Copy link
Member

Veykril commented May 8, 2025

Imo we should not emit these impls by default, we could add a derive helper #[supertype(eq, hash)] to emit them but otherwise the user may want to have variant tag based identity still.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants