Skip to content

Fix evaluating trivial drop glue in constants #57734

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 3 commits into from
Jan 26, 2019

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Jan 18, 2019

struct A;
impl Drop for A {
    fn drop(&mut self) {}
}

const FOO: Option<A> = None;

const BAR: () = (FOO, ()).1;

was erroring with

error: any use of this value will cause an error
 --> src/lib.rs:9:1
  |
9 | const BAR: () = (FOO, ()).1;
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^-^
  |                           |
  |                           calling non-const function `std::ptr::real_drop_in_place::<(std::option::Option<A>, ())> - shim(Some((std::option::Option<A>, ())))`
  |
  = note: #[deny(const_err)] on by default

error: aborting due to previous error

before this PR. According to godbolt this last compiled successfully in 1.27

@rust-highfive
Copy link
Contributor

r? @pnkfelix

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 18, 2019
@pnkfelix
Copy link
Member

pnkfelix commented Jan 23, 2019

Can you add a ui test for the error case showing that const FOO: Option<A> = Some(A); will continue be rejected under this change?

I skimmed over the existing tests and I didn't immediately see anything covering that (where we have the glue for Option around the type implementing Drop).

@pnkfelix
Copy link
Member

r=me with aforementioned test added.

@pnkfelix pnkfelix added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 23, 2019
@oli-obk
Copy link
Contributor Author

oli-obk commented Jan 23, 2019

We do have

static EARLY_DROP_S: i32 = (WithDtor, 0).1;
//~^ ERROR destructors cannot be evaluated at compile-time
const EARLY_DROP_C: i32 = (WithDtor, 0).1;
//~^ ERROR destructors cannot be evaluated at compile-time
and
const F : Foo = (Foo { a : 0 }, Foo { a : 1 }).1;
but neither of these actually use an Option so I added some tests to static-drop-scope.rs

@bors r=pnkfelix

@bors
Copy link
Collaborator

bors commented Jan 23, 2019

📌 Commit 39aa89b has been approved by pnkfelix

@bors
Copy link
Collaborator

bors commented Jan 23, 2019

🌲 The tree is currently closed for pull requests below priority 1000, this pull request will be tested once the tree is reopened

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 23, 2019
Centril added a commit to Centril/rust that referenced this pull request Jan 24, 2019
Fix evaluating trivial drop glue in constants

```rust
struct A;
impl Drop for A {
    fn drop(&mut self) {}
}

const FOO: Option<A> = None;

const BAR: () = (FOO, ()).1;
```

was erroring with

```
error: any use of this value will cause an error
 --> src/lib.rs:9:1
  |
9 | const BAR: () = (FOO, ()).1;
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^-^
  |                           |
  |                           calling non-const function `std::ptr::real_drop_in_place::<(std::option::Option<A>, ())> - shim(Some((std::option::Option<A>, ())))`
  |
  = note: #[deny(const_err)] on by default

error: aborting due to previous error
```

before this PR. According to godbolt this last compiled successfully in 1.27
@Centril
Copy link
Contributor

Centril commented Jan 24, 2019

Failed in rollup, #57874 (comment).
@bors r-

(please please --bless your tests with --compare-mode=nll :) )

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 24, 2019
@oli-obk oli-obk force-pushed the fixes_and_cleanups branch from 39aa89b to 506393e Compare January 25, 2019 08:54
@oli-obk
Copy link
Contributor Author

oli-obk commented Jan 25, 2019

@bors r=pnkfelix

@bors
Copy link
Collaborator

bors commented Jan 25, 2019

📌 Commit 506393e has been approved by pnkfelix

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 25, 2019
Centril added a commit to Centril/rust that referenced this pull request Jan 25, 2019
Fix evaluating trivial drop glue in constants

```rust
struct A;
impl Drop for A {
    fn drop(&mut self) {}
}

const FOO: Option<A> = None;

const BAR: () = (FOO, ()).1;
```

was erroring with

```
error: any use of this value will cause an error
 --> src/lib.rs:9:1
  |
9 | const BAR: () = (FOO, ()).1;
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^-^
  |                           |
  |                           calling non-const function `std::ptr::real_drop_in_place::<(std::option::Option<A>, ())> - shim(Some((std::option::Option<A>, ())))`
  |
  = note: #[deny(const_err)] on by default

error: aborting due to previous error
```

before this PR. According to godbolt this last compiled successfully in 1.27
bors added a commit that referenced this pull request Jan 25, 2019
Rollup of 5 pull requests

Successful merges:

 - #56233 (Miri and miri-related code contains repetitions of `(n << amt) >> amt`)
 - #57645 (distinguish "no data" from "heterogeneous" in ABI)
 - #57734 (Fix evaluating trivial drop glue in constants)
 - #57886 (Add suggestion for moving type declaration before associated type bindings in generic arguments.)
 - #57890 (Fix wording in diagnostics page)

Failed merges:

r? @ghost
@bors bors merged commit 506393e into rust-lang:master Jan 26, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants