Skip to content

Unknown types implement all traits #10454

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

Closed
lnicola opened this issue Oct 4, 2021 · 8 comments · Fixed by #11805
Closed

Unknown types implement all traits #10454

lnicola opened this issue Oct 4, 2021 · 8 comments · Fixed by #11805
Labels
A-ty type system / type inference / traits / method resolution C-bug Category: bug

Comments

@lnicola
Copy link
Member

lnicola commented Oct 4, 2021

image

@lnicola lnicola added A-ty type system / type inference / traits / method resolution C-bug Category: bug labels Oct 4, 2021
@flodiebold
Copy link
Member

flodiebold commented Oct 4, 2021

iterate_method_candidates_with_autoref should probably just return an empty result when called with a free variable as receiver.

@iDawer
Copy link
Contributor

iDawer commented Oct 4, 2021

I guess this happened after #10373 ? 😄
I bet that here
https://github.com/rust-analyzer/rust-analyzer/blob/cf1ea9d0b9241265369b3ba869795ac9bda9f146/crates/hir/src/lib.rs#L2592-L2602
the {error} type gets replaced with bound variable and that may implement all(?) traits in scope. May be shouldn't call replace_errors_with_variables on the {error} type itself?

@flodiebold
Copy link
Member

I guess this happened after #10373 ?

Yes.

May be shouldn't call replace_errors_with_variables on the {error} type itself?

No, that's fine. Note that the type of x in the example is a type variable originally. The problem here is just trying to do method resolution on a method receiver that's a variable, which rustc will just refuse to do.

@lnicola
Copy link
Member Author

lnicola commented Oct 25, 2021

Something like:

diff --git i/crates/hir_ty/src/method_resolution.rs w/crates/hir_ty/src/method_resolution.rs
index 8e6ab8af0..5ba637ef4 100644
--- i/crates/hir_ty/src/method_resolution.rs
+++ w/crates/hir_ty/src/method_resolution.rs
@@ -581,6 +581,10 @@ fn iterate_method_candidates_by_receiver(
     name: Option<&Name>,
     mut callback: &mut dyn FnMut(&Canonical<Ty>, AssocItemId) -> ControlFlow<()>,
 ) -> ControlFlow<()> {
+    match receiver_ty.value.kind(&Interner) {
+        chalk_ir::TyKind::BoundVar(_) => return ControlFlow::Break(()),
+        _ => {}
+    }
     // We're looking for methods with *receiver* type receiver_ty. These could
     // be found in any of the derefs of receiver_ty, so we have to go through
     // that.

Helps in this case, but seems to break for integral types.

(Not sure if that's supposed to be a BoundVar).

@flodiebold
Copy link
Member

Yeah, it's supposed to be a bound var. We can check on the binders of the canonical whether it's a general or an int/float variable.

@iDawer
Copy link
Contributor

iDawer commented Nov 2, 2021

From my limited understanding Chalk treats bound vars as existentially quantified and placeholders as universally quantified, right? Since hir_ty::replace_errors_with_variables uses bound vars it getting ambiguous solution for queries like exists<T> { T: AnyTrait }.

Cant remember but I saw somewhere talking about special handling of sized bound on type vars (or errors?) on Chalk side. So could we hack it on RA side with FromEnv({error}: Sized)? I applied this in hir::Type::iterate_method_candidates_dyn and it seems solved both this and #10297 issues.

Thinking on how the hack could break, the {error} type will satisfy : Sized and : ?Sized bounds. Luckily, : !Sized bounds are impossible 😄

@flodiebold
Copy link
Member

Bound vars can be existentially or universally quantified, it depends on what they refer to. And no, we shouldn't add hacks for the error type; replacing the errors back with variables, which is what they were during type inference, is the correct thing to do.

@kornelski
Copy link
Contributor

Please consider special-casing this for #8468 by disabling auto-imports. Auto-complete suggestions of potentially irrelevant methods isn't a big deal. However, auto-import of definitely incorrect traits that break correctly-picked inherent methods is a real problem.

bors bot added a commit that referenced this issue Mar 23, 2022
11805: fix: Don't try to resolve methods on unknown types r=Veykril a=flodiebold

Fixes #10454, and some type mismatches.

Co-authored-by: Florian Diebold <[email protected]>
@bors bors bot closed this as completed in 8d98f3c Mar 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ty type system / type inference / traits / method resolution C-bug Category: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants