Adding an impl for type defined outside of crate causes every associated method call to be reported as an error
#127798
Comments
udoprog
added
A-diagnostics
|
This seems to be a regression in 1.78 |
|
Bisection points to somewhere here That’s almost certainly cc @oli-obk |
|
This change in test output is actually a bad change that was already visible in the tests when #121113 was merged. The function // Allowed
fn in_impl_Trait_in_return() -> impl IntoIterator<Item = impl IntoIterator> {
vec![vec![0; 10], vec![12; 7], vec![8; 3]]
+ //~^ ERROR: no function or associated item named `into_vec` found for slice `[_]`
}should not gain any errors pointing to it. It's even clearly marked as an “Allowed” case in the comments. |
|
For context, this issue can easily make a single change in a rust crate cause hundreds of unrelated errors at once, as reported on: https://users.rust-lang.org/t/pub-use-breaks-my-compiler/117983 Those hundreds of unrelated errors can then easily hide the actual problem. Looking further through the changed tests in #121113, I also think that the following is a bad change: struct Foo<T: ?Sized>(T);
impl Foo<dyn Send + Sync> {
fn abc() -> bool { //~ ERROR duplicate definitions with name `abc`
false
}
}
impl Foo<dyn Sync + Send> {
fn abc() -> bool {
true
}
}
fn main() {
assert_eq!(<Foo<dyn Send + Sync>>::abc(), false);
+ //~^ ERROR: multiple applicable items in scope
assert_eq!(<Foo<dyn Sync + Send>>::abc(), true);
+ //~^ ERROR: multiple applicable items in scope
}Adding a second definition of |
|
This doesn’t just make methods fail, but all inherent associated items, AFAICT. E.g. here’s an associated constant (inspired by the third bad error I’ve noticed #121113 has added to already existing tests). impl () {}
fn f() {
let x = i8::MIN;
}(among the errors:) |
|
I'll look into it. I agree the existing implementation is a bit goofy. |
|
This is two issues:
|
|
I think, the second issue (ambiguity on overlap) is acceptable either way; at least all of the errors also point to the relevant
This is great to hear, thanks for the quick analysis. Otherwise (if it wasn’t so easily fixed) I would have suggested a revert of #121113 as a quick fix, or at least a partial revert, as far as the |
Rollup merge of rust-lang#130764 - compiler-errors:inherent, r=estebank Separate collection of crate-local inherent impls from error tracking rust-lang#119895 changed the return type of the `crate_inherent_impls` query from `CrateInherentImpls` to `Result<CrateInherentImpls, ErrorGuaranteed>` to avoid needing to use the non-parallel-friendly `track_errors()` to track if an error was reporting from within the query... This was mostly fine until rust-lang#121113, which stopped halting compilation when we hit an `Err(ErrorGuaranteed)` in the `crate_inherent_impls` query. Thus we proceed onwards to typeck, and since a return type of `Result<CrateInherentImpls, ErrorGuaranteed>` means that the query can *either* return one of "the list inherent impls" or "error has been reported", later on when we want to assemble method or associated item candidates for inherent impls, we were just treating any `Err(ErrorGuaranteed)` return value as if Rust had no inherent impls defined anywhere at all! This leads to basically every inherent method call failing with an error, lol, which was reported in rust-lang#127798. This PR changes the `crate_inherent_impls` query to return `(CrateInherentImpls, Result<(), ErrorGuaranteed>)`, i.e. returning the inherent impls collected *and* whether an error was reported in the query itself. It firewalls the latter part of that query into a new `crate_inherent_impls_validity_check` just for the `ensure()` call. This fixes rust-lang#127798.
Separate collection of crate-local inherent impls from error tracking rust-lang#119895 changed the return type of the `crate_inherent_impls` query from `CrateInherentImpls` to `Result<CrateInherentImpls, ErrorGuaranteed>` to avoid needing to use the non-parallel-friendly `track_errors()` to track if an error was reporting from within the query... This was mostly fine until rust-lang#121113, which stopped halting compilation when we hit an `Err(ErrorGuaranteed)` in the `crate_inherent_impls` query. Thus we proceed onwards to typeck, and since a return type of `Result<CrateInherentImpls, ErrorGuaranteed>` means that the query can *either* return one of "the list inherent impls" or "error has been reported", later on when we want to assemble method or associated item candidates for inherent impls, we were just treating any `Err(ErrorGuaranteed)` return value as if Rust had no inherent impls defined anywhere at all! This leads to basically every inherent method call failing with an error, lol, which was reported in rust-lang#127798. This PR changes the `crate_inherent_impls` query to return `(CrateInherentImpls, Result<(), ErrorGuaranteed>)`, i.e. returning the inherent impls collected *and* whether an error was reported in the query itself. It firewalls the latter part of that query into a new `crate_inherent_impls_validity_check` just for the `ensure()` call. This fixes rust-lang#127798.
Code
Current output
But we also get the following "bonus" error:
If for example we add the impl to the tokio crate, we get:
Desired output
Preferably, the
impl for type defined outside of cratebeing an illegal implementation should not cause other seemingly unrelated errors.Rust Version
The text was updated successfully, but these errors were encountered: