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

Unable to click into include!'d sources; possibly spurious need-mut lint #14562

Closed
davidbarsky opened this issue Apr 13, 2023 · 6 comments
Closed
Labels
C-bug Category: bug

Comments

@davidbarsky
Copy link
Contributor

I'm trying out 14561—it's wonderful to be able to get definitions from protoc-generated code!—but unfortunately, I'm getting a bunch of need-mut errors from rust-analyzer at https://github.com/facebook/buck2/blob/f9e261bc956d48f587bfe6777b4de84ec1898bdc/app/buck2_data/src/lib.rs#L161.

(this could be a nightly toolchain-related thing or a prost-related thing; I haven't diagnosed this yet). I'm encountered two issues:

  1. I can't click into the include!'d code. I'm guessing this might be followup work, but it'd be nice!
  2. need-mut might be spurious, since rustc is seemingly fine with it.
    1. This might be related to prost more than anything.

rust-analyzer version: rust-analyzer version: 0.0.0 (901c8a4 2023-04-13)

rustc version: (eg. output of rustc -V) rustc 1.69.0-nightly (c8e6a9e8b 2023-01-23) (it's a RUSTC_BOOTSTRAP. long story.)

@davidbarsky davidbarsky added the C-bug Category: bug label Apr 13, 2023
@davidbarsky davidbarsky changed the title Feedback on #14561 Unable to click into include!'d sources; possibly spurious need-mut lint Apr 13, 2023
@Veykril
Copy link
Member

Veykril commented Apr 13, 2023

I can't click into the include!'d code. I'm guessing this might be followup work, but it'd be nice!

If you mean not having any ide features in the included file, that's to be expected. Handling those as proper input will require #9403 and even with it, it's not 100% certain we'll be able to support that.

need-mut might be spurious, since rustc is seemingly fine with it.

Would be great if you could figure out a repro that causes it if it's not too much work (though I think we don't currently honor the corresponding rustc allow lint, so if thats used there than it might be expected)

@HKalbasi
Copy link
Member

though I think we don't currently honor the corresponding rustc allow lint

That is for unused-mut, as need-mut is a hard error in rustc, so it is always a bug.

@davidbarsky my r-a doesn't work on buck2 repository, should I run something to generate rust-project.json?

@davidbarsky
Copy link
Contributor Author

davidbarsky commented Apr 13, 2023

That is for unused-mut, as need-mut is a hard error in rustc, so it is always a bug.

gotcha.

@davidbarsky my r-a doesn't work on buck2 repository, should I run something to generate rust-project.json?

You shouldn't need to—I'd suggest cding into app/buck2_data/ and opening rust-analyzer there. Cargo should just work, as that's how I was able to discover this issue in the first place.

(I'm working on open-sourcing the tool that generates the rust-project.json right now. I'll let you know when it's out; it'll be under the buck2 repo.)

@HKalbasi
Copy link
Member

Minimized:

fn x() {
    let x = &mut 2;
    || *x = 5;
}

@Veykril
Copy link
Member

Veykril commented Apr 13, 2023

Ah, those are unique immutable borrows https://doc.rust-lang.org/stable/reference/types/closure.html#unique-immutable-borrows-in-captures

bors added a commit that referenced this issue Apr 14, 2023
Fix explicit deref problems in closure capture

fix the `need-mut` part of #14562

Perhaps surprisingly, it wasn't unique immutable borrow. The code still doesn't emit any of them, and I think those won't happen in edition 2021 (which is currently the only thing implemented), since we always capture `&mut *x` instead of `&mut x`. But I'm not very sure about it.
@davidbarsky
Copy link
Contributor Author

If you mean not having any ide features in the included file, that's to be expected. Handling those as proper input will require #9403 and even with it, it's not 100% certain we'll be able to support that.

No, not that—I couldn't enter the file at all, but it seems to be working now. I think that functionality was fixed by #14576.

(I'm working on open-sourcing the tool that generates the rust-project.json right now. I'll let you know when it's out; it'll be under the buck2 repo.)

I've got it open-sourced yesterday here: https://github.com/facebook/buck2/tree/main/integrations/rust-project. There's some fb-specific stuff that lives there at the moment (e.g., locations of the sysroot) which I'll fix today.

Since the issues I've raised have been resolved, I'll close this out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug
Projects
None yet
Development

No branches or pull requests

3 participants