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

Adding a Drop impl to a type is a breaking change #58379

Closed
oli-obk opened this issue Feb 11, 2019 · 3 comments
Closed

Adding a Drop impl to a type is a breaking change #58379

oli-obk opened this issue Feb 11, 2019 · 3 comments
Labels
A-const-fn Area: const fn foo(..) {..}. Pure functions which can be applied at compile time. regression-from-stable-to-stable Performance or correctness regression from one stable version to another. 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 Feb 11, 2019

uncommenting the drop impl in the following program causes it to not compile

struct Foo;

/*
impl Drop for Foo {
    fn drop(&mut self) { }
}
*/

const fn new(_: Foo) { }

if the definition of Foo were in another crate, adding that Drop impl would break compilation of the crate with the const fn.

cc @Centril

I don't think adding a drop impl was a breaking change before const fn. But maybe I just can't come up with an example.

@oli-obk oli-obk added T-lang Relevant to the language team, which will review and decide on the PR/issue. regression-from-stable-to-stable Performance or correctness regression from one stable version to another. A-const-fn Area: const fn foo(..) {..}. Pure functions which can be applied at compile time. labels Feb 11, 2019
@Centril
Copy link
Contributor

Centril commented Feb 11, 2019

Not exactly sure what is wanted from the lang team here; the description here is accurate.

But maybe I just can't come up with an example.

//#[derive(Copy, Clone)]
struct Alpha;

struct Beta;

struct Foo(pub Alpha, pub Beta);

//impl Drop for Foo { fn drop(&mut self) {} }

fn new(x: Foo) {
    let _a = x.0;
}

This needs at least one public field which isn't Copy.

@petrochenkov
Copy link
Contributor

let _a = x.0;

This case is fixable (there was an RFC allowing this in 2017-2018, but I can't find it), but things are currently moving in the opposite direction with that RFC being closed and rust-lang/rfcs#2514 + #56440 being accepted.

@oli-obk
Copy link
Contributor Author

oli-obk commented Feb 12, 2019

Not exactly sure what is wanted from the lang team here

Well, this is a regression in the language, so I assumed the lang team would be the relevant team to figure this out.

Since there is an example that shows that adding Drop impls to types is a breaking change even ignoring const fn, there's nothing to act on here I think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-fn Area: const fn foo(..) {..}. Pure functions which can be applied at compile time. regression-from-stable-to-stable Performance or correctness regression from one stable version to another. 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

3 participants