From cc74de3f9a314724826746aab30566e888e9fcc1 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 7 Apr 2026 14:44:37 +1000 Subject: [PATCH] Remove `HashStable` impl for `[hir::Attribute]`. This impl skips: - All doc comments - A handful of other attributes, mostly `rustc_*` ones related to incremental compilation testing. This skipping originated in #36025 and was extended a couple of times, e.g. in #36370. Those PRs don't have any explanation of why the skipping exists. Perhaps the reasoning was that doc comments should only affect rustdoc and rustdoc doesn't use incremental compilation? But doc comments end up in metadata, and there is a query `attrs_for_def` that returns a `&'tcx [hir::Attribute]`. So skipping some attributes just seems plainly wrong. This commit removes the impl, which means `[hir::Attribute]` hashing falls back to the default impl for `[T]`. This has no noticeable effect on the test suite. It does slightly hurt performance, because of the doc comments. This perf regression seems worth it for the correctness benefits. --- compiler/rustc_middle/src/ich/impls_syntax.rs | 41 ------------------- 1 file changed, 41 deletions(-) diff --git a/compiler/rustc_middle/src/ich/impls_syntax.rs b/compiler/rustc_middle/src/ich/impls_syntax.rs index d932bfdee6ef6..45e9e1c9d09d5 100644 --- a/compiler/rustc_middle/src/ich/impls_syntax.rs +++ b/compiler/rustc_middle/src/ich/impls_syntax.rs @@ -3,9 +3,6 @@ use rustc_ast as ast; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; -use rustc_hir as hir; -use rustc_span::{Symbol, sym}; -use smallvec::SmallVec; use super::StableHashingContext; @@ -16,44 +13,6 @@ impl<'a> HashStable> for ast::NodeId { } } -impl<'a> HashStable> for [hir::Attribute] { - fn hash_stable(&self, hcx: &mut StableHashingContext<'a>, hasher: &mut StableHasher) { - if self.is_empty() { - self.len().hash_stable(hcx, hasher); - return; - } - - // Some attributes are always ignored during hashing. - let filtered: SmallVec<[&hir::Attribute; 8]> = self - .iter() - .filter(|attr| { - attr.is_doc_comment().is_none() - // FIXME(jdonszelmann) have a better way to handle ignored attrs - && !attr.name().is_some_and(|ident| is_ignored_attr(ident)) - }) - .collect(); - - filtered.len().hash_stable(hcx, hasher); - for attr in filtered { - attr.hash_stable(hcx, hasher); - } - } -} - -#[inline] -fn is_ignored_attr(name: Symbol) -> bool { - const IGNORED_ATTRIBUTES: &[Symbol] = &[ - sym::cfg_trace, // FIXME(#138844) should this really be ignored? - sym::rustc_if_this_changed, - sym::rustc_then_this_would_need, - sym::rustc_clean, - sym::rustc_partition_reused, - sym::rustc_partition_codegened, - sym::rustc_expected_cgu_reuse, - ]; - IGNORED_ATTRIBUTES.contains(&name) -} - impl<'tcx> HashStable> for rustc_feature::Features { fn hash_stable(&self, hcx: &mut StableHashingContext<'tcx>, hasher: &mut StableHasher) { // Unfortunately we cannot exhaustively list fields here, since the