-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Chapter 19: advanced features #480
Conversation
src/ch19-02-advanced-lifetimes.md
Outdated
If we wanted to write it out entirely, we'd use this syntax, with `for<>`: | ||
|
||
```rust | ||
fn call_with_ref<F>(some_closure:F) -> i32 |
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.
@steveklabnik it looks like you intended for this code example to compile, but it's not: 3595078
I think I'm going to have to read the chapter to figure out why, but if you know right away and could fix it, that would be awesome ;)
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 uh, thought i tested this....
src/ch19-01-unsafe-rust.md
Outdated
} | ||
``` | ||
|
||
You can only use these features inside of these blocks. This means that you do |
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.
means that if you do
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.
Reviewed some of the early material, up through transmute
. Looks quite good, though I left a couple important suggestions. I'll try to review more soon.
src/ch19-01-unsafe-rust.md
Outdated
make a mistake and something goes wrong, you'll know that it has to be related | ||
to one of the places that you opted into this unsafety. That makes these bugs | ||
much easier to find. Because of this, it's important to contain your unsafe | ||
code to as small of an area as possible. Once you use unsafe inside of a |
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.
Probably good to forward-reference "unsafe abstractions" here -- just the concept that you can "seal up" a use of unsafety behind a safe interface.
src/ch19-01-unsafe-rust.md
Outdated
increments the pointer in memory. We use this function to create the second | ||
slice. | ||
|
||
That's the general idea of unsafe functions, but let's talk about two other |
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 is an important point to make a vital point: that you can wrap up a use of unsafe in a safe function. I'd talk about that right here for this simple example.
src/ch19-01-unsafe-rust.md
Outdated
### `transmute` | ||
|
||
never ever. don't. stop. | ||
The `transmute` function is an unsafe function, but it should really be known | ||
as the most unsafe function, so unsafe that you shouldn't ever use it. What |
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 is a bit over the top :-)
It is true that once unions stabilize, we may be able to deprecate transmute
, but that day hasn't come yet.
@steveklabnik in #482, it looks like you agreed that 19.3 should possibly also have sections on:
There are also a few other topics in that issue that feel like they could fit into this chapter, but maybe not into 19.3 Advanced Traits. Maybe some new sections that cover these topics? 19.4 Advanced Functions
19.5 Advanced Types
Wdyt? If you like these ideas, could you add some content for these please? :) Is there anything else in the "unsure" section of #482 that could go in this chapter somehow? It didn't really look like it to me, but i'd like you to check... |
Yeah, I don't think so. |
I do and I will 😄 |
Oooohh what about putting |
💯
…On Wed, Apr 5, 2017 at 4:38 PM, Carol (Nichols || Goulding) < ***@***.***> wrote:
Oooohh what about putting ! for divergent functions in the Advanced Types
section?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#480 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AABsim6K5WHK4aiXbStA6bfCKI79JKUsks5rs_vTgaJpZM4MNiLi>
.
|
b03b91d
to
55d206d
Compare
let address = 0x012345; | ||
let r = address as *const i32; | ||
|
||
// bad things will happen if you try to use r |
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 comments in this example are overly scary, imo... why shouldn't they run this code? what bad things will happen? I added an unsafe block to test printing out whatever value is at this address, and it just didn't print anything. That's not that scary.
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.
undefined behavior, so you can't say!
probably a segfault. probably.
invalid. | ||
|
||
Furthermore, in these examples, you may have noticed something: we created both | ||
a `*const i32` and a `*mut i32` to the same memory location. With references, |
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.
Wait, what? Two literal 5
values have the same memory location?
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.
Hmmm I think you meant for these examples to be:
let mut num = 5;
let r1 = &num as *const i32;
let r2 = &mut num as *mut i32;
right?
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.
yes 😦
|
||
assert!(mid <= len); | ||
|
||
(slice::from_raw_parts_mut(ptr, mid), |
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.
What's the reason to have all the code in this function within an unsafe
block? Only these last two lines are calling unsafe functions, right?
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.
because that's how it's written in the stdlib
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.
Yeah, but the std lib defines this as a method on [T]
, and we're doing it as a function that only works for slices of i32
...
Put another way, is there any reason not to change this code to only have unsafe
around the last two lines? I'm worried that putting the whole function body inside an unsafe
block makes it confusing about what part is unsafe, and since the first few lines are safe, contradicts with our "make your unsafe code blocks as small as possible" advice in the intro.
@eddyb suggests that @nikomatsakis or @arielb1 should review the DST stuff to make sure I'm not messing it up. Dunno if either of you two have the time; it's pretty basic and small. starts here: https://github.com/rust-lang/book/pull/480/files#diff-bb9474e5f8cdb53e27ccc5295cecc17dR187 |
In case there's been a miscommunication here, when I wrote |
} | ||
} | ||
|
||
fn parse_context<'a>(context: Context<'a>) -> Result<(), &'a str> { |
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.
Whoa whoa whoa whoa whoa. Whoa. You just added lifetimes to the signature of parse_content
too, and didn't explain those. I don't think these are necessary, either, because the elision rules kick in, right? This code compiles for me:
struct Context<'c>(&'c str);
struct Parser<'p, 'c: 'p> {
context: &'p Context<'c>,
}
impl<'p, 'c> Parser<'p, 'c> {
fn parse(&self) -> Result<(), &'c str> {
Err(&self.context.0[1..])
}
}
fn parse_context(context: Context) -> Result<(), &str> {
Parser { context: &context }.parse()
}
Not gonna lie, I'm pretty confused by this example, but I think i'm starting to get it... we'll see when I push changes up.
One unresolved question though, why have parse_context
take ownership of context
? Just to remove the complexity of one more reference? It seems more idiomatic though. (although wait til you see what non idiomatic stuff i'm about to do to this example lol)
If I start back from the point at which parse_context
is introduced, I get a real fun error message, which could be why it's taking ownership instead, but the end result could potentially be easier to understand? not sure yet....
Code:
struct Context<'a>(&'a str);
struct Parser<'a> {
context: &'a Context<'a>,
}
impl<'a> Parser<'a> {
fn parse(&self) -> Result<(), &str> {
Err(&self.context.0[1..])
}
}
fn parse_context(context: &Context) -> Result<(), &str> {
Parser { context: &context }.parse()
}
Error:
error[E0106]: missing lifetime specifier
--> src/main.rs:13:51
|
13 | fn parse_context(context: &Context) -> Result<(), &str> {
| ^ expected lifetime parameter
|
= help: this function's return type contains a borrowed value, but the signature does not
say which one of `context`'s 2 elided lifetimes it is borrowed from
`ptr`. | ||
|
||
The assertion that the `mid` index is within the slice stays the same. Then, | ||
the `slice::from_raw_pts_mut` function does the reverse from the `as_mut_ptr` |
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.
typo: from_raw_parts_mut
.
creating valid pointers from the data this function has access to. | ||
|
||
In contrast, the use of `slice::from_raw_parts_mut` in Listing 19-7 would *not* | ||
be appropriate. This code takes an arbitrary memory location and creates a |
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.
not be appropriate? Do you mean "would be undefined"?
} | ||
``` | ||
|
||
This usage of `extern` does not require `unsafe` |
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.
document #[no_mangle]
?
|
||
This comes up extremely rarely in Rust code. It's an explicit goal of one of | ||
the members of the language design team that you should never need to write an | ||
explicit `for<'a>`, but you can if you'd like to. |
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.
"almost never". If you have something like for<'a> Fn(&'a str, &str) -> &'a str
, there is no way to get rid of the for
. But with trait-objects "anon" and "external" lifetimes are indeed the 99% case.
to add a type parameter to an existing trait, giving it a default will let us | ||
not break that code. | ||
|
||
## Fully qualified syntax |
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.
"associated item projection syntax".
<Type as Trait>::method(receiver, args); | ||
``` | ||
|
||
We only need the `Type as` part if it's ambiguous. And we only need the `<>` |
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.
There's also Type::item
and <Type>::item
, which do type-directed lookup.
be appropriate. This code takes an arbitrary memory location and creates a | ||
slice ten thousand items long: | ||
In contrast, the use of `slice::from_raw_parts_mut` in Listing 19-7 would | ||
result in undefined behavior. This code takes an arbitrary memory location and |
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.
Do we reference "what is undefined behavior"? I would prefer "would likely crash when the slice is used" (or to use the slice and say "would likely crash").
|
||
We've used traits to bound generic types before, but you can also use lifetimes | ||
for those bounds. For example, let's say we wanted to make a wrapper over | ||
references. Using no bounds at all gives an 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.
I think we're going to need a tiny bit more of a story around why we might want to make a wrapper over references-- for what purpose? How do we know we're in this use case?
|
||
Types with no references inside count as `'static`, and since `'static` is | ||
longer than any other lifetime, a type like `T: 'a` can be a type with no | ||
references. |
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'm confused about what this sentence is trying to say-- we've taken T: 'a
out of the example :-/ . Some possible interpretations:
- "a type like
T: 'a
can only be a type with no references" (not true tho, could be'static
references) - "a type like
T: 'static
can be a type with no references" (same not true) - "a type like
T: 'a
can't be a type with no references" (double negative is confusing, not sure this is true/relevant either because'a
is gone)
or did you mean something else?
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 think the sentence itself just... means that T: 'a
allows all types that have no references, along those with references pointing to data that lives longer than or exactly as long as 'a
.
* If you have `&'a X` or `&'a mut X`, then the default is `'a`. | ||
* If you have a single `T: 'a` clause, then the default is `'a`. | ||
* If you have multiple `T: 'a`-like clauses, then there is no default; you must | ||
be explicit. |
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 have many questions:
- In what cases would we have these different scenarios?
- What is
X
here, a generic type parameter? a pseudocode generic for some trait? - Where would we have single or multiple
T: 'a
clauses, in the trait? in the type implementing the trait? in either? - Are these lifetime elision rules in a way?
- Do they relate to the lifetime elision rules?
- How do we reason about this?
- How do we know if we should specify
Box<Foo + 'a>
orBox<Foo + 'static>
?
This says "for any lifetime `'a`." Think of it as similar to how a generic | ||
function says "for any type `T`." | ||
|
||
This comes up extremely rarely in Rust code. It's an explicit goal of one of |
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 think the nomicon explains this slightly better. Wdyt about cutting the HRTB section entirely...?
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.
Given that it's not actually in the current book and we've gotten like, 10 questions over the last two years... seems fine.
|
||
This code compiles just fine, but what about the lifetime here? With the | ||
elision rules, we don't actually *need* to write out the lifetime, but what if | ||
we did? |
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.
If we don't cut the HRTB section, I think we need to come up with an example that actually requires annotating the lifetimes....
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.
for<'a> fn(&'a str, &str) -> &'a str
.
|
||
## Higher ranked trait bounds | ||
TODO: cut this section or work up an example that requires lifetime annotation |
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.
fn needs_annotation<F>(f: F)
where F: for<'a> Fn(&'a str, &'a str) -> &'a str
{
println!("{}", f("hello", "world"));
}
``` | ||
|
||
This should look familiar; it's a trait with one method and an associated type. But | ||
there's one bit of syntax we haven't seen before: `RHS=Self`. What's up with that? |
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.
form, we aren't required to, and if we do not, the type of `RHS` will be the type | ||
of `Self`. | ||
|
||
Let's look at an example. Imagine we have two units, `Feet` and `Inches`. We can |
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 see london, i see france, i see @steveklabnik's american pants
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 thought i switched it on purpose because i wrote it first, then went "oh yeah let's use metric" :(
down with metasyntactic variables!
This is a little weird but it just might work?
fe4faeb
to
a969a88
Compare
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.
MERGINNGGGG
I feel like in the sprint to the end of the book, this is a bit light.
@aturon an extra review by you here would be nice too ❤️