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

Can we add a new StableLintExpectationId type so we can remove unstable Hash impls #101751

Closed
oli-obk opened this issue Sep 13, 2022 · 1 comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. F-lint_reasons `#![feature(lint_reasons)]`

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Sep 13, 2022

Can we add a new StableLintExpectationId type so we can statically prevent the Unstable case? Then we'd have enum LintExpectationId { Unstable { attr_id, lint_index }, Stable(StableLintExpectationId) }, would be a slight pain to rewrite but mostly mechanical.

Might make sense to do it for the return type of normalize() too so we don't accidentally hash the attr_id; that seems worse since it will only invalidate the incremental cache but not be obvious that it's a bug.

Originally posted by @jyn514 in #101620 (comment)

@oli-obk oli-obk added the C-cleanup Category: PRs that clean code up or issues documenting cleanup. label Sep 13, 2022
@xFrednet xFrednet added the F-lint_reasons `#![feature(lint_reasons)]` label Sep 13, 2022
@Urgau
Copy link
Member

Urgau commented Sep 20, 2024

Seems to have been done.

pub enum LintExpectationId {
/// Used for lints emitted during the `EarlyLintPass`. This id is not
/// hash stable and should not be cached.
Unstable { attr_id: AttrId, lint_index: Option<u16> },
/// The [`HirId`] that the lint expectation is attached to. This id is
/// stable and can be cached. The additional index ensures that nodes with
/// several expectations can correctly match diagnostics to the individual
/// expectation.
Stable { hir_id: HirId, attr_index: u16, lint_index: Option<u16> },
}

@Urgau Urgau closed this as completed Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. F-lint_reasons `#![feature(lint_reasons)]`
Projects
None yet
Development

No branches or pull requests

3 participants