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

Tracking issue for unsizing casts in const fns #64992

Closed
oli-obk opened this issue Oct 2, 2019 · 8 comments · Fixed by #85078
Closed

Tracking issue for unsizing casts in const fns #64992

oli-obk opened this issue Oct 2, 2019 · 8 comments · Fixed by #85078
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Oct 2, 2019

Right now we don't allow unsizing casts (array -> slice or type -> dyn Trait) in const fns on stable.

While we can't allow unsizing to dyn Trait until we've figured out trait bounds in const fns, we can easily allow unsizing casts for slices, there's no reason not to have them. The only reason we didn't have them in the initial min_const_fn feature gate was the fact that we preferred overly aggressive rules over accidental stabilization.

The relevant code is

Rvalue::Cast(CastKind::Pointer(PointerCast::Unsize), _, _) => Err((
span,
"unsizing casts are not allowed in const fn".into(),
)),

We need to check the destination and source types for slice and array respectively and permit the cast in that case. We also need to recurse with check_operand on the value that is being casted.

@oli-obk oli-obk added A-const-fn C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC labels Oct 2, 2019
@Centril Centril added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Oct 2, 2019
@jonas-schievink jonas-schievink added the B-unstable Blocker: Implemented in the nightly compiler and unstable. label Nov 26, 2019
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 10, 2020
…fJung

Stabilize casts and coercions to `&[T]` in const fn

Part of rust-lang#64992

There was never a reason to not stabilize this, we just accidentally prevented them when we implemented the `min_const_fn` feature that gave us `const fn` on stable. This PR stabilizes these casts (which are already stable in `const` outside `const fn`), while keeping all other unsizing casts (so `T` -> `dyn Trait`) unstable within const fn.
These casts have no forward compatibility concerns with any future features for const eval and users were able to use them under the `const_fn` feature gate already since at least the miri merger, possibly longer.

r? @rust-lang/lang
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 11, 2020
…fJung

Stabilize casts and coercions to `&[T]` in const fn

Part of rust-lang#64992

There was never a reason to not stabilize this, we just accidentally prevented them when we implemented the `min_const_fn` feature that gave us `const fn` on stable. This PR stabilizes these casts (which are already stable in `const` outside `const fn`), while keeping all other unsizing casts (so `T` -> `dyn Trait`) unstable within const fn.
These casts have no forward compatibility concerns with any future features for const eval and users were able to use them under the `const_fn` feature gate already since at least the miri merger, possibly longer.

r? @rust-lang/lang
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 11, 2020
…fJung

Stabilize casts and coercions to `&[T]` in const fn

Part of rust-lang#64992

There was never a reason to not stabilize this, we just accidentally prevented them when we implemented the `min_const_fn` feature that gave us `const fn` on stable. This PR stabilizes these casts (which are already stable in `const` outside `const fn`), while keeping all other unsizing casts (so `T` -> `dyn Trait`) unstable within const fn.
These casts have no forward compatibility concerns with any future features for const eval and users were able to use them under the `const_fn` feature gate already since at least the miri merger, possibly longer.

r? @rust-lang/lang
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 11, 2020
…fJung

Stabilize casts and coercions to `&[T]` in const fn

Part of rust-lang#64992

There was never a reason to not stabilize this, we just accidentally prevented them when we implemented the `min_const_fn` feature that gave us `const fn` on stable. This PR stabilizes these casts (which are already stable in `const` outside `const fn`), while keeping all other unsizing casts (so `T` -> `dyn Trait`) unstable within const fn.
These casts have no forward compatibility concerns with any future features for const eval and users were able to use them under the `const_fn` feature gate already since at least the miri merger, possibly longer.

r? @rust-lang/lang
@RalfJung
Copy link
Member

Judging from the feature message, nowadays slices seem to be allowed by dyn unsizing is still disallowed -- but only in const fn. We also have no tests using this feature (just some compile-fail tests checking the feature gate).

I am not entirely sure why these casts are disallowed only in const fn, but I assume it is a matter of backwards compatibility. However, this means we already have to be sure that the corresponding Miri code is right, so -- what is blocking us from simply removing this feature gate, and allowing the corresponding code?

@RalfJung
Copy link
Member

RalfJung commented Apr 19, 2021

In fact while trying to write a gate test for #84310, I was unable to find any example of a piece of code that compiles once const_fn_unsize is allowed. I always then additionally get the error about "trait bounds other than Sized".

@oli-obk If we just removed this UnsizingCast in check_consts/ops.rs and all its call sites, is there any additional code that would be accepted?

@RalfJung
Copy link
Member

Ah I found something

const fn test() {
    let _x = NonNull::<[i32; 0]>::dangling() as NonNull<[i32]>;
}

I am not sure why this is gated. It's not a slice directly, but it's also not a lot more complicated.

@oli-obk
Copy link
Contributor Author

oli-obk commented Apr 22, 2021

Slices are OK, trait objects aren't. If the same code snippet but with trait objects fails even with the feature gate, just removing that check is fine by me (needs stab report and lang team approval on the stab PR)

@RalfJung
Copy link
Member

Yeah, there is a rather aggressive check for dyn Trait appearing anywhere that triggers "trait bounds other than Sized are unstable".

@RalfJung
Copy link
Member

RalfJung commented May 8, 2021

@rust-lang/lang I would like to propose stabilizing this feature.

Stabilization report

const_fn_unsize prevents certain unsizing casts from happening in const fn (they were already allowed in const/static bodies):

const fn test() {
    let _x = NonNull::<[i32; 0]>::dangling() as NonNull<[i32]>;
}

Unsizing casts where the type itself has a slice as "unsized tail" were already allowed (since #73862), but when the type is a pointer to something that has a slice as unsized tail, it was forbidden. By stabilizing const_fn_unsize, we allow code like the above. The CTFE engine has supported such code for a long time (and this is battle-tested in standalone Miri), so I see little risk here.

#85078 turns src/test/ui/consts/const_fn_unsize.rs into a test checking correct functionality of this feature.

@joshtriplett
Copy link
Member

FCP started on #85078.

@Mark-Simulacrum
Copy link
Member

Dropping nomination, FCP proceeding smoothly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants