Migrate to pin project lite #595
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Turns IntoStream into a regular struct because pin_project_lite does not and will support tuple structs. https://github.com/taiki-e/pin-project-lite/blob/416be96f7777862c68b567c92a91887f69a8c2b3/src/lib.rs#L401-L408
This enables migration to pin_project_lite, which does not and will not support tuple structs https://github.com/taiki-e/pin-project-lite/blob/416be96f7777862c68b567c92a91887f69a8c2b3/src/lib.rs#L401-L408
Note: the doc comment on AsyncResponseFuture::service was also reduced to a regular comment. This is a known limitation of pin_project_lite that the they have labeled as "help wanted". taiki-e/pin-project-lite#3 (comment)
pin_project_lite has the current limitation of nto supporting doc comments taiki-e/pin-project-lite#3 (comment) pin_project_lite does not and will not support tuple variants https://github.com/taiki-e/pin-project-lite/blob/416be96f7777862c68b567c92a91887f69a8c2b3/src/lib.rs#L401-L408
peak_wma and pending_requests will now properly compile without the "discover" feature enabled.
|
Running the tests on both stable and nightly pass locally for me. On nightly, I do receive multiple warnings about a lint renaming. I'd appreciate any advice on how to diagnose further. On Stable $ rustc --version
rustc 1.53.0 (53cb7b09b 2021-06-17)
$ cargo test --all --all-features
(passes)On Nightly $ rustc +nightly --version
rustc 1.56.0-nightly (08095fc1f 2021-07-26)
$ cargo +nightly test --all --all-features
(passes)The lint rename warning |
8 | #![deny(broken_intra_doc_links)]
| ^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `rustdoc::broken_intra_doc_links`
|
= note: `#[warn(renamed_and_removed_lints)]` on by default |
|
@Michael-J-Ward are you able to rename it? or is it not available on stable yet? |
broken_intra_doc_links has been renamed to rustdoc::broken_intra_doc_links
|
I should have been clearer- the lint warning was not the reason for the failure. This test below is failing in CI but I can't trigger the failure locally. Running tests/spawn_ready/main.rs (target/debug/deps/spawn_ready-102042b79d40a5b4)
running 4 tests
test propagates_trace_spans ... ok
test abort_on_drop ... FAILED
test when_inner_is_not_ready ... ok
test when_inner_fails ... ok
failures:
---- abort_on_drop stdout ----
thread 'abort_on_drop' panicked at 'pending', tower/tests/spawn_ready/main.rs:84:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failures:
abort_on_drop
test result: FAILED. 3 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
error: test failed, to rerun pass '-p tower --test spawn_ready'
Error: The process '/usr/share/rust/.cargo/bin/cargo' failed with exit code 101 |
|
@taiki-e are there any differences in drop behavior between pin project and pin project lite? |
pin_project_lite does support PinnedDrop https://github.com/taiki-e/pin-project-lite/pull/25/files However, it does not support generic trait bounds on the PinnedDrop impl. To workaround this, I removed the T::Error bound from the Worker struct definition, and moved `close_semaphore` to a a new impl without that trait bound.
I don't think so. |
|
After updating cargo and disabling / clearing out sccache, I am able to reproduce the However, |
This test was also failing on master.
Michael-J-Ward
force-pushed
the
migrate-to-pin-project-lite
branch
from
0108b5a
to
5c1c39f
Compare
Michael-J-Ward
changed the title
LucioFranco
approved these changes
Jul 27, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Clears out the easy parts of #594.
Remaining:
util::Either- it is public and the migration requires a breaking change (as described here)buffer::worker- would love to take a shot at the refactor with a little more guidance.