Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support multiple unstable attributes on items #94988

Closed
wants to merge 2 commits into from
Closed

Support multiple unstable attributes on items #94988

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
90a545b
Support multiple unstable attributes
compiler-errors Mar 16, 2022
013ac1a
Remove spans from StabilityLevel, so it can be decoded in ProcMacroData
compiler-errors Mar 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
148 changes: 109 additions & 39 deletions compiler/rustc_attr/src/builtin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,39 +94,54 @@ pub enum OptimizeAttr {
///
/// - `#[stable]`
/// - `#[unstable]`
#[derive(Encodable, Decodable, Copy, Clone, Debug, PartialEq, Eq, Hash)]
#[derive(Encodable, Decodable, Clone, Debug, PartialEq, Eq, Hash)]
#[derive(HashStable_Generic)]
pub struct Stability {
pub level: StabilityLevel,
pub feature: Symbol,
}

/// Represents the `#[rustc_const_unstable]` and `#[rustc_const_stable]` attributes.
#[derive(Encodable, Decodable, Copy, Clone, Debug, PartialEq, Eq, Hash)]
#[derive(Encodable, Decodable, Clone, Debug, PartialEq, Eq, Hash)]
#[derive(HashStable_Generic)]
pub struct ConstStability {
pub level: StabilityLevel,
pub feature: Symbol,
/// whether the function has a `#[rustc_promotable]` attribute
pub promotable: bool,
}

/// The available stability levels.
#[derive(Encodable, Decodable, PartialEq, Copy, Clone, Debug, Eq, Hash)]
#[derive(Encodable, Decodable, PartialEq, Clone, Debug, Eq, Hash)]
#[derive(HashStable_Generic)]
pub enum StabilityLevel {
// Reason for the current stability level and the relevant rust-lang issue
Unstable { reason: Option<Symbol>, issue: Option<NonZeroU32>, is_soft: bool },
Stable { since: Symbol },
Unstable { unstables: Vec<Unstability>, is_soft: bool },
Stable { feature: Symbol, since: Symbol },
}

/// An instance of the `#[unstable]` attribute
#[derive(Encodable, Decodable, PartialEq, Clone, Debug, Eq, Hash)]
#[derive(HashStable_Generic)]
pub struct Unstability {
pub feature: Symbol,
pub issue: Option<NonZeroU32>,
pub reason: Option<Symbol>,
}

impl StabilityLevel {
pub fn is_unstable(&self) -> bool {
matches!(self, StabilityLevel::Unstable { .. })
}

pub fn is_stable(&self) -> bool {
matches!(self, StabilityLevel::Stable { .. })
}

pub fn has_unstable_feature(&self, sym: Symbol) -> bool {
if let StabilityLevel::Unstable { unstables, .. } = &self {
unstables.iter().any(|u| u.feature == sym)
} else {
false
}
}
}

/// Collects stability info from all stability attributes in `attrs`.
Expand All @@ -135,22 +150,22 @@ pub fn find_stability(
sess: &Session,
attrs: &[Attribute],
item_sp: Span,
) -> (Option<(Stability, Span)>, Option<(ConstStability, Span)>) {
) -> (Option<(Stability, Vec<Span>)>, Option<(ConstStability, Vec<Span>)>) {
find_stability_generic(sess, attrs.iter(), item_sp)
}

fn find_stability_generic<'a, I>(
sess: &Session,
attrs_iter: I,
item_sp: Span,
) -> (Option<(Stability, Span)>, Option<(ConstStability, Span)>)
) -> (Option<(Stability, Vec<Span>)>, Option<(ConstStability, Vec<Span>)>)
where
I: Iterator<Item = &'a Attribute>,
{
use StabilityLevel::*;

let mut stab: Option<(Stability, Span)> = None;
let mut const_stab: Option<(ConstStability, Span)> = None;
let mut stab: Option<(Stability, Vec<Span>)> = None;
let mut const_stab: Option<(ConstStability, Vec<Span>)> = None;
let mut promotable = false;

let diagnostic = &sess.parse_sess.span_diagnostic;
Expand Down Expand Up @@ -198,22 +213,6 @@ where
let meta_name = meta.name_or_empty();
match meta_name {
sym::rustc_const_unstable | sym::unstable => {
if meta_name == sym::unstable && stab.is_some() {
handle_errors(
&sess.parse_sess,
attr.span,
AttrError::MultipleStabilityLevels,
);
break;
} else if meta_name == sym::rustc_const_unstable && const_stab.is_some() {
handle_errors(
&sess.parse_sess,
attr.span,
AttrError::MultipleStabilityLevels,
);
break;
}

let mut feature = None;
let mut reason = None;
let mut issue = None;
Expand Down Expand Up @@ -308,14 +307,85 @@ where
);
continue;
}
let level = Unstable { reason, issue: issue_num, is_soft };
let unstability = Unstability { feature, issue: issue_num, reason };
if sym::unstable == meta_name {
stab = Some((Stability { level, feature }, attr.span));
match &mut stab {
stab @ None => {
*stab = Some((
Stability {
level: StabilityLevel::Unstable {
unstables: vec![unstability],
is_soft,
},
},
vec![attr.span],
))
}
// Merge multiple unstability attributes into one
Some((
Stability {
level:
StabilityLevel::Unstable {
unstables,
is_soft: old_is_soft,
},
},
spans,
)) => {
unstables.push(unstability);
spans.push(attr.span);
// FIXME(compiler-errors): Do we want this behavior: is_soft iff all are is_soft?
*old_is_soft &= is_soft;
}
_ => {
handle_errors(
&sess.parse_sess,
attr.span,
AttrError::MultipleStabilityLevels,
);
}
}
} else {
const_stab = Some((
ConstStability { level, feature, promotable: false },
attr.span,
));
match &mut const_stab {
const_stab @ None => {
*const_stab = Some((
ConstStability {
level: StabilityLevel::Unstable {
unstables: vec![unstability],
is_soft,
},
promotable: false,
},
vec![attr.span],
))
}
Some((
ConstStability {
level:
StabilityLevel::Unstable {
unstables: unstability,
is_soft: old_is_soft,
},
..
},
spans,
)) => {
unstability.push(Unstability {
feature,
issue: issue_num,
reason,
});
spans.push(attr.span);
*old_is_soft &= is_soft;
}
_ => {
handle_errors(
&sess.parse_sess,
attr.span,
AttrError::MultipleStabilityLevels,
);
}
}
}
}
(None, _, _) => {
Expand Down Expand Up @@ -386,13 +456,13 @@ where

match (feature, since) {
(Some(feature), Some(since)) => {
let level = Stable { since };
let level = Stable { since, feature };
if sym::stable == meta_name {
stab = Some((Stability { level, feature }, attr.span));
stab = Some((Stability { level }, vec![attr.span]));
} else {
const_stab = Some((
ConstStability { level, feature, promotable: false },
attr.span,
ConstStability { level, promotable: false },
vec![attr.span],
));
}
}
Expand All @@ -413,7 +483,7 @@ where

// Merge the const-unstable info into the stability info
if promotable {
if let Some((ref mut stab, _)) = const_stab {
if let Some((stab, _)) = &mut const_stab {
stab.promotable = promotable;
} else {
struct_span_err!(
Expand Down
11 changes: 0 additions & 11 deletions compiler/rustc_const_eval/src/const_eval/fn_queries.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,8 @@ use rustc_hir as hir;
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_middle::ty::query::Providers;
use rustc_middle::ty::TyCtxt;
use rustc_span::symbol::Symbol;
use rustc_target::spec::abi::Abi;

/// Whether the `def_id` is an unstable const fn and what feature gate is necessary to enable it
pub fn is_unstable_const_fn(tcx: TyCtxt<'_>, def_id: DefId) -> Option<Symbol> {
if tcx.is_const_fn_raw(def_id) {
let const_stab = tcx.lookup_const_stability(def_id)?;
if const_stab.level.is_unstable() { Some(const_stab.feature) } else { None }
} else {
None
}
}

pub fn is_parent_const_impl_raw(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
let hir_id = tcx.hir().local_def_id_to_hir_id(def_id);
let parent_id = tcx.hir().get_parent_node(hir_id);
Expand Down
105 changes: 64 additions & 41 deletions compiler/rustc_const_eval/src/transform/check_consts/check.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ use super::ops::{self, NonConstOp, Status};
use super::qualifs::{self, CustomEq, HasMutInterior, NeedsDrop, NeedsNonConstDrop};
use super::resolver::FlowSensitiveAnalysis;
use super::{ConstCx, Qualif};
use crate::const_eval::is_unstable_const_fn;

type QualifResults<'mir, 'tcx, Q> =
rustc_mir_dataflow::ResultsCursor<'mir, 'tcx, FlowSensitiveAnalysis<'mir, 'mir, 'tcx, Q>>;
Expand Down Expand Up @@ -902,54 +901,78 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
}

// If the `const fn` we are trying to call is not const-stable, ensure that we have
// the proper feature gate enabled.
if let Some(gate) = is_unstable_const_fn(tcx, callee) {
trace!(?gate, "calling unstable const fn");
if self.span.allows_unstable(gate) {
return;
// the proper feature gates enabled.
let gates = if tcx.is_const_fn_raw(callee) {
match tcx.lookup_const_stability(callee) {
Some(rustc_attr::ConstStability {
level: rustc_attr::StabilityLevel::Unstable { unstables, .. },
..
}) => unstables.as_slice(),
_ => &[],
}
} else {
&[]
};

// Calling an unstable function *always* requires that the corresponding gate
// be enabled, even if the function has `#[rustc_allow_const_fn_unstable(the_gate)]`.
if !tcx.features().declared_lib_features.iter().any(|&(sym, _)| sym == gate) {
self.check_op(ops::FnCallUnstable(callee, Some(gate)));
return;
}
if !gates.is_empty() {
trace!(?gates, "calling unstable const fn");
// Features that are not enabled, but must be to make this call const
let mut bad_gates = vec![];
for gate in gates {
let feature = gate.feature;

// If this crate is not using stability attributes, or the caller is not claiming to be a
// stable `const fn`, that is all that is required.
if !self.ccx.is_const_stable_const_fn() {
trace!("crate not using stability attributes or caller not stably const");
return;
}
if self.span.allows_unstable(feature) {
continue;
}

// Otherwise, we are something const-stable calling a const-unstable fn.
// Calling an unstable function *always* requires that the corresponding gate
// be enabled, even if the function has `#[rustc_allow_const_fn_unstable(the_gate)]`.
if !tcx
.features()
.declared_lib_features
.iter()
.any(|&(sym, _)| sym == feature)
{
bad_gates.push(feature);
continue;
}
// If this crate is not using stability attributes, or the caller is not claiming to be a
// stable `const fn`, that is all that is required.
if !self.ccx.is_const_stable_const_fn() {
trace!(
"crate not using stability attributes or caller not stably const"
);
continue;
}
// Otherwise, we are something const-stable calling a const-unstable fn.
if super::rustc_allow_const_fn_unstable(tcx, caller, feature) {
trace!("rustc_allow_const_fn_unstable gate active");
continue;
}

if super::rustc_allow_const_fn_unstable(tcx, caller, gate) {
trace!("rustc_allow_const_fn_unstable gate active");
return;
bad_gates.push(feature);
}

self.check_op(ops::FnCallUnstable(callee, Some(gate)));
return;
}

// FIXME(ecstaticmorse); For compatibility, we consider `unstable` callees that
// have no `rustc_const_stable` attributes to be const-unstable as well. This
// should be fixed later.
let callee_is_unstable_unmarked = tcx.lookup_const_stability(callee).is_none()
&& tcx.lookup_stability(callee).map_or(false, |s| s.level.is_unstable());
if callee_is_unstable_unmarked {
trace!("callee_is_unstable_unmarked");
// We do not use `const` modifiers for intrinsic "functions", as intrinsics are
// `extern` funtions, and these have no way to get marked `const`. So instead we
// use `rustc_const_(un)stable` attributes to mean that the intrinsic is `const`
if self.ccx.is_const_stable_const_fn() || is_intrinsic {
self.check_op(ops::FnCallUnstable(callee, None));
return;
if !bad_gates.is_empty() {
self.check_op(ops::FnCallUnstable(callee, bad_gates));
}
} else {
// FIXME(ecstaticmorse); For compatibility, we consider `unstable` callees that
// have no `rustc_const_stable` attributes to be const-unstable as well. This
// should be fixed later.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this compatibility layer still used?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea. @ecstatic-morse do you know if this FIXME is still valid?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC, these rules proved somewhat contentious, although I don't know if this particular idea was the problematic one. You can take out the FIXME and the "this should be fixed", but please leave the description if it's still correct.

let callee_is_unstable_unmarked = tcx.lookup_const_stability(callee).is_none()
&& tcx.lookup_stability(callee).map_or(false, |s| s.level.is_unstable());
if callee_is_unstable_unmarked {
trace!("callee_is_unstable_unmarked");
// We do not use `const` modifiers for intrinsic "functions", as intrinsics are
// `extern` funtions, and these have no way to get marked `const`. So instead we
// use `rustc_const_(un)stable` attributes to mean that the intrinsic is `const`
if self.ccx.is_const_stable_const_fn() || is_intrinsic {
self.check_op(ops::FnCallUnstable(callee, vec![]));
return;
}
}
trace!("permitting call");
}
trace!("permitting call");
}

// Forbid all `Drop` terminators unless the place being dropped is a local with no
Expand Down
Loading