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

Allow Drop types in const's too, with #![feature(drop_types_in_const)]. #44212

Merged
merged 1 commit into from Sep 9, 2017

Conversation

eddyb
Copy link
Member

@eddyb eddyb commented Aug 31, 2017

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

r? @nikomatsakis

//~^ ERROR statics are not allowed to have destructors

const EARLY_DROP_C: i32 = (WithDtor, 0).1;
//~^ ERROR constants are not allowed to have destructors
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error message is confusing since constants are allowed to have destructors. Instead, they're not allowed to require destruction upon use. The error message should be changed to clarify the issue.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe "not allowed to execute destructors"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please leave a comment on #33156 instead, to do so when stabilizing the feature.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Member Author

@eddyb eddyb Aug 31, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was directing that at @SergioBenitez FWIW, github didn't update, but thanks for updating the issue description!

Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a suggestion for a test -- are there other edge cases to consider? Otherwise seems pretty clean!

@@ -17,7 +17,14 @@ impl Drop for A {
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like we need a test that demonstrates the runtime semantics of constants with DROP. In particular, the destructor does execute when you (e.g.) access BAR or A::BAZ, right?

I want some test that shows when the destructor runs and tests that it did indeed execute (i.e., by observing some effect it had on a global counter).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right, this was the odd thing about the RFC amendment.

@shepmaster shepmaster added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Sep 1, 2017
@eddyb eddyb added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 2, 2017
@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Sep 5, 2017

📌 Commit 1d5c0ba has been approved by nikomatsakis

@arielb1 arielb1 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-review Status: Awaiting review from the assignee but also interested parties. labels Sep 5, 2017
@bors
Copy link
Contributor

bors commented Sep 7, 2017

⌛ Testing commit 1d5c0ba with merge af440b1553833cf28debb302d1c7a38ec60e64d2...

@bors
Copy link
Contributor

bors commented Sep 7, 2017

💔 Test failed - status-travis

@kennytm
Copy link
Member

kennytm commented Sep 7, 2017

@bors retry #40474

x86_64-gnu-incremental failed, crates.io responses with 503.

[00:06:17]  Downloading unicode-segmentation v1.2.0
[00:07:17] warning: spurious network error (2 tries remaining): [28] Timeout was reached (Operation too slow. Less than 10 bytes/sec transferred the last 30 seconds)
[00:07:47] warning: spurious network error (1 tries remaining): failed to get 200 response from `https://crates.io/api/v1/crates/unicode-segmentation/1.2.0/download`, got 503
[00:08:17] error: unable to get packages from source
[00:08:17] 
[00:08:17] Caused by:
[00:08:17]   failed to get 200 response from `https://crates.io/api/v1/crates/unicode-segmentation/1.2.0/download`, got 503
[00:08:17] thread 'main' panicked at 'command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "metadata" "--format-version" "1" "--manifest-path" "/checkout/src/libstd/Cargo.toml"
[00:08:17] expected success, got: exit code: 101', /checkout/src/build_helper/lib.rs:137:8
[00:08:17] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[00:08:17] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap build nonexistent/path/to/trigger/cargo/metadata
[00:08:17] Build completed unsuccessfully in 0:03:33
[00:08:17] Makefile:77: recipe for target 'prepare' failed
[00:08:17] make: *** [prepare] Error 1
[00:08:17] Command failed. Attempt 3/5:
[00:08:17]     Finished dev [unoptimized] target(s) in 0.0 secs
[00:08:18] warning: Package `toml v0.4.5` does not have feature `serde`. It has a required dependency with that name, but only optional dependencies can be used as features. This is currently a warning to ease the transition, but it will become an error in the future.
[00:08:18]  Downloading clap v2.26.0
[00:08:18]  Downloading memchr v0.1.11
[00:08:18]  Downloading debug_unreachable v0.1.1
[00:08:49]  Downloading mac v0.1.1
[00:08:49]  Downloading bitflags v0.9.1
[00:09:49] warning: spurious network error (2 tries remaining): [28] Timeout was reached (Operation too slow. Less than 10 bytes/sec transferred the last 30 seconds)
[00:09:49]  Downloading strsim v0.6.0
[00:10:50] warning: spurious network error (2 tries remaining): [28] Timeout was reached (Operation too slow. Less than 10 bytes/sec transferred the last 30 seconds)
[00:10:50]  Downloading unicode-xid v0.0.3
[00:11:20]  Downloading kuchiki v0.5.1
[00:12:21] warning: spurious network error (2 tries remaining): [28] Timeout was reached (Operation too slow. Less than 10 bytes/sec transferred the last 30 seconds)
[00:12:51] warning: spurious network error (1 tries remaining): failed to get 200 response from `https://crates.io/api/v1/crates/kuchiki/0.5.1/download`, got 503
[00:13:21] error: unable to get packages from source
[00:13:21] 
[00:13:21] Caused by:
[00:13:21]   [28] Timeout was reached (Operation too slow. Less than 10 bytes/sec transferred the last 30 seconds)
[00:13:21] thread 'main' panicked at 'command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "metadata" "--format-version" "1" "--manifest-path" "/checkout/src/libstd/Cargo.toml"
[00:13:21] expected success, got: exit code: 101', /checkout/src/build_helper/lib.rs:137:8
[00:13:21] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[00:13:21] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap build nonexistent/path/to/trigger/cargo/metadata
[00:13:21] Build completed unsuccessfully in 0:05:03
[00:13:21] make: *** [prepare] Error 1
[00:13:21] Makefile:77: recipe for target 'prepare' failed
[00:13:21] Command failed. Attempt 4/5:
[00:13:21]     Finished dev [unoptimized] target(s) in 0.0 secs
[00:13:21] warning: Package `toml v0.4.5` does not have feature `serde`. It has a required dependency with that name, but only optional dependencies can be used as features. This is currently a warning to ease the transition, but it will become an error in the future.
[00:13:21]  Downloading pest v0.3.3
[00:13:22]  Downloading unicode-bidi v0.3.4
[00:14:22] warning: spurious network error (2 tries remaining): [28] Timeout was reached (Operation too slow. Less than 10 bytes/sec transferred the last 30 seconds)
[00:14:52] warning: spurious network error (1 tries remaining): failed to get 200 response from `https://crates.io/api/v1/crates/unicode-bidi/0.3.4/download`, got 503
[00:15:22] error: unable to get packages from source
[00:15:22] 
[00:15:22] Caused by:
[00:15:22]   failed to get 200 response from `https://crates.io/api/v1/crates/unicode-bidi/0.3.4/download`, got 503
[00:15:22] thread 'main' panicked at 'command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "metadata" "--format-version" "1" "--manifest-path" "/checkout/src/libstd/Cargo.toml"
[00:15:22] expected success, got: exit code: 101', /checkout/src/build_helper/lib.rs:137:8
[00:15:22] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[00:15:22] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap build nonexistent/path/to/trigger/cargo/metadata
[00:15:22] Build completed unsuccessfully in 0:02:01
[00:15:22] make: *** [prepare] Error 1
[00:15:22] Makefile:77: recipe for target 'prepare' failed
[00:15:22] Command failed. Attempt 5/5:
[00:15:23]     Finished dev [unoptimized] target(s) in 0.0 secs
[00:15:23] warning: Package `toml v0.4.5` does not have feature `serde`. It has a required dependency with that name, but only optional dependencies can be used as features. This is currently a warning to ease the transition, but it will become an error in the future.
[00:15:23]  Downloading handlebars v0.26.2
[00:15:53] warning: spurious network error (2 tries remaining): [28] Timeout was reached (Operation too slow. Less than 10 bytes/sec transferred the last 30 seconds)
[00:16:23] warning: spurious network error (1 tries remaining): failed to get 200 response from `https://crates.io/api/v1/crates/handlebars/0.26.2/download`, got 503
[00:16:53] error: unable to get packages from source
[00:16:53] 
[00:16:53] Caused by:
[00:16:53]   failed to get 200 response from `https://crates.io/api/v1/crates/handlebars/0.26.2/download`, got 503
[00:16:53] thread 'main' panicked at 'command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "metadata" "--format-version" "1" "--manifest-path" "/checkout/src/libstd/Cargo.toml"
[00:16:53] expected success, got: exit code: 101', /checkout/src/build_helper/lib.rs:137:8
[00:16:53] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[00:16:53] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap build nonexistent/path/to/trigger/cargo/metadata
[00:16:53] Build completed unsuccessfully in 0:01:30
[00:16:53] Makefile:77: recipe for target 'prepare' failed
[00:16:53] make: *** [prepare] Error 1
[00:16:53] The command has failed after 5 attempts.

@bors
Copy link
Contributor

bors commented Sep 7, 2017

⌛ Testing commit 1d5c0ba with merge c760aa74070fd08cf53008dc7c25928f813bc918...

@bors
Copy link
Contributor

bors commented Sep 7, 2017

💔 Test failed - status-travis

@kennytm
Copy link
Member

kennytm commented Sep 7, 2017

@bors retry Travis's problem https://www.traviscistatus.com/incidents/4qylrqvy50gy

@bors
Copy link
Contributor

bors commented Sep 7, 2017

⌛ Testing commit 1d5c0ba with merge 8d30e7d87e56eec1aa1d1a7687ac4ba01ae0ebbb...

@bors
Copy link
Contributor

bors commented Sep 7, 2017

💔 Test failed - status-travis

@alexcrichton
Copy link
Member

@bors: retry

@bors
Copy link
Contributor

bors commented Sep 8, 2017

⌛ Testing commit 1d5c0ba with merge 2e2b1fe...

bors added a commit that referenced this pull request 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
Copy link
Contributor

bors commented Sep 8, 2017

💔 Test failed - status-travis

@alexcrichton
Copy link
Member

@bors: retry

  • the reason we all think it is

@bors
Copy link
Contributor

bors commented Sep 9, 2017

⌛ Testing commit 1d5c0ba with merge 18366f4...

bors added a commit that referenced this pull request 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
@bors
Copy link
Contributor

bors commented Sep 9, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing 18366f4 to master...

@bors bors merged commit 1d5c0ba into rust-lang:master Sep 9, 2017
@eddyb eddyb deleted the drop-const branch September 9, 2017 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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.

None yet

8 participants