NV_low_latency2 Impossible to use timings array after mutably borrowing in GetLatencyMarkerInfoNV

[MarijnS95]

CC @repi @VZout, in case you run into this while incorporating the updated extension bindings in your code. This happens after #816 was merged.

The timings() builder function on GetLatencyMarkerInfoNV mutably borrows a slice of LatencyTimingsFrameReportNV, which itself contains pointers and a lifetime. Rust seems to assume

#[test]
fn lifetime() {
    let mut timings = vec![vk::LatencyTimingsFrameReportNV::default()];
    let info = vk::GetLatencyMarkerInfoNV::default().timings(&mut timings);
    drop(info); // Lifetime wouldn't extend beyond this point if info is no longer used
    dbg!(timings);
}

Results in:

error[E0505]: cannot move out of `timings` because it is borrowed
  --> ash/src/extensions/nv/low_latency2.rs:93:5
   |
90 |     let mut timings = vec![vk::LatencyTimingsFrameReportNV::default()];
   |         ----------- binding `timings` declared here
91 |     let info = vk::GetLatencyMarkerInfoNV::default().timings(&mut timings);
   |                                                              ------------ borrow of `timings` occurs here
92 |     drop(info); // Lifetime wouldn't extend beyond this point if info is no longer used
93 |     dbg!(timings);
   |     ^^^^^^^^^^^^^
   |     |
   |     move out of `timings` occurs here
   |     borrow later used here
   |
   = note: this error originates in the macro `dbg` (in Nightly builds, run with -Z macro-backtrace for more info)

(EDIT: Instead of drop(info);, we could also wrap let info in a new lifetime scope {})

More context: #815 (comment)

Originally posted by @MarijnS95 in #816 (comment)

[Ralith]

This definition for fn timings causes the example test to build:

pub fn timings<'b: 'a>(mut self, timings: &'a mut [LatencyTimingsFrameReportNV<'b>]) -> Self {
    self.timing_count = timings.len() as _;
    self.p_timings = timings.as_mut_ptr().cast();
    self
}

There are two changes: introducing the longer lifetime 'b for LatencyTimingsFrameReportNV, and cast()ing the resulting raw pointer back to ...<'a> to match the struct field. I'm pretty sure that's sound, because 'b: 'a, but I wonder if this indicates that we've got incorrect variance somewhere...

[MarijnS95]

Yeah, separating the lifetime of the slice from the lifetime of the nested borrow in LatencyTimingsFrameReportNV seems to work, and makes sense. It also correctly causes the lifetime of timings to restrict the lifetime of info as expected, i.e. the following snippet correctly does not compile:

    let mut info = vk::GetLatencyMarkerInfoNV::default();
    {
        let mut timings = vec![vk::LatencyTimingsFrameReportNV::default()];
        info = info.timings(&mut timings);
    }
    dbg!(&info);

EDIT: Neither am I allowed to interact with timings ((im)mutably) while info is live (as expected).

That compiler warning suggesting the cast the other way around barely makes any sense, however...

[Ralith]

Yeah, the cast is required because *mut T is invariant in T (https://doc.rust-lang.org/nomicon/subtyping.html). If we used *const T instead (e.g. playground) then no cast is necessary, but that feels wrong because Vulkan is, in fact, writing through this pointer. Raw pointer types are at best a strong suggestion, so maybe I'm reading into this too much.

That compiler warning suggesting the cast the other way around barely makes any sense, however...

Sorry, what warning?

[Ralith]

Incidentally, I think we can omit the explicit lifetime name, since &'a [T<'b>] implies 'b: 'a on its own, so in turn we can elide 'b entirely, as I'd originally hoped: fn timings(mut self, timings: &'a mut [LatencyTimingsFrameReportNV<'_>]). That should make the generator tweaks easier.

[Ralith]

With this definition:

pub fn timings(mut self, timings: &'a mut [LatencyTimingsFrameReportNV<'_>]) -> Self {

Correct use compiles fine, and I get sensible errors for both incorrect use patterns:

timings ref-after-free:

error[E0597]: `timings` does not live long enough
  --> ash/src/extensions/nv/low_latency2.rs:94:29
   |
93 |         let mut timings = vec![vk::LatencyTimingsFrameReportNV::default()];
   |             ----------- binding `timings` declared here
94 |         info = info.timings(&mut timings);
   |                             ^^^^^^^^^^^^ borrowed value does not live long enough
95 |     }
   |     - `timings` dropped here while still borrowed
96 |     _ = info;
   |         ---- borrow later used here

Illegal aliasing:

error[E0502]: cannot borrow `timings` as immutable because it is also borrowed as mutable
  --> ash/src/extensions/nv/low_latency2.rs:93:10
   |
92 |     let info = vk::GetLatencyMarkerInfoNV::default().timings(&mut timings);
   |                                                              ------------ mutable borrow occurs here
93 |     dbg!(&timings);
   |          ^^^^^^^^ immutable borrow occurs here
94 |     _ = info;
   |         ---- mutable borrow later used here

[MarijnS95]

Sorry, what warning?

The error when not using .cast() suggesting that self needs a larger lifetime scope than timings ('a: 'b) while the inverse appears to be true:

1. timings can and must live at least as long as self, for the p_timings reference to remain valid;
2. self cannot live as long as timings, if we wish to (im)mutably use timings after it was temporarily mutably borrowed inside self (to be used inside the get get_latency_timings() call).

[MarijnS95]

That should make the generator tweaks easier.

Do you want to tackle this, or should I?