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.

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

Disable calling all const traits in stable const #132786

Closed
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
68 changes: 19 additions & 49 deletions compiler/rustc_const_eval/src/check_consts/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use rustc_middle::mir::visit::Visitor;
use rustc_middle::mir::*;
use rustc_middle::span_bug;
use rustc_middle::ty::adjustment::PointerCoercion;
use rustc_middle::ty::{self, Instance, InstanceKind, Ty, TypeVisitableExt};
use rustc_middle::ty::{self, Ty, TypeVisitableExt};
use rustc_mir_dataflow::Analysis;
use rustc_mir_dataflow::impls::MaybeStorageLive;
use rustc_mir_dataflow::storage::always_storage_live_locals;
Expand Down Expand Up @@ -627,11 +627,11 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
_ => unreachable!(),
};

let ConstCx { tcx, body, param_env, .. } = *self.ccx;
let ConstCx { tcx, body, .. } = *self.ccx;

let fn_ty = func.ty(body, tcx);

let (mut callee, mut fn_args) = match *fn_ty.kind() {
let (callee, fn_args) = match *fn_ty.kind() {
ty::FnDef(def_id, fn_args) => (def_id, fn_args),

ty::FnPtr(..) => {
Expand All @@ -647,55 +647,25 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {

self.revalidate_conditional_constness(callee, fn_args, call_source, *fn_span);

let mut is_trait = false;
// Attempting to call a trait method?
if let Some(trait_did) = tcx.trait_of_item(callee) {
trace!("attempting to call a trait method");

let trait_is_const = tcx.is_const_trait(trait_did);
// trait method calls are only permitted when `effects` is enabled.
// typeck ensures the conditions for calling a const trait method are met,
// so we only error if the trait isn't const. We try to resolve the trait
// into the concrete method, and uses that for const stability checks.
// FIXME(const_trait_impl) we might consider moving const stability checks
// to typeck as well.
if tcx.features().const_trait_impl() && trait_is_const {
// This skips the check below that ensures we only call `const fn`.
is_trait = true;

if let Ok(Some(instance)) =
Instance::try_resolve(tcx, param_env, callee, fn_args)
&& let InstanceKind::Item(def) = instance.def
{
// Resolve a trait method call to its concrete implementation, which may be in a
// `const` trait impl. This is only used for the const stability check below, since
// we want to look at the concrete impl's stability.
fn_args = instance.args;
callee = def;
}
} else {
// if the trait is const but the user has not enabled the feature(s),
// suggest them.
let feature = if trait_is_const {
Some(if tcx.features().const_trait_impl() {
sym::effects
} else {
sym::const_trait_impl
})
} else {
None
};
self.check_op(ops::FnCallNonConst {
callee,
args: fn_args,
span: *fn_span,
call_source,
feature,
});
// If we allowed this, we're in miri-unleashed mode, so we might
// as well skip the remaining checks.
return;
}
// We are not allowed to use const traits in stable const functions yet.
// FIXME: apply more fine-grained per-trait const stability checks
// (see <https://github.com/rust-lang/project-const-traits/issues/5>).
Comment on lines +655 to +656
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
// FIXME: apply more fine-grained per-trait const stability checks
// (see <https://github.com/rust-lang/project-const-traits/issues/5>).

This is repeated below, no need to have it twice.

self.check_op(ops::FnCallNonConst {
callee,
args: fn_args,
span: *fn_span,
call_source,
feature: tcx.is_const_trait(trait_did).then_some(sym::const_trait_impl),
Copy link
Member

Choose a reason for hiding this comment

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

This is confusing, it conflates two entirely orthogonal checks:

  • is this trait const-callable
  • are we allowed to do const trait calls

Why is the first check even here, didn't revalidate_conditional_constness already check that?

Copy link
Member Author

Choose a reason for hiding this comment

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

revalidate_conditional_constness just checks that if the item is conditionally const then the trait bounds ~const trait bounds hold. For example, if a free function item is not conditionally const, then it will have no ~const trait bounds, so revalidate_conditional_constness is not enforcing anything in that case.

});

// We don't know which function is being called here, so we can't run the checks below.
// FIXME: apply more fine-grained per-trait const stability checks
// (see <https://github.com/rust-lang/project-const-traits/issues/5>).
return;
}

// At this point, we are calling a function, `callee`, whose `DefId` is known...
Expand Down Expand Up @@ -784,7 +754,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
}

// Trait functions are not `const fn` so we have to skip them here.
if !tcx.is_const_fn(callee) && !is_trait {
if !tcx.is_const_fn(callee) {
self.check_op(ops::FnCallNonConst {
callee,
args: fn_args,
Expand Down
11 changes: 11 additions & 0 deletions compiler/rustc_const_eval/src/check_consts/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,17 @@ pub(crate) struct FnCallNonConst<'tcx> {
}

impl<'tcx> NonConstOp<'tcx> for FnCallNonConst<'tcx> {
fn status_in_item(&self, _ccx: &ConstCx<'_, 'tcx>) -> Status {
match self.feature {
Some(feature) => Status::Unstable {
gate: feature,
safe_to_expose_on_stable: false,
is_function_call: false,
},
None => Status::Forbidden,
}
}

// FIXME: make this translatable
#[allow(rustc::diagnostic_outside_of_impl)]
#[allow(rustc::untranslatable_diagnostic)]
Expand Down
20 changes: 4 additions & 16 deletions tests/ui/traits/const-traits/staged-api.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
//@ known-bug: project-const-traits#5
// staged const APIs are intentionally broken pending us deciding how to handle
// recursive const stability checks.

//@ revisions: stable unstable
//@ compile-flags: -Znext-solver

Expand All @@ -19,7 +23,6 @@ pub struct Foo;
#[cfg_attr(unstable, rustc_const_unstable(feature = "local_feature", issue = "none"))]
#[cfg_attr(stable, rustc_const_stable(feature = "local_feature", since = "1.0.0"))]
impl const MyTrait for Foo {
//[stable]~^ ERROR trait implementations cannot be const stable yet
fn func() {}
}

Expand All @@ -32,43 +35,28 @@ fn non_const_context() {
#[unstable(feature = "none", issue = "none")]
const fn const_context() {
Unstable::func();
//[unstable]~^ ERROR cannot use `#[feature(unstable)]`
//[stable]~^^ ERROR not yet stable as a const fn
Copy link
Member

@RalfJung RalfJung Nov 9, 2024

Choose a reason for hiding this comment

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

Something has gone very wrong here. This should complain about the missing feature gate -- const_trait_impl can't be used here since the function does not have #[rustc_const_unstable]. That's the entire point!

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 think you're misinterpreting what I did to this file lol. I marked the whole thing as a known-bug, which requires that the test have no error annotation, because this whole test is basically useless if we have no staged apis for const traits.

Copy link
Member

Choose a reason for hiding this comment

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

We need a test to ensure that we can't call trait functions on stable. Why shouldn't that be this file?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because this test does a lot more than just ensure we can't call trait functions on stable. If we want a test to do that, then it can be like... 10 lines. It's got like... several revisions, an aux-build dep that is also broken due to these changes, etc.

Copy link
Member

Choose a reason for hiding this comment

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

You need an aux build to properly check the cross-crate part of this. And the revisions distinguish whether the trait callee is stable or not, which the old check was sensitive to when it could resolve the callee... I guess for now we don't need the revisions; once we have the ability to do a per-trait decision of "can this be const-called on stable", that will be relevant again.

Foo::func();
//[unstable]~^ ERROR cannot use `#[feature(local_feature)]`
//[stable]~^^ cannot be (indirectly) exposed to stable
// We get the error on `stable` since this is a trait function.
Unstable2::func();
//~^ ERROR not yet stable as a const fn
// ^ fails, because the `unstable2` feature is not active
}

#[stable(feature = "rust1", since = "1.0.0")]
#[cfg_attr(unstable, rustc_const_unstable(feature = "local_feature", issue = "none"))]
pub const fn const_context_not_const_stable() {
//[stable]~^ ERROR function has missing const stability attribute
Unstable::func();
//[stable]~^ ERROR not yet stable as a const fn
Foo::func();
//[stable]~^ cannot be (indirectly) exposed to stable
// We get the error on `stable` since this is a trait function.
Unstable2::func();
//~^ ERROR not yet stable as a const fn
// ^ fails, because the `unstable2` feature is not active
}

#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_stable(feature = "cheese", since = "1.0.0")]
const fn stable_const_context() {
Unstable::func();
//[unstable]~^ ERROR cannot use `#[feature(unstable)]`
//[stable]~^^ ERROR not yet stable as a const fn
Foo::func();
//[unstable]~^ ERROR cannot use `#[feature(local_feature)]`
//[stable]~^^ cannot be (indirectly) exposed to stable
// We get the error on `stable` since this is a trait function.
const_context_not_const_stable()
//[unstable]~^ ERROR cannot use `#[feature(local_feature)]`
}

fn main() {}
131 changes: 101 additions & 30 deletions tests/ui/traits/const-traits/staged-api.stable.stderr
Original file line number Diff line number Diff line change
@@ -1,89 +1,160 @@
error: trait implementations cannot be const stable yet
--> $DIR/staged-api.rs:21:1
--> $DIR/staged-api.rs:25:1
|
LL | / impl const MyTrait for Foo {
LL | |
LL | | fn func() {}
LL | | }
| |_^
|
= note: see issue #67792 <https://github.com/rust-lang/rust/issues/67792> for more information

error: function has missing const stability attribute
--> $DIR/staged-api.rs:48:1
--> $DIR/staged-api.rs:45:1
|
LL | / pub const fn const_context_not_const_stable() {
LL | |
LL | | Unstable::func();
LL | |
... |
LL | | Foo::func();
LL | | // We get the error on `stable` since this is a trait function.
LL | | Unstable2::func();
LL | | // ^ fails, because the `unstable2` feature is not active
LL | | }
| |_^

error: `<staged_api::Unstable as staged_api::MyTrait>::func` is not yet stable as a const fn
--> $DIR/staged-api.rs:34:5
error: const function that might be (indirectly) exposed to stable cannot use `#[feature(const_trait_impl)]`
--> $DIR/staged-api.rs:37:5
|
LL | Unstable::func();
| ^^^^^^^^^^^^^^^^
|
= help: add `#![feature(unstable)]` to the crate attributes to enable
help: if the function is not (yet) meant to be exposed to stable, add `#[rustc_const_unstable]` (this is what you probably want to do)
|
LL + #[rustc_const_unstable(feature = "...", issue = "...")]
LL | const fn const_context() {
|
help: otherwise, as a last resort `#[rustc_allow_const_fn_unstable]` can be used to bypass stability checks (this requires team approval)
|
LL + #[rustc_allow_const_fn_unstable(const_trait_impl)]
LL | const fn const_context() {
|

error: `<Foo as staged_api::MyTrait>::func` cannot be (indirectly) exposed to stable
--> $DIR/staged-api.rs:37:5
error: const function that might be (indirectly) exposed to stable cannot use `#[feature(const_trait_impl)]`
--> $DIR/staged-api.rs:38:5
|
LL | Foo::func();
| ^^^^^^^^^^^
|
= help: either mark the callee as `#[rustc_const_stable_indirect]`, or the caller as `#[rustc_const_unstable]`
help: if the function is not (yet) meant to be exposed to stable, add `#[rustc_const_unstable]` (this is what you probably want to do)
|
LL + #[rustc_const_unstable(feature = "...", issue = "...")]
LL | const fn const_context() {
|
help: otherwise, as a last resort `#[rustc_allow_const_fn_unstable]` can be used to bypass stability checks (this requires team approval)
|
LL + #[rustc_allow_const_fn_unstable(const_trait_impl)]
LL | const fn const_context() {
|

error: `<staged_api::Unstable2 as staged_api::MyTrait>::func` is not yet stable as a const fn
--> $DIR/staged-api.rs:41:5
error: const function that might be (indirectly) exposed to stable cannot use `#[feature(const_trait_impl)]`
--> $DIR/staged-api.rs:39:5
|
LL | Unstable2::func();
| ^^^^^^^^^^^^^^^^^
|
= help: add `#![feature(unstable2)]` to the crate attributes to enable
help: if the function is not (yet) meant to be exposed to stable, add `#[rustc_const_unstable]` (this is what you probably want to do)
|
LL + #[rustc_const_unstable(feature = "...", issue = "...")]
LL | const fn const_context() {
|
help: otherwise, as a last resort `#[rustc_allow_const_fn_unstable]` can be used to bypass stability checks (this requires team approval)
|
LL + #[rustc_allow_const_fn_unstable(const_trait_impl)]
LL | const fn const_context() {
|

error: `<staged_api::Unstable as staged_api::MyTrait>::func` is not yet stable as a const fn
--> $DIR/staged-api.rs:50:5
error: const function that might be (indirectly) exposed to stable cannot use `#[feature(const_trait_impl)]`
--> $DIR/staged-api.rs:46:5
|
LL | Unstable::func();
| ^^^^^^^^^^^^^^^^
|
= help: add `#![feature(unstable)]` to the crate attributes to enable
help: if the function is not (yet) meant to be exposed to stable, add `#[rustc_const_unstable]` (this is what you probably want to do)
|
LL + #[rustc_const_unstable(feature = "...", issue = "...")]
LL | pub const fn const_context_not_const_stable() {
|
help: otherwise, as a last resort `#[rustc_allow_const_fn_unstable]` can be used to bypass stability checks (this requires team approval)
|
LL + #[rustc_allow_const_fn_unstable(const_trait_impl)]
LL | pub const fn const_context_not_const_stable() {
|

error: `<Foo as staged_api::MyTrait>::func` cannot be (indirectly) exposed to stable
--> $DIR/staged-api.rs:52:5
error: const function that might be (indirectly) exposed to stable cannot use `#[feature(const_trait_impl)]`
--> $DIR/staged-api.rs:47:5
|
LL | Foo::func();
| ^^^^^^^^^^^
|
= help: either mark the callee as `#[rustc_const_stable_indirect]`, or the caller as `#[rustc_const_unstable]`
help: if the function is not (yet) meant to be exposed to stable, add `#[rustc_const_unstable]` (this is what you probably want to do)
|
LL + #[rustc_const_unstable(feature = "...", issue = "...")]
LL | pub const fn const_context_not_const_stable() {
|
help: otherwise, as a last resort `#[rustc_allow_const_fn_unstable]` can be used to bypass stability checks (this requires team approval)
|
LL + #[rustc_allow_const_fn_unstable(const_trait_impl)]
LL | pub const fn const_context_not_const_stable() {
|

error: `<staged_api::Unstable2 as staged_api::MyTrait>::func` is not yet stable as a const fn
--> $DIR/staged-api.rs:55:5
error: const function that might be (indirectly) exposed to stable cannot use `#[feature(const_trait_impl)]`
--> $DIR/staged-api.rs:49:5
|
LL | Unstable2::func();
| ^^^^^^^^^^^^^^^^^
|
= help: add `#![feature(unstable2)]` to the crate attributes to enable
help: if the function is not (yet) meant to be exposed to stable, add `#[rustc_const_unstable]` (this is what you probably want to do)
|
LL + #[rustc_const_unstable(feature = "...", issue = "...")]
LL | pub const fn const_context_not_const_stable() {
|
help: otherwise, as a last resort `#[rustc_allow_const_fn_unstable]` can be used to bypass stability checks (this requires team approval)
|
LL + #[rustc_allow_const_fn_unstable(const_trait_impl)]
LL | pub const fn const_context_not_const_stable() {
|

error: `<staged_api::Unstable as staged_api::MyTrait>::func` is not yet stable as a const fn
--> $DIR/staged-api.rs:63:5
error: const function that might be (indirectly) exposed to stable cannot use `#[feature(const_trait_impl)]`
--> $DIR/staged-api.rs:56:5
|
LL | Unstable::func();
| ^^^^^^^^^^^^^^^^
|
= help: add `#![feature(unstable)]` to the crate attributes to enable
help: if the function is not (yet) meant to be exposed to stable, add `#[rustc_const_unstable]` (this is what you probably want to do)
|
LL + #[rustc_const_unstable(feature = "...", issue = "...")]
LL | const fn stable_const_context() {
|
help: otherwise, as a last resort `#[rustc_allow_const_fn_unstable]` can be used to bypass stability checks (this requires team approval)
|
LL + #[rustc_allow_const_fn_unstable(const_trait_impl)]
LL | const fn stable_const_context() {
|

error: `<Foo as staged_api::MyTrait>::func` cannot be (indirectly) exposed to stable
--> $DIR/staged-api.rs:66:5
error: const function that might be (indirectly) exposed to stable cannot use `#[feature(const_trait_impl)]`
--> $DIR/staged-api.rs:57:5
|
LL | Foo::func();
| ^^^^^^^^^^^
|
= help: either mark the callee as `#[rustc_const_stable_indirect]`, or the caller as `#[rustc_const_unstable]`
help: if the function is not (yet) meant to be exposed to stable, add `#[rustc_const_unstable]` (this is what you probably want to do)
|
LL + #[rustc_const_unstable(feature = "...", issue = "...")]
LL | const fn stable_const_context() {
|
help: otherwise, as a last resort `#[rustc_allow_const_fn_unstable]` can be used to bypass stability checks (this requires team approval)
|
LL + #[rustc_allow_const_fn_unstable(const_trait_impl)]
LL | const fn stable_const_context() {
|

error: aborting due to 10 previous errors

Loading
Loading