- 
                Notifications
    
You must be signed in to change notification settings  - Fork 1.9k
 
"borrow checker invariants" section of the "leveraging the type system" chapter #2867
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
base: main
Are you sure you want to change the base?
"borrow checker invariants" section of the "leveraging the type system" chapter #2867
Conversation
…n to lifetime connections
| 
           Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request.  | 
    
        
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/generalizing-ownership.md
          
            Show resolved
            Hide resolved
        
              
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/generalizing-ownership.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/single-use-values.md
          
            Show resolved
            Hide resolved
        
              
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/aliasing-xor-mutability.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/aliasing-xor-mutability.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/aliasing-xor-mutability.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/generalizing-ownership.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/generalizing-ownership.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/generalizing-ownership.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/aliasing-xor-mutability.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/lifetime-connections.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/lifetime-connections.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/lifetime-connections.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/lifetime-connections.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/aliasing-xor-mutability.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/single-use-values.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/single-use-values.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/single-use-values.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/single-use-values.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/single-use-values.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/generalizing-ownership.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/generalizing-ownership.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/generalizing-ownership.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/generalizing-ownership.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/aliasing-xor-mutability.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
      Co-authored-by: Dmitri Gribenko <[email protected]>
        
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/single-use-values.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/single-use-values.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
      | 
               | 
          ||
| Since then, users and developers of the language expanded the use of generics | ||
| to other areas of type-safe API design. | ||
| <!-- TODO: Reference how this was adopted --> | 
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.
Unresolved TODO.
        
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
      | do_something_with_transaction(transaction); | ||
| let assumed_the_transactions_happened_immediately = db.results(); // ❌🔨 | ||
| do_something_with_transaction(transaction); | ||
| // Works, as the lifetime of "transaction" as a reference ended above. | 
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 done yet.
I previously left a comment here:
I might be misunderstanding your intent behind this example, but it does not feel realistic realistic because the database didn't get a notification that the borrow has ended, and thus could not have started executing it. In other words, it is not clear what the DatabaseConnection type achieved by being locked out while the transaction was borrowed out.
Maybe you can fix it by wrapping the reference in a custom wrapper type which implements Drop? Ideally, that drop() would also print a message (like "executing query...") to make it clear to the audience that the database can indeed fill in the result list.
Here's my version of the example that incorporates the suggestion in a different way, without Drop (hopefully even simpler and shorter):
pub struct QueryResult;
pub struct DatabaseConnection { /* fields omitted */ }
impl DatabaseConnection {
    pub fn new() -> Self { Self {} }
    pub fn results(&self) -> &[QueryResult] {
        &[] // fake results
    }
}
pub struct Transaction<'a> {
    connection: &'a mut DatabaseConnection,
}
impl<'a> Transaction<'a> {
    pub fn new(connection: &'a mut DatabaseConnection) -> Self {
        Self { connection }
    }
    pub fn query(&mut self, _query: &str) {
      // Send the query over, but don't wait for results.
    }
    pub fn commit(self) {
      // Finish executing the transaction and retrieve the results.
    }
}
fn main() {
    let mut db = DatabaseConnection::new();
    // The transaction `tx` mutably borrows `db`.
    let mut tx = Transaction::new(&mut db);
    tx.query("SELECT * FROM users");
    // This won't compile because `db` is already mutably borrowed.
    let results = db.results(); // ❌🔨
    // The borrow of `db` ends when `tx` is consumed by `commit`.
    tx.commit();
    // Now it is possible to borrow `db` again.
    let results = db.results();
}        
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/phantomdata.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
      | - Motivation: We want to be able to "tag" structures with different type | ||
| parameters as a way to tell them apart or pass on lifetime information to | ||
| them. | 
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'd prefer that we start with one clear motivation. So tagged types sounds good. But why are we tagging them? Could make the example even more concrete?
Here's my suggestion. Say that we have an app with multiple newtype'd integers.
struct UserId(u64);
impl fmt::Display for UserId { ... }
impl FromStr for UserId { ... }
struct PostId(u64);
impl fmt::Display for UserId { ... }
impl FromStr for UserId { ... }
// Pretend we have 100s of newtypes like that and we don't like code duplication.This works, but we have to repeat impls for every newtype.
Next slide, our attempt at implementing a generic newtype'd integer for our app.
struct TaggedIntegerId<T> {
    value: u64,
}
struct UserTag;
type UserId = TaggedIntegerId<UserTag>;
struct PostTag;
type PostId = TaggedIntegerId<PostTag>;This is cool, but we get error[E0392]: type parameter T is never used.
From here you can connect to the narrative you already wrote.
WDYT?
(I'm not commenting on the rest of the slide because I'm expecting a major restructuring because of my suggestion.)
| user. `Stage<Start>` offers a lot more contextual detail than `StageStart`, | ||
| and implies there is shared behavior between stages. | ||
| 
               | 
          ||
