&raw mut of a static mut treated as simultaneously safe and unsafe

[cbiffle]

On 1.82.0, rust-analyzer seems to have mixed opinions about place expressions in the newly stabilized &raw syntax. This bug also affects uses of the older addr_of/addr_of_mut macros. Specifically, it reports an error that was valid in 1.81 but is not correct in 1.82, and also warns (correctly) if you fix the "error" it describes.

rust-analyzer version: the one distributed with 1.82.0 through rustup (instructions were only given for finding the version through VScode, which I don't use): rust-analyzer 1.82.0 (f6e511e 2024-10-15)

rustc version: rustc 1.82.0 (f6e511eec 2024-10-15)

editor or extension: neovim + rustaceanvim

relevant settings: Not sure if it's relevant, but I am cross-compiling for thumbv7em-none-eabihf on an x86 linux host.

code snippet to reproduce:

static mut BUFFER: [u8; 8] = [u8; 8];
// this line is accepted by rustc but gets a big scary error diagnostic
// from rust-analyzer:
let array = &raw mut BUFFER;
// on this line, rust-analyzer insists the unsafe block is unused
// (which is correct on 1.82 but inconsistent with the above)
let array = unsafe { &raw mut BUFFER };

[ChayimFriedman2]

rust-analyzer does not (yet) natively report unused unsafe (so it must come from rustc), and it's also been updated to not require unsafe from &raw [mut] path like rustc, so this is weird. I also cannot reproduce this. Do you have an outdated version of rust-analyzer?

[cbiffle]

I'd be surprised since rustup is managing it via rust-toolchain.toml, but, let me do some testing just to verify. May not happen for a few hours, sorry.

[thejpster]

Rust 1.81:

Rust 1.82:

There's also a typo on the first line (it should be [0u8; 8]) but that doesn't change what I see.

I'm on VS Code 1.94.2 with extension v0.4.2151 (pre-release).

[cbiffle]

Interesting. So with this code: https://github.com/cbiffle/ra-raw-repro

Using rust-analyzer at: /home/cbiffle/.rustup/toolchains/1.82-x86_64-unknown-linux-gnu/bin/rust-analyzer (so not an old version)

I see this:

(it remains if I fix the unused variable warning.)

neovim reports:

rustfmt 1.7.1-stable (f6e511e 2024-10-15)
rustc 1.82.0 (f6e511eec 2024-10-15)
cargo 1.82.0 (8f40fc59f 2024-08-21)

NVIM v0.10.2
Build type: RelWithDebInfo
LuaJIT 2.1.1727870382

[thejpster]

OK, I see that in your repro.

It goes away if I remove the rust-toolchain.toml file.

[thejpster]

I played with the rust-toolchain.toml file, and even if I use this I see the error.

[toolchain]
channel = "stable"

[ChayimFriedman2]

Probably the r-a on rustup is outdated.

[thejpster]

Ah, I see a difference in versions. With the toolchain file, it says Server Version 1.82 in the pop-out, but without a toolchain file it says Server version 0.4.2151-standalone. So ... a toolchain file makes it use the r-a shipped with Rust 1.82 and not whatever the r-a version the extension is bundling? And the version in Rust 1.82 doesn't know about the new syntax in 1.82?

[cbiffle]

My systems don't have bundled rust-analyzers, I only use the one from rustup and avoid relying on ambient default toolchains. So, that may be why I tripped over this.

Probably the r-a on rustup is outdated.

Well, I posted the git rev output from rust-analyzer on rustup above, is it outdated? I'm a user, I don't know the project or release process.

[ChayimFriedman2]

Yeah, it looks like this version just missed the fix (#18003).

CC @lnicola, do we have something to do here?

[ChayimFriedman2]

As a workaround, you can add missing-unsafe to rust-analyzer.diagnostics.disabled.

[Veykril]

We missed the beta cutoff for 1.82 with the fix on our side, so there is little we can do here aside from point people to use the config that was mentioned or using a non rustup rust-analyzer.

Might be a good idea to raise that to the lang team for stabilization requirements to check this kinda stuff as more and more people seem to be relying on rustup releases now. -> raised https://rust-lang.zulipchat.com/#narrow/channel/213817-t-lang/topic/Stabilization.20requirements.20for.20language.20changes

[workingjubilee]

Might be a good idea to raise that to the lang team for stabilization requirements to check this kinda stuff as more and more people seem to be relying on rustup releases now. -> raised https://rust-lang.zulipchat.com/#narrow/channel/213817-t-lang/topic/Stabilization.20requirements.20for.20language.20changes

T-lang took note of the PR but did not FCP the change, as it was considered a bug fix and not a stabilization. Thus, asking for this to be added to T-lang's stabilization process has the odd consequence that if the process had already been modified, it would not have been applied.

I merely note this because I would be happy to try to make sure some testing is done, but I would not have had my Big Book of Rust Process open to that particular chapter.

[ChayimFriedman2]

Closing as unactionable from our side.

[lnicola]

As a workaround, you can add missing-unsafe to rust-analyzer.diagnostics.disabled.

Another one would be to install nightly and only use r-a from it.

[cbiffle]

So is this the sort of thing that might cause a 1.82.1 to happen, or (for those of us who use rust-analyzer and other tools pinned to our toolchain version) does this just mean 1.82 is permanently not usable? (I don't have a lot of insight into the rustup vs ra vs T-lang dynamics here.)

[lnicola]

I'm personally not sure this is worth an 1.82.1 release. It's limited in scope (the majority of codebases won't use &raw mut), it has three supported workarounds (disabling the diagnostic, using a source/GitHub build, using a nightly r-a), and the rustup component is actually the hardest to support because we've never backported any patches.

We'll try to do more regular syncs, but they're sometimes not possible because of downstream changes.

[workingjubilee]

@cbiffle

• Rustup doesn't decide what toolchains ship, it just pulls what T-release puts into the buckets.
• The rust-analyzer subtree... the thing that is the reason we can regularly ship a rust-analyzer with rustc... only stays updated if someone syncs it into the rust-lang/rust repo.
• T-release is the ultimate decisionmaker on what gets backported and what gets released, however, they generally tend to defer to the teams in question.
• The threshold for backporting nightly patches to the beta toolchain is pretty low, especially on the first week of the beta cycle. As long as the change is fairly contained, we are confident it won't have side effects, and it deals with some sort of problem, it probably gets pulled in. Especially if it's early in the cycle.
• The threshold for backporting nightly patches to stable is very high. The usual reason is "a CVE" or "critical usability bug that breaks tests for large swathes of the ecosystem".

The ideal situation would have been for rust-analyzer to ask for the relevant patch to be backported to the beta toolchain. Which also implies the best time to report this, unfortunately, would have been six weeks ago.

I also use the development configuration you use, @cbiffle, and I do FFI nonsense, so I would notice this... except I'm usually using nightly rustc for development. And I believe rust-analyzer's development team mostly focuses on their own releases via the extension, which are not really affected by this, as lnicola said.

[davidbarsky]

I don't know enough about rustup or T-release, but would it be possible for rustup to pull a rust-analyzer release from a GitHub release? I'm not sure if the whole beta/nightly process that rustc follows is particularly useful for rust-analyzer, especially if rust-analyzer is not in the larger rustc monorepo.

[workingjubilee]

@davidbarsky The exact reason that we started shipping rust-analyzer as a subtree is because doing what you describe would result in rust-analyzer breaking, much more severely, for all nightly users.

[davidbarsky]

@workingjubilee wait, which things specifically would break? The result of the current process is that @cbiffle's workflow is broken, but I'm trying to understand how exactly we got here.

I assume that most of the nightly breakage would come from changing the proc macro ABI, but the proc macro expansion server we use mitigates a large amount of that, no?

[Veykril]

rustup shipping github artifacts would be incredibly confusing to me. The point of rustup's toolchain system is that you get the exact same thing for a given toolchain.

[Veykril]

It's unfortunate that this feature (and another one) is broken on 1.82.0, but unfortunately we did not realize that we fixed this after the beta branching nor did anyone report this issue being on the beta release.

There isn't anything we can do to remedy this aside from improving things such that this doesn't repeat again.

[davidbarsky]

To be clear: I think people should be getting the same exact thing, but it's not clear to me what rust-analyzer gets out of participating in rustc's release train. I personally view https://github.com/rust-lang/rust-analyzer/releases/tag/2024-10-21 + the weekly changelog as the de-facto stable release for rust-analyzer.

[traviscross]

Ideally what it would get out of the participating in the release train is that companies supporting a large number of r-a users would mark a subset of those as "beta-friendly" and move those users over to the beta-branch when that it is cut, six weeks ahead of the release, and then report to us any issues uncovered in that way.

That is, ideally it would get prerelease QA out of this by people testing the beta branch inclusive of r-a.

[lnicola]

Note that six weeks is a relatively long time compared to our one-week release interval. In my experience, most regressions get fixed in the same week they are introduced, and almost all get fixed the next week. Six weeks is enough time to go through multiple cycles of regressions and fixes

[davidbarsky]

Ideally what it would get out of the participating in the release train is that companies supporting a large number of r-a users would mark a subset of those as "beta-friendly" and move those users over to the beta-branch when that it is cut, six weeks ahead of the release, and then report to us any issues uncovered in that way.

That is, ideally it would get prerelease QA out of this by people testing the beta branch inclusive of r-a.

I'm primarily interested in finding mechanisms to ensure that folks like Cliffe doesn't run into issues like these again. I was privately discussing with @workingjubilee about potential options, which include:

1. A checklist item for compiler changes along the lines of "notify r-a if this seems like it affects significant diagnostic support for them"
2. Once rust-analyzer on the new trait solver, I think it'd be pretty feasible to run rust-analyzer against large swathes rustc's UI test suite so that the checklist item is less load-bearing for folks. I know that Lukas disagrees about the feasibility, but I feel like this would be a good thing to pursue, however limited.

I think Jubilee and I still disagree about the utility of a six week bake time for rust-analyzer for the reasons that Laurențiu shared; I would personally like a way to shorten that window.

@traviscross: I'd be happy to discuss the volume and frequency of feedback that my team can offer on Zulip; I don't want to derail this issue any more than necessary.