WIP: Unstable impls #140399

tiif · 2025-04-28T15:13:52Z

r? ghost

tests/ui/internal/unstable-impl-assoc-type.rs

bors · 2025-06-10T21:33:29Z

☔ The latest upstream changes (presumably #142299) made this pull request unmergeable. Please resolve the merge conflicts.

tiif · 2025-06-11T15:49:23Z

tests/ui/internal/unstable_impl_codegen.rs

If we stop making TypingMode::PostAnalysis return Ok(Yes) when proving ClauseKind::UnstableFeature, this test will ice: #142368 (comment)

This means the test is working as intended,

jhpratt · 2025-06-11T23:58:24Z

I've got nothing technical to add (I've not looked over the code). I just wanted to thank @tiif for taking on this task! It's far from trivial and has been a long-standing desire.

… effect

bors · 2025-06-14T08:38:10Z

☔ The latest upstream changes (presumably #142483) made this pull request unmergeable. Please resolve the merge conflicts.

BoxyUwU

gg gamer

BoxyUwU · 2025-06-17T10:52:54Z

compiler/rustc_hir_analysis/src/check/wfcheck.rs

-                continue;
+
+            match pred.kind().skip_binder() {
+                ty::ClauseKind::WellFormed(..) | ty::ClauseKind::UnstableFeature(..) => continue,


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

BoxyUwU · 2025-06-17T10:55:24Z

compiler/rustc_attr_data_structures/src/attributes.rs

@@ -157,7 +157,8 @@ pub enum AttributeKind {

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


Suggested change

AllowUnstableFeature(ThinVec<(Symbol, Span)>),

UnstableFeatureBound(ThinVec<(Symbol, Span)>),

BoxyUwU · 2025-06-17T10:55:32Z

compiler/rustc_attr_parsing/src/attributes/allow_unstable.rs

@@ -24,6 +24,22 @@ impl<S: Stage> CombineAttributeParser<S> for AllowInternalUnstableParser {
    }
 }

+pub(crate) struct AllowUnstableFeatureParser;


Suggested change

pub(crate) struct AllowUnstableFeatureParser;

pub(crate) struct UnstableFeatureBoundParser;

BoxyUwU · 2025-06-17T10:57:01Z

compiler/rustc_hir_analysis/src/collect/predicates_of.rs

@@ -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 =


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 =

BoxyUwU · 2025-06-17T11:11:06Z

compiler/rustc_passes/src/stability.rs

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


I think this won't work if there are multiple unstable_feature_bound attributes. E.g.

#[unstable_feature_bound(feat_foo)] #[unstable_feature_bound(feat_bar)] #[unstable(feature = "feat_bar", issue = "none")] impl Foo for Bar {}

you should be able to do something like:
find_attr!(attrs, AttributeKind::AllowUnstableFeature(i) if i == feature)
to get a yes/no as to whether there's an unstable feature bound on the impl with the same symbol as the stability attr

This reminds me, I am not sure if I handled multiple attributes case correctly in compiler/rustc_hir_analysis/src/collect/predicates_of.rs. I will take a look at that too.

oh, yes, I don't think that's correct either

BoxyUwU · 2025-06-17T11:11:15Z

compiler/rustc_passes/src/stability.rs

+                    // For example:
+                    // ```
+                    // #[unstable_feature_bound(feat_foo)]
+                    // #![unstable(feature = "feat_foo", issue = "none")]


Suggested change

// #![unstable(feature = "feat_foo", issue = "none")]

// #[unstable(feature = "feat_foo", issue = "none")]

BoxyUwU · 2025-06-17T11:12:50Z

compiler/rustc_type_ir/src/flags.rs

@@ -410,6 +409,8 @@ impl<I: Interner> FlagComputation<I> {
                self.add_term(t1);
                self.add_term(t2);
            }
+            ty::PredicateKind::Clause(ty::ClauseKind::UnstableFeature(_))


Suggested change

ty::PredicateKind::Clause(ty::ClauseKind::UnstableFeature(_))

ty::PredicateKind::Clause(ty::ClauseKind::UnstableFeature(_sym))

to make it a little clearer to readers what the field is that we're ignoring here

BoxyUwU · 2025-06-17T11:20:38Z

compiler/rustc_feature/src/unstable.rs

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


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

BoxyUwU · 2025-06-17T11:28:36Z

compiler/rustc_next_trait_solver/src/solve/mod.rs

+        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,
+                ));
+            }
+        }


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.

