rust
rust copied to clipboard
rust-lang
Remove proc-macro back-compat hack for rental
This leaves the hack in place for allsorts-rental - it still relies on this backc-ompat hack to compile, and some crates on crates.io are still using it.
We've been emitting future-incompat report warnings for rental for a while now, and fixing a broken build only requires bumping rental to the latest point release.
Background (in reverse chronological order):
- the hack was restricted to the
rentalandallsorts-rentalcrates back in PR #94063 - the
proc_macro_back_compatwas extended to any use of aprocedural-masqueradecrate in PR #83168 - The
proc_macro_back_comatlint was introduced in PR #83127 (rollup PR #83149)
:hourglass: Trying commit 0c28782ac512786d2b446c9b86d07a72315bfa75 with merge 2f3c33065a16f1030e8d2f90241df3b4bea01e77...
:sunny: Try build successful - checks-actions
Build commit: 2f3c33065a16f1030e8d2f90241df3b4bea01e77 (2f3c33065a16f1030e8d2f90241df3b4bea01e77)
:ok_hand: Experiment pr-106060 created and queued.
:robot: Automatically detected try build 2f3c33065a16f1030e8d2f90241df3b4bea01e77
:mag: You can check out the queue and this experiment's details.
:information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more
:construction: Experiment pr-106060 is now running
:information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more
:rotating_light: Report generation of pr-106060 failed: Failed to upload to "try#2f3c33065a16f1030e8d2f90241df3b4bea01e77/gh/rwatkins.lisp/log.txt": ServiceError { err: PutObjectError { kind: Unhandled(Error { code: Some("InternalError"), message: Some("We encountered an internal error. Please try again."), request_id: Some("ZVVFPJ6CNZT3RZWD"), extras: {"s3_extended_request_id": "89AqWsqHUiB5xXD1dNVPVl3VmABXS/UREAENnXCOqPTPbXcFgAw/j8C/PiaxF319BcEq6/qwG8A="} }), meta: Error { code: Some("InternalError"), message: Some("We encountered an internal error. Please try again."), request_id: Some("ZVVFPJ6CNZT3RZWD"), extras: {"s3_extended_request_id": "89AqWsqHUiB5xXD1dNVPVl3VmABXS/UREAENnXCOqPTPbXcFgAw/j8C/PiaxF319BcEq6/qwG8A="} } }, raw: Response { inner: Response { status: 500, version: HTTP/1.1, headers: {"x-amz-request-id": "ZVVFPJ6CNZT3RZWD", "x-amz-id-2": "89AqWsqHUiB5xXD1dNVPVl3VmABXS/UREAENnXCOqPTPbXcFgAw/j8C/PiaxF319BcEq6/qwG8A=", "content-type": "application/xml", "transfer-encoding": "chunked", "date": "Fri, 23 Dec 2022 20:35:13 GMT", "server": "AmazonS3", "connection": "close"}, body: SdkBody { inner: Once(Some(b"\n<Error><Code>InternalError</Code><Message>We encountered an internal error. Please try again.</Message><RequestId>ZVVFPJ6CNZT3RZWD</RequestId><HostId>89AqWsqHUiB5xXD1dNVPVl3VmABXS/UREAENnXCOqPTPbXcFgAw/j8C/PiaxF319BcEq6/qwG8A=</HostId></Error>")), retryable: true } }, properties: SharedPropertyBag(Mutex { data: PropertyBag, poisoned: false, .. }) } }
:hammer_and_wrench: If the error is fixed use the retry-report command.
:sos: Can someone from the infra team check in on this? @rust-lang/infra :information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more
:hammer_and_wrench: Generation of the report for pr-106060 queued again.
:information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more
:tada: Experiment pr-106060 is completed!
:bar_chart: 167 regressed and 14 fixed (250762 total)
:newspaper: Open the full report.
:warning: If you notice any spurious failure please add them to the blacklist! :information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more
cc @rust-lang/compiler
This change further reduces the scope of a longstanding hack in the procedural macro infrastructure (following up on https://github.com/rust-lang/rust/pull/94063). Currently, we special-case the rental and allsorts-rental crates during pretty-printing tokens for proc macros. This was originally added to prevent widespread ecosystem breakage when we transitioned from pretty-printing/reparsing to preserving the original TokenStream.
This is a horrible hack that we should get rid of. Unfortunately, the allsorts-rental crate still lacks a point release that works without the hack. However, a fixed version of rental (v0.5.6) has been out for over a year now. The Crater run in https://github.com/rust-lang/rust/pull/106060#issuecomment-1364442610 shows that almost all of the breakage is in small unmaintained Github projects. A few crates on crates.io were originally built with older versions of rental, but none of them pin a specific version. Any crates that add a new dependency on them should automatically get the latest version of rental, while crates with an existing old transitive rental dependency can just run cargo update -p rental.
The previous 'tightening' of this hack in https://github.com/rust-lang/rust/pull/94063 landed in 1.66.0 with zero reported regressions on the issue tracker. This lint has been showing up in cargo report-future-compatibilities for a while now, and I think the ecosystem is now at a point where we can almost entirely remove this hack.
I've left in the allsorts-rental hack for now, but we can still make progress by just removing the rental special case. I'd like to try to get a point release of allsorts-rental with a fix before we remove the hack entirely, since the (small) number of downstream dependencies will otherwise need to switch to the normal 'rental' in order to continue compiling.
The Crater run in https://github.com/rust-lang/rust/pull/106060#issuecomment-1364442610 shows that almost all of the breakage is in small unmaintained Github projects.
Let's not forget that crater is a source of signal, not the entirety of Rust code out there.
while crates with an existing old transitive rental dependency can just run
cargo update -p rental.
Can we make it so that any error caused by this hack removal would be explicit about what the user can do to get to working code? What is what users would see otherwise? Because it seems like it might be the following
[INFO] [stdout] error: linking with `cc` failed: exit status: 1
[INFO] [stdout] |
[INFO] [stdout] = note: "cc" "-Wl,--version-script=/tmp/rustcP3ak1D/list" "-m64" "/tmp/rustcP3ak1D/symbols.o" "/opt/rustwide/target/debug/deps/anchor_attribute_program-f4f8af609e88e769.anchor_attribute_program.95bd31b7-cgu.0.rcgu.o" "/opt/rustwide/target/debug/deps/anchor_attribute_program-f4f8af609e88e769.anchor_attribute_program.95bd31b7-cgu.1.rcgu.o" "/opt/rustwide/target/debug/deps/anchor_attribute_program-f4f8af609e88e769.anchor_attribute_program.95bd31b7-cgu.10.rcgu.o" "/opt/rustwide/target/debug/deps/anchor_attribute_program-f4f8af609e88e769.anchor_attribute_program.95bd31b7-cgu.11.rcgu.o" "/opt/rustwide/target/debug/deps/anchor_attribute_program-f4f8af609e88e769.anchor_attribute_program.95bd31b7-cgu.12.rcgu.o" "/opt/rustwide/target/debug/deps/anchor_attribute_program-f4f8af609e88e769.anchor_attribute_program.95bd31b7-cgu.13.rcgu.o" "/opt/rustwide/target/debug/deps/anchor_attribute_program-f4f8af609e88e769.anchor_attribute_program.95bd31b7-cgu.14.rcgu.o" "/opt/rustwide/target/debug/deps/anchor_attribute_program-f4f8af609e88e769.anchor_attribute_program.95bd31b7-cgu.15.rcgu.o" "/opt/rustwide/target/debug/deps/anchor_attribute_program-f4f8af609e88e769.anchor_attribute_program.95bd31b7-cgu.2.rcgu.o" "/opt/rustwide/target/debug/deps/anchor_attribute_program-f4f8af609e88e769.anchor_attribute_program.95bd31b7-cgu.3.rcgu.o" "/opt/rustwide/target/debug/deps/anchor_attribute_program-f4f8af609e88e769.anchor_attribute_program.95bd31b7-cgu.4.rcgu.o" "/opt/rustwide/target/debug/deps/anchor_attribute_program-f4f8af609e88e769.anchor_attribute_program.95bd31b7-cgu.5.rcgu.o" "/opt/rustwide/target/debug/deps/anchor_attribute_program-f4f8af609e88e769.anchor_attribute_program.95bd31b7-cgu.6.rcgu.o" "/opt/rustwide/target/debug/deps/anchor_attribute_program-f4f8af609e88e769.anchor_attribute_program.95bd31b7-cgu.7.rcgu.o" "/opt/rustwide/target/debug/deps/anchor_attribute_program-f4f8af609e88e769.anchor_attribute_program.95bd31b7-cgu.8.rcgu.o" "/opt/rustwide/target/debug/deps/anchor_attribute_program-f4f8af609e88e769.anchor_attribute_program.95bd31b7-cgu.9.rcgu.o" "/opt/rustwide/target/debug/deps/anchor_attribute_program-f4f8af609e88e769.mj16apbzdpert2n.rcgu.rmeta" "/opt/rustwide/target/debug/deps/anchor_attribute_program-f4f8af609e88e769.18e2ty8n1j1r1iqe.rcgu.o" "-Wl,--as-needed" "-L" "/opt/rustwide/target/debug/deps" "-L" "/opt/rustwide/rustup-home/toolchains/2f3c33065a16f1030e8d2f90241df3b4bea01e77/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-Wl,-Bstatic" "/opt/rustwide/target/debug/deps/libanchor_syn-d193c6d3f493e656.rlib" "/opt/rustwide/target/debug/deps/libanyhow-4d54df7b7da98aa1.rlib" "/opt/rustwide/target/debug/deps/libbs58-37124f6621aa4b9a.rlib" "/opt/rustwide/target/debug/deps/libthiserror-74e809d832d82509.rlib" "/opt/rustwide/target/debug/deps/libsha2-3553a565fae7c07d.rlib" "/opt/rustwide/target/debug/deps/libcpufeatures-c38b0b0f1a8d0c39.rlib" "/opt/rustwide/target/debug/deps/libcfg_if-90e3626e3c41eec3.rlib" "/opt/rustwide/target/debug/deps/libopaque_debug-b3a531c9eb400c1f.rlib" "/opt/rustwide/target/debug/deps/libdigest-27b36b676cb111e5.rlib" "/opt/rustwide/target/debug/deps/libblock_buffer-dc427d38b7b7a36c.rlib" "/opt/rustwide/target/debug/deps/libblock_padding-b1d56c780acc9039.rlib" "/opt/rustwide/target/debug/deps/libgeneric_array-54cbe522c2621d86.rlib" "/opt/rustwide/target/debug/deps/libtypenum-ae91be759e5f8db3.rlib" "/opt/rustwide/target/debug/deps/libserde-d266035027feb283.rlib" "/opt/rustwide/target/debug/deps/libproc_macro2_diagnostics-5462a86590838ff2.rlib" "/opt/rustwide/target/debug/deps/libheck-3d868c7c7b3422bd.rlib" "/opt/rustwide/target/debug/deps/libunicode_segmentation-9603a48f7d2a6a8a.rlib" "/opt/rustwide/target/debug/deps/libsyn-edbbf51d50b9a7bb.rlib" "/opt/rustwide/target/debug/deps/libquote-4040e84c0ddb8645.rlib" "/opt/rustwide/target/debug/deps/libproc_macro2-d937f1f82ffb437d.rlib" "/opt/rustwide/target/debug/deps/libunicode_xid-3c8a88541efd8c80.rlib" "/opt/rustwide/rustup-home/toolchains/2f3c33065a16f1030e8d2f90241df3b4bea01e77/lib/rustlib/x86_64-unknown-linux-gnu/lib/libproc_macro-3de055f562eb2812.rlib" "/opt/rustwide/rustup-home/toolchains/2f3c33065a16f1030e8d2f90241df3b4bea01e77/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd-62c4894b82797b30.rlib" "/opt/rustwide/rustup-home/toolchains/2f3c33065a16f1030e8d2f90241df3b4bea01e77/lib/rustlib/x86_64-unknown-linux-gnu/lib/libpanic_unwind-21d882eb82e74d58.rlib" "/opt/rustwide/rustup-home/toolchains/2f3c33065a16f1030e8d2f90241df3b4bea01e77/lib/rustlib/x86_64-unknown-linux-gnu/lib/libobject-a7a4a5c38e3da2ad.rlib" "/opt/rustwide/rustup-home/toolchains/2f3c33065a16f1030e8d2f90241df3b4bea01e77/lib/rustlib/x86_64-unknown-linux-gnu/lib/libmemchr-e2b1fec37c9c19cd.rlib" "/opt/rustwide/rustup-home/toolchains/2f3c33065a16f1030e8d2f90241df3b4bea01e77/lib/rustlib/x86_64-unknown-linux-gnu/lib/libaddr2line-f5128b1419f0c95a.rlib" "/opt/rustwide/rustup-home/toolchains/2f3c33065a16f1030e8d2f90241df3b4bea01e77/lib/rustlib/x86_64-unknown-linux-gnu/lib/libgimli-41e9355081407ce3.rlib" "/opt/rustwide/rustup-home/toolchains/2f3c33065a16f1030e8d2f90241df3b4bea01e77/lib/rustlib/x86_64-unknown-linux-gnu/lib/librustc_demangle-3d69bc2ce2ff7508.rlib" "/opt/rustwide/rustup-home/toolchains/2f3c33065a16f1030e8d2f90241df3b4bea01e77/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd_detect-bb4c6139d02b6b90.rlib" "/opt/rustwide/rustup-home/toolchains/2f3c33065a16f1030e8d2f90241df3b4bea01e77/lib/rustlib/x86_64-unknown-linux-gnu/lib/libhashbrown-3990de10d3f2460e.rlib" "/opt/rustwide/rustup-home/toolchains/2f3c33065a16f1030e8d2f90241df3b4bea01e77/lib/rustlib/x86_64-unknown-linux-gnu/lib/libminiz_oxide-3a23c05350b4d45c.rlib" "/opt/rustwide/rustup-home/toolchains/2f3c33065a16f1030e8d2f90241df3b4bea01e77/lib/rustlib/x86_64-unknown-linux-gnu/lib/libadler-c2c33bc29289b145.rlib" "/opt/rustwide/rustup-home/toolchains/2f3c33065a16f1030e8d2f90241df3b4bea01e77/lib/rustlib/x86_64-unknown-linux-gnu/lib/librustc_std_workspace_alloc-b3d80be3be44960e.rlib" "/opt/rustwide/rustup-home/toolchains/2f3c33065a16f1030e8d2f90241df3b4bea01e77/lib/rustlib/x86_64-unknown-linux-gnu/lib/libunwind-d2581a381e26c54b.rlib" "/opt/rustwide/rustup-home/toolchains/2f3c33065a16f1030e8d2f90241df3b4bea01e77/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcfg_if-0ff401eab4233ffd.rlib" "/opt/rustwide/rustup-home/toolchains/2f3c33065a16f1030e8d2f90241df3b4bea01e77/lib/rustlib/x86_64-unknown-linux-gnu/lib/liblibc-6d46d38f739892fe.rlib" "/opt/rustwide/rustup-home/toolchains/2f3c33065a16f1030e8d2f90241df3b4bea01e77/lib/rustlib/x86_64-unknown-linux-gnu/lib/liballoc-cb19371b39fc63d8.rlib" "/opt/rustwide/rustup-home/toolchains/2f3c33065a16f1030e8d2f90241df3b4bea01e77/lib/rustlib/x86_64-unknown-linux-gnu/lib/librustc_std_workspace_core-522518611024dce5.rlib" "/opt/rustwide/rustup-home/toolchains/2f3c33065a16f1030e8d2f90241df3b4bea01e77/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcore-05898138a596088a.rlib" "/opt/rustwide/rustup-home/toolchains/2f3c33065a16f1030e8d2f90241df3b4bea01e77/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcompiler_builtins-66b9c3ae5ff29c13.rlib" "-Wl,-Bdynamic" "-lgcc_s" "-lutil" "-lrt" "-lpthread" "-lm" "-ldl" "-lc" "-Wl,--eh-frame-hdr" "-Wl,-znoexecstack" "-L" "/opt/rustwide/rustup-home/toolchains/2f3c33065a16f1030e8d2f90241df3b4bea01e77/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-o" "/opt/rustwide/target/debug/deps/libanchor_attribute_program-f4f8af609e88e769.so" "-Wl,--gc-sections" "-shared" "-Wl,-zrelro,-znow" "-nodefaultlibs"
[INFO] [stdout] = note: collect2: fatal error: ld terminated with signal 9 [Killed]
[INFO] [stdout] compilation terminated.
[INFO] [stdout]
[INFO] [stdout]
[INFO] [stdout]
[INFO] [stdout] error: aborting due to previous error
[INFO] [stdout]
[INFO] [stdout]
[INFO] [stderr] error: could not compile `anchor-attribute-program` due to 2 previous errors
[INFO] [stderr] warning: build failed, waiting for other jobs to finish...
[INFO] [stderr] error: could not compile `libsecp256k1`
[INFO] [stderr]
[INFO] [stderr] Caused by:
[INFO] [stderr] process didn't exit successfully: `rustc --crate-name libsecp256k1 --edition=2018 /opt/rustwide/cargo-home/registry/src/github.com-1ecc6299db9ec823/libsecp256k1-0.6.0/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --crate-type lib --emit=dep-info,metadata -C embed-bitcode=no -C debuginfo=2 --cfg 'feature="default"' --cfg 'feature="hmac"' --cfg 'feature="hmac-drbg"' --cfg 'feature="sha2"' --cfg 'feature="static-context"' --cfg 'feature="std"' --cfg 'feature="typenum"' -C metadata=137ff93ecf18f915 -C extra-filename=-137ff93ecf18f915 --out-dir /opt/rustwide/target/debug/deps -L dependency=/opt/rustwide/target/debug/deps --extern arrayref=/opt/rustwide/target/debug/deps/libarrayref-e3c1ab0608993ffe.rmeta --extern base64=/opt/rustwide/target/debug/deps/libbase64-0fa0330413000f5a.rmeta --extern digest=/opt/rustwide/target/debug/deps/libdigest-309fdf9e086ac0b2.rmeta --extern hmac_drbg=/opt/rustwide/target/debug/deps/libhmac_drbg-ffe7e774c72e1cbd.rmeta --extern libsecp256k1_core=/opt/rustwide/target/debug/deps/liblibsecp256k1_core-d320d82f1a57f1e7.rmeta --extern rand=/opt/rustwide/target/debug/deps/librand-1e54a45746a0c82c.rmeta --extern serde=/opt/rustwide/target/debug/deps/libserde-f9a1d1b038fa4aba.rmeta --extern sha2=/opt/rustwide/target/debug/deps/libsha2-7b24a61d5c3e1883.rmeta --extern typenum=/opt/rustwide/target/debug/deps/libtypenum-fa5adaf555b55ba1.rmeta --cap-lints allow --cap-lints=forbid` (signal: 9, SIGKILL: kill)
which isn't very illuminating. Instead of removing the hack entirely, can we turn the lint into a hard error for a few releases instead as an intermediary step?
I'd like to try to get a point release of allsorts-rental with a fix before we remove the hack entirely, since the (small) number of downstream dependencies will otherwise need to switch to the normal 'rental' in order to continue compiling.
Should we make sure that there are dot releases for every published major version of these two crates? That way anyone depending on allsorts-rental 0.4.* would still have an update path.
Let's not forget that crater is a source of signal, not the entirety of Rust code out there.
That's true - however, the total lack of reported issues gives me more confidence. Compare this to the various incremental-compilation issues, which resulted in a large number of reported issues from affected projects (despite being non-deterministic and difficult to trigger).
Can we make it so that any error caused by this hack removal would be explicit about what the user can do to get to working code? What is what users would see otherwise? Because it seems like it might be the following
That cc error is spurious - I believe it's caused by the Crater runner exhausting its memory and killing the process. This change can only ever result in panics during proc-macro execution - it has no effect on linking.
which isn't very illuminating. Instead of removing the hack entirely, can we turn the lint into a hard error for a few releases instead as an intermediary step?
We could, but I don't think it would be very useful. In practice, I think all of the breakage is going to be caused by users upgrading directly from a very old compiler (which has no warnings at all around this) to a future compiler (with the hack removed entirely). Making it a hard error for a little while won't really change this situation, since users might still upgrade directly to a future version where the hard error logic has been removed.
Should we make sure that there are dot releases for every published major version of these two crates? That way anyone depending on allsorts-rental 0.4.* would still have an update path.
allsorts-rental is a weird crate with exactly one published version.
Can we make it so that any error caused by this hack removal would be explicit about what the user can do to get to working code? What is what users would see otherwise? Because it seems like it might be the following
Unfortunately, this isn't possible. With the hack removed, old versions of the rental proc-macro will start panicking due to the stringified input tokens failing to match the expected string. Since we'll be invoking this macro exactly like any other proc-macro, the only way to detect this would be to keep a special case for those specific crate names.
This was probably discussed before, but why not:
- remove hack from compiler
- add warning/error for specific crates/versions/hashes to cargo.
So if cargo found that blacklisted crates, it suggest user to update (assuming, that update available), or live in the doomed world they have created, without digging which part broken exactly.
@klensy: That's an interesting idea. I would prefer to remove all special-casing based on crate names, but having a check in cargo does seem less hacky than having rustc alter its behavior like it does now.
This was probably discussed before, but why not:
remove hack from compiler
add warning/error for specific crates/versions/hashes to cargo.
So if cargo found that blacklisted crates, it suggest user to update (assuming, that update available), or live in the doomed world they have created, without digging which part broken exactly.
I like the theme of this proposal.
I'm curious about people's thoughts about overall timing. Namely: Should we actually block removing the hack from the compiler until after the cargo change has been made?
(Or, to rephrase: If the hack were removed from the compiler today, with no preceding change to cargo, would we have any motivation to subsequently put the warning/error into cargo later on? Or is it something where the warning/error only makes sense as a kind of future-incompat warning, and once the hack is removed and the breakage hits compiler's users, will they still benefit from the higher level cargo perspective?)
The reason I ask is that I'm trying to figure out if makes sense to try to contribute the cargo change, in order to unblock this, or if we can just go ahead and land this without worrying if the cargo change has been implemented yet.
Adding in a cargo warning afterwards could still be beneficial. If users update directly from an old compiler to the latest stable or nightly (containing the cargo change), they'll have an improved experience if they're using a crate broken by the upgrade
Okay. I think for the time being we can land this as is
~~It has enough info in its diagnostic as it stands to tell people what to do. (I'm going to double-check that understanding with a local build; after I've confirmed that, I plan to leave an review plus marker.)~~ Well, the struck-out statement that opens this paragraph is debatable at best.
Before this branch:
log of the current error message you get if you force rental 0.5.5 via `rental = "=0.5.5"` in Cargo.toml dependencies
% cargo +nightly build
Compiling proc-macro2 v1.0.51
Compiling unicode-ident v1.0.6
Compiling quote v1.0.23
Compiling syn v1.0.107
Compiling stable_deref_trait v1.2.0
Compiling rental-impl v0.5.5
Compiling rental v0.5.5
Compiling exercise_issue_106060 v0.1.0 (/tmp/exercise_issue_106060)
Finished dev [unoptimized + debuginfo] target(s) in 5.76s
warning: the following packages contain code that will be rejected by a future version of Rust: rental v0.5.5
note: to see what the problems were, use the option `--future-incompat-report`, or run `cargo report future-incompatibilities --id 1`
% cargo +nightly build --future-incompat-report
Finished dev [unoptimized + debuginfo] target(s) in 0.00s
warning: the following packages contain code that will be rejected by a future version of Rust: rental v0.5.5
note:
To solve this problem, you can try the following approaches:
- Some affected dependencies have newer versions available.
You may want to consider updating them to a newer version to see if the issue has been fixed.
rental v0.5.5 has the following newer versions available: 0.5.6
- If the issue is not solved by updating the dependencies, a fix has to be
implemented by those dependencies. You can help with that by notifying the
maintainers of this problem (e.g. by creating a bug report) or by proposing a
fix to the maintainers (e.g. by creating a pull request):
- [email protected]
- Repository: https://github.com/jpernst/rental
- Detailed warning command: `cargo report future-incompatibilities --id 2 --package [email protected]`
- If waiting for an upstream fix is not an option, you can use the `[patch]`
section in `Cargo.toml` to use your own version of the dependency. For more
information, see:
https://doc.rust-lang.org/cargo/reference/overriding-dependencies.html#the-patch-section
note: this report can be shown with `cargo report future-incompatibilities --id 1`
After:
log of this branch's error message you get if you force rental 0.5.5 via `rental = "=0.5.5"` in Cargo.toml dependencies
% RUSTC=/media/pnkfelix/Rust/rust.git/objdir/build/x86_64-unknown-linux-gnu/stage1/bin/rustc cargo build
Compiling rental v0.5.5
error: proc-macro derive panicked
--> /home/pnkfelix/.cargo/registry/src/github.com-1ecc6299db9ec823/rental-0.5.5/src/lib.rs:93:12
|
93 | #[derive(__rental_traits)]
| ^^^^^^^^^^^^^^^
...
117 | define_rental_traits!(32);
| ------------------------- in this macro invocation
|
= help: message: expected suffix ").0," not found in "#[allow(unused)] enum ProceduralMasqueradeDummyType\n{ Input = (0, stringify! (32)).0 }"
= note: this error originates in the macro `define_rental_traits` (in Nightly builds, run with -Z macro-backtrace for more info)
error: proc-macro derive panicked
--> /home/pnkfelix/.cargo/registry/src/github.com-1ecc6299db9ec823/rental-0.5.5/src/lib.rs:257:13
|
257 | #[derive(__rental_structs_and_impls)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
...
285 | / rental! {
286 | | /// Example types that demonstrate the API generated by the rental macro.
287 | | pub mod examples {
288 | | use std::sync;
... |
345 | | }
346 | | }
| |_- in this macro invocation
|
= help: message: expected suffix ").0," not found in "#[allow(unused)] enum ProceduralMasqueradeDummyType\n{\n Input =\n (0, stringify!\n (use std :: s\
ync ;\n #[doc =\n r\" The simplest shared rental. The head is a boxed integer, and the suffix is a ref to that integer. This struct demonstrates the basic API\
that all shared rental structs have. See [`SimpleMut`](struct.SimpleMut.html) for the mutable analog.\"]\n #[rental] pub struct SimpleRef { head : Box < i32 >, \
iref : & 'head i32, }\n #[doc =\n r\" The simplest mutable rental. Mutable rentals have a slightly different API; compare this struct to [`SimpleRef`](struct.\
SimpleRef.html) for the clearest picture of how they differ.\"]\n #[rental_mut] pub struct SimpleMut\n { head : Box < i32 >, iref : & 'head mut i32, }\n #[\
doc =\n r\" Identical to [`SimpleRef`](struct.SimpleRef.html), but with the `debug` flag enabled. This will provide a `Debug` impl for the struct as long as all \
of the fields are `Debug`.\"]\n #[rental(debug)] pub struct SimpleRefDebug\n { head : Box < i32 >, iref : & 'head i32, }\n #[doc =\n r\" Similar to [`Si\
mpleRef`](struct.SimpleRef.html), but with the `clone` flag enabled. This will provide a `Clone` impl for the struct as long as the prefix fields are `CloneStableDe\
ref` and the suffix is `Clone` . Notice that the head is an `Arc`, since a clone of an `Arc` will deref to the same object as the original.\"]\n #[rental(clone)]\
pub struct SimpleRefClone\n { head : sync :: Arc < i32 >, iref : & 'head i32, }\n #[doc =\n r\" Identical to [`SimpleRef`](struct.SimpleRef.html), but wit\
h the `deref_suffix` flag enabled. This will provide a `Deref` impl for the struct, which will in turn deref the suffix. Notice that this flag also removes the `sel\
f` param from all methods, replacing it with an explicit param. This prevents any rental methods from blocking deref.\"]\n #[rental(deref_suffix)] pub struct Sim\
pleRefDeref\n { head : Box < i32 >, iref : & 'head i32, }\n #[doc =\n r\" Identical to [`SimpleMut`](struct.SimpleMut.html), but with the `deref_mut_suffix\
` flag enabled. This will provide a `DerefMut` impl for the struct, which will in turn deref the suffix.Notice that this flag also removes the `self` param from all\
methods, replacing it with an explicit param. This prevents any rental methods from blocking deref.\"]\n #[rental_mut(deref_mut_suffix)] pub struct SimpleMutDer\
ef\n { head : Box < i32 >, iref : & 'head mut i32, }\n #[doc =\n r\" Identical to [`SimpleRef`](struct.SimpleRef.html), but with the `covariant` flag enabl\
ed. For rental structs where the field types have covariant lifetimes, this will allow you to directly borrow the fields, as they can be safely reborrowed to a shor\
ter lifetime. See the [`all`](struct.SimpleRefCovariant.html#method.all) and [`suffix`](struct.SimpleRefCovariant.html#method.suffix) methods.\"]\n #[rental(cova\
riant)] pub struct SimpleRefCovariant\n { head : Box < i32 >, iref : & 'head i32, }\n #[doc =\n r\" Identical to [`SimpleRef`](struct.SimpleRef.html), but \
with the `map_suffix` flag enabled. This will allow the type of the suffix to be changed by mapping it to another instantiation of the same struct with the differen\
t type param. See the [`map`](struct.SimpleRefMap.html#method.map), [`try_map`](struct.SimpleRefMap.html#method.try_map), and [`try_map_or_drop`](struct.SimpleRefMa\
p.html#method.try_map_or_drop) methods.\"]\n #[rental(map_suffix = \"T\")] pub struct SimpleRefMap < T : 'static >\n { head : Box < i32 >, iref : & 'head T, }\
)).0\n}"
= note: this error originates in the macro `rental` (in Nightly builds, run with -Z macro-backtrace for more info)
error: proc-macro derive panicked
--> /home/pnkfelix/.cargo/registry/src/github.com-1ecc6299db9ec823/rental-0.5.5/src/lib.rs:257:13
|
257 | #[derive(__rental_structs_and_impls)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
...
350 | / rental! {
351 | | /// Premade types for the most common use cases.
352 | | pub mod common {
353 | | use std::ops::DerefMut;
... |
484 | | }
485 | | }
| |_- in this macro invocation
|
= help: message: expected suffix ").0," not found in "#[allow(unused)] enum ProceduralMasqueradeDummyType\n{\n Input =\n (0, stringify!\n (use std :: o\
ps :: DerefMut ; use stable_deref_trait :: StableDeref ; use\n std :: cell ; use std :: sync ;\n #[doc = r\" Stores an owner and a shared reference in the sam\
e struct.\"]\n #[doc = r\"\"] #[doc = r\" ```rust\"] #[doc = r\" # extern crate rental;\"]\n #[doc = r\" # use rental::common::RentRef;\"] #[doc = r\" # fn ma\
in() {\"]\n #[doc = r\" let r = RentRef::new(Box::new(5), |i| &*i);\"]\n #[doc = r\" assert_eq!(*r, RentRef::rent(&r, |iref| **iref));\"]\n #[doc = r\" # }\
\"] #[doc = r\" ```\"]\n #[rental(debug, clone, deref_suffix, covariant, map_suffix = \"T\")] pub\n struct RentRef < H : 'static + StableDeref, T : 'static >\\
n { head : H, suffix : & 'head T, }\n #[doc = r\" Stores an owner and a mutable reference in the same struct.\"]\n #[doc = r\"\"] #[doc = r\" ```rust\"] #[\
doc = r\" # extern crate rental;\"]\n #[doc = r\" # use rental::common::RentMut;\"] #[doc = r\" # fn main() {\"]\n #[doc = r\" let mut r = RentMut::new(Box::n\
ew(5), |i| &mut *i);\"]\n #[doc = r\" *r = 12;\"]\n #[doc = r\" assert_eq!(12, RentMut::rent(&mut r, |iref| **iref));\"]\n #[doc = r\" # }\"] #[doc = r\" `\
``\"]\n #[rental_mut(debug, deref_mut_suffix, covariant, map_suffix = \"T\")] pub\n struct RentMut < H : 'static + StableDeref + DerefMut, T : 'static >\n \
{ head : H, suffix : & 'head mut T, }\n #[doc = r\" Stores a `RefCell` and a `Ref` in the same struct.\"]\n #[doc = r\"\"] #[doc = r\" ```rust\"] #[doc = r\" \
# extern crate rental;\"]\n #[doc = r\" # use rental::common::RentRefCell;\"] #[doc = r\" # fn main() {\"]\n #[doc = r\" use std::cell;\"] #[doc = r\"\"]\n \
#[doc =\n r\" let r = RentRefCell::new(Box::new(cell::RefCell::new(5)), |c| c.borrow());\"]\n #[doc = r\" assert_eq!(*r, RentRefCell::rent(&r, |c| **c));\"]\\
n #[doc = r\" # }\"] #[doc = r\" ```\"]\n #[rental(debug, clone, deref_suffix, covariant, map_suffix = \"T\")] pub\n struct RentRefCell < H : 'static + Sta\
bleDeref, T : 'static >\n { head : H, suffix : cell :: Ref < 'head, T >, }\n #[doc = r\" Stores a `RefCell` and a `RefMut` in the same struct.\"]\n #[doc =\
r\"\"] #[doc = r\" ```rust\"] #[doc = r\" # extern crate rental;\"]\n #[doc = r\" # use rental::common::RentRefCellMut;\"]\n #[doc = r\" # fn main() {\"] #[d\
oc = r\" use std::cell;\"] #[doc = r\"\"]\n #[doc =\n r\" let mut r = RentRefCellMut::new(Box::new(cell::RefCell::new(5)), |c| c.borrow_mut());\"]\n #[doc \
= r\" *r = 12;\"]\n #[doc = r\" assert_eq!(12, RentRefCellMut::rent(&r, |c| **c));\"]\n #[doc = r\" # }\"] #[doc = r\" ```\"]\n #[rental_mut(debug, deref_m\
ut_suffix, covariant, map_suffix = \"T\")] pub\n struct RentRefCellMut < H : 'static + StableDeref + DerefMut, T : 'static\n > { head : H, suffix : cell :: Re\
fMut < 'head, T >, }\n #[doc = r\" Stores a `Mutex` and a `MutexGuard` in the same struct.\"]\n #[doc = r\"\"] #[doc = r\" ```rust\"] #[doc = r\" # extern cra\
te rental;\"]\n #[doc = r\" # use rental::common::RentMutex;\"] #[doc = r\" # fn main() {\"]\n #[doc = r\" use std::sync;\"] #[doc = r\"\"]\n #[doc =\n \
r\" let mut r = RentMutex::new(Box::new(sync::Mutex::new(5)), |c| c.lock().unwrap());\"]\n #[doc = r\" *r = 12;\"]\n #[doc = r\" assert_eq!(12, RentMutex::ren\
t(&r, |c| **c));\"]\n #[doc = r\" # }\"] #[doc = r\" ```\"]\n #[rental(debug, clone, deref_mut_suffix, covariant, map_suffix = \"T\")] pub\n struct RentMut\
ex < H : 'static + StableDeref + DerefMut, T : 'static >\n { head : H, suffix : sync :: MutexGuard < 'head, T >, }\n #[doc =\n r\" Stores an `RwLock` and a\
n `RwLockReadGuard` in the same struct.\"]\n #[doc = r\"\"] #[doc = r\" ```rust\"] #[doc = r\" # extern crate rental;\"]\n #[doc = r\" # use rental::common::R\
entRwLock;\"] #[doc = r\" # fn main() {\"]\n #[doc = r\" use std::sync;\"] #[doc = r\"\"]\n #[doc =\n r\" let r = RentRwLock::new(Box::new(sync::RwLock::ne\
w(5)), |c| c.read().unwrap());\"]\n #[doc = r\" assert_eq!(*r, RentRwLock::rent(&r, |c| **c));\"]\n #[doc = r\" # }\"] #[doc = r\" ```\"]\n #[rental(debug,\
clone, deref_suffix, covariant, map_suffix = \"T\")] pub\n struct RentRwLock < H : 'static + StableDeref, T : 'static >\n { head : H, suffix : sync :: RwLock\
ReadGuard < 'head, T >, }\n #[doc =\n r\" Stores an `RwLock` and an `RwLockWriteGuard` in the same struct.\"]\n #[doc = r\"\"] #[doc = r\" ```rust\"] #[doc\
= r\" # extern crate rental;\"]\n #[doc = r\" # use rental::common::RentRwLockMut;\"]\n #[doc = r\" # fn main() {\"] #[doc = r\" use std::sync;\"] #[doc = r\\
"\"]\n #[doc =\n r\" let mut r = RentRwLockMut::new(Box::new(sync::RwLock::new(5)), |c| c.write().unwrap());\"]\n #[doc = r\" *r = 12;\"]\n #[doc = r\" \
assert_eq!(12, RentRwLockMut::rent(&r, |c| **c));\"]\n #[doc = r\" # }\"] #[doc = r\" ```\"]\n #[rental(debug, clone, deref_mut_suffix, covariant, map_suffix \
= \"T\")] pub\n struct RentRwLockMut < H : 'static + StableDeref, T : 'static >\n { head : H, suffix : sync :: RwLockWriteGuard < 'head, T >, })).0\n}"
= note: this error originates in the macro `rental` (in Nightly builds, run with -Z macro-backtrace for more info)
error: could not compile `rental` due to 3 previous errors
Having said that, we have had the future-incompat message in there for quite a long time; it was restricted to focus on rental and allsorts-rental over a year ago. (So I still intend to leave a review-plus marker; I just want to run this by @estebank first.)
Longer term, I would like us to look into shifting these messages into cargo instead of leaving them embedded in the rustc source, but that need not block this PR landing, as discussed above.
What is the cost to promote the existing lint to a hard error? It doesn't seem to be adding too big of a maintenance burden, does it? I fear people trying to build a 3 year old project a year from now and just getting (seemingly) gibberish.
Longer term, I would like us to look into shifting these messages into cargo instead of leaving them embedded in the rustc source, but that need not block this PR landing, as discussed above.
I believe that to handle cases like these, where a proc-macro crate can produce invalid code that has been deprecated and fixed in subsequent versions, the ideal DX would be for cargo to have a database of such issues, and pass it down to rustc along with crate name and version. That way the compatibility error would only be shown on the appropriate error, and if never hit because of how the user used the crate then nothing is emitted. For what is worth, I've been wanting cargo to pass down the crate names and versions for a while now, to better handle the "expected Foo, found Foo" category of errors.
Having said that, I think that we can have cargo hold a list of "unconditionally warn/error if this crate-version is used if rustc --version > NN ". The maintenance burden of such a list should be low, and having it be a separate file (or even downloadable!) could allow distros to keep that list around and give better errors for the times someone inevitably tries to build a project off github in 2029 when running today's Debian.
If we are set on completely removing this logic for rental, I'd like it if we could coordinate to have something on cargo before this reaches users.
We discussed this issue in today's T-compiler triage meeting
Sorry, got interrupted before I finished my thought there.
- For the short term, we think we need to retain some kind of special-case treatment of
rental. It doesn't mean this PR cannot land; it just means that we want to keep detectingrental(perhaps in a different place) and spitting out guidance for the user in that case. - For the long term, something like @estebank 's suggestion of a database that our tools (e.g.
cargo) can query for known broken builds sounds like the right path for then completely removing these hacks entirely. We don't want to block landing this PR on such a database landing, but we do want to keep some kind of crate-specific error message until we have such a mitigation measure available elsewhere.
Changing this PR to S-waiting-on-author to reflect our request for a more specific error message. (Or, if you would rather just close this PR for now and wait for the cargo work, I think that would be fine too, based on how I read the room in the meeting today.)
@rustbot label: -S-waiting-on-team +S-waiting-on-author
:umbrella: The latest upstream changes (presumably #111925) made this pull request unmergeable. Please resolve the merge conflicts.
@Aaron1011 ping from triage - can you post your status on this PR? There hasn't been an update in a few months. Thanks!
I think the benefit of an external database that cargo pulls is that users can find out about these situations now and prepare. This avoids the problem of a user having a bad dependency on an old toolchain and then skipping over any of the transitional toolchains when they do their next upgrade. Instead, once the toolchain with support for the external database is their old version, they can find out about these dependencies then and upgrade that dependency before upgrading their toolchain.
We do have a database today: yanked crates. I don't know how crates.io feels about shortcutting the maintainer for yanking these "bad" versions. And I wish we could attach a yanked reason to it (https://github.com/rust-lang/cargo/issues/2608). There is a PR open to allow forcing the use of a yanked version.
rustsec also has a database. Their focus is on security but that bleeds over a little when it comes to "unmaintained crates" (e.g. its a small step to say "suggest structopt users switch to clap despite structopt being 'maintained'"). It is also all through third-party subcommands atm.
Whether its hard coded or not, the cargo team is relatively cautious when it comes to warnings until we have https://github.com/rust-lang/cargo/issues/12235. However, I think a case like this is likely warranted for an unconditional warning until then.
Sorry, @epage , just to clarify: How should I translate your comment for the specific case of rental 0.5.5?
As far as I can tell, rental 0.5.5 is not yanked, and I'm as clueless as you are about the policy of crates.io about non-owners trying to arrange for "bad" crates to get yanked.
So maybe I should interpret your comment as saying "See if you can get rustsec to add rental 0.5.5 to their database" ?
Or the very end of your comment mentioned "whether its hard-coded or not" -- so maybe you're saying, for the medium term, the cargo team might be willing to accept a PR that includes special case handling of rental 0.5.5, rather than trying to do the upfront design of the separate database that would store these cases?