Stream: t-compiler/wg-rls-2.0

Topic: rust-project.json


Benjamin Brittain (Feb 13 2020 at 15:45, on Zulip):

also, is it possible to use rust-project.json to describe hundreds of various binary/library targets? or would there need to be one per crate like Cargo.toml files?

matklad (Feb 13 2020 at 15:46, on Zulip):

@Benjamin Brittain it's not docummented, there's only a test. You need one project.json file which descibes the whole web of crates you are compiling

matklad (Feb 13 2020 at 15:47, on Zulip):

What are your use-case for project.json ?

matklad (Feb 13 2020 at 15:48, on Zulip):

The problem with it is that I really want to support non-CArgo based proejcts (mostly to make sure that the abstraction is right), but, as almost everyone uses Cargo, this functionality is not really well tested at all

Benjamin Brittain (Feb 13 2020 at 15:48, on Zulip):

I'm reworking the Rust rules in Fuchsia, and I'm so tired of our Cargo.toml hacks

Benjamin Brittain (Feb 13 2020 at 15:49, on Zulip):

I'm probably like the only person who needs this :P

matklad (Feb 13 2020 at 15:49, on Zulip):

Oh yeah, this is exactly use-case for project.json

matklad (Feb 13 2020 at 15:49, on Zulip):

@Benjamin Brittain I think you are rougthly 50% off the audience, the other half should be Facebook :D

Benjamin Brittain (Feb 13 2020 at 15:49, on Zulip):

sounds about right :laughing:

Benjamin Brittain (Feb 13 2020 at 15:50, on Zulip):

ok, I might have questions then. Can you point me to the test?

Benjamin Brittain (Feb 13 2020 at 15:50, on Zulip):

If it's not straightforward I'll try to write up some docs in parallel

matklad (Feb 13 2020 at 15:51, on Zulip):

So, rust-analyzer works with a set of crates at a time (in contrast with rustc, which works crate at a time).

So what project.json does is it describes the graph of crates.

https://github.com/rust-analyzer/rust-analyzer/blob/4444192b05c107a40a5a05ea3c9091ad8f8cbbcc/crates/ra_lsp_server/tests/heavy_tests/main.rs#L380

Benjamin Brittain (Feb 13 2020 at 15:52, on Zulip):

can rust-analyzer handle ~2000 crates?

matklad (Feb 13 2020 at 15:52, on Zulip):

In particular, the nodes in graph are anonymous (names that you see in extern crate foo are written on edges), and you can "instantiate" sevetal crates from a single file.

matklad (Feb 13 2020 at 15:53, on Zulip):

can rust-analyzer handle ~2000 crates?

In theory, yes, in practice, I hanve't tried that big projects

Benjamin Brittain (Feb 13 2020 at 15:53, on Zulip):

cool, I'm ready to be a stress test

matklad (Feb 13 2020 at 15:53, on Zulip):

but it can handle rustc, for examle

matklad (Feb 13 2020 at 15:53, on Zulip):

In general, the processing of rust-analyzer should be sublinear, so in theory we should scale pretty well.

Benjamin Brittain (Feb 13 2020 at 15:53, on Zulip):

fancy

matklad (Feb 13 2020 at 15:54, on Zulip):

A major limitation in practice is that we do keep all the data in memory at the moment, so several gig for rust-analyzer is not unheard of

Benjamin Brittain (Feb 13 2020 at 15:54, on Zulip):

my workstation has 196Gb of RAM. I think I'll be Ok

matklad (Feb 13 2020 at 15:54, on Zulip):

yup, that should be fine

Benjamin Brittain (Feb 13 2020 at 15:55, on Zulip):

but I'd like people who do OSS work (who usually don't have as powerful machines) to be able to use this too :(

are there plans to move some things to disk?

matklad (Feb 13 2020 at 15:56, on Zulip):

Eventual plans, not immediate plans

matklad (Feb 13 2020 at 15:56, on Zulip):

and it might make more sense to just optimize data structures though

Benjamin Brittain (Feb 13 2020 at 15:56, on Zulip):

sounds good! Thanks for the super rapid answers to all my questions :D

matklad (Feb 13 2020 at 15:56, on Zulip):

basically, we are roughtly 30% into "make it work" phase, we haven't started on "make it fast/make it smol" yet :)

Benjamin Brittain (Feb 13 2020 at 15:58, on Zulip):

unfortunately, I still need to generate Cargo.toml files for people using RLS :(
but this should be a superior workflow

matklad (Feb 13 2020 at 16:01, on Zulip):

That's not officially decided, but we might want to deprecate RLS soon

Benjamin Brittain (Feb 13 2020 at 16:02, on Zulip):

I would like you to come and tell people here that their workflow is broken. :upside_down:

Benjamin Brittain (Feb 13 2020 at 16:02, on Zulip):

but yeah, that totally makes sense

Benjamin Brittain (Feb 13 2020 at 16:02, on Zulip):

the people using intelij are not gonna be happy when I remove Cargo.toml support though

matklad (Feb 13 2020 at 16:03, on Zulip):

It should be not that hard to add rust-project.json support to IntelliJ

cc @Vlad , wdyt about quasi-standard Cargo-independent way to describe rust-projects?

std::Veetaha (Feb 13 2020 at 16:15, on Zulip):

Just as a sidenote: I would suggest not using .json format for such config files, .toml or as a last resort .yaml are much better for this ...

Benjamin Brittain (Feb 13 2020 at 16:17, on Zulip):

why?

matklad (Feb 13 2020 at 16:17, on Zulip):

This is intended to be a machine-generated files, and it's usually easier to produce and consume JSON than human readable format.

Benjamin Brittain (Feb 13 2020 at 16:17, on Zulip):

I actively war against yaml :P

Benjamin Brittain (Feb 13 2020 at 16:17, on Zulip):

even for human readable

matklad (Feb 13 2020 at 16:17, on Zulip):

Though, we probably could use TOML, as the nesting is shallow

Benjamin Brittain (Feb 13 2020 at 16:18, on Zulip):

I'd really prefer json tbh. Most build system tooling already has a way to consume JSON

Benjamin Brittain (Feb 13 2020 at 16:18, on Zulip):

TOML is very rust ecosystem centric

matklad (Feb 13 2020 at 16:18, on Zulip):

and yeah, yaml is not my cup of tea. At least 30% of my "fix CI" commits are about fixing yaml syntax

matklad (Feb 13 2020 at 16:18, on Zulip):

TOML is very rust ecosystem centric

I think Go uses Toml as well though?

matklad (Feb 13 2020 at 16:19, on Zulip):

but yeah, agree that JSON is less weird option

Benjamin Brittain (Feb 13 2020 at 16:19, on Zulip):

I just turned around and asked someone who works on Go

"It does not"

std::Veetaha (Feb 13 2020 at 16:20, on Zulip):

If this is machine-generated and nothing more, than alright...
It's just .json doesn't natively support comments, may be quite verbose and it's just my gory experience of .json programming (i.e. imitating function call syntax and in fact writing programs with logic in .json files...)

Benjamin Brittain (Feb 13 2020 at 16:21, on Zulip):

back to the quasi-standard, that would make me very happy

matklad (Feb 13 2020 at 16:21, on Zulip):

ah, that was go dep, which was subsumed by go mod: https://github.com/golang/dep/blob/master/Gopkg.toml

Benjamin Brittain (Feb 13 2020 at 16:21, on Zulip):

I've basically removed Cargo usage from our project with the exception of vendoring

Benjamin Brittain (Feb 13 2020 at 16:21, on Zulip):

and needing any form of the files makes my life very frustrating

std::Veetaha (Feb 13 2020 at 16:22, on Zulip):

@Benjamin Brittain , might I ask why do you drop cargo?

Benjamin Brittain (Feb 13 2020 at 16:23, on Zulip):

because we have a lot of cross-language dependencies and a large amount of infrastructure dedicated to other build systems

Benjamin Brittain (Feb 13 2020 at 16:23, on Zulip):

Cargo also has many pain points once you grow to be a large project

Benjamin Brittain (Feb 13 2020 at 16:24, on Zulip):

where large means 10k+ individual targets

std::Veetaha (Feb 13 2020 at 16:26, on Zulip):

And what do you do you to replace cargo?

std::Veetaha (Feb 13 2020 at 16:27, on Zulip):

I thought cargo was supposed to simplify all that build systems configuration stuff

Benjamin Brittain (Feb 13 2020 at 16:27, on Zulip):

most people aren't building operating systems

Benjamin Brittain (Feb 13 2020 at 16:27, on Zulip):

in this case, GN

Benjamin Brittain (Feb 13 2020 at 16:27, on Zulip):

https://gn.googlesource.com/gn

matklad (Feb 13 2020 at 16:27, on Zulip):

See also https://rust-lang.github.io/rfcs/2136-build-systems.html

matklad (Feb 13 2020 at 16:28, on Zulip):

Basically, "integration with other build system" is a problem on which Cargo team spend a couple of years on and off, and which wasn't resolved in a satisfactory way.

Benjamin Brittain (Feb 13 2020 at 16:28, on Zulip):

It's also really hard to justify working on

Benjamin Brittain (Feb 13 2020 at 16:29, on Zulip):

like, we can just throw people at the problem to make it go away

Benjamin Brittain (Feb 13 2020 at 16:29, on Zulip):

and most people using their own build system can do that

Benjamin Brittain (Feb 13 2020 at 17:03, on Zulip):
    let project = json!({
        "roots": [path],
        "crates": [ {
            "root_module": path.join("src/lib.rs"),
            "deps": [],
            "edition": "2015",
            "atom_cfgs": [],
            "key_value_cfgs": {}
        } ]
    });
Benjamin Brittain (Feb 13 2020 at 17:04, on Zulip):

ok, some of that is obvious

Benjamin Brittain (Feb 13 2020 at 17:04, on Zulip):

what is atom_cfgs & key_value_cfgs

matklad (Feb 13 2020 at 17:04, on Zulip):

that's --cfg flags you pass on the command line

Benjamin Brittain (Feb 13 2020 at 17:05, on Zulip):

NICE

matklad (Feb 13 2020 at 17:05, on Zulip):

which are either --cfg=foo, or --cfg=foo=bar

Benjamin Brittain (Feb 13 2020 at 17:05, on Zulip):

yeah, that makes total sense now

matklad (Feb 13 2020 at 17:05, on Zulip):

So, in theory, you can instantiate the same crate with two sets of different cfgs

Benjamin Brittain (Feb 13 2020 at 17:05, on Zulip):

uh, I do need to do that :upside_down:

Benjamin Brittain (Feb 13 2020 at 17:06, on Zulip):

host side & target side compilation is very common

matklad (Feb 13 2020 at 17:07, on Zulip):

Yeah, in theory you should be able to have diffrent target-trippled crates in the same graph

matklad (Feb 13 2020 at 17:08, on Zulip):

Though i am not sure if that would just work in practice. We do some sniffing of rustc --print cfg, whcih just uses HOST, but I think rust-project.json should override that

Benjamin Brittain (Feb 13 2020 at 18:52, on Zulip):

ok, about to take a crack at this.

deps are just the crate_name and r-a handles the resolution?

Benjamin Brittain (Feb 13 2020 at 18:53, on Zulip):

and roots is a scratch space?

Florian Diebold (Feb 13 2020 at 19:10, on Zulip):

each dep is something like {"crate": 0, "name": "crate_name"}, where the 0 is the index of the crate in the crates array, and the "crate_name" is the name of the dependency (think rename in cargo)

Florian Diebold (Feb 13 2020 at 19:10, on Zulip):

so no, you have to resolve it

Benjamin Brittain (Feb 13 2020 at 19:12, on Zulip):

interesting. so hypothetically It can handle crates with the same name?

Florian Diebold (Feb 13 2020 at 19:12, on Zulip):

roots are the directories containing rust crates; RA will watch all rust code in each of these. All the crates must be in one of these, I think. I don't know if this affects anything else; RA has a notion of library roots, but I don't know how that maps to this

Florian Diebold (Feb 13 2020 at 19:13, on Zulip):

interesting. so hypothetically It can handle crates with the same name?

well, basically crates just have no name in this model. the name is a property of the dependency edge

Florian Diebold (Feb 13 2020 at 19:14, on Zulip):

RA has a notion of library roots

(the effect of this is that certain optimizations are activated that assume that the code in that root usually won't change)

Benjamin Brittain (Feb 13 2020 at 19:14, on Zulip):

I'll write a small test rust-project.json and see if it works the way I think

Benjamin Brittain (Feb 13 2020 at 19:15, on Zulip):

I'm very excited

Florian Diebold (Feb 13 2020 at 19:15, on Zulip):

ok, from reading the code it looks like we just consider all roots non-library for json projects, which is probably fair

Florian Diebold (Feb 13 2020 at 19:16, on Zulip):

note that you need to include the sysroot crates as well (i.e. std and so on), they aren't automatically added

Benjamin Brittain (Feb 13 2020 at 19:16, on Zulip):

yeah, that's fine

Benjamin Brittain (Feb 13 2020 at 19:16, on Zulip):

makes sense

Benjamin Brittain (Feb 13 2020 at 19:32, on Zulip):

how do I see R-A logs and whatnot?

Benjamin Brittain (Feb 13 2020 at 19:32, on Zulip):

I don't trust vscode not to be doing funky things >_<

Benjamin Brittain (Feb 13 2020 at 21:20, on Zulip):

I started writting out a simple one by hand

    "roots": ["/usr/local/google/home/bwb/fuchsia/src/lib/zircon/rust/fuchsia-zircon-status/", "/usr/local/google/home/bwb/fuchsia/out/default.zircon/user.vdso-x64-clang.shlib/gen/zircon/syscalls/"],
    "crates": [
      {
        "root_module": "/usr/local/google/home/bwb/fuchsia/out/default.zircon/user.vdso-x64-clang.shlib/gen/zircon/syscalls/definitions.rs",
        "deps": [],
        "edition": "2018",
        "atom_cfgs": [],
        "key_value_cfgs": {}
      },
      {
        "root_module": "/usr/local/google/home/bwb/fuchsia/src/lib/zircon/rust/fuchsia-zircon-status/src/lib.rs",
        "deps": [ {"crate": 0, "name": "fuchsia_zircon_sys"} ],
        "edition": "2018",
        "atom_cfgs": [],
        "key_value_cfgs": {}
      }
   ]
}
Benjamin Brittain (Feb 13 2020 at 21:21, on Zulip):

but it doesn't seem to pick up sys

Benjamin Brittain (Feb 13 2020 at 21:21, on Zulip):

also if you say fuchsia-zircon-sys r-a is _not happy_ :upside_down:

Benjamin Brittain (Feb 13 2020 at 21:22, on Zulip):

anything I should fix before I go digging through the source code?

Benjamin Brittain (Feb 13 2020 at 21:24, on Zulip):

oh wait, I think it does work!

Benjamin Brittain (Feb 13 2020 at 21:24, on Zulip):

I just had a silly typo and it didn't surface the error

Benjamin Brittain (Feb 13 2020 at 21:31, on Zulip):

this is gonna be really slick

Benjamin Brittain (Feb 13 2020 at 21:32, on Zulip):

I'm very happy with this format

matklad (Feb 13 2020 at 22:20, on Zulip):

also if you say fuchsia-zircon-sys r-a is _not happy_ :upside_down:

Because that's not a valid identifier. The name in the dep is literarly the identifier you'll use in an .rs file.

Benjamin Brittain (Feb 13 2020 at 22:20, on Zulip):

oh yeah, I know that

Benjamin Brittain (Feb 13 2020 at 22:21, on Zulip):

but we use dashes everywhere so I typed it in reflexivly

Benjamin Brittain (Feb 13 2020 at 22:21, on Zulip):

our build rules convert - to _

Benjamin Brittain (Feb 13 2020 at 23:59, on Zulip):

AND IT WORKS \o/

Benjamin Brittain (Feb 13 2020 at 23:59, on Zulip):

it's amazing

Benjamin Brittain (Feb 13 2020 at 23:59, on Zulip):

@matklad thank you so much

matklad (Feb 14 2020 at 00:03, on Zulip):

I am extremely glad to receive the first report about rust-project.json actually working about a year after the feature was implemented :D

Benjamin Brittain (Feb 14 2020 at 00:13, on Zulip):

It's a little slow on startup, not sure how long r-a usually takes

Benjamin Brittain (Feb 14 2020 at 00:13, on Zulip):

only ~2k targets right now

Benjamin Brittain (Feb 14 2020 at 00:14, on Zulip):

we have more, but I've been using a subset

matklad (Feb 14 2020 at 00:14, on Zulip):

Yeah, as we don't save anything to disk, we need to run some analyzis on the startup

matklad (Feb 14 2020 at 00:14, on Zulip):

Though, it still should be significanlty faster than compiling the code

Benjamin Brittain (Feb 14 2020 at 00:19, on Zulip):

several orders of magnitude faster :)

Benjamin Brittain (Feb 14 2020 at 00:48, on Zulip):

Is there an easy way to convert Cargo.toml output into this format?

Benjamin Brittain (Feb 14 2020 at 00:49, on Zulip):

although I'm about to remove it, our third_party code builds with Cargo right now

Benjamin Brittain (Feb 14 2020 at 00:50, on Zulip):

I have a plan that will almost work, but it'd be nice to leverage cargo to do the work for me

matklad (Feb 14 2020 at 00:53, on Zulip):

Nope, there isn't such a way

Benjamin Brittain (Feb 14 2020 at 00:54, on Zulip):

that's fine. that's _very niche_

Benjamin Brittain (Feb 14 2020 at 00:54, on Zulip):

even more than using this format at all

Benjamin Brittain (Feb 14 2020 at 02:03, on Zulip):

ok, it was surprisingly easy to walk a vendored cargo.toml setup and generate this

Benjamin Brittain (Feb 14 2020 at 20:16, on Zulip):

So it looks like everything is working, but it debug prints unresolved imports. Any way to get access to that so I can figure out how to make this perfct?

matklad (Feb 14 2020 at 20:16, on Zulip):

but it debug prints unresolved imports

Hm, not sure I understad what happes

matklad (Feb 14 2020 at 20:16, on Zulip):

Could you show what exactly is printed where?

Benjamin Brittain (Feb 14 2020 at 20:17, on Zulip):

"name resolution is stuck"

Benjamin Brittain (Feb 14 2020 at 20:17, on Zulip):

which looks like it happens when resolving imports
[ERROR ra_hir_def::nameres::collector] name resolution is stuck

matklad (Feb 14 2020 at 20:18, on Zulip):

oh, interesting

Benjamin Brittain (Feb 14 2020 at 20:19, on Zulip):

It tried resolving imports 1000 times (any reason for this constant?) , then gave up

matklad (Feb 14 2020 at 20:19, on Zulip):

THat's basically a rust-analyzer requivalent of ICE

matklad (Feb 14 2020 at 20:19, on Zulip):

This should not be happening

Benjamin Brittain (Feb 14 2020 at 20:19, on Zulip):

hahaha

Benjamin Brittain (Feb 14 2020 at 20:19, on Zulip):

I make rustc ICE regularly. this doesn't seem atypical to me :P

matklad (Feb 14 2020 at 20:19, on Zulip):

It tried resolving imports 1000 times (any reason for this constant?) , then gave up

That's just a safeguard: we can not afford to spin indefinitelly even in the case of bugs.

Benjamin Brittain (Feb 14 2020 at 20:20, on Zulip):

how many times is this loop usually called?

Benjamin Brittain (Feb 14 2020 at 20:20, on Zulip):

is this just because of the 2k+ crates?

Benjamin Brittain (Feb 14 2020 at 20:21, on Zulip):

I don't know anything about how this is architected, sorry for the potentially silly questions

matklad (Feb 14 2020 at 20:21, on Zulip):

IDE sees all kinds of wired incomplete code code, so it tends to crash more often than a compiler. Moreover, the cost of IDE crash is much higher, because it's a long-lived process, and not a batch one. For this reason, we embrace the existance of bugs and try to mitigate their consequences

Benjamin Brittain (Feb 14 2020 at 20:22, on Zulip):

this might just be that I don't have CFGs wired properly then and it thinks a lot of code is perpetually incomplete

matklad (Feb 14 2020 at 20:22, on Zulip):

21:20

how many times is this loop usually called?

I haen't measured, but I'd expect less than a dozen calls for typical usecases. I don't think this relates to the number of crates. Rather, it's more likely that, among all that code, there's a particular code pattern that exposes a buggy behavior in our name resolution

matklad (Feb 14 2020 at 20:23, on Zulip):

this might just be that I don't have CFGs wired properly then and it thinks a lot of code is perpetually incomplete

Even if the code is completely broken, this loop should terminated

Benjamin Brittain (Feb 14 2020 at 20:24, on Zulip):

any guidance on gathering a repro?

matklad (Feb 14 2020 at 20:24, on Zulip):

Unfortunatelly we don't have infra for automatic minimization of such problems yet :(

Benjamin Brittain (Feb 14 2020 at 20:24, on Zulip):

aw :(

matklad (Feb 14 2020 at 20:24, on Zulip):

so yeah, at the moment we just manually minize such bugs, which is a huge time sink

bjorn3 (Feb 14 2020 at 20:25, on Zulip):

http://blog.pnkfx.org/blog/2019/11/18/rust-bug-minimization-patterns/

Benjamin Brittain (Feb 14 2020 at 20:26, on Zulip):

that doesn't seem relevant to logic level errors like this, that seems more targeted at compilation failures

bjorn3 (Feb 14 2020 at 20:27, on Zulip):

I also remember a program that does the first step of minimizing rust code, but I can't remember it's name.

Benjamin Brittain (Feb 14 2020 at 20:28, on Zulip):

I have written no rust code to generate this behaviour

bjorn3 (Feb 14 2020 at 20:30, on Zulip):

Csmith is supposedly also useful, even though it is technically made for C, not Rust.

Edit: wrong name, meant creduce

bjorn3 (Feb 14 2020 at 20:32, on Zulip):

bjorn3 said:

I also remember a program that does the first step of minimizing rust code, but I can't remember it's name.

Found it: https://github.com/jethrogb/rust-reduce

matklad (Feb 14 2020 at 20:33, on Zulip):

Another thing that helps here is batch mode for rust-analyzer: https://github.com/rust-analyzer/rust-analyzer/blob/6711335173f138d444049357495fb2785c2bdd0b/crates/ra_cli/src/main.rs#L83

matklad (Feb 14 2020 at 20:33, on Zulip):

Basically, it asks ra to fully type-check the crate, so it's easy to check if things are broken without restarting LSP server

matklad (Feb 14 2020 at 20:34, on Zulip):

But it is hard-coded for cargo atm: https://github.com/rust-analyzer/rust-analyzer/blob/6fb36dfdcb91f67c28f51e51514ebe420ec3aa22/crates/ra_cli/src/analysis_stats.rs#L24

Benjamin Brittain (Feb 14 2020 at 20:36, on Zulip):

cool. I'll give that a try shortly, want to land a few things first

Benjamin Brittain (Feb 14 2020 at 20:38, on Zulip):

should also be straightforward to break this down to subsections of our build graph programatically

Benjamin Brittain (Feb 14 2020 at 20:38, on Zulip):

that could be helpful

matklad (Feb 14 2020 at 20:57, on Zulip):

Opened https://github.com/rust-analyzer/rust-analyzer/issues/3149

matklad (Feb 14 2020 at 20:59, on Zulip):

@Benjamin Brittain one thing I would try first is probably just bumping that limit to 10k. Like, my understanding of the code is that this loop should be really really shallow, but it might be the case that my understanding is wrong, and you ineed hit the limit due to share size. I think this is unlikely, but it is much cheaper to check than doing actual minimization.

Benjamin Brittain (Feb 14 2020 at 20:59, on Zulip):

very true :grinning_face_with_smiling_eyes:

Benjamin Brittain (Feb 14 2020 at 21:11, on Zulip):

I bumped it up to 10k and the error hasn't show up yet

matklad (Feb 14 2020 at 21:12, on Zulip):

Wow, that means that my understanding is waaaay offf. We need to completely rewrite that bit of code anyway....

Benjamin Brittain (Feb 14 2020 at 21:17, on Zulip):

as a temporary workaround: https://github.com/rust-analyzer/rust-analyzer/pull/3150

Benjamin Brittain (Feb 14 2020 at 21:22, on Zulip):

I look forward to submitting repros in the future though :)

Edwin Cheng (Feb 15 2020 at 04:06, on Zulip):

IIRC,that loop also handle recursive macro expansion in item scopes

Benjamin Brittain (Mar 18 2020 at 19:20, on Zulip):

Ok! So people are actually using it now

Benjamin Brittain (Mar 18 2020 at 19:20, on Zulip):

and are pretty happy

Benjamin Brittain (Mar 18 2020 at 19:20, on Zulip):

@matklad

Benjamin Brittain (Mar 18 2020 at 19:21, on Zulip):

Only the early adopters though, so let's see what people say when I force their workflows to change :grimacing:

Benjamin Brittain (Mar 18 2020 at 19:27, on Zulip):

actually, I'm personally attempting to use LSP for the first time.

Is there a way to make rust-analyzer hang around? I use vim and I open and close all the time

Benjamin Brittain (Mar 18 2020 at 19:28, on Zulip):

I'd rather not have it reindex every single time, it takes a while

matklad (Mar 18 2020 at 19:29, on Zulip):

@Benjamin Brittain I think this needs to be fixed on the editor side: rust-analyzer is indeed designed to stick around, vim should somehow persist it.

Benjamin Brittain (Mar 18 2020 at 19:30, on Zulip):

makes perfect sense! I'll write some horrible vimscript then

Benjamin Brittain (Mar 18 2020 at 19:30, on Zulip):

I resent having any knowledge of vimscript

matklad (Mar 18 2020 at 19:32, on Zulip):

I think ideally it should be handled by the LSP plugin on the vim's side... Don't know whom to cc for vim+rust-analyzer expertise though, I can only give advice about VS Code and Emacs :Ь

Benjamin Brittain (Mar 18 2020 at 19:33, on Zulip):

honestly, all the vim lsp options are pretty horrible looking

matklad (Mar 18 2020 at 19:33, on Zulip):

I've heard good things about coc.rust-analyzer

matklad (Mar 18 2020 at 19:34, on Zulip):

https://github.com/fannheyward/coc-rust-analyzer

Benjamin Brittain (Mar 18 2020 at 19:34, on Zulip):

I'm not willing to let it shell out to node unfortunately

Benjamin Brittain (Mar 18 2020 at 19:35, on Zulip):

I've tried switching to the pre-release version ofnvim 0.5 since it has built-in lsp. It seems to be working well actually

matklad (Mar 18 2020 at 19:42, on Zulip):

Yeah, totally understandable. This actually is I think a problem with LSP adoption across editors different from VS Code.

Microsoft are doing something very far-sighted here. They provide high-level API to VS Code for plugins for things like completion and goto definition. They maintain lsp-vscode library, which is a, well, library to bind VS Code API to LSP processes. For each language, community maintains a language-specific plugin which uses the library. So, the maintenance is distributed, for each language you need to install a separate plugin, which can be pretty high-quality and specialized.

Other editors in contrast try to do a single universal things: an LSP plugin which simultaneously supports all languages and maps LSP concepts directly to low-level editor's UI elements.

mark-i-m (Mar 18 2020 at 19:46, on Zulip):

Yeah, I've been using coc.vim but I really resent the node server running in the background

mark-i-m (Mar 18 2020 at 19:46, on Zulip):

@Benjamin Brittain how did you get the nvim 0.5 lsp to work? I never figured it out

mark-i-m (Mar 18 2020 at 19:47, on Zulip):

Also, I agree with everything you said

Benjamin Brittain (Mar 18 2020 at 19:48, on Zulip):
" lsp support
lua << EOF

local nvim_lsp = require'nvim_lsp'
local util = require 'nvim_lsp/util'

require'nvim_lsp'.rust_analyzer.setup({
  root_dir = util.root_pattern("rust-project.json");
  log_level = vim.lsp.protocol.MessageType.Log;
  message_level = vim.lsp.protocol.MessageType.Log;
})
EOF
autocmd Filetype rust setlocal omnifunc=v:lua.vim.lsp.omnifunc

nnoremap <silent> K     <cmd>lua vim.lsp.buf.hover()<CR>
Benjamin Brittain (Mar 18 2020 at 19:49, on Zulip):

@mark-i-m That's what's in my config right now

Benjamin Brittain (Mar 18 2020 at 19:49, on Zulip):

although, you don't want that root_dir line

Benjamin Brittain (Mar 18 2020 at 19:50, on Zulip):

I'm trying to figure out what the other vim.lsp.buf.* commands do. Basically nothing is documented

mark-i-m (Mar 18 2020 at 19:54, on Zulip):

Thanks I will try that out

mark-i-m (Mar 18 2020 at 19:55, on Zulip):

This is what I currently use for coc.vim: https://github.com/mark-i-m/dotfiles/blob/464b7f46e52a72896dd485ea2bf63d5b94881cc8/vim/.vimrc#L24-L59

mark-i-m (Mar 18 2020 at 19:58, on Zulip):

I'm sure you've seen it by now, but :help lsp contains some info

Benjamin Brittain (Mar 18 2020 at 19:59, on Zulip):

yeah, it's helpful

Benjamin Brittain (Mar 18 2020 at 20:00, on Zulip):

what's a good heuristic for shutting down rust-analyzer :thinking:

Benjamin Brittain (Mar 18 2020 at 20:00, on Zulip):

running it in the background is great, but I'd like it to turn itself off it's not used for a while

Benjamin Brittain (Mar 18 2020 at 20:01, on Zulip):

especially since it's a bit of a RAM hog

mark-i-m (Mar 18 2020 at 20:01, on Zulip):

Perhaps just do something simple... no commands in N min -> shut down

mark-i-m (Mar 18 2020 at 20:01, on Zulip):

It's a starting point

Benjamin Brittain (Mar 18 2020 at 20:37, on Zulip):

https://github.com/neovim/nvim-lsp/pull/175

mark-i-m (Mar 18 2020 at 21:18, on Zulip):

@Benjamin Brittain I just came across this https://github.com/neovim/neovim/issues/11389... still looking through but perhaps there are some useful examples

Benjamin Brittain (Mar 18 2020 at 21:23, on Zulip):

oh my, I'm excited to figure out rename :D

Benjamin Brittain (Mar 18 2020 at 21:24, on Zulip):

does rust-analyzer have a singleton pattern? If you launch it twice what happens?

Benjamin Brittain (Mar 18 2020 at 21:24, on Zulip):
bwb@bwb ~ $ rust-analyzer --help
Error: Invalid flags: --help

   0: core::result::Result<T,E>::or_else
   1: rust_analyzer::args::Args::parse
   2: rust_analyzer::main
   3: std::rt::lang_start::{{closure}}
   4: std::rt::lang_start_internal::{{closure}}
             at src/libstd/rt.rs:52
      std::panicking::try::do_call
             at src/libstd/panicking.rs:303
   5: __rust_maybe_catch_panic
             at src/libpanic_unwind/lib.rs:86
   6: std::panicking::try
             at src/libstd/panicking.rs:281
      std::panic::catch_unwind
             at src/libstd/panic.rs:394
      std::rt::lang_start_internal
             at src/libstd/rt.rs:51
   7: main
   8: __libc_start_main
   9: _start
Benjamin Brittain (Mar 18 2020 at 21:24, on Zulip):

:(

Laurențiu Nicola (Mar 18 2020 at 21:30, on Zulip):

rust-analyzer help

Benjamin Brittain (Mar 18 2020 at 21:31, on Zulip):
bwb@bwb ~ $ rust-analyzer help
ra-cli

USAGE:
    rust-analyzer <SUBCOMMAND>

FLAGS:
    -h, --help        Prints help information

:thinking:

Laurențiu Nicola (Mar 18 2020 at 21:32, on Zulip):

Yeah, small bug. And it says ra-cli instead of rust-analyzer

Benjamin Brittain (Mar 18 2020 at 21:33, on Zulip):

what happens if two rust-analyzers are running at the same time?

Laurențiu Nicola (Mar 18 2020 at 21:34, on Zulip):

They don't interact. With LSP the client talks to the server via the standard input and output, and rust-analyzer doesn't fork to the background or anything like that.

Benjamin Brittain (Mar 18 2020 at 21:34, on Zulip):

oh, that's somewhat upsetting

Benjamin Brittain (Mar 18 2020 at 21:34, on Zulip):

good to know though

Benjamin Brittain (Mar 18 2020 at 21:35, on Zulip):

I didn't realize LSP used stdin/stdout. That makes sense for the behavior I've seen though

Laurențiu Nicola (Mar 18 2020 at 21:37, on Zulip):

You _could_ write a wrapper that keeps one running instance and forwards requests to it, but it would probably be easy to end up with inconsistencies

Benjamin Brittain (Mar 18 2020 at 21:38, on Zulip):

There isn't really another option right now. Using vim frequently involves opening and closing buffers rapidly. rust-analyzer can take up to 30 seconds to be responsive on our codebase

Laurențiu Nicola (Mar 18 2020 at 21:39, on Zulip):

E.g. the server can send notifications to the editor (there's one for "workspace loaded") and if you start a second client at a later time there will be no workspace loaded notification for it.

Laurențiu Nicola (Mar 18 2020 at 21:39, on Zulip):

Does that still happen in nvim 0.5?

Benjamin Brittain (Mar 18 2020 at 21:40, on Zulip):

the speed is related to r-a, not nvim

Laurențiu Nicola (Mar 18 2020 at 21:41, on Zulip):

I meant restarting the server when you open a file

Benjamin Brittain (Mar 18 2020 at 21:42, on Zulip):

sorry, using vim, at least how I do, frequently involves closing all of vim, running some commands, then loading it again

Benjamin Brittain (Mar 18 2020 at 21:42, on Zulip):

not just buffer manipulation

Laurențiu Nicola (Mar 18 2020 at 21:43, on Zulip):

Can you.. suspend it? :smiley:

Benjamin Brittain (Mar 18 2020 at 21:43, on Zulip):

haaa

Benjamin Brittain (Mar 18 2020 at 21:43, on Zulip):

I guess, but what if I'm opening a different file? :)

Laurențiu Nicola (Mar 18 2020 at 21:44, on Zulip):

Or use one of those fancy file pickers?

Benjamin Brittain (Mar 18 2020 at 21:44, on Zulip):

oh noes. stop trying to turn my vim into an ide :upside_down:

Benjamin Brittain (Mar 18 2020 at 21:44, on Zulip):

I recognize lsp was very much designed for an IDE experience though

Laurențiu Nicola (Mar 18 2020 at 21:46, on Zulip):

I hope we'll be able to either dump the database on exit or always keep it on disk, but it hasn't been a priority

Laurențiu Nicola (Mar 18 2020 at 21:48, on Zulip):

It's a bit of a can of worms and other features might be more important in the long run, like making it faster even during the initial analysis

Benjamin Brittain (Mar 18 2020 at 21:48, on Zulip):

yeah, it seems like a bad idea to commit to right now

Benjamin Brittain (Mar 18 2020 at 21:48, on Zulip):

eventually, for sure

Benjamin Brittain (Mar 18 2020 at 21:49, on Zulip):

oh no. I crashed rust-analyzer :(

Benjamin Brittain (Mar 18 2020 at 21:50, on Zulip):
[ ERROR ] 2020-03-18T14:47:57Z-0700 ] /usr/local/share/nvim/runtime/lua/vim/lsp/rpc.lua:308 ]   "rpc"   "rust-analyzer""stderr"        "thread 'main' panicked at 'Invalid request\nMethod: textDocument/rename\n error: missing field `newName`', crates/rust-analyzer/src/lib.rs"
Benjamin Brittain (Mar 18 2020 at 21:50, on Zulip):

to be fair, I did not give it a "NewName"

std::Veetaha (Mar 18 2020 at 21:51, on Zulip):

yeah, we don't expect people to violate the lsp protocol ;D

Laurențiu Nicola (Mar 18 2020 at 21:51, on Zulip):

Does it really crash, or does it panic and carry on?

Benjamin Brittain (Mar 18 2020 at 21:52, on Zulip):

nope, really crashes

Benjamin Brittain (Mar 18 2020 at 21:52, on Zulip):

panic and carry on seems ok to me

matklad (Mar 18 2020 at 21:56, on Zulip):

Yeah, we deliberately assume trusted client which sends correct requests at
us. This is to create more pressure to actually fix clients/servers.

Benjamin Brittain (Mar 18 2020 at 21:57, on Zulip):

to be clear, when you say "trusted client" you don't mean a security boundary, you mean a protocol trust boundary?

std::Veetaha (Mar 18 2020 at 21:58, on Zulip):

He means the human trust...

Benjamin Brittain (Mar 18 2020 at 21:59, on Zulip):

there's more to think about than humans when clients provide arbitrary buffers to the lsp server

Laurențiu Nicola (Mar 18 2020 at 21:59, on Zulip):

But in some clients you can pretty much manually type the LSP requests. Crashing is a bit harsh then :-)

std::Veetaha (Mar 18 2020 at 21:59, on Zulip):

E.g. if you mess up the settings.json with inappropriate configuration type value, we are going to crash very loudly and in some weird place

std::Veetaha (Mar 18 2020 at 21:59, on Zulip):

Its about vscode*

Benjamin Brittain (Mar 18 2020 at 22:00, on Zulip):

however, to recap, my concern is with the words "trusted client"

Input is still validated, right?

Laurențiu Nicola (Mar 18 2020 at 22:02, on Zulip):

I think it means "non-buggy client". I'm not sure if there are any security implications. If you can run vim and run rust-analyzer, even if you trick the latter into doing something bad, it's nothing you couldn't have done in vim already.

Benjamin Brittain (Mar 18 2020 at 22:06, on Zulip):

I hate that article with a burning passion

Benjamin Brittain (Mar 18 2020 at 22:06, on Zulip):

software is composed with other bits of software in arbitrary and complex ways. Saying that it's not exploitable now is short-sighted

Benjamin Brittain (Mar 18 2020 at 22:07, on Zulip):

I'm not super worried about the security of the LSP engine in my life

Benjamin Brittain (Mar 18 2020 at 22:08, on Zulip):

but "trusted client" means something quite specific to me, which causes concern

matklad (Mar 18 2020 at 22:16, on Zulip):

So, we validate that requests confirm to the protocol and “crash
gracefully” (via assert and stack unwinding) if it is not the case. We can
replace those asserts with error bubbling and reporting, but it doesn’t
really make sense to me. “Typing requests by hand” is a use case where it
actually might make sense to not terminate outright, but this is pretty
niche.

Security-wise, we also assume that everything is trusted. We don’t directly
execute code today, but we, eg, run cargo check, which can run build.rs. At
some point we’ll start executing proc macros, which are also arbitrary
code. Memory-safety wise, rust-analyzer has very little unsafe in general,
except for syntax trees. Syntax trees are 100% cursed crazy unsafe code
internally though, and, while safe interface should be fine, we haven’t
done security oriented testing to make sure that everything is really
as memory safe as we think.

TL;DR it’s probably not a good idea to expose rust-analyzer via an
Internet-visible port.

Benjamin Brittain (Mar 18 2020 at 22:18, on Zulip):

but this is pretty niche.

is it? I'm not doing anything atypical right now I think, just setting up vim

Benjamin Brittain (Mar 18 2020 at 22:19, on Zulip):

I disabled cargo check immediately :) but mostly because we don't have cargo

Benjamin Brittain (Mar 18 2020 at 22:20, on Zulip):

my point is that text editors are increasingly becoming "internet-visible" things

matklad (Mar 18 2020 at 22:28, on Zulip):

Yeah, that’s true, but we need to support arbitrary code execution for
proc macros, and that restricts the threat model to “everything should be
fine”. If proc macros move to some kind of wasm sandbox, rust-analyzer
will be able to provide good security, as it’ll guarantee no I/O except for
stdin/stdout/stderr and memory safety, but for the time being we are not
explicitly testing for that.

Benjamin Brittain (Mar 18 2020 at 22:31, on Zulip):

ugh, build.rs and proc-macros. The banes of my existence

Benjamin Brittain (Mar 18 2020 at 22:31, on Zulip):

yup, that makes sense to me

Benjamin Brittain (Mar 18 2020 at 22:32, on Zulip):

I wouldn't say trusted client though, compliant client doesn't set the same alarm bells off

mark-i-m (Mar 18 2020 at 22:34, on Zulip):

@Benjamin Brittain I've given up on nvim-lsp again for now... but I would love to hear if you make progress on stuff!

Benjamin Brittain (Mar 18 2020 at 22:35, on Zulip):

It's working fantastic for me! I'm trying to figure out how to send arbitrary stuff right now

