Temporary lifetime testing strategy

[Mark-Simulacrum]

I wanted to drop a thread here around our experience with making changes to dbg! and the difficulty (I think still unfinished) of getting a consistent experience on the temporary lifetimes. @dianne was already involved so has some additional context, but I thought it'd be a useful point to keep in mind more broadly for this group.

The collection of PRs:

• std: avoid tearing dbg! prints #149869 -- this failed with 1.95 beta regression: "temporary value dropped while borrowed" #153850
• the fix (don't drop arguments' temporaries in dbg! #154074) then caused 1.96 beta regression: "lua does not live long enough" #154988 and 1.96 beta regression: "lua does not live long enough", take 2 #155902
• which we attempted to fix in don't leak internal temporaries from dbg!, even on false unwind paths #155915, but eventually gave up and just reverted the original change.

(I may have missed one or two attempts there).

I think this is still pending in #157576 which uses super let to implement it, but as that PR says the result is not great in terms of saying "yes that's doing what I expect".

$(let $bound);+;
$(super let _ = ($bound = $processed));+;

However, the main goal in writing this up is that I think we're missing some good way for us to test this. Most of our temporary lifetimes are inherently implicit, and describing what happens at what scope in a way that can be guaranteed constant across refactoring is hard. Writing tests for language features (e.g., let-else, if-let, ...) takes a good deal of work to remember each case (e.g., see https://github.com/rust-lang/rust/blob/main/tests/ui/let-else/let-else-temporary-lifetime.rs, https://github.com/rust-lang/rust/blob/main/tests/ui/lifetimes/temporary-lifetime-extension.rs). Regardless of the syntax to express (super let, some macros, etc.), I think it would be very useful to have some way to exhaustively cover the cases a macro author (at least) or ideally language feature author needs to concern themselves with.

I'm not sure exactly what form that should take. Maybe the right answer is a collection of macros in core that you need to can use and then assert against a template -- e.g., something like this hypothetical:

temporary_tests!(dbg!($arg1, $arg2));

would expand into something that at least exhaustively enumerates cases and can be snapshot-tested across changes to the internals of the dbg! macro. In principle, that same strategy could be used for a language feature, e.g., something like this?

temporary_tests!(let $pat = $arg1 else { $arg2 });

I've not thought too deeply on the specific semantics, but I suspect the right shape is that each separate case would get its own test case produced from the template. It can then be compared (via execution / Drop prints / something else?) against canonical examples (e.g., "this language feature should behave exactly the same as this alternative feature) or the previous version of a library (snapshot testing) to prevent repeated regressions like the ones we worked through with dbg!.

Cutting an issue per some short discussion with @traviscross offline to keep track of this conversation.

[traviscross]

Thanks @Mark-Simulacrum for writing this up.

cc @dianne

[traviscross]

See also:

• Stabilize if let guards (feature(if_let_guard)) #141295 (comment)

[dianne]

Testing expression/statement/etc. templates for equivalence sounds right to me. In addition to comparing runtime drop order (and drop order relative to execution order), we may also want to have tests where both cases fail to borrow-check. I don't have a comprehensive answer for what contexts to put templates in and what to fill them with, though. There's a bunch of obvious things we could test, and I have some subtler ones in mind, but I haven't managed to think of a better strategy yet than adding more tests as we encounter regressions. Maybe there's a minimal set of tests we'd need to demonstrate equivalence? But the possibility space is large enough that I'm not really sure how I'd find it at this point.

About the drop order issue with if let guards, that's an interesting case: as I recall, there were actually tests for drop order equivalence between a match with an if let guard and a match with if let in an arm body. It just didn't compare the drop order of the guard bindings/temporaries with the arm's pattern's bindings. Maybe if they were written as templates it'd've been easier to notice the omission, at least? But we wouldn't be able to make the template fully general, since a match with an if let guard isn't fully equivalent to a match with an if let in an arm body. I expect automation would make it significantly easier, but it'd still be up to us to judge whether we're doing sufficient testing for novel language features and manually write tests if needed.

cc @steffahn maybe? I recall you expressing interest in this at RustWeek.

[Mark-Simulacrum]

it'd still be up to us to judge whether we're doing sufficient testing for novel language features and manually write tests if needed.

My mental model (quite possibly an incorrect model) is that it should be possible for the compiler to guarantee such tests are exhaustive. I think if we have the grammar for the relevant construct, every site that either takes a pattern or an expression should be covered, and ideally we ought to be able to mechanically check that.

I think that approach ought to at least allow exhaustive coverage of a feature (i.e. should prevent regressions due to unintentional changes in behavior), but it probably doesn't allow comparisons between feature that aren't 1:1 equivalent.

[dianne]

it probably doesn't allow comparisons between feature that aren't 1:1 equivalent.

Right, I was referring to the discussion in #141295 about a drop order quirk that as far as I know had been in if let guards from the start and nearly was included in its stabilization. Since it was a novel feature, 1:1 comparison wasn't an option, but comparing to other features was still helpful shorthand to express the properties we wanted from it.

If we're not comparing to existing features as shorthand, the thing I'd imagine testing against is a specification of how a feature's scopes should work. To automate that, I guess we'd need a specification language? That comes with extra maintenance burden, of course (both in tooling and in understanding the tests).

I think that approach ought to at least allow exhaustive coverage of a feature (i.e. should prevent regressions due to unintentional changes in behavior)

Assuming we can generate a comprehensive enough test suite, I'd hope we'd be able to catch regressions that way, yeah. Though I'm not sure I follow regarding mechanically checking coverage.