Skip to content

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

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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
# for free to join this conversation on GitHub. Already have an account? # 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