BoxyUwU · 2025-06-17T11:30:11Z

compiler/rustc_middle/src/ty/context.rs

+    fn enabled(self, symbol: <TyCtxt<'tcx> as Interner>::Symbol) -> bool {
+        self.enabled(symbol)
+    }


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)

}

BoxyUwU · 2025-06-17T11:38:45Z

compiler/rustc_passes/src/check_attr.rs

@@ -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 => {}


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 => {}

rust-cloud-vms bot force-pushed the unstable_impl branch from d01f12a to 9e139e1 Compare May 23, 2025 10:26

This comment has been minimized.

Sign in to view

BoxyUwU self-assigned this May 27, 2025

This comment has been minimized.

Sign in to view

rust-cloud-vms bot force-pushed the unstable_impl branch from 486d3d3 to 02f780f Compare May 30, 2025 09:52

This comment has been minimized.

Sign in to view

BoxyUwU mentioned this pull request Jun 2, 2025

Impl stability is not checked #55436

Open

This comment has been minimized.

Sign in to view

rust-cloud-vms bot force-pushed the unstable_impl branch from 567ec89 to 80fd239 Compare June 9, 2025 09:06

BoxyUwU reviewed Jun 10, 2025

View reviewed changes

tests/ui/internal/unstable-impl-assoc-type.rs Outdated Show resolved Hide resolved

rust-cloud-vms bot force-pushed the unstable_impl branch from 850e3e1 to 27d9e21 Compare June 11, 2025 13:39

tiif mentioned this pull request Jun 11, 2025

Try: stop special casing codegen for unstable impl #142368

Closed

tiif commented Jun 11, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

tiif added 20 commits June 13, 2025 16:08

Setup feature gate and attribute

54b0b68

Lower the attribute to UnstableFeature predicate

fc9a0e6

Add the core logic in old solver and new solver

1ca27cc

Other logics

a7c61c0

Make stability attribute not to error if unstable_feature_bound is in…

3b5713c

… effect

Tests

0da5266

Add the test description

25cfd96

Add coherence test

3ca0475

Add codegen test

4ba8acb

Bless test

9e24e10

tidy

9583e1f

Fix codegen test

c6976b2

Fix rebase

bd1072b

Fix codegen test by removing T:Trait bound

e88370f

Clean up aux crate

fc2926f

Use run-pass test

cdb2773

Add test for using unstable_feature_bound on method

788d9be

Implement check_attr

b2eabf2

Add test for disallowed attribute

4e816e1

tidy

6e4c902

rust-cloud-vms bot force-pushed the unstable_impl branch from a77d746 to 6e4c902 Compare June 13, 2025 16:09

This comment has been minimized.

Sign in to view

Fix build error

b6bfa2d

BoxyUwU reviewed Jun 17, 2025

View reviewed changes

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

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

-    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 =

	// #![unstable(feature = "feat_foo", issue = "none")]
	// #[unstable(feature = "feat_foo", issue = "none")]

	ty::PredicateKind::Clause(ty::ClauseKind::UnstableFeature(_))
	ty::PredicateKind::Clause(ty::ClauseKind::UnstableFeature(_sym))

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

-    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)
+    }

WIP: Unstable impls #140399

Are you sure you want to change the base?

WIP: Unstable impls #140399

Conversation

tiif commented Apr 28, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Uh oh!

bors commented Jun 10, 2025

Uh oh!

tiif Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jhpratt commented Jun 11, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

bors commented Jun 14, 2025

Uh oh!

BoxyUwU left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tiif Jun 11, 2025 •

edited

Loading