Stream: t-compiler/wg-rls-2.0

Topic: Explore hiding cfg excluded code


Paul Faria (Jun 17 2020 at 13:42, on Zulip):

Hi @matklad , I was thinking about looking into dimming code that is not compiled with cfg, so, e.g. if you have some code that's

#[cfg(not(test))]
struct A { ... }

#[cfg(test)]
struct A { ... }

then one of the two could be hidden with styling. At least for vs code, that's not currently possible outside of Diagnostic::Unused, which doesn't seem like an appropriate usage of that tag. Do you think it's still worth that effort or add in the initial support as a possible motivator to get some support in vs code for some additional dimming outside of the diagnostic tag?

Paul Faria (Jun 17 2020 at 13:45, on Zulip):

Implementation wise, I think I could build off the work the work that was done with doctests in terms of replacing the highlighting stacks.

matklad (Jun 17 2020 at 13:53, on Zulip):

I prefer to think not in terms of VS Code, but in terms of some hypothetical "perfect" IDE client. It's better if clients catch up with the server, than vice verse :)

For cfg, I think we need to do two things:

matklad (Jun 17 2020 at 13:54, on Zulip):

The harder bit is figuring out which code is disabled, and what does it mean at all...

Like, you can have a single crate both with and without --test enabled. SHould this disable #[cfg(test)] code?

Jonas Schievink (Jun 17 2020 at 14:00, on Zulip):

In my ideal world, it would disable #[cfg(test)] code when the cursor is in an item that does not have #[cfg(test)] on it

Paul Faria (Jun 17 2020 at 14:07, on Zulip):

This actually relates to a conversation I was having with @woody77 . We were thinking of a way to toggle cfg items on and off through some kind of command rather than changing a settings config. I've run into this myself in some of my own projects where some feature flags conflict with each other, and --all-features isn't a possible solution.

matklad (Jun 17 2020 at 14:10, on Zulip):

The ideal UI woudl be https://github.com/intellij-rust/intellij-rust/pull/5189

Paul Faria (Jun 17 2020 at 14:11, on Zulip):

Oh wow I was thinking of checkboxes too! but I didn't think to place it in Cargo.toml

Paul Faria (Jun 17 2020 at 14:11, on Zulip):

One area where this could get confusing is if you're navigating multiple crates

Paul Faria (Jun 17 2020 at 14:11, on Zulip):

For example, in one of my crates I have subfolders with test crates that depend on the main crate with different features toggled on and off.

Paul Faria (Jun 17 2020 at 14:12, on Zulip):

Navigating through the code there, my preference would be that the context of the flags in the crates that I'm jumping to would depend on where I jumped from. But from an internals perspective, that sounds like something that's very hard to get right.

Paul Faria (Jun 17 2020 at 14:14, on Zulip):

Unless maybe there's a way to say, "show all code as if this crate was the main crate". I imagine multi-crate scenarios like this are somewhat rare

matklad (Jun 17 2020 at 14:15, on Zulip):

But from an internals perspective, that sounds like something that's very hard to get right.

That's actually how it is implemnted internally already

matklad (Jun 17 2020 at 14:15, on Zulip):

The impl parts are not that difficult, you just have to allow for several instances of a crate in the crate graph

Paul Faria (Jun 17 2020 at 14:16, on Zulip):

Oh I had no clue that was in place already.

matklad (Jun 17 2020 at 14:16, on Zulip):

The hard thing is UI: reconsiling the fact that the user sees "a struct" with a fact that this struct corresponds to 2-3 hir::Struct in different crates internally

Paul Faria (Jun 17 2020 at 14:17, on Zulip):

Which part of the code should I read to get a better understanding of how this works?

Paul Faria (Jun 17 2020 at 14:18, on Zulip):

I also realized, regarding the checkbox UI you linked from intellij, we'd need an alternate implementation for supporting json projects.

matklad (Jun 17 2020 at 14:19, on Zulip):

So, the reason why it works is that we don't restrict CrateGraph to only have a single Crate for each root modules

matklad (Jun 17 2020 at 14:20, on Zulip):

And if you are already in the context of a specific Crate, then you get correct dependnecies, etc

matklad (Jun 17 2020 at 14:20, on Zulip):

The only trick happens when you translate a raw position (file foo.rs, line 92, offset 62) into a crate with this file

matklad (Jun 17 2020 at 14:21, on Zulip):

Here, at the moment we basically "guess": pick the first crate which matches

matklad (Jun 17 2020 at 14:21, on Zulip):

https://github.com/rust-analyzer/rust-analyzer/blob/master/crates/ra_hir/src/semantics/source_to_def.rs#L30-L39

woody77 (Jun 24 2020 at 23:47, on Zulip):

matklad said:

The impl parts are not that difficult, you just have to allow for several instances of a crate in the crate graph

Catching up _very_ belatedly... This has been on my mind a bunch, recently, as for us, our (current) rust-project.json is a dependency graph of crates to compile, and crates can be listed multiple times, with different sets of enabled features. I've been trying to decide if we should compress that down or not into something that looks more like Cargo.toml, where each crate is listed as a set of potential features, or the summation of all enabled features in all uses (but then what to do with conflicts).

I really like the ability we currently seem like we have, where we can put the stdlib into the crate graph multiple times, for different architectures (since we're building both host and embedded-target (aka cross-compilation) in the same tree), and that the graph can know which particular target arch/os we're compiling for.

But the UI side is hard. In the graph, it seems easy to know that for a struct that's only in the graph once, that all of its symbols are quite clear. But once you jump from that struct, to one of the types that it uses, which is in a file that matches multiple places in the graph... now what?

Hence @Paul Faria and I talking about how we'd go about marking which config sets to enable (although with a compiled-target approach, RA is able to know how many places in the graph that something exists, and but I'm not sure if we can send back multiple options based on features (vs. files that might match).

Last update: Sep 27 2020 at 13:30UTC