Skip to content

Update Yokeable, including removing unnecessary methods #6784

@robofinch

Description

@robofinch

Brief summary

After some discussions about Yokeable (and Yoke) and after reading some related issues, there's a bunch I want to tackle. I could have a PR for each change, but it seems tedious, when some changes need two PR's (one non-breaking and one followup breaking PR). I intend to tackle five issues (#3507, #5368, #6057, this one, and #6786) in two PR's (non-breaking and breaking).

Edit: I'll only be doing the non-breaking one, but will leave the list of tasks for a breaking PR for future reference.

Immediate issue

The Yokeable::make and Yokeable::transform_mut methods each require that implementations are equivalent to a transmute, and equivalent to passing the result of a transmute to a callback, respectively. In other words, there is only one implementation which fulfills the safety contract for implementors, and they should not be trait methods.

We should instead provide helper free functions for these uses. I don't believe that any part of the implementation safety contracts for make or transform_mut would need to remain in Yokeable; the existing safety section on Yokeable should be sufficient, since the functions take Yokeables as input. make keeps its safety contract for users.

Initially discussed in #5368 (comment).

Related issues

Closing this issue would be a breaking change to Yokeable, removing two of its methods. It might be wise, then, to also make other breaking changes in the same update, and provide related functionality.

Another helper function

If we provide make as a free function, we could tackle #3507 at the same time, and provide something like the upgrade_output function discussed there (essentially, a safe version of make for when 'a is 'static).

GATs

We could close #6057 by removing the <'a> from Yokeable, and making its output type be Output<'a>.

Bikeshedding

If Yokeable::make and Yokeable::transform_mut are made standalone functions, it seems like a good idea to rename them to make_yokeable and transform_mut_yokeable, or similar.

I don't know if the function from #3507 should have "upgrade" in its name, when the safety contract says that if Y: Yokeable<'static>, then Y::Output is Y. Maybe cast_yokeable, since it should be a practically no-op type cast from Y::Output to Y (if the compiler notices the types are the same). Or, from the GAT version of a 'static Y::Output to Y.

Should a Yokeable::Output<'a> GAT still be called "Output", when it's no longer the output of feeding a lifetime to a Yokeable<'a>? There are probably better options, possibly WithLifetime<'a>.

Other decisions to make

I assume we want to publicly export the three helper functions for dealing with Yokeables. Do we want to export them in the root of yoke, or put them in a public module?

Should map_project (and friends) be renamed to map in the next major release?

Plan

I'll write a non-breaking PR (handles things I'm personally interested in).

Not sure if I'll be around for the breaking PR, but someone else might find the plan useful.

  • Bikeshedding for first PR

    • Name the three free functions (currently: make_yokeable, transform_mut_yokeable, and cast_yokeable)
    • Decide where to publicly export them from. For now I'm going to put them in a private yokeable_utils module and publicly reexport them from the yoke root.
  • Bikeshedding for second PR

    • Consider changing the name of Yokeable::Output<'a>. Currently: Yokeable::WithLifetime<'a>.
  • Non-breaking PR

  • Breaking PR

    • Remove Yokeable::make and Yokeable::transform_mut. Possibly, add checks to the derive macro that the free functions work for the type Yokeable is derived on.
    • Close Add return type to Yoke::with_mut and Yokeable::transform_mut #5368 by replacing Yoke::with_mut with the implementation of Yoke::with_mut_return, and remove the latter. (Maybe the change to Yoke::with_mut could be done in one step and be considered sufficiently nonbreaking. Adding a return type certainly supersedes the old behavior, but I think that breaks type inference in some silly edge cases. Maybe the slight breakage would be considered worth avoiding the temporary Yoke::with_mut_return, but I'm defaulting to putting it in the breaking PR.)
    • Close Move Yoke over to GATs #6057 by making Yokeable::Output a GAT. (Renamed to Yokeable::WithLifetime<'a> or whatever it's bikeshed to.)
    • Update anything in yoke affected by above changes, e.g. trait bounds, the derive macro, documentation.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions