Sunset RLS

[ehuss]

This removes RLS per the plan outlined in https://blog.rust-lang.org/2022/07/01/RLS-deprecation.html. This replaces the rls executable with a small program which will display an alert telling the user that RLS is no longer available.

An overview of the changes here:

• Removes the rls submodule and replaces it with a small stub program.
• rls is removed from ./x.py install. I do not think users running install will need the stub.
• rls is removed from ./x.py test, it doesn't have any tests.

Other things of note:

• I kept DIST_REQUIRE_ALL_TOOLS even though it is no longer needed, with the thought that it could be useful in the feature. However, I could remove it if desired.
• I kept extra_deps in tool_extended (which allows tools to depend on other things), even though it is no longer needed. This can also be removed if desired.
• This keeps RLS in the macOS pkg installer and the Windows msi installer. I kinda lean towards removing it from those, but I'm not sure? RLS has been removed from pkg and msi.
• This does not remove the analysis component. It is not clear if we should keep this or not. RLS was the primary user, but I think there are a few users that have made tools around it.
• There will be several followup steps after this PR:
  • Updating the toolstate repo to remove RLS.
  • Updating documentation, such as rustc-dev-guide, to remove references to RLS.

The alert looks like this in VSCode:

In Sublime it looks similar.

I would appreciate if others could help test with other editors such as vim or Emacs.

[rust-highfive]

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

[rustbot]

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

[bors]

☔ The latest upstream changes (presumably #100904) made this pull request unmergeable. Please resolve the merge conflicts.

[klensy]

Few updates here and lower sneaked in, but i guess it's ok?

[klensy]

Probably not all removed crates should be removed, as some of them still used somewhere else.

[ehuss]

These were removed based on extensive testing to ensure they aren't used elsewhere. There is some possibility that one of the more obscure platforms may enable one of these features, but that can be resolved via CI.

[klensy]

Uhh, i don't understand. For example, proc-macro2 with friends of quote and syn used literally everywhere: rustc\rustdoc\clippy\cargo\rustfmt.

Yes, removing this entries probably don't break things now, until some of crates decide to add\remove some of features (at which point CI will fail and contributor will be forced to return back here and rollback something).

[ehuss]

That is true of any shared dependency between the tools. Features can be adjusted at any time. The rustc-workspace-hack doesn't cover all of the shared dependencies (nor should it, since not all tools use the same dependencies). I don't think rustc-workspace-hack should proactively add dependencies in the anticipation that they might be needed in the future. Adding new entries is relatively easy.

As for proc-macro2 and quote, I doubt those will ever appear without their default features. It is not really clear from the history why those were added.

[klensy]

Also this can be removed?

rust/compiler/rustc_driver/src/lib.rs

Lines 163 to 180 in 1cff564

	/// Emit diagnostics to the specified location.
	///
	/// Used by RLS.
	pub fn set_emitter(&mut self, emitter: Option<Box<dyn Write + Send>>) -> &mut Self {
	self.emitter = emitter;
	self
	}
	/// Load files from sources other than the file system.
	///
	/// Used by RLS.
	pub fn set_file_loader(
	&mut self,
	file_loader: Option<Box<dyn FileLoader + Send + Sync>>,
	) -> &mut Self {
	self.file_loader = file_loader;
	self
	}

[Mark-Simulacrum]

This looks largely good to me; I think leaving further cleanups of the more interesting decisions (e.g., DIST_REQUIRE_ALL_TOOLS, other things listed in your list) to future PRs should be fine. Definitely great to see a bunch of simplifications.

I think dropping RLS from the pkg and msi installers in this PR makes sense though; usage on those is probably quite low and I'm not sure an "upgrade" there is typically done without checking back to blog posts etc (unlike may be the case with rustup).

A few smaller comments to resolve left inline.

[ehuss]

think dropping RLS from the pkg and msi installers in this PR makes sense though;

Pushed an update that removes it from the msi and pkg installers.

[Mark-Simulacrum]

@bors r+ p=1 rollup=iffy

[bors]

📌 Commit 9b66f46f36d9cb2189a177854e96af54f920bd19 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

[bjorn3]

Also this can be removed?

rust/compiler/rustc_driver/src/lib.rs

Lines 163 to 180 in 1cff564

	/// Emit diagnostics to the specified location.
	///
	/// Used by RLS.
	pub fn set_emitter(&mut self, emitter: Option<Box<dyn Write + Send>>) -> &mut Self {
	self.emitter = emitter;
	self
	}
	/// Load files from sources other than the file system.
	///
	/// Used by RLS.
	pub fn set_file_loader(
	&mut self,
	file_loader: Option<Box<dyn FileLoader + Send + Sync>>,
	) -> &mut Self {
	self.file_loader = file_loader;
	self
	}

No, I don't think so. While RLS is the main user, it is also useful outside of RLS. For example my (not updated for current nightly) https://github.com/bjorn3/rust_read_rlib uses set_file_loader to generate a fake lib.rs without having to write anything to the disk. They may also be used to fix rust-lang/rust-analyzer#3107 at some point in the future.

[bors]

⌛ Testing commit 9b66f46f36d9cb2189a177854e96af54f920bd19 with merge 01264cdc8b1ca0f56700ff335685ca5fe6a1c4b3...

[bors]

💔 Test failed - checks-actions

[eddyb]

This does not remove the analysis component. It is not clear if we should keep this or not. RLS was the primary user, but I think there are a few users that have made tools around it.

First off: is there a tracking issue that we could discuss this on? I will move this comment wherever necessary.

---

I believe searchfox's Rust indexer might be one of the more important users but I don't really know much about it, sadly (@emilio's handle shows up in the comments, maybe they can shed some light?).

(EDIT: ignore this collapsed section, @bjorn3's suggestion below of using RA is much better)

My rough suggestion would be to look at what information it reads and try to reduce the -Z save-analysis implementation in the compiler so that it doesn't outright break, but we put less effort into "offering detail".
(imagine things like pretending module::function and Type::method are the same thing, or that there is no difference between enum/structs and type aliases, etc.)

I would expect we may be able to shrink it all the way to "dump all use->def relations where we have unique Span mappings" style approach (roughly structured like a lint over the HIR that wants to see both direct paths, and inferred e.g. method callees, using the "current typeck results" pattern available to lints, though other passes also mimic their own - rustc_privacy would be a great example if it were much smaller).

AFAIK, this information would also be very similar to what rustdoc source annotation needs (cc @GuillaumeGomez @Manishearth), so there's maybe some sharing opportunity there? (e.g. make a query computing something like this, -Z save-analysis' only job becomes dumping it into some JSON format)

[bjorn3]

Rust-analyzer has support for emitting LSIF formatted analysis results as well as Sourcegraph's SCIP. This might be a viable alternative for the save-analysis results for most users.

[ehuss]

I have no intent on making changes to save-analysis. If you want to start a discussion on that, I would imagine #43606 or something like an MCP or internals discussion would be a good start.

[eddyb]

(EDIT: moved to the save-analysis tracking issue, #43606)

Ah, thanks for the keywords @bjorn3, I was able to find LSIF mentioned twice already wrt searchfox:

• rust-analyze: only stick with the deps/save-analysis file, instead of all of them. mozsearch/mozsearch#81 (comment)
• Bug 1633260 - Add Java AST parser using JavaParser mozsearch/mozsearch#314 (comment)

Based on https://bugzilla.mozilla.org/show_bug.cgi?id=1761287 I'm guessing we should coordinate with @asutherland and eventually remove save-analysis in favor of RA+LSIF.>

Will continue this discussion #43606 (saw @ehuss' comment as I was writing this, heh).

[rust-timer]

Finished benchmarking commit (d5ef528): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean1	range	count2
Regressions ❌ (primary)	2.6%	[2.6%, 2.6%]	1
Regressions ❌ (secondary)	1.9%	[1.9%, 1.9%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-6.9%	[-6.9%, -6.9%]	1
All ❌✅ (primary)	2.6%	[2.6%, 2.6%]	1

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean1	range	count2
Regressions ❌ (primary)	2.1%	[2.0%, 2.1%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.8%	[-3.8%, -3.8%]	1
All ❌✅ (primary)	2.1%	[2.0%, 2.1%]	2

Footnotes

1. the arithmetic mean of the percent change ↩ ↩²

2. number of relevant changes ↩ ↩²

[Xanewok]

Thanks a lot @ehuss, appreciate it!