-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Allow Drop types in const's too, with #![feature(drop_types_in_const)]. #44212
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,14 +10,32 @@ | |
|
||
#![feature(drop_types_in_const)] | ||
|
||
use std::sync::atomic::{AtomicUsize, ATOMIC_USIZE_INIT, Ordering}; | ||
|
||
static DROP_COUNTER: AtomicUsize = ATOMIC_USIZE_INIT; | ||
|
||
struct A(i32); | ||
|
||
impl Drop for A { | ||
fn drop(&mut self) {} | ||
fn drop(&mut self) { | ||
// update global drop count | ||
DROP_COUNTER.fetch_add(1, Ordering::SeqCst); | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, right, this was the odd thing about the RFC amendment. |
||
|
||
static FOO: A = A(123); | ||
const BAR: A = A(456); | ||
|
||
impl A { | ||
const BAZ: A = A(789); | ||
} | ||
|
||
fn main() { | ||
println!("{}", &FOO.0); | ||
assert_eq!(DROP_COUNTER.load(Ordering::SeqCst), 0); | ||
assert_eq!(&FOO.0, &123); | ||
assert_eq!(DROP_COUNTER.load(Ordering::SeqCst), 0); | ||
assert_eq!(BAR.0, 456); | ||
assert_eq!(DROP_COUNTER.load(Ordering::SeqCst), 1); | ||
assert_eq!(A::BAZ.0, 789); | ||
assert_eq!(DROP_COUNTER.load(Ordering::SeqCst), 2); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,11 @@ | ||
error[E0493]: constants are not allowed to have destructors | ||
--> $DIR/E0493.rs:27:17 | ||
--> $DIR/E0493.rs:29:17 | ||
| | ||
16 | fn drop(&mut self) {} | ||
18 | fn drop(&mut self) {} | ||
| --------------------- destructor defined here | ||
... | ||
27 | const F : Foo = Foo { a : 0 }; | ||
| ^^^^^^^^^^^^^ constants cannot have destructors | ||
29 | const F : Foo = (Foo { a : 0 }, Foo { a : 1 }).1; | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constants cannot have destructors | ||
|
||
error: aborting due to previous error | ||
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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"?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
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!