Skip to content

Commit

Permalink
passes: load defined_lib_features query less
Browse files Browse the repository at this point in the history
Re-structure the stability checks for library features to avoid calling
`defined_lib_features` for any more crates than necessary for each of
the implications or local feature attributes that need validation.
  • Loading branch information
davidtwco committed Aug 9, 2022
1 parent cc4dd6f commit 5e2e478
Showing 1 changed file with 91 additions and 36 deletions.
127 changes: 91 additions & 36 deletions compiler/rustc_passes/src/stability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -949,58 +949,113 @@ pub fn check_unused_or_stable_features(tcx: TyCtxt<'_>) {
remaining_lib_features.remove(&sym::libc);
remaining_lib_features.remove(&sym::test);

// We always collect the lib features declared in the current crate, even if there are
// no unknown features, because the collection also does feature attribute validation.
let local_defined_features = tcx.lib_features(());
let mut all_lib_features: FxHashMap<_, _> =
local_defined_features.to_vec().iter().map(|el| *el).collect();
let mut implications = tcx.stability_implications(rustc_hir::def_id::LOCAL_CRATE).clone();
for &cnum in tcx.crates(()) {
implications.extend(tcx.stability_implications(cnum));
all_lib_features.extend(tcx.defined_lib_features(cnum).iter().map(|el| *el));
}

// Check that every feature referenced by an `implied_by` exists (for features defined in the
// local crate).
for (implied_by, feature) in tcx.stability_implications(rustc_hir::def_id::LOCAL_CRATE) {
// Only `implied_by` needs to be checked, `feature` is guaranteed to exist.
if !all_lib_features.contains_key(implied_by) {
let span = local_defined_features
.stable
.get(feature)
.map(|(_, span)| span)
.or_else(|| local_defined_features.unstable.get(feature))
.expect("feature that implied another does not exist");
tcx.sess
.struct_span_err(
*span,
format!("feature `{implied_by}` implying `{feature}` does not exist"),
)
.emit();
}
}

if !remaining_lib_features.is_empty() {
for (feature, since) in all_lib_features.iter() {
/// For each feature in `defined_features`..
///
/// - If it is in `remaining_lib_features` (those features with `#![feature(..)]` attributes in
/// the current crate), check if it is stable (or partially stable) and thus an unnecessary
/// attribute.
/// - If it is in `remaining_implications` (a feature that is referenced by an `implied_by`
/// from the current crate), then remove it from the remaining implications.
///
/// Once this function has been invoked for every feature (local crate and all extern crates),
/// then..
///
/// - If features remain in `remaining_lib_features`, then the user has enabled a feature that
/// does not exist.
/// - If features remain in `remaining_implications`, the `implied_by` refers to a feature that
/// does not exist.
///
/// By structuring the code in this way: checking the features defined from each crate one at a
/// time, less loading from metadata is performed and thus compiler performance is improved.
fn check_features<'tcx>(
tcx: TyCtxt<'tcx>,
remaining_lib_features: &mut FxIndexMap<&Symbol, Span>,
remaining_implications: &mut FxHashMap<Symbol, Symbol>,
defined_features: &[(Symbol, Option<Symbol>)],
all_implications: &FxHashMap<Symbol, Symbol>,
) {
for (feature, since) in defined_features {
if let Some(since) = since && let Some(span) = remaining_lib_features.get(&feature) {
// Warn if the user has enabled an already-stable lib feature.
if let Some(implies) = implications.get(&feature) {
if let Some(implies) = all_implications.get(&feature) {
unnecessary_partially_stable_feature_lint(tcx, *span, *feature, *implies, *since);
} else {
unnecessary_stable_feature_lint(tcx, *span, *feature, *since);
}

}
remaining_lib_features.remove(&feature);
if remaining_lib_features.is_empty() {
remaining_lib_features.remove(feature);

// `feature` is the feature doing the implying, but `implied_by` is the feature with
// the attribute that establishes this relationship. `implied_by` is guaranteed to be a
// feature defined in the local crate because `remaining_implications` is only the
// implications from this crate.
remaining_implications.remove(feature);

if remaining_lib_features.is_empty() && remaining_implications.is_empty() {
break;
}
}
}

// All local crate implications need to have the feature that implies it confirmed to exist.
let mut remaining_implications =
tcx.stability_implications(rustc_hir::def_id::LOCAL_CRATE).clone();

// We always collect the lib features declared in the current crate, even if there are
// no unknown features, because the collection also does feature attribute validation.
let local_defined_features = tcx.lib_features(()).to_vec();
if !remaining_lib_features.is_empty() || !remaining_implications.is_empty() {
// Loading the implications of all crates is unavoidable to be able to emit the partial
// stabilization diagnostic, but it can be avoided when there are no
// `remaining_lib_features`.
let mut all_implications = remaining_implications.clone();
for &cnum in tcx.crates(()) {
all_implications.extend(tcx.stability_implications(cnum));
}

check_features(
tcx,
&mut remaining_lib_features,
&mut remaining_implications,
local_defined_features.as_slice(),
&all_implications,
);

for &cnum in tcx.crates(()) {
if remaining_lib_features.is_empty() && remaining_implications.is_empty() {
break;
}
check_features(
tcx,
&mut remaining_lib_features,
&mut remaining_implications,
tcx.defined_lib_features(cnum).to_vec().as_slice(),
&all_implications,
);
}
}

for (feature, span) in remaining_lib_features {
struct_span_err!(tcx.sess, span, E0635, "unknown feature `{}`", feature).emit();
}

for (implied_by, feature) in remaining_implications {
let local_defined_features = tcx.lib_features(());
let span = local_defined_features
.stable
.get(&feature)
.map(|(_, span)| span)
.or_else(|| local_defined_features.unstable.get(&feature))
.expect("feature that implied another does not exist");
tcx.sess
.struct_span_err(
*span,
format!("feature `{implied_by}` implying `{feature}` does not exist"),
)
.emit();
}

// FIXME(#44232): the `used_features` table no longer exists, so we
// don't lint about unused features. We should re-enable this one day!
}
Expand Down

0 comments on commit 5e2e478

Please sign in to comment.