rental future compat warning

[est31]

If I clone rental and run cargo +beta test on it, I get t he following warning:

> cargo +beta check
warning: using `procedural-masquerade` crate
   --> src/lib.rs:94:8
    |
94  |         enum ProceduralMasqueradeDummyType {
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
117 |     define_rental_traits!(32);
    |     -------------------------- in this macro invocation
    |
    = note: `#[warn(proc_macro_back_compat)]` on by default
    = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
    = note: for more information, see issue #83125 <https:/rust-lang/rust/issues/83125>
    = note: The `procedural-masquerade` crate has been unnecessary since Rust 1.30.0. Versions of this crate below 0.1.7 will eventually stop compiling.
    = note: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

This also occurs in crates that use rental, e.g. one of glium's examples.

cc #83125 (tracking issue)

The warning itself has been added by PR #83168, whose justification was that the procedural-masquerade crate sees little use anyways. And indeed, on crates.io it "only" has 27,419 recent downloads. Rental on the other hand has 193,341 recent downloads, or almost 10 times as many. Furthermore, you can fix the warning on your side by just removing use of that crate as it's become redundant. Rental on the other hand is not redundant, so your only option is to switch to an alternative, which might be expensive.

I would approach fixing of this issue with the following steps:

• Fixing the issue in rental, releasing a new version with the fix. This help from the maintainer. cc @jpernst
• Improvement of the error message to include rental: Only apply procedural-masquerade back-compat hack to rental #85387

cc @Aaron1011 @petrochenkov

[glandium]

This causes problems on crates that use rental too, which is the case of Firefox (and since things are vendored in Firefox, anyone building Firefox doesn't count towards the rental download counts).

[est31]

I've only glossed over the message that rental is no longer maintained:

Rental can be considered "frozen" in its current state, and any further development will need to take place under a fork for whoever wishes to do so.

So I guess instead of attempting to contact the author, we should maybe follow the wishes of the maintainer, create a fork, and put any fixes/changes there.

There are also a bunch of rental alternatives:

• selfstack
• self_cell
• ouroboros

I think even if the intent is to not continue development, a drop-in replacement of rental would be helpful as it's easier for crates to move to it.

[Stargateur]

as far as I remember I tried alternative of rental but no one seem to do what I needed.

[Aaron1011]

I don't think forking rental will allow us to remove the underlying pretty-print hack in any reasonable amount of time. crates.io shows hundreds/thousands of daily downloads - all of those users will need to switch to the fork (directly, or by updating whatever crate pulls it in as a transitive dependency). Considering all of the difficulties that we've had getting crates like syn and proc-macro-hack bumped (which just requires cargo update), this doesn't seem feasible.

An alternative would be to try to restore the 'nicer' pretty-printing behavior for all crates. That is, we would try to do a better job about keeping spacing information in the TokenStream, without relying on pretty-printing a captured AST node. Unfortunately, this information cannot survive 'deep recollection' from a proc-macro, since the necessary information is not exposed through the proc-macro API. However, I think we could make it work in the absence of recollection (i.e. better best-effort pretty printing), which should be good enough to prevent the regressions.

[est31]

idk proc-macro-hack and syn seem to have far more users than rental. It's doable, also given that users should switch to something that has at least some basic level of maintenance anyways.

[Aaron1011]

idk proc-macro-hack and syn seem to have far more users than rental.

We still haven't completed the migration for proc-macro-hack - a recent attempt showed almost 100 regressions (#81863). Note that the the PR mentions time-macros-impl, but that's because the issue with proc-macro-hack ended up producing an error when compiling the time-macros-impl.

I can do a Crater run that just disables things from rental, to see how the impact compares to proc-macro-hack. However, I think 'all versions of this crate stop compiling entirely with newer Rust versions' is a pretty severe violation of the spirit of our backwards-incompatibility policy (as opposed to 'you must run cargo update -p <affected_crate>), even if the breakage could be considered a bugfix. Considering that this isn't a soundness bug, I think we should consider all other possible solutions (including getting the maintainer to make 'one last release') before using this option.

[Aaron1011]

Unfortunately, rental is checking for a trailing comma in the pretty-printed input: https:/jpernst/rental/blob/213671ab3aab3452efd7e2290c6bb714ee327014/rental-impl/src/lib.rs#L29

However, that comma does not actually exist in the original input: https:/jpernst/rental/blob/213671ab3aab3452efd7e2290c6bb714ee327014/src/lib.rs#L94-L96

A pretty-printing change along can't fix this - while it would be fine for us to improve pretty-printing for all crates, we should not be synthesizing fake commas that aren't present in the original macro input.

However, this does suggest an combination of approaches we could take to solving this problem:

1. Change the pretty-printing behavior of all TokenStreams to avoid inserting 'extra' spaces in more places. This will make the pretty-printed output look nicer for all crates, and makes the output look closer to what rental expects.
2. Get the maintainer to make a point release of rental, which just adds trailing commas to the Input = (0, stringify!($max_arity)).0 variants (e.g. Input = (0, stringify!($max_arity)).0,). This would avoid the need to make any substantive changes to the macro code (which I could see the maintainer opposing on the grounds that it might introduce additional issues for an unmaintained crate).

I've locally verified that the combination of these two changes is enough to keep rental compiling with the pretty-print hack removed.

[Aaron1011]

cc @jpernst - I would normally post this on the rental issue tracker, but since the repository is archived, I can't open any new issues.

TL;DR - it would be extremely helpful if you could make one final point release of rental, adding trailing commas after all of the Input = variants. This would be an enormous help to the Rust ecosystem, and would be the only change needed to keep rental working. I'd be happy to open a pull request containing the exact 4-line change needed, after which the repository can be marked 'Archived' again.

Background:

rental-impl contains code which stringifies (via TokenStream::to_string()) and parses the input TokenStream: https:/jpernst/rental/blob/213671ab3aab3452efd7e2290c6bb714ee327014/rental-impl/src/lib.rs#L18-L37

Historically, the Rust compiler did not always preserve the exact input to a proc-macro (in this case, the item annotated with #[derive(__rental_traits)]). Recently, work has been done to ensure that we preserve the exact tokens written by the user, passing them along to the proc-macro without any changes.

The problem
When Rust started preserving the original TokenStream, the output of TokenStream::to_string() (the 'pretty-printed' output) changed in some cases. Most notably, we no longer insert trailing commas that did not exist in the original input. In the case of rental, the definition of ProceduralMasqueradeDummyType does not contain a trailing comma after Input = (0, stringify!($max_arity)).0. However, rental-impl expects to see a trailing comma - specifically, it requires that that pretty-printed output look like Input = (0, stringify!(32)).0,.

In order to avoid breaking a large number of crates in the ecosystem, the Rust compiler added a backwards-compatibility hack, which keeps the original pretty-printed output for crates that use a type named ProceduralMasqueradeDummyType. However, this behavior is a huge hack - eventually, we'd like to have the compiler invoke all proc-macros in the same way, regardless of whether or not a type happens to be named ProceduralMasqueradeDummyType

As it stands now, the Rust compiler cannot remove this hack without making all versions of rental unuseable (rental-impl will start panicking, stopping compilation). Even though rental is unmaintained, I'd like to avoid inflicting this kind of breakage on rental's users (and the wider Rust ecosystem).

The solution

I believe that the simplest solution is to make a new, final release of ruffle. The only change in this release would be adding a trailing comma to the following lines:

• https:/jpernst/rental/blob/213671ab3aab3452efd7e2290c6bb714ee327014/src/lib.rs#L95
• https:/jpernst/rental/blob/213671ab3aab3452efd7e2290c6bb714ee327014/src/lib.rs#L241
• https:/jpernst/rental/blob/213671ab3aab3452efd7e2290c6bb714ee327014/src/lib.rs#L259
• https:/jpernst/rental/blob/213671ab3aab3452efd7e2290c6bb714ee327014/src/lib.rs#L277

I've verified that this is sufficient to allow Rust to remove the hack, allowing rental users to continue using the crate by running cargo update -p rental.

I understand that the rental crate is no longer maintained or supported, and that this request may sound like a request for continued long-term maintanence of rental. However, this is a one-time request that will allow rental to keep compiling. Once Rust is able to remove the hack, the pretty-printed output seen by rental will not contain any 'synthetic' characters (e.g. trailing commas) that are not present in the original source. Any further issues would be Rust's responsibility to fix.

Thank you for your work creating rental!

[Aaron1011]

A new release of rental has been made that adds the necessary trailing commas. The next step is to adjust Rust's pretty-printing behavior to align with what rental expects - I've opened PR #84920 to do this.

[est31]

This is resolved now I think due to #84920 being merged?

[Stargateur]

To future reader, ouroborus can probably be use to replace rental