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

Simpler diagnostic when passing arg to closure and missing borrow #76362

Closed
wants to merge 1 commit into from
Closed

Simpler diagnostic when passing arg to closure and missing borrow #76362

wants to merge 1 commit into from

Conversation

krupitskas
Copy link

Hey! It's a working draft for the #64915 issue.
I've started it with help from @estebank but at this point, I have some questions, which I will be glad if someone will little clear it up a bit.
As @estebank suggested, I've done a small initial approach and the next steps are:

If you look there that has an FnDecl with a field inputs that has data for each closure arg
But this is on the HIR, which is the internal representation of what the user wrote, where the error deals in ty::Ty
So we need to look at those, identify what the difference was
Go back to the hir
Find the argument type
And extract the span of that type to add a & in front of it in the same way you already did

So I've got this FnDecl with an array of Ty's (but how to understand which ty from this array should I get, i mean borrow problem can be 2nd 3rd.. any argument in array?)

error deals in ty::Ty
As I see, Ty has TyKind and I can be asked is it Rptr, but where to get another Ty to compare it kinds?

Go back to the hir, Find the argument type
This is about TyKind, right?

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nikomatsakis (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 5, 2020
@krupitskas krupitskas closed this Sep 14, 2020
Manishearth added a commit to Manishearth/rust that referenced this pull request Nov 11, 2022
…stic_when_passing_arg_to_closure_and_missing_borrow, r=estebank

Simpler diagnostic when passing arg to closure and missing borrow

fixes rust-lang#64915

I followed roughly the instructions and the older PR rust-lang#76362.
The number of references for the expected and the found types will be compared and depending on which has more the diagnostic will be emitted.

I'm not quite sure if my approach with the many `span_bug!`s is good, it could lead to some ICEs. Would it be better if  those errors are ignored?

As far as I know the following code works similarly but in a different context. Is this probably reusable since it looks like it would emit better diagnostics?
https://github.com/rust-lang/rust/blob/a688a0305fad9219505a8f2576446510601bafe8/compiler/rustc_hir_analysis/src/check/demand.rs#L713-L1061

When running the tests locally, a codegen test failed. Is there something I can/ should do about that?

If you have some improvements/ corrections please say so and I will happily include them.

r? `@estebank` (as you added the mentoring instructions to the issue)
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 13, 2022
…ic_when_passing_arg_to_closure_and_missing_borrow, r=estebank

Simpler diagnostic when passing arg to closure and missing borrow

fixes rust-lang#64915

I followed roughly the instructions and the older PR rust-lang#76362.
The number of references for the expected and the found types will be compared and depending on which has more the diagnostic will be emitted.

I'm not quite sure if my approach with the many `span_bug!`s is good, it could lead to some ICEs. Would it be better if  those errors are ignored?

As far as I know the following code works similarly but in a different context. Is this probably reusable since it looks like it would emit better diagnostics?
https://github.com/rust-lang/rust/blob/a688a0305fad9219505a8f2576446510601bafe8/compiler/rustc_hir_analysis/src/check/demand.rs#L713-L1061

When running the tests locally, a codegen test failed. Is there something I can/ should do about that?

If you have some improvements/ corrections please say so and I will happily include them.

r? `@estebank` (as you added the mentoring instructions to the issue)
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.

3 participants