Skip to content

Introduce DST-enabled Rc as unstable under a feature flag #25531

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

Closed
bluss opened this issue May 17, 2015 · 7 comments
Closed

Introduce DST-enabled Rc as unstable under a feature flag #25531

bluss opened this issue May 17, 2015 · 7 comments
Labels
A-stability Area: `#[stable]`, `#[unstable]` etc.

Comments

@bluss
Copy link
Member

bluss commented May 17, 2015

It's irresponsible not to do it.

See #25515.

@Thiez
Copy link
Contributor

Thiez commented May 17, 2015

Must we really have feature flags for everything? If the bug turns out to have an easy fix, surely just fixing the bug would be preferable to introducing yet another feature flag. Let's wait for a day so the bug can get discussed, and if it turns out to be a very hard problem the feature flag can always be introduced then. Alternatively, temporarily change the line

impl<T: ?Sized+Unsize<U>, U: ?Sized> CoerceUnsized<Rc<U>> for Rc<T> {}

to

impl<T: ?Sized+Copy+Unsize<U>, U: ?Sized> CoerceUnsized<Rc<U>> for Rc<T> {}

because the Copy bound will prevent types with a Drop implementation from being coerced for now (why don't we have a Pod trait anymore?), and removing the bound is probably backwards compatible.

@bluss
Copy link
Member Author

bluss commented May 17, 2015

We have feature flags for everything that's unstable. That's not really the problem, I just want it to be unstable while in development. We only have 6-week cycles now and why rush out something that's broken?

I think the default should be that big features are introduced under feature flag.

cc @nikomatsakis since you introduced it, what do you think? If you have 100% belief in the API that's fine, but sometimes we need to adjust APIs to fix soundness bugs.

@alexcrichton
Copy link
Member

The current system of stability attributes is known to not be bullet proof and doesn't cover the stability of impl blocks, for example, so I don't think we need to make sure that everything comes in under a feature gate, adding functionality like this is fine to land without it. Although there may be bugs no one's going to rely on the bug of double-drop, for example.

@bluss
Copy link
Member Author

bluss commented May 18, 2015

I think I'm overreacting a bit, I'm sorry for that. Especially given that the bug is not in Rc-DST at all!

@alexcrichton Stabilizing precludes changes to API, if those are needed to provide a sound interface for say for example Rc-DST. See my question to niko.

@steveklabnik steveklabnik added A-libs A-stability Area: `#[stable]`, `#[unstable]` etc. labels May 18, 2015
@bluss
Copy link
Member Author

bluss commented Jun 17, 2015

Another data point: see bug #26352.

The feature is broken in the beta, which is unfortunate. With a feature flag we could have quality assured it before we stabilized it.

@apasel422
Copy link
Contributor

Rc is DST-enabled:

use std::rc::Rc;

fn main() {
    let x: Rc<[i32]> = Rc::new([1, 2, 3]);
}

I think this can be closed.

@steveklabnik
Copy link
Member

Yup, this should have been okay as of 1.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-stability Area: `#[stable]`, `#[unstable]` etc.
Projects
None yet
Development

No branches or pull requests

5 participants