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

permit Drop types in constants (tracking issue for RFC #1440) #33156

Closed
1 of 2 tasks
nikomatsakis opened this issue Apr 22, 2016 · 18 comments
Closed
1 of 2 tasks

permit Drop types in constants (tracking issue for RFC #1440) #33156

nikomatsakis opened this issue Apr 22, 2016 · 18 comments
Assignees
Labels
B-RFC-implemented Blocker: Approved by a merged RFC and implemented. B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Apr 22, 2016

Tracking issue for rust-lang/rfcs#1440: "Allow Drop types in statics/const functions"

@nikomatsakis nikomatsakis added T-lang Relevant to the language team, which will review and decide on the PR/issue. B-unstable Blocker: Implemented in the nightly compiler and unstable. labels Apr 22, 2016
@eddyb eddyb self-assigned this Apr 25, 2016
bors added a commit that referenced this issue May 7, 2016
Implement constant support in MIR.

All of the intended features in `trans::consts` are now supported by `mir::constant`.
The implementation is considered a temporary measure until `miri` replaces it.

A `-Z orbit` bootstrap build will only translate LLVM IR from AST for `#[rustc_no_mir]` functions.

Furthermore, almost all checks of constant expressions have been moved to MIR.
In non-`const` functions, trees of temporaries are promoted, as per RFC 1414 (rvalue promotion).
Promotion before MIR borrowck would allow reasoning about promoted values' lifetimes.

The improved checking comes at the cost of four `[breaking-change]`s:
* repeat counts must contain a constant expression, e.g.:
`let arr = [0; { println!("foo"); 5 }];` used to be allowed (it behaved like `let arr = [0; 5];`)
* dereference of a reference to a `static` cannot be used in another `static`, e.g.:
`static X: [u8; 1] = [1]; static Y: u8 = (&X)[0];` was unintentionally allowed before
* the type of a `static` *must* be `Sync`, irrespective of the initializer, e.g.
`static FOO: *const T = &BAR;` worked as `&T` is `Sync`, but it shouldn't because `*const T` isn't
* a `static` cannot wrap `UnsafeCell` around a type that *may* need drop, e.g.
`static X: MakeSync<UnsafeCell<Option<String>>> = MakeSync(UnsafeCell::new(None));`
was previously allowed based on the fact `None` alone doesn't need drop, but in `UnsafeCell`
it can be later changed to `Some(String)` which *does* need dropping

The drop restrictions are relaxed by RFC 1440 (#33156), which is implemented, but feature-gated.
However, creating `UnsafeCell` from constants is unstable, so users can just enable the feature gate.
bors added a commit that referenced this issue May 8, 2016
Implement constant support in MIR.

All of the intended features in `trans::consts` are now supported by `mir::constant`.
The implementation is considered a temporary measure until `miri` replaces it.

A `-Z orbit` bootstrap build will only translate LLVM IR from AST for `#[rustc_no_mir]` functions.

Furthermore, almost all checks of constant expressions have been moved to MIR.
In non-`const` functions, trees of temporaries are promoted, as per RFC 1414 (rvalue promotion).
Promotion before MIR borrowck would allow reasoning about promoted values' lifetimes.

The improved checking comes at the cost of four `[breaking-change]`s:
* repeat counts must contain a constant expression, e.g.:
`let arr = [0; { println!("foo"); 5 }];` used to be allowed (it behaved like `let arr = [0; 5];`)
* dereference of a reference to a `static` cannot be used in another `static`, e.g.:
`static X: [u8; 1] = [1]; static Y: u8 = (&X)[0];` was unintentionally allowed before
* the type of a `static` *must* be `Sync`, irrespective of the initializer, e.g.
`static FOO: *const T = &BAR;` worked as `&T` is `Sync`, but it shouldn't because `*const T` isn't
* a `static` cannot wrap `UnsafeCell` around a type that *may* need drop, e.g.
`static X: MakeSync<UnsafeCell<Option<String>>> = MakeSync(UnsafeCell::new(None));`
was previously allowed based on the fact `None` alone doesn't need drop, but in `UnsafeCell`
it can be later changed to `Some(String)` which *does* need dropping

The drop restrictions are relaxed by RFC 1440 (#33156), which is implemented, but feature-gated.
However, creating `UnsafeCell` from constants is unstable, so users can just enable the feature gate.
@kornholi
Copy link

kornholi commented Jun 3, 2016

Ran into an ICE: #34053

@nrc nrc added B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. B-RFC-implemented Blocker: Approved by a merged RFC and implemented. and removed B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. labels Aug 29, 2016
@brson
Copy link
Contributor

brson commented Jan 11, 2017

Is there anything holding up this feature? No activity.

This is used by Rocket.

@coder543
Copy link

coder543 commented Feb 6, 2017

Bumping this as well. It seems like a good candidate for stabilization?

@SimonSapin
Copy link
Contributor

SimonSapin commented Feb 26, 2017

+1 for stabilization.

Servo has a hack to work around this: a separate type with no destructor for using a static phf map, and a conversion during lookup. (This is in a crate used by Firefox, which runs on stable Rust.) Stabilizing would allow removing this hack.

@nikomatsakis
Copy link
Contributor Author

I feel ok with stabilizing drop types in statics specifically.

@nikomatsakis
Copy link
Contributor Author

@rfcbot fcp merge

I propose that we stabilize this feature, which allows types that implement Drop to appear in static and const fn. It does not (yet) permit them in const initializers, though there is a pending RFC rust-lang/rfcs#1817 on that topic (which has a few complications).

@rfcbot
Copy link

rfcbot commented Mar 2, 2017

Team member @nikomatsakis has proposed to merge this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@eddyb
Copy link
Member

eddyb commented Mar 2, 2017

@rfcbot fcp concern We can't stabilize this until we ban temporaries that "would drop" (see #40036).

@aturon
Copy link
Member

aturon commented Mar 14, 2017

@rfcbot fcp cancel

I'm cancelling FCP due to @eddy's comment, just to get it cleared out of the queue. Let's revisit when #40036 is closed.

@rfcbot
Copy link

rfcbot commented Mar 14, 2017

@aturon proposal cancelled.

@brson
Copy link
Contributor

brson commented May 22, 2017

No progress here lately. @eddyb what's the likelihood of fixing #40036 any time soon?

@Mark-Simulacrum Mark-Simulacrum added the C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. label Jul 22, 2017
@eddyb
Copy link
Member

eddyb commented Jul 25, 2017

I have a branch with the forwards compatibility checks for #40036, with no known stable code breakages, and one unstable intended regression (ironically, in @SergioBenitez's Rocket).
The catch is that it depends on rvalue promotion to be backwards compatible.
There was no reason to let this sit around this long, but I think we can move forward this release cycle.

Ryan1729 added a commit to Ryan1729/live-code-sdl2-opengl-2_1-template that referenced this issue Aug 10, 2017
hack was necessary because `Drop` tyes are currently not allowed in statics.
see rust-lang/rust#33156

Signed-off-by: Ryan1729 <Ryan1729@gmail.com>
@eddyb eddyb changed the title permit Drop types in statics (tracking issue for RFC #1440) permit Drop types in constants (tracking issue for RFC #1440) Aug 28, 2017
@eddyb
Copy link
Member

eddyb commented Aug 28, 2017

With rvalue promotion stabilized, #43932 produces no regressions and is waiting on review.
Once it's merged, we can implement the amendment RFC (for consts, including associated ones).
Do we need to delay stabilizing this feature, including the amendment, @rust-lang/lang?
Otherwise I'll propose to implement the amendment and stabilize it at once.

@aturon
Copy link
Member

aturon commented Aug 28, 2017

@eddyb I think it's totally fine to stabilize the original feature at this point, but am slightly wary of stabilizing the amendment immediately upon implementation. OTOH, it's a little hard to imagine that we'll get much feedback on nightly about it.

@eddyb
Copy link
Member

eddyb commented Aug 31, 2017

@rfcbot fcp merge

I propose that we implement the amendment and stabilize this feature now that the main blocker has been resolved - see #33156 (comment) also.

@rfcbot
Copy link

rfcbot commented Aug 31, 2017

Team member @eddyb has proposed to merge this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Aug 31, 2017
@rfcbot
Copy link

rfcbot commented Sep 1, 2017

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Sep 1, 2017
bors added a commit that referenced this issue Sep 8, 2017
Allow Drop types in const's too, with #![feature(drop_types_in_const)].

Implements the remaining amendment, see #33156. cc @SergioBenitez

r? @nikomatsakis
bors added a commit that referenced this issue Sep 9, 2017
Allow Drop types in const's too, with #![feature(drop_types_in_const)].

Implements the remaining amendment, see #33156. cc @SergioBenitez

r? @nikomatsakis
@rfcbot
Copy link

rfcbot commented Sep 11, 2017

The final comment period is now complete.

bors added a commit that referenced this issue Sep 13, 2017
Stabilize drop_types_in_const.

Closes #33156, stabilizing the new, revised, rules, and improving the error message.

r? @nikomatsakis cc @SergioBenitez
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-RFC-implemented Blocker: Approved by a merged RFC and implemented. B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

10 participants