Skip to content

WIP: Unstable impls #140399

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.

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

Draft
wants to merge 21 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion compiler/rustc_attr_data_structures/src/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,8 @@ pub enum AttributeKind {

/// Represents `#[allow_internal_unstable]`.
AllowInternalUnstable(ThinVec<(Symbol, Span)>),

/// Represents `#[unstable_feature_bound]`.
AllowUnstableFeature(ThinVec<(Symbol, Span)>),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
AllowUnstableFeature(ThinVec<(Symbol, Span)>),
UnstableFeatureBound(ThinVec<(Symbol, Span)>),

/// Represents `#[rustc_default_body_unstable]`.
BodyStability {
stability: DefaultBodyStability,
Expand Down
16 changes: 16 additions & 0 deletions compiler/rustc_attr_parsing/src/attributes/allow_unstable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,22 @@ impl<S: Stage> CombineAttributeParser<S> for AllowInternalUnstableParser {
}
}

pub(crate) struct AllowUnstableFeatureParser;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pub(crate) struct AllowUnstableFeatureParser;
pub(crate) struct UnstableFeatureBoundParser;

impl<S: Stage> CombineAttributeParser<S> for AllowUnstableFeatureParser {
const PATH: &'static [rustc_span::Symbol] = &[sym::unstable_feature_bound];
type Item = (Symbol, Span);
const CONVERT: ConvertFn<Self::Item> = AttributeKind::AllowUnstableFeature;

fn extend<'c>(
cx: &'c mut AcceptContext<'_, '_, S>,
args: &'c ArgParser<'_>,
) -> impl IntoIterator<Item = Self::Item> {
parse_unstable(cx, args, <Self as CombineAttributeParser<S>>::PATH[0])
.into_iter()
.zip(iter::repeat(cx.attr_span))
}
}

pub(crate) struct AllowConstFnUnstableParser;
impl<S: Stage> CombineAttributeParser<S> for AllowConstFnUnstableParser {
const PATH: &[Symbol] = &[sym::rustc_allow_const_fn_unstable];
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_attr_parsing/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ use rustc_hir::{AttrArgs, AttrItem, AttrPath, Attribute, HashIgnoredAttrId, HirI
use rustc_session::Session;
use rustc_span::{DUMMY_SP, ErrorGuaranteed, Span, Symbol, sym};

use crate::attributes::allow_unstable::{AllowConstFnUnstableParser, AllowInternalUnstableParser};
use crate::attributes::allow_unstable::{
AllowConstFnUnstableParser, AllowInternalUnstableParser, AllowUnstableFeatureParser,
};
use crate::attributes::confusables::ConfusablesParser;
use crate::attributes::deprecation::DeprecationParser;
use crate::attributes::repr::ReprParser;
Expand Down Expand Up @@ -98,6 +100,7 @@ attribute_parsers!(
// tidy-alphabetical-start
Combine<AllowConstFnUnstableParser>,
Combine<AllowInternalUnstableParser>,
Combine<AllowUnstableFeatureParser>,
Combine<ReprParser>,
// tidy-alphabetical-end

Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -670,6 +670,11 @@ pub static BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
DuplicatesOk, EncodeCrossCrate::Yes,
"allow_internal_unstable side-steps feature gating and stability checks",
),
gated!(
unstable_feature_bound, Normal, template!(Word, List: "feat1, feat2, ..."),
DuplicatesOk, EncodeCrossCrate::No, impl_stability,
"used internally to mark impl as unstable",
),
gated!(
allow_internal_unsafe, Normal, template!(Word), WarnFollowing,
EncodeCrossCrate::No, "allow_internal_unsafe side-steps the unsafe_code lint",
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_feature/src/unstable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,8 @@ declare_features! (
(internal, custom_mir, "1.65.0", None),
/// Outputs useful `assert!` messages
(unstable, generic_assert, "1.63.0", None),
/// Allow declaring an impl as unstable.
(internal, impl_stability, "CURRENT_RUSTC_VERSION", None),
Comment on lines +216 to +217
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// Allow declaring an impl as unstable.
(internal, impl_stability, "CURRENT_RUSTC_VERSION", None),

I think that unstable feature bounds should just live under the staged_api feature instead of a separate feature gate. Might be a somewhat involved change to go and change everywhere to use staged_api instead of impl_stability everywhere :> my bad

/// Allows using the #[rustc_intrinsic] attribute.
(internal, intrinsics, "1.0.0", None),
/// Allows using `#[lang = ".."]` attribute for linking items to special compiler logic.
Expand Down
7 changes: 5 additions & 2 deletions compiler/rustc_hir_analysis/src/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2368,9 +2368,12 @@ impl<'tcx> WfCheckingCtxt<'_, 'tcx> {
// We lower empty bounds like `Vec<dyn Copy>:` as
// `WellFormed(Vec<dyn Copy>)`, which will later get checked by
// regular WF checking
if let ty::ClauseKind::WellFormed(..) = pred.kind().skip_binder() {
continue;

match pred.kind().skip_binder() {
ty::ClauseKind::WellFormed(..) | ty::ClauseKind::UnstableFeature(..) => continue,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ty::ClauseKind::WellFormed(..) | ty::ClauseKind::UnstableFeature(..) => continue,
ty::ClauseKind::WellFormed(..)
// Unstable feature goals cannot be proven in an empty environment so skip them
| ty::ClauseKind::UnstableFeature(..) => continue,

and then can u move the comment about WellFormed onto the WellFormed pattern instead of the whole match

_ => {}
}

// Match the existing behavior.
if pred.is_global() && !pred.has_type_flags(TypeFlags::HAS_BINDER_VARS) {
let pred = self.normalize(span, None, pred);
Expand Down
13 changes: 13 additions & 0 deletions compiler/rustc_hir_analysis/src/collect/predicates_of.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::assert_matches::assert_matches;

use hir::Node;
use rustc_attr_data_structures::{AttributeKind, find_attr};
use rustc_data_structures::fx::FxIndexSet;
use rustc_hir as hir;
use rustc_hir::def::DefKind;
Expand Down Expand Up @@ -318,6 +319,16 @@ fn gather_explicit_predicates_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Gen
predicates.extend(const_evaluatable_predicates_of(tcx, def_id, &predicates));
}

let attrs = tcx.hir_attrs(tcx.local_def_id_to_hir_id(def_id));
let allow_unstable_feature_attr =
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
let allow_unstable_feature_attr =
// FIXME(staged_api): We might want to look at the normal stability attributes too but
// first we would need a way to let std/core use APIs with unstable feature bounds from
// within stable APIs.
let allow_unstable_feature_attr =

find_attr!(attrs, AttributeKind::AllowUnstableFeature(i) => i)
.map(|i| i.as_slice())
.unwrap_or_default();

for (feat_name, span) in allow_unstable_feature_attr {
predicates.insert((ty::ClauseKind::UnstableFeature(*feat_name).upcast(tcx), *span));
}

let mut predicates: Vec<_> = predicates.into_iter().collect();

// Subtle: before we store the predicates into the tcx, we
Expand Down Expand Up @@ -747,6 +758,7 @@ pub(super) fn assert_only_contains_predicates_from<'tcx>(
ty::ClauseKind::RegionOutlives(_)
| ty::ClauseKind::ConstArgHasType(_, _)
| ty::ClauseKind::WellFormed(_)
| ty::ClauseKind::UnstableFeature(_)
| ty::ClauseKind::ConstEvaluatable(_) => {
bug!(
"unexpected non-`Self` predicate when computing \
Expand Down Expand Up @@ -774,6 +786,7 @@ pub(super) fn assert_only_contains_predicates_from<'tcx>(
| ty::ClauseKind::ConstArgHasType(_, _)
| ty::ClauseKind::WellFormed(_)
| ty::ClauseKind::ConstEvaluatable(_)
| ty::ClauseKind::UnstableFeature(_)
| ty::ClauseKind::HostEffect(..) => {
bug!(
"unexpected non-`Self` predicate when computing \
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,7 @@ fn trait_specialization_kind<'tcx>(
| ty::ClauseKind::ConstArgHasType(..)
| ty::ClauseKind::WellFormed(_)
| ty::ClauseKind::ConstEvaluatable(..)
| ty::ClauseKind::UnstableFeature(_)
| ty::ClauseKind::HostEffect(..) => None,
}
}
1 change: 1 addition & 0 deletions compiler/rustc_hir_analysis/src/outlives/explicit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ impl<'tcx> ExplicitPredicatesMap<'tcx> {
| ty::ClauseKind::ConstArgHasType(_, _)
| ty::ClauseKind::WellFormed(_)
| ty::ClauseKind::ConstEvaluatable(_)
| ty::ClauseKind::UnstableFeature(_)
| ty::ClauseKind::HostEffect(..) => {}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
| ty::PredicateKind::Clause(ty::ClauseKind::ConstEvaluatable(..))
| ty::PredicateKind::ConstEquate(..)
| ty::PredicateKind::Clause(ty::ClauseKind::HostEffect(..))
| ty::PredicateKind::Clause(ty::ClauseKind::UnstableFeature(_))
| ty::PredicateKind::Ambiguous => false,
}
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir_typeck/src/method/probe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -926,6 +926,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
| ty::ClauseKind::ConstArgHasType(_, _)
| ty::ClauseKind::WellFormed(_)
| ty::ClauseKind::ConstEvaluatable(_)
| ty::ClauseKind::UnstableFeature(_)
| ty::ClauseKind::HostEffect(..) => None,
}
});
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_lint/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1521,8 +1521,9 @@ impl<'tcx> LateLintPass<'tcx> for TrivialConstraints {
ClauseKind::TypeOutlives(..) |
ClauseKind::RegionOutlives(..) => "lifetime",

ClauseKind::UnstableFeature(_)
// `ConstArgHasType` is never global as `ct` is always a param
ClauseKind::ConstArgHasType(..)
| ClauseKind::ConstArgHasType(..)
// Ignore projections, as they can only be global
// if the trait bound is global
| ClauseKind::Projection(..)
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ impl<'tcx> Interner for TyCtxt<'tcx> {
type FnInputTys = &'tcx [Ty<'tcx>];
type ParamTy = ParamTy;
type BoundTy = ty::BoundTy;
type Symbol = Symbol;

type PlaceholderTy = ty::PlaceholderType;
type ErrorGuaranteed = ErrorGuaranteed;
Expand Down Expand Up @@ -831,6 +832,14 @@ impl<'tcx> rustc_type_ir::inherent::Features<TyCtxt<'tcx>> for &'tcx rustc_featu
fn associated_const_equality(self) -> bool {
self.associated_const_equality()
}

fn impl_stability(self) -> bool {
self.impl_stability()
}

fn enabled(self, symbol: <TyCtxt<'tcx> as Interner>::Symbol) -> bool {
self.enabled(symbol)
}
Comment on lines +840 to +842
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
fn enabled(self, symbol: <TyCtxt<'tcx> as Interner>::Symbol) -> bool {
self.enabled(symbol)
}
fn feature_bound_holds_in_crate(self, symbol: <TyCtxt<'tcx> as Interner>::Symbol) -> bool {
// We don't consider feature bounds to hold in the crate if we `staged_api` feature is
// even when the feature is enabled. This is to prevent accidentally leaking unstable APIs
// to stable.
!self.staged_api() && self.enabled(symbol)
}

}

impl<'tcx> rustc_type_ir::inherent::Span<TyCtxt<'tcx>> for Span {
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_middle/src/ty/predicate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ impl<'tcx> Predicate<'tcx> {
| PredicateKind::Clause(ClauseKind::TypeOutlives(_))
| PredicateKind::Clause(ClauseKind::Projection(_))
| PredicateKind::Clause(ClauseKind::ConstArgHasType(..))
| PredicateKind::Clause(ClauseKind::UnstableFeature(_))
| PredicateKind::DynCompatible(_)
| PredicateKind::Subtype(_)
| PredicateKind::Coerce(_)
Expand Down Expand Up @@ -649,6 +650,7 @@ impl<'tcx> Predicate<'tcx> {
PredicateKind::Clause(ClauseKind::Projection(..))
| PredicateKind::Clause(ClauseKind::HostEffect(..))
| PredicateKind::Clause(ClauseKind::ConstArgHasType(..))
| PredicateKind::Clause(ClauseKind::UnstableFeature(_))
| PredicateKind::NormalizesTo(..)
| PredicateKind::AliasRelate(..)
| PredicateKind::Subtype(..)
Expand All @@ -670,6 +672,7 @@ impl<'tcx> Predicate<'tcx> {
PredicateKind::Clause(ClauseKind::Trait(..))
| PredicateKind::Clause(ClauseKind::HostEffect(..))
| PredicateKind::Clause(ClauseKind::ConstArgHasType(..))
| PredicateKind::Clause(ClauseKind::UnstableFeature(_))
| PredicateKind::NormalizesTo(..)
| PredicateKind::AliasRelate(..)
| PredicateKind::Subtype(..)
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/ty/print/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3250,6 +3250,7 @@ define_print! {
ty::ClauseKind::ConstEvaluatable(ct) => {
p!("the constant `", print(ct), "` can be evaluated")
}
ty::ClauseKind::UnstableFeature(symbol) => p!("unstable feature: ", write("`{}`", symbol)),
}
}

Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -607,6 +607,9 @@ where
ty::PredicateKind::Clause(ty::ClauseKind::ConstArgHasType(ct, ty)) => {
self.compute_const_arg_has_type_goal(Goal { param_env, predicate: (ct, ty) })
}
ty::PredicateKind::Clause(ty::ClauseKind::UnstableFeature(symbol)) => {
self.compute_unstable_feature_goal(param_env, symbol)
}
ty::PredicateKind::Subtype(predicate) => {
self.compute_subtype_goal(Goal { param_env, predicate })
}
Expand Down
34 changes: 34 additions & 0 deletions compiler/rustc_next_trait_solver/src/solve/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,40 @@ where
}
}

fn compute_unstable_feature_goal(
&mut self,
param_env: <I as Interner>::ParamEnv,
symbol: <I as Interner>::Symbol,
) -> QueryResult<I> {
// Iterate through all goals in param_env to find the one that has the same symbol.
for pred in param_env.caller_bounds().iter() {
if let ty::ClauseKind::UnstableFeature(sym) = pred.kind().skip_binder() {
if sym == symbol {
return self.evaluate_added_goals_and_make_canonical_response(Certainty::Yes);
}
}
}

if self.cx().features().impl_stability() {
// If we are in std/core, and the feature is not enabled through #[unstable_feature_bound(..)]
return self.evaluate_added_goals_and_make_canonical_response(Certainty::Maybe(
MaybeCause::Ambiguity,
));
} else {
// Outside of std/core, check if feature is enabled at crate level with #[feature(..)]
// or if we are currently in codegen.
if self.cx().features().enabled(symbol)
|| (self.typing_mode() == TypingMode::PostAnalysis)
{
return self.evaluate_added_goals_and_make_canonical_response(Certainty::Yes);
} else {
return self.evaluate_added_goals_and_make_canonical_response(Certainty::Maybe(
MaybeCause::Ambiguity,
));
}
}
Comment on lines +165 to +182
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if self.cx().features().impl_stability() {
// If we are in std/core, and the feature is not enabled through #[unstable_feature_bound(..)]
return self.evaluate_added_goals_and_make_canonical_response(Certainty::Maybe(
MaybeCause::Ambiguity,
));
} else {
// Outside of std/core, check if feature is enabled at crate level with #[feature(..)]
// or if we are currently in codegen.
if self.cx().features().enabled(symbol)
|| (self.typing_mode() == TypingMode::PostAnalysis)
{
return self.evaluate_added_goals_and_make_canonical_response(Certainty::Yes);
} else {
return self.evaluate_added_goals_and_make_canonical_response(Certainty::Maybe(
MaybeCause::Ambiguity,
));
}
}
// During codegen we must assume that all feature bounds hold as we may be
// monomorphizing a body from an upstream crate which had an unstable feature
// enabled that we do not.
//
// Note: `feature_bound_holds_in_crate` does not consider a feature to be enabled
// if we are in std/core even if there is a corresponding `feature` attribute on the crate.
if self.cx().features().feature_bound_holds_in_crate(symbol)
|| (self.typing_mode() == TypingMode::PostAnalysis)
{
return self.evaluate_added_goals_and_make_canonical_response(Certainty::Yes);
} else {
return self.evaluate_added_goals_and_make_canonical_response(Certainty::Maybe(
MaybeCause::Ambiguity,
));
}

Minor structural change here. Moving the "are we in std/core" check to within the feature_bound_holds_in_crate method. I think it makes the code a little nicer and also makes the enabled feature only suitable for use by unstable feature bounds which will prevent people using the API elsewhere.

}

#[instrument(level = "trace", skip(self))]
fn compute_const_evaluatable_goal(
&mut self,
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_passes/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,10 @@ passes_rustc_std_internal_symbol =
attribute should be applied to functions or statics
.label = not a function or static

passes_rustc_unstable_feature_bound =
attribute should be applied to `impl` or free function outside of any `impl` or trait
.label = not an `impl` or free function

passes_should_be_applied_to_fn =
attribute should be applied to a function definition
.label = {$on_crate ->
Expand Down
42 changes: 42 additions & 0 deletions compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,9 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
}
Attribute::Parsed(AttributeKind::Repr(_)) => { /* handled below this loop and elsewhere */
}
Attribute::Parsed(AttributeKind::AllowUnstableFeature(syms)) => {
self.check_unstable_feature_bound(syms.first().unwrap().1, span, target)
}
Attribute::Parsed(
AttributeKind::BodyStability { .. }
| AttributeKind::ConstStabilityIndirect
Expand Down Expand Up @@ -2274,6 +2277,45 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
}
}

fn check_unstable_feature_bound(&self, attr_span: Span, span: Span, target: Target) {
match target {
Target::Fn | Target::Impl => {}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Target::Fn | Target::Impl => {}
// FIXME(staged_api): There's no reason we can't support more targets here. We're just
// being conservative to begin with.
Target::Fn | Target::Impl => {}

Target::ExternCrate
| Target::Use
| Target::Static
| Target::Const
| Target::Closure
| Target::Mod
| Target::ForeignMod
| Target::GlobalAsm
| Target::TyAlias
| Target::Enum
| Target::Variant
| Target::Struct
| Target::Field
| Target::Union
| Target::Trait
| Target::TraitAlias
| Target::Expression
| Target::Statement
| Target::Arm
| Target::AssocConst
| Target::Method(_)
| Target::AssocTy
| Target::ForeignFn
| Target::ForeignStatic
| Target::ForeignTy
| Target::GenericParam(_)
| Target::MacroDef
| Target::Param
| Target::PatField
| Target::ExprField
| Target::WherePredicate => {
self.tcx.dcx().emit_err(errors::RustcUnstableFeatureBound { attr_span, span });
}
}
}

fn check_rustc_std_internal_symbol(&self, attr: &Attribute, span: Span, target: Target) {
match target {
Target::Fn | Target::Static | Target::ForeignFn | Target::ForeignStatic => {}
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_passes/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -662,6 +662,15 @@ pub(crate) struct RustcAllowConstFnUnstable {
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(passes_rustc_unstable_feature_bound)]
pub(crate) struct RustcUnstableFeatureBound {
#[primary_span]
pub attr_span: Span,
#[label]
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(passes_rustc_std_internal_symbol)]
pub(crate) struct RustcStdInternalSymbol {
Expand Down
Loading
Loading