-
-
Notifications
You must be signed in to change notification settings - Fork 14.2k
Stable inline const using unstable intrinsics #149817
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -243,7 +243,22 @@ impl<S: Stage> AttributeParser<S> for ConstStabilityParser { | |
| this.promotable = true; | ||
| }), | ||
| ]; | ||
| const ALLOWED_TARGETS: AllowedTargets = ALLOWED_TARGETS; | ||
| const ALLOWED_TARGETS: AllowedTargets = AllowedTargets::AllowList(&[ | ||
| Allow(Target::Fn), | ||
| Allow(Target::Method(MethodKind::Inherent)), | ||
| Allow(Target::Method(MethodKind::TraitImpl)), | ||
| Allow(Target::Method(MethodKind::Trait { body: true })), | ||
| Allow(Target::Impl { of_trait: false }), | ||
| Allow(Target::Impl { of_trait: true }), | ||
| Allow(Target::Use), // FIXME I don't think this does anything? | ||
| Allow(Target::Const), | ||
| Allow(Target::AssocConst), | ||
| Allow(Target::Trait), | ||
| Allow(Target::Static), | ||
| Allow(Target::Expression), // FIXME: we really only want to allow inline consts | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we could make up a new target for that 🤷♀️ . This would be its current only use but it's not that hard to do otherwise
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yea I had that, but I felt I already changed enough in this PR 😅 |
||
| Allow(Target::Crate), | ||
| Allow(Target::MacroDef), // FIXME(oli-obk): remove this and eliminate the manual check for it | ||
| ]); | ||
|
|
||
| fn finalize(mut self, cx: &FinalizeContext<'_, '_, S>) -> Option<AttributeKind> { | ||
| if self.promotable { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -54,9 +54,11 @@ impl<'mir, 'tcx> ConstCx<'mir, 'tcx> { | |
| pub fn enforce_recursive_const_stability(&self) -> bool { | ||
| // We can skip this if neither `staged_api` nor `-Zforce-unstable-if-unmarked` are enabled, | ||
| // since in such crates `lookup_const_stability` will always be `None`. | ||
| self.const_kind == Some(hir::ConstContext::ConstFn) | ||
| && (self.tcx.features().staged_api() | ||
| || self.tcx.sess.opts.unstable_opts.force_unstable_if_unmarked) | ||
| matches!( | ||
| self.const_kind, | ||
| Some(hir::ConstContext::ConstFn | hir::ConstContext::Const { inline: true }) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Only a partial fix. I can remove the inline limitation here, but it has a lot of fallout that I haven't analyzed yet. I'll do that in a follow-up |
||
| ) && (self.tcx.features().staged_api() | ||
| || self.tcx.sess.opts.unstable_opts.force_unstable_if_unmarked) | ||
| && is_fn_or_trait_safe_to_expose_on_stable(self.tcx, self.def_id().to_def_id()) | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -864,7 +864,10 @@ impl fmt::Debug for TypeId { | |
| #[stable(feature = "type_name", since = "1.38.0")] | ||
| #[rustc_const_unstable(feature = "const_type_name", issue = "63084")] | ||
| pub const fn type_name<T: ?Sized>() -> &'static str { | ||
| const { intrinsics::type_name::<T>() } | ||
| #[rustc_const_unstable(feature = "const_type_name", issue = "63084")] | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What does It would help to see e.g. a patch updating the dev guide on these attributes, to understand the intended behavior without having to reverse engineer it from the implementation. |
||
| const { | ||
| intrinsics::type_name::<T>() | ||
| } | ||
| } | ||
|
|
||
| /// Returns the type name of the pointed-to value as a string slice. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2881,6 +2881,7 @@ pub const fn type_name<T: ?Sized>() -> &'static str; | |
| #[rustc_nounwind] | ||
| #[unstable(feature = "core_intrinsics", issue = "none")] | ||
| #[rustc_intrinsic] | ||
| #[rustc_intrinsic_const_stable_indirect] | ||
| pub const fn type_id<T: ?Sized + 'static>() -> crate::any::TypeId; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And neither is this const-stable.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is const stable. TypeId::of is const stable
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah fair, somehow I didn't find that one.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For the record, the t-lang FCP that justifies adding this attribute passed here. Maybe we should have a separate PR just adding that attribute? This PR shouldn't add any more since we don't have FCP for any of the others. |
||
|
|
||
| /// Tests (at compile-time) if two [`crate::any::TypeId`] instances identify the | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| //! Ensure we reject invalid combinations of feature gating inline consts | ||
|
|
||
| #![feature(staged_api, abi_unadjusted)] | ||
| #![stable(feature = "rust_test", since = "1.0.0")] | ||
|
|
||
| #[unstable(feature = "abi_unadjusted", issue = "42")] | ||
| #[rustc_const_unstable(feature = "abi_unadjusted", issue = "42")] | ||
| const fn my_fun() {} | ||
|
|
||
| #[stable(feature = "asdf", since = "99.0.0")] | ||
| #[rustc_const_stable(feature = "asdf", since = "99.0.0")] | ||
| const fn my_fun2() { | ||
| #[rustc_const_unstable(feature = "abi_unadjusted", issue = "42")] | ||
| //~^ ERROR: must match const stability of containing item | ||
| const { | ||
| my_fun() | ||
| } | ||
| } | ||
|
|
||
| // Check that const stable const blocks can only call const stable things | ||
| #[stable(feature = "asdf", since = "99.0.0")] | ||
| #[rustc_const_stable(feature = "asdf", since = "99.0.0")] | ||
| const fn my_fun3() { | ||
| #[rustc_const_stable(feature = "asdf", since = "99.0.0")] | ||
| const { | ||
| my_fun() | ||
| //~^ ERROR: (indirectly) exposed to stable | ||
| } | ||
| } | ||
|
|
||
| // Check that const stable const blocks can only call const stable things | ||
| #[stable(feature = "asdf", since = "99.0.0")] | ||
| #[rustc_const_stable(feature = "asdf", since = "99.0.0")] | ||
| const fn my_fun4() { | ||
| const { | ||
| my_fun() | ||
| //~^ ERROR: (indirectly) exposed to stable | ||
| } | ||
| } | ||
|
|
||
| fn main() { | ||
| #[rustc_const_unstable(feature = "abi_unadjusted", issue = "42")] | ||
| //~^ ERROR: must match const stability of containing item | ||
| const { | ||
| my_fun2() | ||
| }; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| error: const stability of inline consts must match const stability of containing item. | ||
| --> $DIR/const-fn-lang-feature-inline-const.rs:13:5 | ||
| | | ||
| LL | #[rustc_const_unstable(feature = "abi_unadjusted", issue = "42")] | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| | | ||
| help: did you mean to use `rustc_allow_const_fn_unstable`? | ||
| --> $DIR/const-fn-lang-feature-inline-const.rs:13:5 | ||
| | | ||
| LL | #[rustc_const_unstable(feature = "abi_unadjusted", issue = "42")] | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| note: stability marker of containing item defined here | ||
| --> $DIR/const-fn-lang-feature-inline-const.rs:11:1 | ||
| | | ||
| LL | #[rustc_const_stable(feature = "asdf", since = "99.0.0")] | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
|
||
| error: const stability of inline consts must match const stability of containing item. | ||
| --> $DIR/const-fn-lang-feature-inline-const.rs:42:5 | ||
| | | ||
| LL | #[rustc_const_unstable(feature = "abi_unadjusted", issue = "42")] | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| | | ||
| help: did you mean to use `rustc_allow_const_fn_unstable`? | ||
| --> $DIR/const-fn-lang-feature-inline-const.rs:42:5 | ||
| | | ||
| LL | #[rustc_const_unstable(feature = "abi_unadjusted", issue = "42")] | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
|
||
| error: const function that might be (indirectly) exposed to stable cannot use `#[feature(abi_unadjusted)]` | ||
| --> $DIR/const-fn-lang-feature-inline-const.rs:26:9 | ||
| | | ||
| LL | my_fun() | ||
| | ^^^^^^^^ | ||
| | | ||
| = help: mark the callee as `#[rustc_const_stable_indirect]` if it does not itself require any unstable features | ||
| help: if the caller is not (yet) meant to be exposed to stable const contexts, add `#[rustc_const_unstable]` | ||
| | | ||
| LL | const #[rustc_const_unstable(feature = "...", issue = "...")] | ||
| | +++++++++++++++++++++++++++++++++++++++++++++++++++++++ | ||
|
|
||
| error: const function that might be (indirectly) exposed to stable cannot use `#[feature(abi_unadjusted)]` | ||
| --> $DIR/const-fn-lang-feature-inline-const.rs:36:9 | ||
| | | ||
| LL | my_fun() | ||
| | ^^^^^^^^ | ||
| | | ||
| = help: mark the callee as `#[rustc_const_stable_indirect]` if it does not itself require any unstable features | ||
| help: if the caller is not (yet) meant to be exposed to stable const contexts, add `#[rustc_const_unstable]` | ||
| | | ||
| LL | const #[rustc_const_unstable(feature = "...", issue = "...")] | ||
| | +++++++++++++++++++++++++++++++++++++++++++++++++++++++ | ||
|
|
||
| error: aborting due to 4 previous errors | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can put at least some atributes on const operands. Iirc recently cfg was stabilized here? Though that one is of course expanded out early. Still wonder where those attributes go and whether they should be forwarded here 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
even if in this case it only matters for their stability attrs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know where the attribute would go. If it's on the anon const, it will end up as part of its body's expression. You can't add attrs on operands, at least the ast has nowhere to put them. This change should be behaviour-preserving, but I could add a FIXME