Skip to content

Conversation

lowr
Copy link
Contributor

@lowr lowr commented Dec 24, 2022

Fixes #13838

The problem is similar to #13223: we've been skipping non-empty binders, letting lifetime bound variables escape.

I ended up refactoring hir_ty::callable_sig_from_fnonce(). Like #13223, I chose to make use of InferenceTable which is capable of handling variables (I feel we should always use it when we solve trait-related stuff instead of manually building obligations/queries).

I couldn't make up a test that crashes without this patch (since the function I'm fixing is only used outside hir-ty, simple hir-ty test wouldn't cause crash), but at least I tested with my local build and made sure it doesn't crash with the code in the original issue. I'd appreciate any help to find a regression test.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 24, 2022
@flodiebold
Copy link
Member

I couldn't make up a test that crashes without this patch (since the function I'm fixing is only used outside hir-ty, simple hir-ty test wouldn't cause crash), but at least I tested with my local build and made sure it doesn't crash with the code in the original issue. I'd appreciate any help to find a regression test.

I think it'd be fine to add a syntax highlighting test as a regression test for this.

(I feel we should always use it when we solve trait-related stuff instead of manually building obligations/queries)

👍

@bors delegate+

@bors
Copy link
Contributor

bors commented Dec 25, 2022

✌️ @lowr can now approve this pull request

@lowr
Copy link
Contributor Author

lowr commented Dec 25, 2022

I tried adding a syntax highlighting test as well without success actually - turns out I had to add impl<A, F> FnOnce<A> for &F in order to cause crash. You know, I'm something of a beginner myself 😅

@bors r+

@bors
Copy link
Contributor

bors commented Dec 25, 2022

📌 Commit a1a4083 has been approved by lowr

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Dec 25, 2022

⌛ Testing commit a1a4083 with merge 74ae2dd...

@bors
Copy link
Contributor

bors commented Dec 25, 2022

☀️ Test successful - checks-actions
Approved by: lowr
Pushing 74ae2dd to master...

@bors bors merged commit 74ae2dd into rust-lang:master Dec 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Panic when analyzing &fn(&dyn Trait)
4 participants