fix: Wrong closure kind inferences for closures with predicates

[ShoyuVanilla]

Currently, the Rust Analyzer infers the kind(Fn, FnMut or FnOnce) of a closure only with its captures.
But the Rust compiler utilizes obligations and predicates(with here) to infer its kind whenever available (for example, coercing into a bounded argument), and only uses capture informations when not.
So, this makes false positive errors as we can see in #16421,
or more simpler cases like;

which is compiled and runs without any error on Rust compiler.

This PR gathers FnTrait predicates from obligations and uses them inside infer_closures to infer closure kinds, and fixes some wrong impl FnX inferences in preexisting test codes.

Fixes #16421

[rustbot]

There are merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged.

You can start a rebase with the following commands:

$ # rebase
$ git rebase -i master
$ # delete any merge commits in the editor that appears
$ git push --force-with-lease

The following commits are merge commits:

• d31f22d

[bors]

☔ The latest upstream changes (presumably #16454) made this pull request unmergeable. Please resolve the merge conflicts.

[Veykril]

The compiler also inspects the expected type first and foremost, and only then does it deduce_closure_signature_from_predicates if the expected type is a type variable or opaque alias. We probably should do the same (as much as we can, given our infra is not the same), https://github.com/rust-lang/rust/blob/11f32b73e0dc9287e305b5b9980d24aecdc8c17f/compiler/rustc_hir_typeck/src/closure.rs#L214

[Veykril]

Should use unwrap_or_else(|| self.closure_kind_from_capture()) for lazyiness

[ShoyuVanilla]

I was planning to implement a diagnostic here as the comments above says. I'll try it and will convert into unwrap_or_else if it is not possible

[Veykril]

Not too happy with this, can't we just iterate through the pending obligations when necessary? (as is what rustc does)

[ShoyuVanilla]

The chalk resolves obligations like FnOnce immediately, so they are not remained in the pending obligations. The most problematic thing is that it infers the closures with FnOnce predicate as implementing FnMut and Fn

[ShoyuVanilla]

Maybe I should look into it more, but I'm not sure that it is intended chalk functionality or not

[ShoyuVanilla]

Anyway, I'm gonna take a deep look into the chalk closure inferences. I might fix this from upstream or maybe RA is asking chalk without some details

[ShoyuVanilla]

@Veykril I had a closer into RA and chalk, and I think that we couldn't use pending obligations like rustc does.

When the chalk has to solve a goal that whether some closure implements some FnTrait;

1. It asks RustIrDatabase for the kind of our closure and checkes if the kind of closure is compatible with the given FnTrait
2. If so, it pushes the clause for the "FACT" that our closure implements that trait.

So, solving whether the given closure implements FnTrait is heavily depends on RustIrDatabase and I think that's reasonable for it to do so because what chalk wants to be(though it is not going to be actively maintained due to new trait solver in rustc) is an abstract trait solver, not a very definite rustc inference maker.

But since RA's ChalkContext, which is our RustIrDatabase implementation, always returns Fn as the kind of a closure. (I think that this is sort of a heuristic to avoid having other inferences and coercions delayed or unresolved.)
Thus, the obligations like closure{n}[...] implements FnX are always solved into Unique solutions and not remain as "pending" because they are immediately solved.

Giving the "correct" kind of a closure doesn't work for this because;

1. We need to use "correct" pending obligations for closure kind inferencing
2. But the "correct" pending obligations are what we are going to do with "correct" kind of a closure, a cyclic dependency!

I've tried just giving FnOnce as a closure_kind but aside from that it makes lots of inferences in our test cases unresolved - if the later were to worked correctly I would have tried fixing those inferences -, it still does not "pend" obligations.
The obligations for closure implementation FnMut and Fn are solved as None, and they are not pended as well.

So, I have concluded that we would have to modify lots of RA inferencing code to do this as rustc does, and this was why I wrote somewhat sad codes for similar thing 😢 (When I wrote it, I hadn't debugged the chalk much, but tried using pending obligations and it was always empty; so I had similar conclusion without much evidence then 😅 )

[ShoyuVanilla]

The compiler also inspects the expected type first and foremost, and only then does it deduce_closure_signature_from_predicates if the expected type is a type variable or opaque alias. We probably should do the same (as much as we can, given our infra is not the same), https://github.com/rust-lang/rust/blob/11f32b73e0dc9287e305b5b9980d24aecdc8c17f/compiler/rustc_hir_typeck/src/closure.rs#L214

Okay, I'll delve into it more

[bors]

☔ The latest upstream changes (presumably #16527) made this pull request unmergeable. Please resolve the merge conflicts.

[ShoyuVanilla]

After some investigations, I found out that I had made some terrible misunderstandings on what rustc does this 😱;

While this is compiled and runs as my previous understandings for this issue;

struct Foo<T: FnOnce()>(T);

fn main() {
    let mut x = 0;
    // let a = || { x = 1; };
    let b = Foo(|| { x = 1 });
    (b.0)();
}

but this is not compiled;

struct Foo<T: FnOnce()>(T);

fn main() {
    let mut x = 0;
    let a = || { x = 1; };
    let b = Foo(a);
    (b.0)();
}

contrary to my previous misunderstandings, as seen in my wrong test cases.

I'll close this and soon reopen new PR with proper fix, and I think that I could fix the thing in similar way the rustc does, as you said.
Sorry for wasting your time 😢

[Veykril]

No problem, and thanks for investigating!