Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression in rustc nightly #69629

Closed
rex4539 opened this issue Mar 2, 2020 · 12 comments
Closed

Regression in rustc nightly #69629

rex4539 opened this issue Mar 2, 2020 · 12 comments
Assignees
Labels
E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@rex4539
Copy link

rex4539 commented Mar 2, 2020

Steps:
Compile tower-balance.

What happened:

Compiling tower-balance v0.3.0
error[E0034]: multiple applicable items in scope
   --> /Users/rex/.cargo/registry/src/github.com-1ecc6299db9ec823/tower-balance-0.3.0/src/pool/mod.rs:373:47
    |
373 |         if let Poll::Ready(()) = self.balance.poll_ready(cx)? {
    |                                               ^^^^^^^^^^ multiple `poll_ready` found
    |
note: candidate #1 is defined in an impl of the trait `tower_service::Service` for the type `p2c::service::Balance<_, _>`
   --> /Users/rex/.cargo/registry/src/github.com-1ecc6299db9ec823/tower-balance-0.3.0/src/p2c/service.rs:234:5
    |
234 |     fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    = note: candidate #2 is defined in an impl of the trait `tower_make::make_service::MakeService` for the type `_`
help: disambiguate the method call for candidate #1
    |
373 |         if let Poll::Ready(()) = tower_service::Service::poll_ready(&mut self.balance, cx)? {
    |                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: disambiguate the method call for candidate #2
    |
373 |         if let Poll::Ready(()) = tower_make::make_service::MakeService::poll_ready(&mut self.balance, cx)? {
    |                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0034]: multiple applicable items in scope
   --> /Users/rex/.cargo/registry/src/github.com-1ecc6299db9ec823/tower-balance-0.3.0/src/pool/mod.rs:418:37
    |
418 |                 return self.balance.poll_ready(cx);
    |                                     ^^^^^^^^^^ multiple `poll_ready` found
    |
note: candidate #1 is defined in an impl of the trait `tower_service::Service` for the type `p2c::service::Balance<_, _>`
   --> /Users/rex/.cargo/registry/src/github.com-1ecc6299db9ec823/tower-balance-0.3.0/src/p2c/service.rs:234:5
    |
234 |     fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    = note: candidate #2 is defined in an impl of the trait `tower_make::make_service::MakeService` for the type `_`
help: disambiguate the method call for candidate #1
    |
418 |                 return tower_service::Service::poll_ready(&mut self.balance, cx);
    |                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: disambiguate the method call for candidate #2
    |
418 |                 return tower_make::make_service::MakeService::poll_ready(&mut self.balance, cx);
    |                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0034`.
error: could not compile `tower-balance`.

Expected result:
No regression.

Notes:
See tower-rs/tower#423 (comment)

@Sherlock-Holo
Copy link

it happened to me compile error

@Centril Centril added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. and removed C-bug Category: This is a bug. labels Mar 2, 2020
@estebank estebank added E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc I-nominated labels Mar 3, 2020
@jonhoo
Copy link
Contributor

jonhoo commented Mar 3, 2020

I ran regression with cargo-bisect-rust, and got:

checking nightly-2020-02-28
std for x86_64-unknown-linux-gnu: 17.52 MB / 17.52 MB [==========================================================================================================================================================================================================================] 100.00 % 15.98 MB/s uninstalling nightly-2020-02-28
verifying the start of the range does not reproduce the regression
std for x86_64-unknown-linux-gnu: 17.52 MB / 17.52 MB [==========================================================================================================================================================================================================================] 100.00 % 15.78 MB/s uninstalling nightly-2020-02-28
tested nightly-2020-02-28, got No
confirmed the start of the range does not reproduce the regression
verifying the end of the range reproduces the regression
uninstalling nightly-2020-02-29
tested nightly-2020-02-29, got Yes
confirmed the end of the range reproduces the regression
searched toolchains nightly-2020-02-28 through nightly-2020-02-29
uninstalling nightly-2020-02-29
regression in nightly-2020-02-29
fetching https://static.rust-lang.org/dist/2020-02-29/channel-rust-nightly-git-commit-hash.txt
nightly manifest 2020-02-29: 40 B / 40 B [======================================================================================================================================================================================================================================] 100.00 % 176.06 KB/s converted 2020-02-29 to 0eb878d2aa6e3a1cb315f3f328681b26bb4bffdb
fetching https://static.rust-lang.org/dist/2020-02-28/channel-rust-nightly-git-commit-hash.txt
nightly manifest 2020-02-28: 40 B / 40 B [======================================================================================================================================================================================================================================] 100.00 % 624.42 KB/s converted 2020-02-28 to 6d69caba110c0c2fb90180df1cbc8be5033b91d4
looking for regression commit between 2020-02-29 and 2020-02-28
fetching commits from 6d69caba110c0c2fb90180df1cbc8be5033b91d4 to 0eb878d2aa6e3a1cb315f3f328681b26bb4bffdb
opening existing repository at "/home/jon/dev/others/rust"
refreshing repository
looking up first commit
looking up second commit
checking that commits are by bors and thus have ci artifacts...
finding bors merge commits
found 7 bors merge commits in the specified range
opening existing repository at "/home/jon/dev/others/rust"
refreshing repository
validated commits found, specifying toolchains
testing commits
verifying the start of the range does not reproduce the regression
installing 6d69caba110c0c2fb90180df1cbc8be5033b91d4
std for x86_64-unknown-linux-gnu: 17.52 MB / 17.52 MB [=========================================================================================================================================================================================================================] 100.00 % 713.64 KB/s testing 6d69caba110c0c2fb90180df1cbc8be5033b91d4
tested 6d69caba110c0c2fb90180df1cbc8be5033b91d4, got No
uninstalling 6d69caba110c0c2fb90180df1cbc8be5033b91d4
confirmed the start of the range does not reproduce the regression
verifying the end of the range reproduces the regression
installing 0eb878d2aa6e3a1cb315f3f328681b26bb4bffdb
std for x86_64-unknown-linux-gnu: 17.53 MB / 17.53 MB [=========================================================================================================================================================================================================================] 100.00 % 605.20 KB/s testing 0eb878d2aa6e3a1cb315f3f328681b26bb4bffdb
tested 0eb878d2aa6e3a1cb315f3f328681b26bb4bffdb, got No
uninstalling 0eb878d2aa6e3a1cb315f3f328681b26bb4bffdb
thread 'main' panicked at 'the end of the range to test must reproduce the regression', /home/jon/.cargo/registry/src/github.com-1ecc6299db9ec823/cargo-bisect-rustc-0.3.0/src/least_satisfying.rs:34:14

Which suggests that the regression is present in nightly-2020-02-29, but somehow not in 0eb878d ? I'm working on bisecting further.

@jonhoo
Copy link
Contributor

jonhoo commented Mar 3, 2020

cargo-bisect-rust claims that the regression was caused by 55aee8d (#69255) by @estebank:

bisecting ci builds
starting at 0eb878d2aa6e3a1cb315f3f328681b26bb4bffdb, ending at origin/master
fetching commits from 0eb878d2aa6e3a1cb315f3f328681b26bb4bffdb to origin/master
opening existing repository at "/home/jon/dev/others/rust"
refreshing repository
looking up first commit
looking up second commit
checking that commits are by bors and thus have ci artifacts...
finding bors merge commits
found 25 bors merge commits in the specified range
opening existing repository at "/home/jon/dev/others/rust"
refreshing repository
validated commits found, specifying toolchains
testing commits
verifying the start of the range does not reproduce the regression
installing 0eb878d2aa6e3a1cb315f3f328681b26bb4bffdb
std for x86_64-unknown-linux-gnu: 17.53 MB / 17.53 MB [======================================================================================================================================================================================================================] 100.00 % 679.77 KB/s 0s testing 0eb878d2aa6e3a1cb315f3f328681b26bb4bffdb
tested 0eb878d2aa6e3a1cb315f3f328681b26bb4bffdb, got No
uninstalling 0eb878d2aa6e3a1cb315f3f328681b26bb4bffdb
confirmed the start of the range does not reproduce the regression
verifying the end of the range reproduces the regression
installing 3c5b1b7d63f55ac96fc7cd06df01e0f0e4f49d47
std for x86_64-unknown-linux-gnu: 17.57 MB / 17.57 MB [=========================================================================================================================================================================================================================] 100.00 % 939.29 KB/s testing 3c5b1b7d63f55ac96fc7cd06df01e0f0e4f49d47
tested 3c5b1b7d63f55ac96fc7cd06df01e0f0e4f49d47, got Yes
uninstalling 3c5b1b7d63f55ac96fc7cd06df01e0f0e4f49d47
confirmed the end of the range reproduces the regression
installing 360e42de82152c4e1a6e70d2f228dd3748c50c8d
std for x86_64-unknown-linux-gnu: 17.62 MB / 17.62 MB [=========================================================================================================================================================================================================================] 100.00 % 749.47 KB/s testing 360e42de82152c4e1a6e70d2f228dd3748c50c8d
tested 360e42de82152c4e1a6e70d2f228dd3748c50c8d, got Yes
uninstalling 360e42de82152c4e1a6e70d2f228dd3748c50c8d
installing 4f0edbdfe5f111c43a5e06f68186b95141d1f6c8
std for x86_64-unknown-linux-gnu: 17.57 MB / 17.57 MB [=========================================================================================================================================================================================================================] 100.00 % 953.92 KB/s testing 4f0edbdfe5f111c43a5e06f68186b95141d1f6c8
tested 4f0edbdfe5f111c43a5e06f68186b95141d1f6c8, got Yes
uninstalling 4f0edbdfe5f111c43a5e06f68186b95141d1f6c8
installing 04e7f96dd89b1f0ad615dff1c85d11d4c4c64cb4
std for x86_64-unknown-linux-gnu: 17.54 MB / 17.54 MB [=========================================================================================================================================================================================================================] 100.00 % 708.05 KB/s testing 04e7f96dd89b1f0ad615dff1c85d11d4c4c64cb4
tested 04e7f96dd89b1f0ad615dff1c85d11d4c4c64cb4, got Yes
uninstalling 04e7f96dd89b1f0ad615dff1c85d11d4c4c64cb4
installing 5703b7aafb70e77547e8f03876a5911a2e89a2a5
std for x86_64-unknown-linux-gnu: 17.57 MB / 17.57 MB [=========================================================================================================================================================================================================================] 100.00 % 906.34 KB/s testing 5703b7aafb70e77547e8f03876a5911a2e89a2a5
tested 5703b7aafb70e77547e8f03876a5911a2e89a2a5, got No
uninstalling 5703b7aafb70e77547e8f03876a5911a2e89a2a5
installing 55aee8d49628ae8218e91745c388d5dc36771248
std for x86_64-unknown-linux-gnu: 17.50 MB / 17.50 MB [===========================================================================================================================================================================================================================] 100.00 % 4.52 MB/s testing 55aee8d49628ae8218e91745c388d5dc36771248
tested 55aee8d49628ae8218e91745c388d5dc36771248, got Yes
uninstalling 55aee8d49628ae8218e91745c388d5dc36771248
searched toolchains 0eb878d2aa6e3a1cb315f3f328681b26bb4bffdb through 3c5b1b7d63f55ac96fc7cd06df01e0f0e4f49d47
regression in 55aee8d49628ae8218e91745c388d5dc36771248

@jonhoo
Copy link
Contributor

jonhoo commented Mar 3, 2020

You can replicate by cloning tower and running

$ cargo check -p tower-balance

@jonhoo
Copy link
Contributor

jonhoo commented Mar 4, 2020

I wonder if this may somehow be a new manifestation of #60375?

@pnkfelix
Copy link
Member

pnkfelix commented Mar 4, 2020

triage: P-high, removing nomination. Assigning to @estebank initially (but feel free to clear assignment or reassign if you are overloaded)

@pnkfelix pnkfelix added P-high High priority and removed E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc I-nominated labels Mar 4, 2020
@pnkfelix
Copy link
Member

pnkfelix commented Mar 4, 2020

by the way, under the new priority scheme being considered for next generation triage, I would consider #69629 a P-critical bug. The regressing PR took code that used to compile successfully, and now produced an unexpected static error.

@estebank
Copy link
Contributor

estebank commented Mar 4, 2020

Can confirm that the issue was introduced in the following code https://github.com/rust-lang/rust/pull/69255/files#diff-dd3531c645441fc4588d997838e391f5R1406-R1447

@estebank
Copy link
Contributor

estebank commented Mar 4, 2020

I have a fix in #69717. Tested with tower locally.

@estebank estebank added the E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. label Mar 5, 2020
bors added a commit that referenced this issue Mar 5, 2020
Correctly reject `TraitCandidate` in all cases

Follow up to #69255, addresses #69629.

When `self.select_trait_candidate(trait_ref)` returned `Err(_)`, `result` wasn't being set to `NoMatch`, causing invalid methods to be selected.
@bors bors closed this as completed in 8a32729 Mar 5, 2020
@lqd
Copy link
Member

lqd commented Mar 5, 2020

(Reopening: the issue was closed automatically by the commit message rather than the PR description, but still needs a test)

@lqd lqd reopened this Mar 5, 2020
@Sherlock-Holo
Copy link

I test and build success with rustc 1.43.0-nightly (96bb8b31c 2020-03-05)

@rex4539
Copy link
Author

rex4539 commented Mar 6, 2020

Verified with rustc 1.43.0-nightly (96bb8b31c 2020-03-05).

Closing.

@rex4539 rex4539 closed this as completed Mar 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants