Stream: t-compiler/rust-analyzer

Topic: Expanding attribute macros


Aaron Hill (May 30 2021 at 23:18, on Zulip):

I'm interested in contributing to https://github.com/rust-analyzer/rust-analyzer/issues/8971 - is there already ongoing work on it?

Jonas Schievink [he/him] (May 30 2021 at 23:20, on Zulip):

I have done some work on the prerequisites

Jonas Schievink [he/him] (May 30 2021 at 23:21, on Zulip):

The next steps here are:

Jonas Schievink [he/him] (May 30 2021 at 23:24, on Zulip):

Re: the attribute API – this can possibly be skipped for an MVP, but is probably already causing issues with how we strip #[derive] attributes today.

Basically the issue is that we try to uniquely identify attributes without accounting for cfg_attr, which can expand to any number of attributes. This then leads to us stripping the entire cfg_attr instead of just one of the expanded attributes. It has also causes hangs, where I assumed that attribute IDs are unique (I've fixed or papered over those now).

Jonas Schievink [he/him] (May 30 2021 at 23:25, on Zulip):

One thing that would also help rust-analyzer would be if we didn't have to duplicate this file but could somehow reuse it: https://github.com/rust-analyzer/rust-analyzer/blob/master/crates/hir_def/src/builtin_attr.rs

Jonas Schievink [he/him] (May 30 2021 at 23:26, on Zulip):

(the EXTRA_ATTRIBUTES const is new, that would go away if we add proper support for the builtin attribute macros - it's also very incomplete apparently :laughing: )

Aaron Hill (May 30 2021 at 23:28, on Zulip):

@Jonas Schievink [he/him] Are you able to depend on rustc_feature directly?

Jonas Schievink [he/him] (May 30 2021 at 23:29, on Zulip):

not directly, since we target stable

Jonas Schievink [he/him] (May 30 2021 at 23:30, on Zulip):

rustc_feature also uses rustc's list of known symbols, which we don't have

Aaron Hill (May 30 2021 at 23:30, on Zulip):

does rust-analyzer depend on any rustc internal crates?

Jonas Schievink [he/him] (May 30 2021 at 23:34, on Zulip):

on the lexer, yeah

Jonas Schievink [he/him] (May 30 2021 at 23:34, on Zulip):

outside of that, no, but we do use chalk, and have vendored the proc_macro bridge

Jonas Schievink [he/him] (May 30 2021 at 23:35, on Zulip):

(I'd also like to stop vendoring the proc_macro code since it tends to break often, but this requires some more complicated changes I think)

matklad (May 31 2021 at 09:37, on Zulip):

To expand on "depend on rustc", we totally can and want to depend on rustc crates, as long as they are build with stable. So, extracting the attribute data table into a separate, "builds on stable" crate would help us, yeah. To be clear, we don't need the APIs to be stable, we are 100% comfortable following whatever changes happen in rustc, as long as "builds on stable" is observed.

Also, @Aaron Hill excited to see experienced rustc hackers taking a look at rust-analyzer. If you need anything to mover forward, feel free to ping us!

bjorn3 (May 31 2021 at 09:42, on Zulip):

rustc_features uses the once_cell feature gate and depends on rustc_data_structures and rustc_span. The rustc_data_structures dependency is easy to remove as it only uses FxHashMap. The rustc_span dependency is used for Edition, Symbol, Span and sym, making it much harder to remove. rustc_span uses several feature gates.

matklad (May 31 2021 at 10:17, on Zulip):

Yeah, there's no analogue to spans in rust-analyzer, so any sharing needs to be done in such a way that the callee supplies the span info.

Jonas Schievink [he/him] (May 31 2021 at 11:16, on Zulip):

sooo, it turns out we already have most things in place to let this "work", for some values of "work"

screenshot-2021-05-31-131459.png screenshot-2021-05-31-131510.png

Jonas Schievink [he/him] (May 31 2021 at 11:16, on Zulip):

everything else breaks tho

Florian Diebold (May 31 2021 at 11:20, on Zulip):

ship it? :see_no_evil:

Florian Diebold (May 31 2021 at 11:21, on Zulip):

(what's 'everything else'?)

Jonas Schievink [he/him] (May 31 2021 at 11:24, on Zulip):

all the salsa macros apparently expand to thin air

Florian Diebold (May 31 2021 at 11:24, on Zulip):

ah

Jonas Schievink [he/him] (May 31 2021 at 11:25, on Zulip):

I think we're just missing the attribute arguments here

Jonas Schievink [he/him] (May 31 2021 at 11:25, on Zulip):

but I did expect at least some sort of diagnostic

Jonas Schievink [he/him] (May 31 2021 at 11:26, on Zulip):

also I think the TokenMap isn't working, and maybe child_by_source also doesn't

Jonas Schievink [he/him] (May 31 2021 at 11:26, on Zulip):

so there's still quite a bit of work to do

Jonas Schievink [he/him] (May 31 2021 at 11:44, on Zulip):

pushed the code here for now: https://github.com/jonas-schievink/rust-analyzer/tree/expand-attr-macros

Aaron Hill (May 31 2021 at 14:47, on Zulip):

@matklad Can you elaborate on the motivation behind the compiles-on-stable requirement? Given the extensive use of nightly features in compiler crates, it seems like that requirement causes (or will cause) a large amount of duplication between rustc and rust-analyzer. I can see the value in having independent implementations of different parts of the compiler, but it sounds like you would potentially depend on many more rustc crates if you were able to.

Aaron Hill (May 31 2021 at 14:50, on Zulip):

If rust-analyzer were to be made a subtreee (I don't know if there's been any discussion of this), then in my (perhaps naive) view, it could function similar to clippy

Jonas Schievink [he/him] (May 31 2021 at 14:51, on Zulip):

Personally I would be fine with targeting nightly instead, if we use a rust-toolchain file we should get roughly the same contributor experience

Aaron Hill (May 31 2021 at 14:51, on Zulip):

It requires nightly, uses many compiler crates, but AFAIK that doesn't cause problems

Aaron Hill (May 31 2021 at 14:51, on Zulip):

of course, clippy is also distributed via rustup

matklad (May 31 2021 at 14:53, on Zulip):

So there are two parts here:

1) just using unstable features
2) relying on unstable features for bootstraping purposes

Both have a disadvantage that they make iterating on rust-analyzer itself significantly harder, especially for new contributors. Fast, reliable build & test process is something we optimize for a lot, to make code base more accessible, and to be able to ship more things faster ourselves.

Long term, when we start actually sharing significant parts of code with rustc, I can see us forgoing 1). I'd fight tooth and nail for 2). It's not that I can't be defeated on that one, but I'll fight :-)

Before we reach the situation where a lot of code is shared, I think it makes sense to at least try to stick with stable. chalk is a good example to learn from here.

matklad (May 31 2021 at 14:55, on Zulip):

I guess, ther's also

3) not using rustc_private crates

That I think is hardest requirenment, as that goes directly the idea of proper, re-usable libraries.

Jonas Schievink [he/him] (Jun 03 2021 at 13:31, on Zulip):

Do we have any features in the hir crates that can be controlled by the settings?

Jonas Schievink [he/him] (Jun 03 2021 at 13:31, on Zulip):

Because gating attribute macros behind one currently seems pretty annoying to do

Lukas Wirth (Jun 03 2021 at 13:33, on Zulip):

I don't think we do

Florian Diebold (Jun 03 2021 at 13:40, on Zulip):

the setting would just be another input query, I guess?

Jonas Schievink [he/him] (Jun 03 2021 at 13:41, on Zulip):

hmm, yeah, that's pretty simple actually

Jonas Schievink [he/him] (Jun 03 2021 at 13:41, on Zulip):

we just can't forget to set it (does salsa have default values for input queries?)

Florian Diebold (Jun 03 2021 at 13:41, on Zulip):

hmm I don't know

Jonas Schievink [he/him] (Jun 03 2021 at 14:14, on Zulip):

okay, that was relatively straightforward

Last update: Jul 26 2021 at 12:15UTC