| - Demonstrate: We can also capture lifetime parameters with `PhantomData` | 
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.
Could we split this onto a separate slide and also provide a more concrete motivation?
I think this is also a better example than the abstract one currently in lifetime-connections.md.
For example:
/// Direct FFI to a database library in C.
/// We got this API as is, we have no influence over it.
mod ffi {
use DatabaseHandle = u8; // maximum 255 databases open at the same time
fn database_open(name: *const c_char) -> DatabaseHandle;
// ... etc.
}
struct DatabaseConnection(DatabaseHandle);
struct Transaction<'a>(&mut DatabaseConnection);
impl DatabaseConnection {
  fn new_transaction<'a>(&'a mut self) -> Transaction<'a> {
    Transaction(&mut self)
  }
}While the Transaction exists, nothing else in Rust can disturb the database, good. But Transaction contains a reference to the database - 8 bytes on a 64-bit platform, whereas the database handle is just 1 byte. That's wasted memory, and an extra indirection. Why don't we simply copy the handle into the transaction? That would save 7 bytes, and avoid jumping through a pointer to access the integer handle value.
struct DatabaseConnection(DatabaseHandle);
struct Transaction<'a>(DatabaseHandle);Oh, that does not compile. PhantomData to the rescue.
| ```rust,editable,compile_fail | ||
| use std::marker::PhantomData; | ||
| 
               | 
          ||
| pub struct BorrowedButOwned<'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.
This example (StringOrigin, BorrowedButOwned) is abstract and a bit confusing. It's not immediately clear what real-world problem this code could solve. The names are generic, and the act of cloning a string to create a "borrowed" value feels counter-intuitive without a concrete motivation (even though that's exactly what would happen in a real-world application!)
I suggest to rewrite this slide to be the motivation and explanation of using PhantomData with generic lifetime parameters (base it upon my suggested struct Transaction(DatabaseHandle) example). Leave the phantomdata.md slide to be purely about using PhantomData with generic type parameters.
| 
           @tall-vase Thank you for the latest round of improvements! I've just completed a review, and my main feedback is focused on strengthening the narrative flow and making the examples more concrete and motivating for students. The core ideas are excellent, but we can improve the examples and the overall flow. I have tried to leave detailed, actionable comments, used suggested edits and provide code for new less abstract examples.  | 
    
Co-authored-by: Dmitri Gribenko <[email protected]>
Co-authored-by: Dmitri Gribenko <[email protected]>
        
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
      | The borrow checker, while added to enforce memory ownership, can be leveraged | ||
| model other problems and prevent API misuse. | 
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 borrow checker, while added to enforce memory ownership, can be leveraged | |
| model other problems and prevent API misuse. | |
| * The borrow checker was introduced in Rust to guarantee memory safety. | |
| * It can model other API contracts, turning logic bugs into compile-time errors. | 
Larger edit idea: Splitting the message into simpler, more scannable statements for the slide.
| 
               | 
          ||
| # Generalizing Ownership: Lifetimes Refresher | ||
| 
               | 
          ||
| The logic of the borrow checker, while modelled off "memory ownership", can be | 
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 logic of the borrow checker, while modelled off "memory ownership", can be | |
| The logic of the borrow checker, while modelled off memory ownership, can be | 
No need for scare quotes here.
        
          
                src/idiomatic/leveraging-the-type-system/borrow-checker-invariants/phantomdata-02-lifetimes.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
      | But this isn't the way database connections tend to work in practice. We can | ||
| have multiple connections. | 
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 makes it sound like there was something wrong with the previous example. There wasn't.
We merely haven't shown what was inside the database connection, that's all.
Adds materials on the "leveraging the type system/borrow checker invariants" subject.
I'm still calibrating what's expected subject-and-style wise, so do spell out things where I've drifted off mark.