Benjamin Brittain (Mar 18 2020 at 22:35, on Zulip):

but I haven't touched lua in years and I don't know the lsp protocol :)

Benjamin Brittain (Mar 18 2020 at 22:36, on Zulip):

I'll post my config here when I'm all done

Benjamin Brittain (Mar 18 2020 at 23:20, on Zulip):

every time I open a file now my fans goes crazy :laughing:

matt rice (Mar 22 2020 at 23:16, on Zulip):

This seems interesting. I have been wanting to try and automatically toml -> json but add on json-ld schema in the conversion process, I wonder how open you are to having a json-ld schema for rust-project.json?

matklad (Mar 23 2020 at 09:04, on Zulip):

I'd start with just documenting rust-project.json. I don't think its stable or complex enough to really benefit from a formal schema

std::Veetaha (Mar 23 2020 at 11:03, on Zulip):

Not heard about json-ld, but making a json-schema will be beneficial for the user experience. VSCode provides a json schema validation contribution point. By supplying that, the editor will be able to give the users IntelliSense in the rust-project.json file similarly to how it does in settings.json

matt rice (Mar 23 2020 at 14:32, on Zulip):

Glad I asked first then. Not really familiar with json-schema, what I like about json-ld, is the ability to avoid fields like cargo's package.metadata, because its validating the contents rather than the file as a whole thing, so you can tell this field belongs to this schema and some other field another. I don't really see that from the few json-schema examples i've seen. Anyhow I don't want to press the issue really.

Last update: Sep 22 2020 at 02:15UTC