[RFC] demo: Replace 'static with explicit lifetime 'a

[Patiga]

Problem I want to solve

Currently, you can only pass 'static readers/writers into the demo readers/writers.
This works for Files, but not for writing into Vec<u8>.
As we require io::Seek for both reader and writer, we would not write into Vec<u8> directly, but a Cursor<&mut Vec<u8>> should work.

Why RFC

I'm not sure about the implications of this change.
Introducing a lifetime typically makes using it more difficult.
This is apparent with the lifetime issues that arose.

Issues

New lifetime issues which I wasn't able to solve, I guess I disagree with the compiler on some level.

1. Weird lifetime issue in tools/demo_read_write.
2. Lifetime issue in tools/teehistorian2demo, where I tried to actually write into a Cursor<Vec<u8>>. (I also tried Cursor<&mut Vec<u8>> and that didn't work either.

[heinrich5991]

Suggested change

	data: Box<dyn SeekableRead<'a>>,
	data: Box<dyn SeekableRead + 'a>,

I believe changing the trait itself isn't necessary, the + 'a here should suffice.

[Patiga]

Interesting. Could you elaborate how this solved the weird lifetime issue in tools/demo_read_write?
(I unfortunately already squashed the commits. From memory: The writer was initialized with references from the reader. Then the borrow checker complained when the writer was used later)

[heinrich5991]

There are a couple of ways you can recover squashed commits:

• In your git reflog.
• In this pull request, ctrl-f "force-pushed".
• Using GitHub events if it's only on a remote branch: https://neodyme.io/en/blog/github_secrets/.

I couldn't quickly see why the code wasn't accepted before. SeekableRead<'a> is different from SeekableRead + 'a because the compiler cannot assume that a type also implements SeekableRead<'b> if it implements SeekableRead<'a> and 'b is shorter than 'a. But of course every type that is SeekableRead + 'a is also SeekableRead + 'b.

[Patiga]

Neat tool :)
The lifetime issue is in 1703cab

Compiler error:

error[E0597]: `reader` does not live long enough
  --> tools/src/bin/demo_read_write.rs:73:9
   |
70 |     let mut reader = ddnet::DemoReader::<DDNet>::new(input_file, &mut warn::Log)?;
   |         ---------- binding `reader` declared here
...
73 |         reader.inner().net_version(),
   |         ^^^^^^ borrowed value does not live long enough
...
95 | }
   | -
   | |
   | `reader` dropped here while still borrowed
   | borrow might be used here, when `reader` is dropped and runs the destructor for type `DemoReader<'_, Protocol>`

error[E0502]: cannot borrow `reader` as mutable because it is also borrowed as immutable
  --> tools/src/bin/demo_read_write.rs:83:29
   |
73 |         reader.inner().net_version(),
   |         ------ immutable borrow occurs here
...
83 |     while let Some(chunk) = reader.next_chunk(&mut warn::Log)? {
   |                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here
...
95 | }
   | - immutable borrow might be used here, when `reader` is dropped and runs the destructor for type `DemoReader<'_, Protocol>`

[Patiga]

The only change here is that the lifetime is also in the trait.
The lifetime of the trait is the same as the file parameter. The other parameters, which cause the issue here, shouldn't be taken into account afaik

[Patiga]

The suggested changes solved all the issues 👍
This change did not require any changes in tools.
It should only lessen the restriction on the parameter (and make the code a little more complicated).

[heinrich5991]

Thanks!