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

Functions Returning Unresolved Types Wrapped In A Tuple Wrapped In impl Try Causes Strange Errors When ?ing & Destructuring #16801

Closed
danii opened this issue Mar 10, 2024 · 9 comments · Fixed by #16968
Labels
A-diagnostics diagnostics / error reporting A-ty type system / type inference / traits / method resolution C-bug Category: bug

Comments

@danii
Copy link
Contributor

danii commented Mar 10, 2024

This bug requires a very specific condition to occur. First, you must have a function that returns an Option that contains a tuple that contains a type that is not resolved. Once you have that, you must use the function, apply the ? operator on to it's result, and destructure the tuple. After doing that, Rust Analyzer will throw a bogus error on the let statement stating that "non-exhaustive pattern: _ not covered".

This title is quite the mouth full, but this is a rather obscure bug. I'm looking to implement a fix for this issue, so let me know if the team wants this issue fixed; there's a chance this doesn't need to be fixed as this bug inherently requires another bug to trigger.

Minimum Reproducible Code

Here is the minimum reproducible code.

#![allow(dead_code, unused_variables)]

struct Thing;

mod module {
	// This uses another bug to cause rust-analyzer to not resolve `Thing`.
	// More information at issue #16800.
	use self::super::Thing;

	fn create() -> Option<(i32, Thing)> {
		Some((69420, Thing))
	}

	fn consume() -> Option<()> {
		// Error triggered on next line.
		let (number, thing) = create()?;
		Some(())
	}
}

fn main() {}

Here's the output from cargo check on the code.
image

Here's what happens when you hover over the let statement.
image

Version Information

rust-analyzer version: 1.76.0 (07dca48 2024-02-04)
rustc version: 1.76.0 (07dca489a 2024-02-04)

@danii danii added the C-bug Category: bug label Mar 10, 2024
@lnicola
Copy link
Member

lnicola commented Mar 14, 2024

expected <Option<(i32, {unknown})> as Try>::Output, found ({unknown}, {unknown})

I thought we ignore these errors now?

@lnicola lnicola added A-ty type system / type inference / traits / method resolution A-diagnostics diagnostics / error reporting labels Mar 14, 2024
@Veykril
Copy link
Member

Veykril commented Mar 14, 2024

Those types have different constructors so we don't silence them. We only silence mismatches where the types differ in {unknown} vs some other type (i.e. (u32, u32) vs ({unknown}, u32) would get silenced).

@flodiebold
Copy link
Member

Silencing mismatches between unresolved projections and anything else also makes sense though

@roife
Copy link
Contributor

roife commented Mar 22, 2024

I believe this issue is the same as #16800, which has been addressed. So, we can close this issue now.

@lnicola
Copy link
Member

lnicola commented Mar 22, 2024

I read #16800 as "self::super::Thing doesn't resolve" and this issue as "we show a bogus type error when we don't resolve self::super::Thing" (cf. Florian's comment above).

@Veykril
Copy link
Member

Veykril commented Mar 22, 2024

Yes, the cause of this issue was #16800, but the issue here is more about doing

Silencing mismatches between unresolved projections and anything else also makes sense though

We can probably repurpose the UnknownMismatch zipper here

type_mismatches.retain(|_, mismatch| {
mismatch.expected = table.resolve_completely(mismatch.expected.clone());
mismatch.actual = table.resolve_completely(mismatch.actual.clone());
chalk_ir::zip::Zip::zip_with(
&mut UnknownMismatch(self.db),
Variance::Invariant,
&mismatch.expected,
&mismatch.actual,
)
.is_ok()
});
to also just bail out when one side an unresolved projection

@roife
Copy link
Contributor

roife commented Mar 23, 2024

Oh, my apologies, I misunderstood what this issue was about🙊

Following the discussion above, I want to make sure my interpretation is accurate: when one side of mismatch includes an unknown type, we ought to remove it from type_mismatches.

I've implemented this, and it seems addressed the problem described in the issue. If my understanding is correct, I will proceed to submit a PR.

@Veykril
Copy link
Member

Veykril commented Mar 23, 2024

If you mean with unknown type unresolved projections (aka <Foo as Bar>::Assoc being unresolved) then yes, we want to filter those diagnostics.

@roife
Copy link
Contributor

roife commented Mar 24, 2024

I've read some materials on chalk and noted that it presents two methods for expressing projection: AssociatedType(AssocTypeId, Substitution) and Alias(Projection(ProjectionTy { AssocTypeId, Substitution })). And in the issue discussed, <Option<(i32, {unknown})> as Try>::Output is represented as an AssociatedType rather than an Alias.

Should we take both AssociatedType and ProjectionTy into consideration?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics diagnostics / error reporting 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.

5 participants