Skip to content

Commit

Permalink
Auto merge of #130036 - weiznich:diagnostic_unstable_tracking, r=comp…
Browse files Browse the repository at this point in the history
…iler-errors

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`.

r​? `@compiler-errors`
  • Loading branch information
bors committed Sep 8, 2024
2 parents adf8d16 + 7c9e818 commit 085744b
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 9 deletions.
8 changes: 8 additions & 0 deletions compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1186,3 +1186,11 @@ pub static BUILTIN_ATTRIBUTE_MAP: LazyLock<FxHashMap<Symbol, &BuiltinAttribute>>
}
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,
}
}
5 changes: 3 additions & 2 deletions compiler/rustc_feature/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,9 @@ pub fn find_feature_issue(feature: Symbol, issue: GateIssue) -> Option<NonZero<u
pub use accepted::ACCEPTED_FEATURES;
pub use builtin_attrs::{
deprecated_attributes, encode_cross_crate, find_gated_cfg, is_builtin_attr_name,
is_valid_for_get_attr, AttributeDuplicates, AttributeGate, AttributeSafety, AttributeTemplate,
AttributeType, BuiltinAttribute, GatedCfg, BUILTIN_ATTRIBUTES, BUILTIN_ATTRIBUTE_MAP,
is_stable_diagnostic_attribute, is_valid_for_get_attr, AttributeDuplicates, AttributeGate,
AttributeSafety, AttributeTemplate, AttributeType, BuiltinAttribute, GatedCfg,
BUILTIN_ATTRIBUTES, BUILTIN_ATTRIBUTE_MAP,
};
pub use removed::REMOVED_FEATURES;
pub use unstable::{Features, INCOMPATIBLE_FEATURES, UNSTABLE_FEATURES};
2 changes: 0 additions & 2 deletions compiler/rustc_hir_analysis/src/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1698,8 +1698,6 @@ fn impl_trait_header(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Option<ty::ImplTrai
trait_ref: ty::EarlyBinder::bind(trait_ref),
safety: impl_.safety,
polarity: polarity_of_impl(tcx, def_id, impl_, item.span),
do_not_recommend: tcx.features().do_not_recommend
&& tcx.has_attrs_with_path(def_id, &[sym::diagnostic, sym::do_not_recommend]),
}
})
}
Expand Down
10 changes: 8 additions & 2 deletions compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use rustc_data_structures::fx::{FxIndexMap, FxIndexSet};
use rustc_data_structures::memmap::{Mmap, MmapMut};
use rustc_data_structures::sync::{join, par_for_each_in, Lrc};
use rustc_data_structures::temp_dir::MaybeTempDir;
use rustc_feature::Features;
use rustc_hir as hir;
use rustc_hir::def_id::{LocalDefId, LocalDefIdSet, CRATE_DEF_ID, CRATE_DEF_INDEX, LOCAL_CRATE};
use rustc_hir::definitions::DefPathData;
Expand Down Expand Up @@ -797,9 +798,10 @@ impl<'a, 'tcx> 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
Expand All @@ -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.
Expand All @@ -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;
}
Expand Down Expand Up @@ -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()
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}

Expand Down
32 changes: 31 additions & 1 deletion compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,6 @@ pub struct ImplTraitHeader<'tcx> {
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)]
Expand Down Expand Up @@ -1797,6 +1796,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<DefId>,
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,
Expand Down

0 comments on commit 085744b

Please sign in to comment.