From 717a11788d49a960c94509ec97d35114fa4b3a7f Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Fri, 6 Sep 2024 18:47:22 +0200 Subject: [PATCH 1/2] Correctly handle stability of `#[diagnostic]` attributes This commit changes the way we treat the stability of attributes in the `#[diagnostic]` namespace. Instead of relaying on ad-hoc checks to ensure at call side that a certain attribute is really usable at that location it centralises the logic to one place. For diagnostic attributes comming from other crates it just skips serializing attributes that are not stable and that do not have the corresponding feature enabled. For attributes from the current crate we can just use the feature information provided by `TyCtx`. --- compiler/rustc_feature/src/builtin_attrs.rs | 8 +++++ compiler/rustc_feature/src/lib.rs | 5 ++-- compiler/rustc_metadata/src/rmeta/encoder.rs | 10 +++++-- compiler/rustc_middle/src/ty/context.rs | 3 +- compiler/rustc_middle/src/ty/mod.rs | 31 ++++++++++++++++++++ 5 files changed, 51 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_feature/src/builtin_attrs.rs b/compiler/rustc_feature/src/builtin_attrs.rs index e86421f2150db..3b7e0d82d0f62 100644 --- a/compiler/rustc_feature/src/builtin_attrs.rs +++ b/compiler/rustc_feature/src/builtin_attrs.rs @@ -1186,3 +1186,11 @@ pub static BUILTIN_ATTRIBUTE_MAP: LazyLock> } map }); + +pub fn is_stable_diagnostic_attribute(sym: Symbol, features: &Features) -> bool { + match sym { + sym::on_unimplemented => true, + sym::do_not_recommend => features.do_not_recommend, + _ => false, + } +} diff --git a/compiler/rustc_feature/src/lib.rs b/compiler/rustc_feature/src/lib.rs index adaaba3cd235e..fe12930e6b9d6 100644 --- a/compiler/rustc_feature/src/lib.rs +++ b/compiler/rustc_feature/src/lib.rs @@ -130,8 +130,9 @@ pub fn find_feature_issue(feature: Symbol, issue: GateIssue) -> Option EncodeContext<'a, 'tcx> { } } -struct AnalyzeAttrState { +struct AnalyzeAttrState<'a> { is_exported: bool, is_doc_hidden: bool, + features: &'a Features, } /// Returns whether an attribute needs to be recorded in metadata, that is, if it's usable and @@ -812,7 +814,7 @@ struct AnalyzeAttrState { /// visibility: this is a piece of data that can be computed once per defid, and not once per /// attribute. Some attributes would only be usable downstream if they are public. #[inline] -fn analyze_attr(attr: &Attribute, state: &mut AnalyzeAttrState) -> bool { +fn analyze_attr(attr: &Attribute, state: &mut AnalyzeAttrState<'_>) -> bool { let mut should_encode = false; if !rustc_feature::encode_cross_crate(attr.name_or_empty()) { // Attributes not marked encode-cross-crate don't need to be encoded for downstream crates. @@ -837,6 +839,9 @@ fn analyze_attr(attr: &Attribute, state: &mut AnalyzeAttrState) -> bool { } } } + } else if attr.path().starts_with(&[sym::diagnostic]) && attr.path().len() == 2 { + should_encode = + rustc_feature::is_stable_diagnostic_attribute(attr.path()[1], state.features); } else { should_encode = true; } @@ -1343,6 +1348,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { let mut state = AnalyzeAttrState { is_exported: tcx.effective_visibilities(()).is_exported(def_id), is_doc_hidden: false, + features: &tcx.features(), }; let attr_iter = tcx .hir() diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index 20828067c469c..ff34555920de5 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -3183,8 +3183,7 @@ impl<'tcx> TyCtxt<'tcx> { /// Whether this is a trait implementation that has `#[diagnostic::do_not_recommend]` pub fn do_not_recommend_impl(self, def_id: DefId) -> bool { - matches!(self.def_kind(def_id), DefKind::Impl { of_trait: true }) - && self.impl_trait_header(def_id).is_some_and(|header| header.do_not_recommend) + self.get_diagnostic_attr(def_id, sym::do_not_recommend).is_some() } } diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index ee70a6346d920..ee4f67f0e939e 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -1797,6 +1797,37 @@ impl<'tcx> TyCtxt<'tcx> { } } + /// Get an attribute from the diagnostic attribute namespace + /// + /// This function requests an attribute with the following structure: + /// + /// `#[diagnostic::$attr]` + /// + /// This function performs feature checking, so if an attribute is returned + /// it can be used by the consumer + pub fn get_diagnostic_attr( + self, + did: impl Into, + attr: Symbol, + ) -> Option<&'tcx ast::Attribute> { + let did: DefId = did.into(); + if did.as_local().is_some() { + // it's a crate local item, we need to check feature flags + if rustc_feature::is_stable_diagnostic_attribute(attr, self.features()) { + self.get_attrs_by_path(did, &[sym::diagnostic, sym::do_not_recommend]).next() + } else { + None + } + } else { + // we filter out unstable diagnostic attributes before + // encoding attributes + debug_assert!(rustc_feature::encode_cross_crate(attr)); + self.item_attrs(did) + .iter() + .find(|a| matches!(a.path().as_ref(), [sym::diagnostic, a] if *a == attr)) + } + } + pub fn get_attrs_by_path<'attr>( self, did: DefId, From 7c9e818f028a899cf4597f80c67bfe64b7e1a4ed Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Fri, 6 Sep 2024 19:06:59 +0200 Subject: [PATCH 2/2] Revert ed7bdbb17b9c03fe3530e5e3f21b7c6c7879dbca --- compiler/rustc_hir_analysis/src/collect.rs | 2 -- compiler/rustc_middle/src/ty/mod.rs | 1 - 2 files changed, 3 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/collect.rs b/compiler/rustc_hir_analysis/src/collect.rs index 92111805ab422..ac9976148e25c 100644 --- a/compiler/rustc_hir_analysis/src/collect.rs +++ b/compiler/rustc_hir_analysis/src/collect.rs @@ -1698,8 +1698,6 @@ fn impl_trait_header(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Option { pub trait_ref: ty::EarlyBinder<'tcx, ty::TraitRef<'tcx>>, pub polarity: ImplPolarity, pub safety: hir::Safety, - pub do_not_recommend: bool, } #[derive(Copy, Clone, PartialEq, Eq, Debug, TypeFoldable, TypeVisitable)]