-
Notifications
You must be signed in to change notification settings - Fork 13.3k
DST coercions and DST fields in structs #14397
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 |
---|---|---|
|
@@ -1088,7 +1088,8 @@ mod tests { | |
let n = list_from([1i,2,3]); | ||
spawn(proc() { | ||
check_links(&n); | ||
assert_eq!(&[&1,&2,&3], n.iter().collect::<Vec<&int>>().as_slice()); | ||
let a: &[_] = &[&1,&2,&3]; | ||
assert_eq!(a, n.iter().collect::<Vec<&int>>().as_slice()); | ||
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. here and elsewhere: Does the E.g. could we here instead write I am not sure if it would actually make things look nicer, but it might help stress that a coercion is happening here, not just a type assertion. 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. It does not. From what I could tell at the time, the 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 might be using the term "coercion" incorrectly here. anyway the point is whether we have any expression form, other than a 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. @nick29581 Anyway, if possible, when you squash your commits together, I would appreciate it if you put these preparatory refactorings on a separate early commit, so that people looking at the history can easily focus in on the substansive changes. |
||
}); | ||
} | ||
|
||
|
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.
The drop glue has the responsibility to not do anything if the entire value is zeroed - structures with a
Drop
impl have a drop flag by default, and pointers use null values.For
~T
/Box<T>
, drop glue makes the null check. I would've expected~[T]
/Box<[T]>
to be even more like that after this change.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.
Drop glue should make the null check, but it was not trivial to do (probably easy, but not trivial - I got bugs which I didn't start to investigate), so I left it for a follow up (the 'hard' case is when the vec has length 0, then I use a null pointer inside the fat pointer).
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.
nit: put an octothorpe (
#
) in front of the number in the fixme.