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

Rust-Analyzer shows no diagnostics from check build #13057

Closed
RalfJung opened this issue Aug 18, 2022 · 24 comments
Closed

Rust-Analyzer shows no diagnostics from check build #13057

RalfJung opened this issue Aug 18, 2022 · 24 comments
Assignees
Labels
A-diagnostics diagnostics / error reporting

Comments

@RalfJung
Copy link
Member

RalfJung commented Aug 18, 2022

Sometimes when I open Miri in vsocde, I get no diagnostics and RA says build scripts don't work. I now found this in the error log:

[ERROR rust_analyzer::main_loop] File with cargo diagnostic not found in VFS: file not found: /home/r/src/rust/miri/cargo-miri/src/stacked_borrows/mod.rs

(and a lot of similar ones)

This is correct, there is no such file. The correct location is /home/r/src/rust/miri/src/stacked_borrows/mod.rs.

I think this is related to #10793: the workspace settings are

    "rust-analyzer.linkedProjects": [
        "./Cargo.toml",
        "./cargo-miri/Cargo.toml"
    ],
    "rust-analyzer.checkOnSave.overrideCommand": [
        "./miri",
        "check",
        "--message-format=json"
    ],

To work around #10793, we have ./miri not just in the root folder but also in cargo-miri. They both run the same script. But unfortunately sometimes that means RA interprets the filenames in the diagnostics as being relative to the cargo-miri subdir, which they are not.

We can make cargo-miri/miri a NOP, but then RA shows no diagnostics at all inside cargo-miri. Running the checkOnSave.overrideCommand will produce those diagnostics, but RA usually ignores them. So it seems sometimes diagnostics for 'other' projects are ignored, so we need both ./miri and ./cargo-miri/miri to produce output, but sometimes they are not ignored, which then breaks RA entirely.

Is there any way at all to reliably use RA with multiple linked projects in different directories, and a relative path in checkOnSave.overrideCommand? Currently, all work-arounds that I know entirely fail sometimes. :( (This affects not only Miri, but also rustc itself, where RA does not work in the crates that are not in the main rustc workspace, such as bootstrap or codegen_cranelift.)

@Veykril Veykril added the A-diagnostics diagnostics / error reporting label Aug 18, 2022
@RalfJung
Copy link
Member Author

Oh, actually, maybe those errors are not the actual cause of the problem? I made cargo-miri/miri a NOP, which made these errors go away, but RA still says build scripts don't work, and it shows no diagnostics. The server logs are not very helpful:

[ERROR rust_analyzer::lsp_utils] failed to run build scripts

I guess it's time to disable build scripts again -- when they fail (which happens fairly regularly for me), they seem to take all the rest of the RA features down with them.

@RalfJung
Copy link
Member Author

RalfJung commented Aug 18, 2022

Hm, no... no build scripts, no error message, but also no diagnostics. ./miri check fails but vscode just refuses to show the errors. I have no idea what's going on. I just wanted to quickly do some hacking and now I am debugging the IDE again...

The weirdest thing is, it worked fine half an hour ago. I have not changed any configuration.

@Veykril
Copy link
Member

Veykril commented Aug 18, 2022

I'll try to get the miri script running on my windows machine tomorrow (that is get bash to run I suppose) given you seem to find a lot of peculiar issues with r-a with that setup that we don't seem to see reported elsewhere so I can dig into those myself.

@RalfJung
Copy link
Member Author

I am told it works with the bash in git-for-windows.

@RalfJung
Copy link
Member Author

RalfJung commented Aug 18, 2022

But note that most of these issues also apply to rustc itself, and its ./x.py. It's not really different from that.

What happens this time though where it just silently drops all errors and pretends like nothing happens... no idea, I don't think I saw that before.

@Veykril
Copy link
Member

Veykril commented Aug 18, 2022

I'd assume so, it's more the question how to get r-a to invoke it via that

@Veykril
Copy link
Member

Veykril commented Aug 18, 2022

Rustc usually works fine for me, though I don't hack on the codebase, I only use r-a to browse there which mostly works.

@RalfJung
Copy link
Member Author

RalfJung commented Aug 18, 2022

Ah, the issue also affects the rustc repo. Ctrl-S does nothing (a check-build of rustc after changing libcore takes at least a minute, so it should be sitting there for a bit telling me it is doing a check-build, but it does not even flicker), and no errors are shown. So I assume an RA update was auto-installed in the last half an hour and that broke everything.

Strangely, in Miri, when I do Ctrl-S it does pretend to run ./miri ..... It just doesn't show the results. In contrast, in rustc I don't even see it running anything.

@RalfJung RalfJung changed the title Rust-Analyzer interprets diagnostics for the wrong working directory Rust-Analyzer shows no diagnostics from check build Aug 18, 2022
@Veykril
Copy link
Member

Veykril commented Aug 18, 2022

Can reproduce it in miri, native diagnostics don't work for me there either. Will go dig around what the cause might (probably don't have the time to tonight)

@RalfJung
Copy link
Member Author

Downgrading to v0.3.1162 doesn't help either though. Why did this suddenly just break when I restarted vscode...?

@RalfJung
Copy link
Member Author

Is there some component of RA that is tracked and updated separately? The status message when doing the check-on-save build used to be cargo check even when I had another command configured. Now it shows ./miri .... Even after downgrading it still does that. Looks like it did not completely downgrade everything?

@RalfJung
Copy link
Member Author

Oh but now it shows diagnostics again. 🙏

@Veykril
Copy link
Member

Veykril commented Aug 18, 2022

The status message when doing the check-on-save build used to be cargo check even when I had another command configured. Now it shows ./miri ....

This behavior should be quite a bit older already though I believe. I wonder if VSCode mixed your installs, iirc there have been very few occurences where the client and server versions did not match up properly for some people for some reason

@Veykril
Copy link
Member

Veykril commented Aug 18, 2022

OKay, turns out using git bash did not work which fooled me instead. Managed to get it running and now I also see a ton of File with cargo diagnostic not found in VFS: messages, I do see diagnostics in the src directory though. OTOH I don't see any in cargo-miri.

@RalfJung
Copy link
Member Author

RalfJung commented Aug 18, 2022

This behavior should be quite a bit older already though I believe.

Strange, I am pretty sure it was cargo check still this morning.

Well, after downgrading and re-upgrading, diagnostics in Miri seem to work again.

In rustc turns out this was my fault, I had changed the checkOnSave command to x ... when it should have been ./x .... So that also works again.

The issue disappeared as mysteriously as it appeared...

@RalfJung
Copy link
Member Author

cargo-miri diagnostics work for me with these settings

// Place your settings in this file to overwrite default and user settings.
{
    "rust-analyzer.rustc.source": "discover",
    "rust-analyzer.linkedProjects": [
        "./Cargo.toml",
        "./cargo-miri/Cargo.toml"
    ],
    "rust-analyzer.checkOnSave.overrideCommand": [
        "./miri",
        "clippy",
        "--message-format=json"
    ],
    // Contrary to what the name suggests, this also affects proc macros.
    "rust-analyzer.cargo.buildScripts.overrideCommand": [
        "./miri",
        "check",
        "--message-format=json",
    ],
}

They only show up when main Miri builds successfully though, I think.

@RalfJung
Copy link
Member Author

RalfJung commented Aug 18, 2022

Strange, I am pretty sure it was cargo check still this morning.

Ah! It's cargo check whenever there are multiple linked projects. I had changed that earlier while furiously debugging how to make the diagnostics show up again in Miri.

@Veykril
Copy link
Member

Veykril commented Aug 18, 2022

Strange, I am pretty sure it was cargo check still this morning.

Ye I just checked, we show the override command, unless more than one check processes are running, then we show cargo check (#1) etc.

@Veykril Veykril self-assigned this Aug 18, 2022
@Veykril
Copy link
Member

Veykril commented Aug 19, 2022

Alright, so the main issue here (things not working at all) fixed itself after switching around with your extension versions if I understand correctly?

The other issue that spawns the error log messages is #10793 #5962 I believe if I see this right. Which is what I would investigate now, or rather think about how to solve as I see it its a lack of configurability.

They only show up when main Miri builds successfully though, I think.

If you save a file we now only trigger checks for the workspaces that contain the file. The cargo-miri workspace won't get diagnostic no matter what due to the file not found issue

So regarding #10793 #5962, the issue here is that the miri check invoked in cargo-miri reports diagnostics of the top-level src directory which we then remap onto the cargo-miri workspace path resulting in non-existant paths. How are you supposed to run miri check on the cargo-miri workspace?

@RalfJung
Copy link
Member Author

Alright, so the main issue here (things not working at all) fixed itself after switching around with your extension versions if I understand correctly?

Looks like it, yes...

If you save a file we now only trigger checks for the workspaces that contain the file. The cargo-miri workspace won't get diagnostic no matter what due to the file not found issue

Right, but I did save a file in cargo-miri.

However I realized now that ./miri only moves on to cargo-miri after miri succeeded, so I guess this is expected.

How are you supposed to run miri check on the cargo-miri workspace?

./miri check checks both of them. So only one command is needed to cover both workspaces. When working on the terminal, that is the most convenient way. It is also how x.py check works (which currently even builds things from 3 workspaces).

However I can see how that is problematic for RA... errors in cargo-miri show up as

error[E0599]: no method named `unwraps` found for enum `std::option::Option` in the current scope
  --> src/main.rs:21:17

and there is no way for RA to know what that filename is relative to. (This is annoying even for humans.)

@RalfJung
Copy link
Member Author

So what I was hoping for originally is some way to set a single command to be used across all linked projects/crates/workspaces -- rather than what RA currently does, which is to run this command separately for each workspace. But while rust-lang/cargo#11007 remains unsolved, this might not be possible?

The problem with running the command for all workspaces is that the command doesn't even know which workspace to build, and a miri script needs to exist in the root directory of each workspace. (And rustc has the same problem, we'd like to have RA support in the bootstrap workspace but nobody figured out a way to do that yet.)

For Miri what I could imagine is that rather than overwriting the entire command, we could have a way to wrap the existing command. Miri could then use that to set the environment variables it needs to set, and forward the rest to cargo, thus allowing the caller to select the project they want to have built. I am not sure if that would work for rustc, though...

@RalfJung
Copy link
Member Author

RalfJung commented Aug 19, 2022

I think rust-lang/miri#2495 helps with this on the Miri side.

IMO what RA could do here is, rather than changing the working directory to go check the different linked workspaces, it could stay in the same working directory and pass --manifest-path. That would avoid the need for the cargo-miri/miri script. However that would require some way for overrideCommand to indicate where the manifest path should go... also it'd break anyone who set cargo clippy --message-form=json as their command. It could just hope that appending --manifest-path ... to the end works, but that certainly won't work for rustc's x.py...

At the least, it would be good to extend the docs for rust-analyzer.checkOnSave.overrideCommand to say that with multiple projects, this command will be invoked for each of them, with the working directory set to the project root.

So for Miri we have a pretty good work-around now I think, that should avoid any ignored diagnostics by only building the project that RA asked to build. But for rustc (in particular, rustc bootstrap) we don't have a solution. The --manifest-path thing could work, it'd just require adding support for that flag to x.py.

@RalfJung
Copy link
Member Author

For what we are discussing here, this should probably be closed as a duplicate of #10793 ?

@Veykril
Copy link
Member

Veykril commented Aug 19, 2022

Ye I think we can close this as a dupe and continue discussion in that issue. (I'll read through this and drop my input in the other issue after the weekend)

@Veykril Veykril closed this as completed Aug 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics diagnostics / error reporting
Projects
None yet
Development

No branches or pull requests

2 participants