From b9af36e671aff4c11808673aed1fa120c881f67a Mon Sep 17 00:00:00 2001 From: dianne Date: Sun, 27 Oct 2024 23:32:38 -0700 Subject: [PATCH] Clarify/fix const-unstable semantics in the presence of multiple unstable features --- compiler/rustc_attr/src/builtin.rs | 18 +++---- .../src/check_consts/check.rs | 47 +++++++++---------- .../rustc_const_eval/src/check_consts/mod.rs | 2 +- .../rustc_const_eval/src/check_consts/ops.rs | 20 +++++--- compiler/rustc_middle/src/middle/stability.rs | 2 +- compiler/rustc_passes/src/stability.rs | 2 +- src/librustdoc/html/format.rs | 6 +-- src/librustdoc/html/render/mod.rs | 2 +- 8 files changed, 50 insertions(+), 49 deletions(-) diff --git a/compiler/rustc_attr/src/builtin.rs b/compiler/rustc_attr/src/builtin.rs index 7b84fd5fad9c..977e1817456e 100644 --- a/compiler/rustc_attr/src/builtin.rs +++ b/compiler/rustc_attr/src/builtin.rs @@ -96,6 +96,7 @@ impl Stability { } /// Represents the `#[rustc_const_unstable]` and `#[rustc_const_stable]` attributes. +/// For details see [the dev guide](https://rustc-dev-guide.rust-lang.org/stability.html#rustc_const_unstable). #[derive(Encodable, Decodable, Clone, Debug, PartialEq, Eq, Hash)] #[derive(HashStable_Generic)] pub struct ConstStability { @@ -115,8 +116,8 @@ impl ConstStability { self.level.is_const_stable() } - pub fn is_implicit(&self) -> bool { - self.level.is_implicit() + pub fn is_implicit_unstable(&self) -> bool { + self.level.is_implicit_unstable() } pub fn unstable_features(&self) -> impl Iterator + use<'_> { @@ -154,7 +155,6 @@ pub enum StabilityLevel { } /// The available const-stability levels for const functions. -/// For details see [#131349](https://github.com/rust-lang/rust/pull/131349). #[derive(Encodable, Decodable, PartialEq, Clone, Debug, Eq, Hash)] #[derive(HashStable_Generic)] pub enum ConstStabilityLevel { @@ -169,7 +169,7 @@ pub enum ConstStabilityLevel { }, /// For functions with no explicit const-stability attribute that require checking recursive /// const stability. This is either an unmarked const fn or a `const_stable_indirect` intrinsic. - Implicit, + ImplicitUnstable, } /// Rust release in which a feature is stabilized. @@ -224,15 +224,15 @@ impl StabilityLevel { impl ConstStabilityLevel { pub fn is_const_unstable(&self) -> bool { - matches!(self, ConstStabilityLevel::Unstable { .. }) + matches!(self, ConstStabilityLevel::Unstable { .. } | ConstStabilityLevel::ImplicitUnstable) } pub fn is_const_stable(&self) -> bool { matches!(self, ConstStabilityLevel::Stable { .. }) } - pub fn is_implicit(&self) -> bool { - matches!(self, ConstStabilityLevel::Implicit) + pub fn is_implicit_unstable(&self) -> bool { + matches!(self, ConstStabilityLevel::ImplicitUnstable) } pub fn unstable_features(&self) -> impl Iterator + use<'_> { @@ -428,10 +428,10 @@ pub fn find_const_stability( // record this, and so that's what we do for all `const fn` in a staged_api crate. if is_const_fn || const_stable_indirect.is_some() { stab_spans = ConstStabilitySpans(smallvec![( - ConstStabilityLevel::Implicit, + ConstStabilityLevel::ImplicitUnstable, const_stable_indirect.unwrap_or(DUMMY_SP) )]); - ConstStabilityLevel::Implicit + ConstStabilityLevel::ImplicitUnstable } else { return None; } diff --git a/compiler/rustc_const_eval/src/check_consts/check.rs b/compiler/rustc_const_eval/src/check_consts/check.rs index 9f54da1a2b67..b00a9edaf4e6 100644 --- a/compiler/rustc_const_eval/src/check_consts/check.rs +++ b/compiler/rustc_const_eval/src/check_consts/check.rs @@ -23,7 +23,6 @@ use rustc_span::{Span, Symbol, sym}; use rustc_trait_selection::traits::{ Obligation, ObligationCause, ObligationCauseCode, ObligationCtxt, }; -use smallvec::SmallVec; use tracing::{debug, instrument, trace}; use super::ops::{self, NonConstOp, Status}; @@ -761,7 +760,10 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> { // Non-const intrinsic. self.check_op(ops::IntrinsicNonConst { name: intrinsic.name }); } - Some(ConstStability { level: ConstStabilityLevel::Implicit, .. }) => { + Some(ConstStability { + level: ConstStabilityLevel::ImplicitUnstable, + .. + }) => { // Intrinsic does not need a separate feature gate (we rely on the // regular stability checker). However, we have to worry about recursive // const stability. @@ -815,7 +817,10 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> { Some(ConstStability { level: ConstStabilityLevel::Stable { .. }, .. }) => { // All good. } - None | Some(ConstStability { level: ConstStabilityLevel::Implicit, .. }) => { + None + | Some(ConstStability { + level: ConstStabilityLevel::ImplicitUnstable, .. + }) => { // This doesn't need a separate const-stability check -- const-stability equals // regular stability, and regular stability is checked separately. // However, we *do* have to worry about *recursive* const stability. @@ -838,14 +843,12 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> { // We only honor `span.allows_unstable` aka `#[allow_internal_unstable]` if // the callee is safe to expose, to avoid bypassing recursive stability. - let is_allowed_unstable = |u: &rustc_attr::Unstability| { - callee_safe_to_expose_on_stable - && (self.span.allows_unstable(u.feature) - || u.implied_by.is_some_and(|f| self.span.allows_unstable(f))) - }; - let mut needs_check = - unstables.iter().filter(|u| !is_allowed_unstable(u)).peekable(); - if needs_check.peek().is_none() { + if callee_safe_to_expose_on_stable + && unstables.iter().all(|u| { + self.span.allows_unstable(u.feature) + || u.implied_by.is_some_and(|f| self.span.allows_unstable(f)) + }) + { return; } @@ -853,28 +856,20 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> { // the logic is a bit different than elsewhere: local functions don't need // the feature gate, and there might be an "implied" gate that also suffices // to allow this. - let is_feature_enabled = |u: &rustc_attr::Unstability| { - callee.is_local() - || tcx.features().enabled(u.feature) - || u.implied_by.is_some_and(|f| tcx.features().enabled(f)) - }; + let features_enabled = callee.is_local() + || unstables.iter().all(|u| { + tcx.features().enabled(u.feature) + || u.implied_by.is_some_and(|f| tcx.features().enabled(f)) + }); // We do *not* honor this if we are in the "danger zone": we have to enforce // recursive const-stability and the callee is not safe-to-expose. In that // case we need `check_op` to do the check. let danger_zone = !callee_safe_to_expose_on_stable && self.enforce_recursive_const_stability(); - let missing_features: SmallVec<[_; 1]> = if danger_zone { - needs_check.map(|u| u.into()).collect() - } else { - needs_check - .filter(|u| !is_feature_enabled(u)) - .map(|u| u.into()) - .collect() - }; - if !missing_features.is_empty() { + if danger_zone || !features_enabled { self.check_op(ops::FnCallUnstable { def_id: callee, - features: missing_features, + features: unstables.iter().map(|u| u.into()).collect(), reason: *reason, safe_to_expose_on_stable: callee_safe_to_expose_on_stable, }); diff --git a/compiler/rustc_const_eval/src/check_consts/mod.rs b/compiler/rustc_const_eval/src/check_consts/mod.rs index fdcf44d9c462..d4410a7e15b1 100644 --- a/compiler/rustc_const_eval/src/check_consts/mod.rs +++ b/compiler/rustc_const_eval/src/check_consts/mod.rs @@ -116,7 +116,7 @@ pub fn is_safe_to_expose_on_stable_const_fn(tcx: TyCtxt<'_>, def_id: DefId) -> b Some(stab) => { // We consider things safe-to-expose if they are stable, if they don't have any explicit // const stability attribute, or if they are marked as `const_stable_indirect`. - stab.is_const_stable() || stab.is_implicit() || stab.const_stable_indirect + stab.is_const_stable() || stab.is_implicit_unstable() || stab.const_stable_indirect } } } diff --git a/compiler/rustc_const_eval/src/check_consts/ops.rs b/compiler/rustc_const_eval/src/check_consts/ops.rs index a3fcb2effac2..241177d9df77 100644 --- a/compiler/rustc_const_eval/src/check_consts/ops.rs +++ b/compiler/rustc_const_eval/src/check_consts/ops.rs @@ -325,15 +325,19 @@ impl<'tcx> NonConstOp<'tcx> for FnCallUnstable { } fn build_error(&self, ccx: &ConstCx<'_, 'tcx>, span: Span) -> Diag<'tcx> { + // Only report the features that aren't already enabled. + let missing_features = Vec::from_iter( + self.features.iter().filter(|d| !ccx.tcx.features().enabled(d.feature)).copied(), + ); ccx.dcx().create_err(errors::UnstableConstFn { span, def_path: ccx.tcx.def_path_str(self.def_id), - features: stability::unstable_message(&self.features, self.reason.to_opt_reason()) + features: stability::unstable_message(&missing_features, self.reason.to_opt_reason()) .hide_features_on_nightly(&ccx.tcx.sess), - issues: stability::unstable_issues(&self.features), + issues: stability::unstable_issues(&missing_features), nightly_subdiags: stability::unstable_nightly_subdiags( &ccx.tcx.sess, - &self.features, + &missing_features, None, ), }) @@ -377,15 +381,19 @@ impl<'tcx> NonConstOp<'tcx> for IntrinsicUnstable { } fn build_error(&self, ccx: &ConstCx<'_, 'tcx>, span: Span) -> Diag<'tcx> { + // Only report the features that aren't already enabled. + let missing_features = Vec::from_iter( + self.features.iter().filter(|d| !ccx.tcx.features().enabled(d.feature)).copied(), + ); ccx.dcx().create_err(errors::UnstableIntrinsic { span, name: self.name, - features: stability::unstable_message(&self.features, self.reason.to_opt_reason()) + features: stability::unstable_message(&missing_features, self.reason.to_opt_reason()) .hide_features_on_nightly(&ccx.tcx.sess), - issues: stability::unstable_issues(&self.features), + issues: stability::unstable_issues(&missing_features), nightly_subdiags: stability::unstable_nightly_subdiags( &ccx.tcx.sess, - &self.features, + &missing_features, None, ), }) diff --git a/compiler/rustc_middle/src/middle/stability.rs b/compiler/rustc_middle/src/middle/stability.rs index 9d08336ebc91..d32acfca19df 100644 --- a/compiler/rustc_middle/src/middle/stability.rs +++ b/compiler/rustc_middle/src/middle/stability.rs @@ -362,7 +362,7 @@ pub enum EvalResult { } /// An instance of a disabled feature required for an unstable item -#[derive(Debug)] +#[derive(Clone, Copy, Debug)] pub struct EvalDenial { pub feature: Symbol, pub issue: Option>, diff --git a/compiler/rustc_passes/src/stability.rs b/compiler/rustc_passes/src/stability.rs index 4434c44b820f..2b45ce861fc1 100644 --- a/compiler/rustc_passes/src/stability.rs +++ b/compiler/rustc_passes/src/stability.rs @@ -600,7 +600,7 @@ impl<'tcx> MissingStabilityAnnotations<'tcx> { let is_stable = self.tcx.lookup_stability(def_id).is_some_and(|stability| stability.level.is_stable()); let missing_const_stability_attribute = - self.tcx.lookup_const_stability(def_id).is_none_or(|s| s.is_implicit()); + self.tcx.lookup_const_stability(def_id).is_none_or(|s| s.is_implicit_unstable()); if is_const && is_stable && missing_const_stability_attribute { let descr = self.tcx.def_descr(def_id.to_def_id()); diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index ba118f8a1eda..d86096cf6711 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -1700,14 +1700,12 @@ pub(crate) fn print_constness_with_space( | (_, None) // ...or when const unstable, but overall unstable too... | (None, Some(ConstStability { level: ConstStabilityLevel::Unstable { .. }, .. })) - // ...or when a const stability level was not explicitly provided, and overall unstable - | (None, Some(ConstStability { level: ConstStabilityLevel::Implicit, .. })) => { + | (None, Some(ConstStability { level: ConstStabilityLevel::ImplicitUnstable, .. })) => { "const " } // const unstable (and overall stable)... (Some(_), Some(ConstStability { level: ConstStabilityLevel::Unstable { .. }, .. })) - // ...or without an explicit const stability level (and overall stable) - | (Some(_), Some(ConstStability { level: ConstStabilityLevel::Implicit, .. })) => "", + | (Some(_), Some(ConstStability { level: ConstStabilityLevel::ImplicitUnstable, .. })) => "", }, // not const hir::Constness::NotConst => "", diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index 636b1426f13e..a77f58544ae4 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -1041,7 +1041,7 @@ fn render_stability_since_raw_with_extra( Some((String::from("const unstable"), format!("const: {unstable}"))) } } - Some(ConstStability { level: ConstStabilityLevel::Implicit, .. }) => { + Some(ConstStability { level: ConstStabilityLevel::ImplicitUnstable, .. }) => { // No explicit const-stability annotation was provided. Treat it as const-unstable. if stable_version.is_none() { None