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 1.56.0 - Trait bound not satisfied error even though the impl exists #90662

Closed
AzureMarker opened this issue Nov 6, 2021 · 8 comments · Fixed by #92041
Closed
Labels
A-traits Area: Trait system C-bug Category: This is a bug. P-high High priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another.

Comments

@AzureMarker
Copy link
Member

Code

I tried this code (minimized from this test in my library):

trait HasProvider<T: ?Sized> {}
trait Provider<M> {
    type Interface: ?Sized;
}

trait Repository {}
trait Service {}

struct DbConnection;
impl<M> Provider<M> for DbConnection {
    type Interface = DbConnection;
}

struct RepositoryImpl;
impl<M: HasProvider<DbConnection>> Provider<M> for RepositoryImpl {
    type Interface = dyn Repository;
}

struct ServiceImpl;
impl<M: HasProvider<dyn Repository>> Provider<M> for ServiceImpl {
    type Interface = dyn Service;
}

struct TestModule;
impl HasProvider<<DbConnection as Provider<Self>>::Interface> for TestModule {}
impl HasProvider<<RepositoryImpl as Provider<Self>>::Interface> for TestModule {}
impl HasProvider<<ServiceImpl as Provider<Self>>::Interface> for TestModule {}

fn main() {}

I expected to see this happen: Compile without errors.

Instead, this happened: Compiled with errors:

error[E0277]: the trait bound `TestModule: HasProvider<(dyn Repository + 'static)>` is not satisfied
  --> shaku/tests/debug_rust_1_56.rs:26:6
   |
26 | impl HasProvider<<RepositoryImpl as Provider<Self>>::Interface> for TestModule {}
   |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `HasProvider<(dyn Repository + 'static)>` is not implemented for `TestModule`
   |
   = help: the following implementations were found:
             <TestModule as HasProvider<(dyn Repository + 'static)>>
             <TestModule as HasProvider<<ServiceImpl as Provider<TestModule>>::Interface>>
             <TestModule as HasProvider<DbConnection>>
note: required by a bound in `HasProvider`
  --> shaku/tests/debug_rust_1_56.rs:1:1
   |
1  | trait HasProvider<T: ?Sized> {}
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `HasProvider`

error[E0277]: the trait bound `TestModule: HasProvider<(dyn Repository + 'static)>` is not satisfied
  --> shaku/tests/debug_rust_1_56.rs:27:6
   |
27 | impl HasProvider<<ServiceImpl as Provider<Self>>::Interface> for TestModule {}
   |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `HasProvider<(dyn Repository + 'static)>` is not implemented for `TestModule`
   |
   = help: the following implementations were found:
             <TestModule as HasProvider<(dyn Repository + 'static)>>
             <TestModule as HasProvider<<ServiceImpl as Provider<TestModule>>::Interface>>
             <TestModule as HasProvider<DbConnection>>
note: required because of the requirements on the impl of `Provider<TestModule>` for `ServiceImpl`
  --> shaku/tests/debug_rust_1_56.rs:20:38
   |
20 | impl<M: HasProvider<dyn Repository>> Provider<M> for ServiceImpl {
   |                                      ^^^^^^^^^^^     ^^^^^^^^^^^

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0277`.

Note that it says HasProvider<(dyn Repository + 'static)> is not implemented for TestModule, but then lists it as an existing implementation.

Version it worked on

It most recently worked on: 1.55.0

Version with regression

rustc --version --verbose:

rustc 1.56.0 (09c42c458 2021-10-18)
binary: rustc
commit-hash: 09c42c45858d5f3aedfa670698275303a3d19afa
commit-date: 2021-10-18
host: x86_64-unknown-linux-gnu
release: 1.56.0
LLVM version: 13.0.0
@AzureMarker AzureMarker added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Nov 6, 2021
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. regression-untriaged Untriaged performance or correctness regression. labels Nov 6, 2021
@AzureMarker
Copy link
Member Author

@rustbot modify labels: +regression-from-stable-to-stable

@rustbot rustbot added regression-from-stable-to-stable Performance or correctness regression from one stable version to another. I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Nov 6, 2021
@camelid camelid added the A-traits Area: Trait system label Nov 6, 2021
@camelid
Copy link
Member

camelid commented Nov 6, 2021

Bisects to #85868. cc @Aaron1011 @jackh726


searched nightlies: from nightly-2021-07-01 to nightly-2021-10-01
regressed nightly: nightly-2021-09-03
searched commits: from 50171c3 to 371f3cd
regressed commit: 371f3cd

bisected with cargo-bisect-rustc v0.6.0

Host triple: x86_64-apple-darwin
Reproduce with:

cargo bisect-rustc --preserve --regress=error --start=2021-07-01 --end=2021-10-01 -- check 

@apiraino
Copy link
Contributor

apiraino commented Nov 9, 2021

Assigning priority as discussed in the Zulip thread of the Prioritization Working Group.

@rustbot label -I-prioritize +P-high

@rustbot rustbot added P-high High priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Nov 9, 2021
@elertan
Copy link

elertan commented Nov 18, 2021

Any advancements on this issue? I currently can't upgrade to Rust 1.56 due to this exact problem

@AzureMarker
Copy link
Member Author

I might try my hand at diagnosing it, but someone with more context would definitely be faster at it.

@Aaron1011
Copy link
Member

Sorry for the delay in getting to this. I've opened #91183 with a fix.

Aaron1011 added a commit to Aaron1011/rust that referenced this issue Nov 26, 2021
The existing `InferCtxt` may already have in-progress projections
for some of the predicates we may (recursively evaluate). This can
cause us to incorrect produce (and cache!) `EvaluatedToRecur`, leading
us to incorrectly mark a predicate as unimplemented.

We now create a fresh `InferCtxt` for each sub-obligation that we
'speculatively' evaluate. This causes us to miss out on some
legitimate caching opportunities, but ensures that our speculative
evaluation cannot pollute any of the caches from the 'real' `InferCtxt`.
The evaluation can still update *global* caches, but our global caches
don't have any notion of 'in progress', so this is fine.

Fixes rust-lang#90662
@AzureMarker
Copy link
Member Author

I noticed that this regression was found in the 1.56 beta crater run:
https://crater-reports.s3.amazonaws.com/beta-1.56-3/index.html

Was it just overlooked at that time?

bors added a commit to rust-lang-ci/rust that referenced this issue Dec 20, 2021
…on, r=jackh726

Remove 'speculative evaluation' of predicates

Performing 'speculative evaluation' introduces caching bugs that
cannot be fixed without invasive changes to projection.

Hopefully, we can win back most of the performance lost by
re-adding 'cache completion'

Fixes rust-lang#90662
@bors bors closed this as completed in eee09ec Dec 20, 2021
@DanielJoyce
Copy link

Seems kinda major to

I noticed that this regression was found in the 1.56 beta crater run: https://crater-reports.s3.amazonaws.com/beta-1.56-3/index.html

Was it just overlooked at that time?

Yeah, this seems rather significant. It's right there in the results.

https://crater-reports.s3.amazonaws.com/beta-1.56-3/beta-2021-10-15/reg/shaku-0.6.1/log.txt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-traits Area: Trait system C-bug Category: This is a bug. P-high High priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another.
Projects
None yet
7 participants