Skip to content

Commit

Permalink
Clarify/fix const-unstable semantics in the presence of multiple unst…
Browse files Browse the repository at this point in the history
…able features
  • Loading branch information
dianne committed Nov 6, 2024
1 parent d9915da commit b9af36e
Show file tree
Hide file tree
Showing 8 changed files with 50 additions and 49 deletions.
18 changes: 9 additions & 9 deletions compiler/rustc_attr/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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<Item = Symbol> + use<'_> {
Expand Down Expand Up @@ -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 {
Expand All @@ -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.
Expand Down Expand Up @@ -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<Item = Symbol> + use<'_> {
Expand Down Expand Up @@ -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;
}
Expand Down
47 changes: 21 additions & 26 deletions compiler/rustc_const_eval/src/check_consts/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand All @@ -838,43 +843,33 @@ 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;
}

// We can't use `check_op` to check whether the feature is enabled because
// 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,
});
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/check_consts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}
20 changes: 14 additions & 6 deletions compiler/rustc_const_eval/src/check_consts/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
),
})
Expand Down Expand Up @@ -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,
),
})
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/middle/stability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<NonZero<u32>>,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_passes/src/stability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
6 changes: 2 additions & 4 deletions src/librustdoc/html/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 => "",
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/html/render/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit b9af36e

Please sign in to comment.