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

Need new test for incomplete compute_implied_outlives_bounds fulfillment #101694

Open
jackh726 opened this issue Sep 11, 2022 · 2 comments
Open
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jackh726
Copy link
Member

In #101680 (af619eb#diff-f699f7fa1d6499f96d393ebaf6be4031125d026a7b6a18d6ecb8ee4e875fa197R161), I changed the Err(NoSolution) to a bug!(). The issue-42552 test originally covered this case, but seems like that's not the case anymore. Instead, when attempting a try run the hyper-0.14.18 benchmark in rustc-perf failed to build.

This issue is basically a call to minimize that failure and add a test to the test suite so we properly capture that bug path.

To do this, you will need to make a local change like the above, calling bug!() instead of returning Err(NoSolution). Then, you can minimize the hyper-0.14.18 benchmark.

@jackh726 jackh726 added the E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example label Sep 11, 2022
@jackh726
Copy link
Member Author

jackh726 commented Sep 11, 2022

This actually might not be the cause, will update later.

(That PR does have problems with hyper but not the test suite, though. So there is an untested bug path.)

@jackh726
Copy link
Member Author

Okay, so the failure was actually caused by the other changes in that PR (specifically, that project predicates without bound vars were accidently not getting registered).

In hyper, it's this method that ICEs: https://github.com/rust-lang/rustc-perf/blob/ff4e6fb9246ac08771107f4498bdb4e3f84007a0/collector/benchmarks/hyper-0.14.18/src/service/make.rs#L84

The key predicate is the T: for<'a> Service<&'a Target, Error = ME, Response = S, Future = F>, which must be registered to not ICE (and wasn't because it has bound vars). This should help to make a MCVE.

I'm keeping the buggy code in a branch here: https://github.com/jackh726/rust/tree/implied-cleanup-old. This passes the rustc test suite but fails to build hyper.

@Enselic Enselic added A-testsuite Area: The testsuite used to check the correctness of rustc T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example 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

2 participants