Lifetimes in Structs that Contain both a piece of data and the data being referenced

Question

I am writing a little Cards Against Humanity clone for personal use and am using it as a chance to learn Rust. I currently have the following structs:

// game.rs
pub struct Game<'c> {
    deck: Deck,
    players: Vec<Player<'c>>,
    current_czar_index: usize,
}

// player.rs
pub struct Player<'c> {
    pub hand: Vec<&'c Card>,
    pub max_hand_size: usize,
    pub is_czar: bool,
    pub score: u8,
    pub name: String,
}

// deck.rs
struct DiscardPile {
    pub white: Mutex<Vec<usize>>,
    pub black: Mutex<Vec<usize>>,
}

pub struct Deck {
    white: Vec<Card>,
    black: Vec<Card>,
    pub used_sets: Vec<String>,
    discard_pile: DiscardPile,
}

// card.rs
pub struct Card {
    pub text: String,
    pub uuid: Uuid,
    pub pick: Option<u8>,
}

There are a few others (namely set which is used for reading in JSON files), but they aren't relevant.

I have a function in game.rs

    pub fn deal_hands(&self) -> Result<(), Box<dyn std::error::Error>> {
        for player in &mut self.players {
            while player.hand.len() < player.max_hand_size {
                player.add_card(self.deck.draw_white().unwrap())?;
            }
        }
        Ok(())
    }

that is giving me the following error:

error[E0495]: cannot infer an appropriate lifetime for autoref due to conflicting requirements
  --> src/game.rs:42:31
   |
42 |                 player.add_card(self.deck.draw_white().unwrap())?;
   |                                           ^^^^^^^^^^
   |

It specifically says that it is expecting &mut Player<'_> but found a lifetime of &mut Player<'c>.

HERE if you want to see the code firsthand.

How do I fix such a lifetime error, or do I need to rethink my architecture?

The answers that you have gotten are unfortunately misleading. Rethinking your architecture is most likely the way to go here. The major red flag is `Game` where `'c` is the lifetime of *something inside* `Game` (the deck) -- that's a pattern that rarely pans out the way you want in Rust. The linked question has more information. — trent, Oct 30 '20 at 15:10
In a program like this I don't see a good reason to use references at all -- why shouldn't the `Card`s be moved from the `Deck` to the `Player`s' `hand`s and eventually to the `DiscardPile`? Why do cards get "discarded" as soon as they're drawn? — trent, Oct 30 '20 at 15:21
That actually seems to be a better plan indeed. Initially I wanted to keep them in the deck so that it would be easier to refill the deck if the cards were exhausted, but your method seems easier in the long run. — Matthew Krohn, Oct 31 '20 at 22:55

score 0 · Answer 1 · answered Oct 30 '20 at 05:42

The architecture seems fine to me. You are probably just missing the passing of a lifetime parameter somewhere.

I'm not totally certain, but I believe issue may be that .draw_white() is returning Option<&Card> and it's not clear what the lifetime of the contained Card should be. I believe it is the case that anytime you return a borrowed value, you must attach a lifetime to the borrowed value. Otherwise, the borrow checker cannot tell how long the borrowed value will live.

Maybe try the following definition for .draw_white() instead

pub fn draw_white<'a>(&self) -> Option<&'a Card> {

You will then need to tweak deal_hands() to be something like the following. (Working on figuring out the right syntax, but I think it's something like this.)

    pub fn deal_hands(&self) -> Result<(), Box<dyn std::error::Error>> {
        for player<'c> in &mut self.players {
            while player.hand.len() < player.max_hand_size {
                player.add_card(self.deck.draw_white<'c>().unwrap())?;
            }
        }
        Ok(())
    }

No dice; since there is no parameter to "link" the lifetime of the `Option` to, it mismatches with the returned reference which has the same lifetime as `self`. — Matthew Krohn, Oct 30 '20 at 06:34

score -1 · Accepted Answer · answered Oct 30 '20 at 06:51

I think I got it sorted out. Luckily everything builds just fine after pushing through a handful of errors.

Original Error

Ok. Here is the full error I saw when I tried to build your crate.

error[E0495]: cannot infer an appropriate lifetime for autoref due to conflicting requirements
  --> src/game.rs:42:31
   |
42 |                 player.add_card(self.deck.draw_white().unwrap())?;
   |                                           ^^^^^^^^^^
   |
note: first, the lifetime cannot outlive the anonymous lifetime #1 defined on the method body at 39:2...
  --> src/game.rs:39:2
   |
39 |     pub fn deal_hands(&self) -> Result<(), Box<dyn std::error::Error>> {
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...so that reference does not outlive borrowed content
  --> src/game.rs:42:21
   |
42 |                 player.add_card(self.deck.draw_white().unwrap())?;
   |                                 ^^^^^^^^^
note: but, the lifetime must be valid for the lifetime `'c` as defined on the impl at 20:6...
  --> src/game.rs:20:6
   |
20 | impl<'c> Game<'c> {
   |      ^^
note: ...so that the types are compatible
  --> src/game.rs:42:12
   |
42 |                 player.add_card(self.deck.draw_white().unwrap())?;
   |                        ^^^^^^^^
   = note: expected `&mut Player<'_>`
              found `&mut Player<'c>`

It looks like the problem is actually with deal_hands(). Rather than allowing the lifetime of &self to be elided, you need to specify the lifetime as &'c mut self. (The mut is because you are mutating self.players.) I'm not totally sure why Rust doesn't elide the lifetime of self to 'c, but that's probably another topic.

Next Error

The next error I ran into was

error: captured variable cannot escape `FnMut` closure body                                          
   --> src/game.rs:106:17                                                                            
    |                                                                                                
106 |                     .map(|num| player.play_card_by_index(num as usize))                        
    |                              - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ returns a reference to a
 captured variable which escapes the closure body                                                    
    |                              |                                                                 
    |                              inferred to be a `FnMut` closure                                  
    |                                                                                                
    = note: `FnMut` closures only have access to their captured variables while they are executing...
    = note: ...therefore, they cannot allow references to captured variables to escape

For this one, the problem is actually not in the error message. You need to specify the lifetime of Card in the return value of .play_card_by_index() because otherwise it's not clear that the reference to Card actually outlives the closure. This means you need to adjust .play_card_by_index() to return &'c Card' instead of &Card`.

Final Error

There was a trivial error in your .get_player_names(). You just need to use .iter() instead of .into_iter() because the former will not take ownership of self. Also, you probably want to .clone() the player names too for a similar reason.

Full Diff

Here's the full diff

diff --git a/src/game.rs b/src/game.rs
index 573d09d..5ccbf6b 100644
--- a/src/game.rs
+++ b/src/game.rs
@@ -36,7 +36,7 @@ impl<'c> Game<'c> {
 
        /// Hands need to be filled on every turn, so this lets us fill everyones'
        /// hands at once.
-       pub fn deal_hands(&self) -> Result<(), Box<dyn std::error::Error>> {
+       pub fn deal_hands(&'c mut self) -> Result<(), Box<dyn std::error::Error>> {
                for player in &mut self.players {
                        while player.hand.len() < player.max_hand_size {
                                player.add_card(self.deck.draw_white().unwrap())?;
@@ -124,7 +124,10 @@ impl<'c> Game<'c> {
        }
 
        pub fn get_player_names(&self) -> Vec<String> {
-               self.players.into_iter().map(|player| player.name).collect()
+               self.players
+                       .iter()
+                       .map(|player| player.name.clone())
+                       .collect()
        }
 }
 
diff --git a/src/player.rs b/src/player.rs
index 4bd6848..9f95373 100644
--- a/src/player.rs
+++ b/src/player.rs
@@ -46,7 +46,7 @@ impl<'c> Player<'c> {
 
        /// Used in the TUI, we can play a card by index and get the reference
        /// to the card we played
-       pub fn play_card_by_index(&mut self, index: usize) -> & Card {
+       pub fn play_card_by_index(&mut self, index: usize) -> &'c Card {
                self.hand.remove(index)
        }
 
@@ -56,7 +56,7 @@ impl<'c> Player<'c> {
        /// instead of making the card on the client and sending the full card info,
        /// we can just send the UUID[s] of the cards that were played with some
        /// data about the order they were played in.
-       pub fn play_card_by_uuid(&mut self, uuid: Uuid) -> Option<& Card> {
+       pub fn play_card_by_uuid(&mut self, uuid: Uuid) -> Option<&Card> {
                // TODO: Find a better way to do this
                let mut index_of_card: usize = self.hand.len() + 1;
                for (index, card) in self.hand.iter().enumerate() {

Good Luck!

This looks like a fun project. I hope you continue to build it in Rust!

This all did the trick; I had just stumbled upon the `into_iter` error myself as I had tried some explicit lifetimes and wrapping `players` in a mutex. — Matthew Krohn, Oct 30 '20 at 07:00
Nevermind, only a temporary fix as my attempt to call `deal_hands` before passing my `Game` instance into Rocket causes some issues related to the mutable borrow living longer than it should (specifically for the lifetime `'c`). Will let my brain idle on it for a while before I determine whether I need to open another question. — Matthew Krohn, Oct 30 '20 at 07:25
`&'c mut self` (where `'c` is a parameter of `Self`) is never the solution. This (bad) advice always seems to pop up because it can make trivial examples compile, but at the expense of making `self` self-borrowing, so it can never be used again -- this is never what anyone wants, and writing it is a sure sign that you don't yet understand lifetimes. — trent, Oct 30 '20 at 15:06
As someone who does understand lifetimes a bit better, what is a better solution here, trentctl? In particular, I wasn't sure what to make of "note: first, the lifetime cannot outlive the anonymous lifetime #1 defined on the method body at 39:2...". — chckyn, Oct 30 '20 at 15:47
Self-referential structs are inherently immobile and immutable, because if you can move or mutate the struct you can invalidate the reference and cause undefined behavior. That said, Rust is actually fairly lenient: you can create self-referential structs, as long as you don't try to mutate or move them, and you can mutate structs as long as you don't try to make them self-referencing. Adding `'c` to `&mut self` moves the error from "creation time" to "use time" but it does not allow you to actually create and mutate a self-referencing struct, which is not possible in safe code. — trent, Oct 30 '20 at 19:06
It's sometimes possible to work around the inherent unsoundness using safe code and a crate like `owning_ref` or `ouroboros`, like the linked question mentions. These crates work by only allowing accesses that don't permit unsound mutation, so you have to be prepared to rework your data structures -- it's not possible to just fix up the lifetimes. It's possible one of these approaches would work for OP, but the usual advice is the best: don't create self-referential structs in the first place. — trent, Oct 30 '20 at 19:16
Thank you providing for such a thorough explanation. Just when I thought I handle on lifetimes, I realized I was totally wrong! The linked question has definitely aided my understanding as well. — chckyn, Oct 31 '20 at 04:34