Skip to content

Strengthen lifetime tests to exercise non-trivial lifetime relationships in lifetime_conversion and lifetime_duration modules #62

Description

@sourcery-ai

Summary

The current tests for lifetime-related derives/macros (notably in the lifetime_conversion and lifetime_duration modules) only use values that share the same lifetime. As a result, the tests do not actually exercise or validate the intended lifetime relationships encoded in the where-clauses (e.g., 'pass: 't, 'fail: 'e, 't: 'o, 'e: 'f, 'input: 't / 'input: 'e).

This issue tracks improving those tests to construct references with different lifetimes (including nested scopes and separate stack frames) so that:

  • Over-constrained derives (that accidentally extend lifetimes too far) are caught by the compiler, and
  • Under-constrained derives (that fail to enforce required lifetime relationships) are also caught.

Problem

In the existing lifetime_conversion and lifetime_duration tests:

  • All &i32 values used in assertions are created within a single scope per test.
  • This means all references effectively have the same lifetime.
  • Constraints like 'pass: 't, 'fail: 'e, 't: 'o, 'e: 'f, and 'input: 't/'input: 'e are only exercised in the degenerate “all lifetimes are equal” situation.

Because of that, the tests cannot:

  • Detect if a derive accidentally allows a reference to outlive its source, or
  • Verify that a derive correctly encodes relationships such as one lifetime strictly outliving another.

Desired Behavior

Tests should:

  1. Create references (okval, errval, etc.) with different, nested lifetimes so that one clearly outlives the other.
  2. Use helper functions in a separate stack frame to model realistic uses and force the compiler to check the lifetime constraints as encoded by the macros/derives.
  3. Wire these helpers to the actual conversion/result types used by the macros (e.g., Try / Try_ConvertResult or custom enums), not just trivial tuples.
  4. Store extracted references in outer-scope variables so the compiler detects any lifetime over-/under-extension.

Suggested Approach

Below is a sketch of the kind of tests that should be added to lifetime_conversion (and analogously to lifetime_duration). This is intentionally written in terms of simple tuples but should be adapted to use the real types in this crate.

mod lifetime_conversion {
    use super::*;

    // Helper functions to exercise lifetime relationships across stack frames.
    // Replace the return types with the real conversion/derive types used in
    // this module (e.g., Try / Try_ConvertResult / custom enums).
    fn convert_ok_err<'t, 'e>(ok: &'t i32, err: &'e i32) -> (&'t i32, &'e i32) {
        (ok, err)
    }

    fn convert_err_ok<'t, 'e>(ok: &'t i32, err: &'e i32) -> (&'e i32, &'t i32) {
        (err, ok)
    }

    #[test]
    fn ok_outlives_err_nested_scope() {
        // 'outer: lifetime of okval
        let ok_storage = 1;
        let okval = &ok_storage;

        // Bind in outer scope so any returned references must be valid for 'outer.
        let (out_ok, out_err): (&i32, &i32);

        {
            // 'inner: lifetime of errval, strictly shorter than 'outer
            let err_storage = 2;
            let errval = &err_storage;

            // This should reflect constraints like 't: 'e or 'input: 't, depending
            // on the actual conversion types.
            let (ok_res, err_res) = convert_ok_err(okval, errval);

            assert_eq!(*ok_res, 1);
            assert_eq!(*err_res, 2);

            out_ok = ok_res;
            out_err = err_res;
        }

        // out_ok must still be valid here (okval outlives errval).
        assert_eq!(*out_ok, 1);

        // out_err is only sound here if the conversion did NOT extend the
        // lifetime of errval beyond its scope. Once wired to the real types,
        // the compiler should reject any incorrect derive that over-constrains.
        assert_eq!(*out_err, 2);
    }

    #[test]
    fn err_outlives_ok_nested_scope() {
        // 'outer: lifetime of errval
        let err_storage = 10;
        let errval = &err_storage;

        let (out_ok, out_err): (&i32, &i32);

        {
            // 'inner: lifetime of okval, strictly shorter than 'outer
            let ok_storage = 20;
            let okval = &ok_storage;

            // Exercises the opposite direction of the lifetime relationship.
            let (err_res, ok_res) = convert_err_ok(okval, errval);

            assert_eq!(*ok_res, 20);
            assert_eq!(*err_res, 10);

            out_ok = ok_res;
            out_err = err_res;
        }

        // out_err must still be valid here (errval outlives okval).
        assert_eq!(*out_err, 10);

        // When wired to the real conversion types, any under-constrained derive
        // that fails to enforce 'fail: 'e-like relationships should be rejected.
        assert_eq!(*out_ok, 20);
    }
}
``

### Integration Steps
To fully exercise the actual macros/derives instead of simple tuples:
1. **Replace helper return types**
   - Change `convert_ok_err` / `convert_err_ok` to return the real conversion/result types used in `lifetime_conversion` (e.g., a type implementing `Try` or a custom enum with the `derive` under test).
   - Ensure these types preserve the `'t` and `'e` lifetimes in their generic parameters as intended.

2. **Construct real conversion values inside helpers**
   - In `convert_ok_err` and `convert_err_ok`, construct the actual conversion/derive types using `ok` and `err` and return those.

3. **Adjust assertions**
   - Instead of directly destructuring tuples, pattern-match on the real types or use accessor methods to extract the `ok`/`err` references.
   - Store the extracted references in `out_ok` / `out_err` (bound in the outer scope) so the compiler is forced to verify the lifetime constraints.

4. **Mirror for `lifetime_duration`**
   - Add analogous tests in the `lifetime_duration` module that:
     - Create `okval` and `errval` (or analogous values) in nested scopes with differing lifetimes.
     - Use helper functions in separate stack frames.
     - Use the real duration-related types/macros being tested there.

## Action Items
- [ ] Identify all lifetime-sensitive test modules (at least `lifetime_conversion` and `lifetime_duration`).
- [ ] Add nested-scope tests for the case where the `ok` value outlives the `err` value.
- [ ] Add nested-scope tests for the case where the `err` value outlives the `ok` value.
- [ ] Implement helper functions that:
  - Take references with explicit lifetime parameters (`'t`, `'e`, etc.).
  - Return the actual macro/derive-generated types that encode those lifetimes.
- [ ] Update assertions to operate on the real conversion/duration types, not tuples.
- [ ] Ensure the compiler catches incorrect lifetime constraints by verifying that the tests fail to compile (or require `compile_fail` tests) if derives are over- or under-constrained.

By implementing these changes, the test suite will validate the true lifetime behavior encoded by the macros/derives instead of only the trivial case where all references share the same lifetime.

---

I created this issue for @MusicalNinjaDad from https://github.com/MusicalNinjaDad/try_v2/pull/61#discussion_r3068136567.

<details>
<summary>Tips and commands</summary>

#### Getting Help

- [Contact our support team](mailto:support@sourcery.ai) for questions or feedback.
- Visit our [documentation](https://docs.sourcery.ai) for detailed guides and information.
- Keep in touch with the Sourcery team by following us on [X/Twitter](https://x.com/SourceryAI), [LinkedIn](https://www.linkedin.com/company/sourcery-ai/) or [GitHub](https://github.com/sourcery-ai).

</details>

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions