Stream: t-compiler/wg-rls-2.0

Topic: save-analysis based precise analysis


matklad (Nov 19 2019 at 15:39, on Zulip):

cc @Igor Matuszewski, this is the thread to discuss how we add save-analysis to rust-analyzer.I think a reasonable first step is to review analysis-related crates, fix any API issues and publish them to crates.io.

Specifically, I think there might be two things to fix:

Igor Matuszewski (Nov 19 2019 at 16:07, on Zulip):

@matklad thanks for starting a topic! I was just thinking about it when you posted :big_smile:

Igor Matuszewski (Nov 19 2019 at 16:08, on Zulip):

Re errors, we definitely should either expand the definition and catch more errors or use the dynamic one, I’m good with either

Igor Matuszewski (Nov 19 2019 at 16:09, on Zulip):

Re mutex, I’d say we should just move it out. It also messes with recursive processing of the data as well

Igor Matuszewski (Nov 19 2019 at 16:10, on Zulip):

I had a patch somewhere but didn’t go further with that, will need to bring it back

Igor Matuszewski (Nov 19 2019 at 16:12, on Zulip):

Exploring the stateless approach is a good idea, as it strikes a balance between optimizing for subsequent queries and latency stemming from preprocessing the data

Igor Matuszewski (Nov 19 2019 at 20:16, on Zulip):

Exploring the stateless approach is a good idea, as it strikes a balance between optimizing for subsequent queries and latency stemming from preprocessing the data

nikomatsakis (Dec 16 2019 at 21:31, on Zulip):

So @matklad and @Igor Matuszewski I've been talking to @eddyb. They were making a case for why it makes more sense to use a "LSP server" instead of "command-line tools". The TL;DR, I think, is a few things:

I feel like it'd be good maybe to make a hackmd and try to enumerate the arguments in various directions in a bit more depth. I suspect there are a lot of details -- eg., what exactly do we forward? (notably, I would expect we'd not forward details from open files.)

matklad (Dec 17 2019 at 09:25, on Zulip):

@nikomatsakis , @eddyb hack md sounds nice, I've started one here: https://hackmd.io/sMospvYyTtisAnLMdlrUgQ

matklad (Dec 17 2019 at 10:10, on Zulip):

Filled in the document!

nikomatsakis (Dec 17 2019 at 14:43, on Zulip):

@matklad great! I left a few comments

nikomatsakis (Dec 17 2019 at 14:43, on Zulip):

I had a question though

nikomatsakis (Dec 17 2019 at 14:43, on Zulip):

It seems like there's a "third option" too

nikomatsakis (Dec 17 2019 at 14:44, on Zulip):

Maybe I didn't really understand what you were proposing :)

matklad (Dec 17 2019 at 14:45, on Zulip):

The third is "CLI tool to replace RLS"?

nikomatsakis (Dec 17 2019 at 14:46, on Zulip):

Yes. I'm not sure it's better than the other two.

nikomatsakis (Dec 17 2019 at 14:46, on Zulip):

Maybe it has the combined downsides of both :)

matklad (Dec 17 2019 at 14:46, on Zulip):

Yeah, I guess it's a possible extension to the second one

nikomatsakis (Dec 17 2019 at 14:46, on Zulip):

does my description make sense?

matklad (Dec 17 2019 at 14:47, on Zulip):

yes

nikomatsakis (Dec 17 2019 at 14:47, on Zulip):

I guess the key question is whether rust-analyzer actually has knowledge of save-analysis directly

nikomatsakis (Dec 17 2019 at 14:47, on Zulip):

or not

nikomatsakis (Dec 17 2019 at 14:48, on Zulip):

that's the main difference between the two CLI options, I think

matklad (Dec 17 2019 at 14:48, on Zulip):

Hm, I think if you actually distribute CLI tools, you have to make them stable for real

matklad (Dec 17 2019 at 14:48, on Zulip):

while using save-analyzis internall can be sort-of an impl detail

matklad (Dec 17 2019 at 14:49, on Zulip):

Stability guarantees seems like a bigger difference here

nikomatsakis (Dec 17 2019 at 14:49, on Zulip):

yeah that was first in my list of drawbacks

nikomatsakis (Dec 17 2019 at 14:49, on Zulip):

I'm not sure I agree that we have to support it, but it's a risk for sure

nikomatsakis (Dec 17 2019 at 14:50, on Zulip):

what do you think about distributing rust-analyzer with rustc as the "RLS component"? I guess I kind of think we would want to do that anyway

matklad (Dec 17 2019 at 14:50, on Zulip):

Oh, if we can publish a tool and then just remove it from distribution that would probably be the sweet spot!n

matklad (Dec 17 2019 at 14:50, on Zulip):

Like, rust-analyzer wouldn't have to know about save-analysis, the tool can, in the future, use rustc API directly, etc etc.

I love it!

nikomatsakis (Dec 17 2019 at 14:51, on Zulip):

in the limit this could be like rustc -Zsomething

matklad (Dec 17 2019 at 14:51, on Zulip):

The hard question here is who will be responsible for merging analysis of several crates into a single analysis of a workspace

nikomatsakis (Dec 17 2019 at 14:51, on Zulip):

but I think that will be hard to figure out because of workspaces

nikomatsakis (Dec 17 2019 at 14:51, on Zulip):

I guess that's what you just said

matklad (Dec 17 2019 at 14:52, on Zulip):

Yeah. At the moment, rls library which loads save analysis is responsible for merging sa from several crates

matklad (Dec 17 2019 at 14:52, on Zulip):

it would be hard to put that logic directly into rustc, b/c it sort-of depends on Cargo as well

nikomatsakis (Dec 17 2019 at 14:52, on Zulip):

what is the answer to this

nikomatsakis (Dec 17 2019 at 14:52, on Zulip):

nikomatsakis: The LSP server itself can't require the client to save all modified documents, right? So this would require all editor plugins to be aware of these changes?

nikomatsakis (Dec 17 2019 at 14:52, on Zulip):

referring to forcing things to save to disk

nikomatsakis (Dec 17 2019 at 14:52, on Zulip):

I think that's a pretty significant drawback worth highlighting :)

nikomatsakis (Dec 17 2019 at 14:54, on Zulip):

The hard question here is who will be responsible for merging analysis of several crates into a single analysis of a workspace

I guess I feel like the goal should just be to complete rust-analyzer and remove the save-analysis dependency that way

matklad (Dec 17 2019 at 14:54, on Zulip):

Yeah, that's true

matklad (Dec 17 2019 at 14:54, on Zulip):

there should be custom per-client work for this

matklad (Dec 17 2019 at 14:55, on Zulip):

(though, we already ship custom clients for emacs and VS Code, and some folks ship custom clients for Vims, so I don't see this as a super-major issue)

nikomatsakis (Dec 17 2019 at 14:55, on Zulip):

how hard to do you think it would be to spin up the RLS and forward requests for these specific commands back and forth?

nikomatsakis (Dec 17 2019 at 14:55, on Zulip):

it doesn't, conceptually, seem very hard

matklad (Dec 17 2019 at 14:55, on Zulip):

(and, like, we don't save docs for cargo watch at all at the moment, and it somehow works ok )

matklad (Dec 17 2019 at 14:56, on Zulip):

medium annoying I'd say

matklad (Dec 17 2019 at 14:56, on Zulip):

like you'll need to deal with all kinds of io-related errors and state synchronization between the two processes, but that's not a super big deal

nikomatsakis (Dec 17 2019 at 14:57, on Zulip):

(there are also the drawbacks of the existing RLS design, where it doesn't play as well with custom build environments and so forth)

matklad (Dec 17 2019 at 14:57, on Zulip):

it definitelly would be a minor issue in comparison to keeping RLS itself building and working

matklad (Dec 17 2019 at 14:57, on Zulip):

(I'll need to run soon
btw)

nikomatsakis (Dec 17 2019 at 14:57, on Zulip):

you could of course imagine simplifying the RLS to "just" run cargo to get its results -- ah, but I guess that would have the same limitations regarding the VFS

nikomatsakis (Dec 17 2019 at 14:58, on Zulip):

it definitelly would be a minor issue in comparison to keeping RLS itself building and working

well, I'm not sure about this

nikomatsakis (Dec 17 2019 at 14:58, on Zulip):

that is

nikomatsakis (Dec 17 2019 at 14:58, on Zulip):

I think that @Igor Matuszewski can speak more to it, but a lot of that work is basically maintaining save analysis

nikomatsakis (Dec 17 2019 at 14:58, on Zulip):

at least that's what I remember

nikomatsakis (Dec 17 2019 at 14:58, on Zulip):

which seems like it'll be required either way

matklad (Dec 17 2019 at 15:01, on Zulip):

Hm, maybee. But, there's still this whole separate RLS build, submodule and toolstate

matklad (Dec 17 2019 at 15:02, on Zulip):

although, if we really strip RLS down, we might just move it into rustc repo, which simplifies that a lot!

nikomatsakis (Dec 17 2019 at 15:27, on Zulip):

Yes, plausibly

Last update: Jan 28 2020 at 07:40UTC