Skip to content

Remove impl_stable_hash_via_hash!() #96013

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

Closed
michaelwoerister opened this issue Apr 13, 2022 · 1 comment · Fixed by #96082
Closed

Remove impl_stable_hash_via_hash!() #96013

michaelwoerister opened this issue Apr 13, 2022 · 1 comment · Fixed by #96082
Labels
A-incr-comp Area: Incremental compilation C-bug Category: This is a bug. C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-incr-comp Working group: Incremental compilation

Comments

@michaelwoerister
Copy link
Member

I came across impl_stable_hash_via_hash!() the other day and it seems very problematic because one can easily generate invalid HashStable implementations without even noticing:

#[derive(Hash)]
struct Foo<'tcx> {
    def_id: DefId, // numeric value of def_id ends up in fingerprint,
    ptr: *const c_void, // memory address ends up in fingerprint,
    bar: Interned<'tcx, Bar>, // ditto
}

// Yay, three separate bugs in a single line!
impl_stable_hash_via_hash!(Foo);

I think we should just remove the macro (or keep it local to the module that implements HashStable for basic types). #[derive(HashStable_Generic)] is available as a safe alternative.

rustc_lint_defs::Level looks like a case where this is already a problem because of the HirId in the contained LintExpectationId.

cc @rust-lang/wg-incr-comp

@michaelwoerister michaelwoerister added C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-incr-comp Area: Incremental compilation C-bug Category: This is a bug. WG-incr-comp Working group: Incremental compilation labels Apr 13, 2022
@michaelwoerister
Copy link
Member Author

I plan to fix this on Friday if nobody beats me to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-incr-comp Area: Incremental compilation C-bug Category: This is a bug. C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-incr-comp Working group: Incremental compilation
Projects
None yet
1 participant