7891: Improve handling of rustc_private r=matklad a=DJMcNab

This PR changes how `rust-analyzer` handles `rustc_private`. In particular, packages now must opt-in to using `rustc_private` in `Cargo.toml`, by adding:
```toml
[package.metadata.rust-analyzer]
rustc_private=true
```

This means that depending on crates which also use `rustc_private` will be significantly improved, since their dependencies on the `rustc_private` crates will be resolved properly.

A similar approach could be used in #6714 to allow annotating that your package uses the `test` crate, although I have not yet handled that in this PR.

Additionally, we now only index the crates which are transitive dependencies of `rustc_driver` in the `rustcSource` directory. This should not cause any change in behaviour when using `rustcSource: "discover"`, as the source used then will only be a partial clone. However, if `rustcSource` pointing at a local checkout of rustc, this should significantly improve the memory usage and lower indexing time. This is because we avoids indexing all crates in `src/tools/`, which includes `rust-analyzer` itself.

Furthermore, we also prefer named dependencies over dependencies from `rustcSource`. This ensures that feature resolution for crates which are depended on by both `rustc` and your crate uses the correct set for analysing your crate.

See also [introductory zulip stream](https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Fwg-rls-2.2E0/topic/Fixed.20crate.20graphs.20and.20optional.20builtin.20crates/near/229086673)

I have tested this in [priroda](https://github.com/oli-obk/priroda/), and it provides a significant improvement to the development experience (once I give `miri` the required data in `Cargo.toml`)

Todo:
- [ ] Documentation

This is ready to review, and I will add documentation if this would be accepted (or if I get time to do so anyway)

Co-authored-by: Daniel McNab <36049421+DJMcNab@users.noreply.github.com>
This commit is contained in:
bors[bot] 2021-03-08 16:51:20 +00:00 committed by GitHub
commit d57c9f7980
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 113 additions and 53 deletions

View file

@ -9,6 +9,8 @@ use cargo_metadata::{CargoOpt, MetadataCommand};
use la_arena::{Arena, Idx};
use paths::{AbsPath, AbsPathBuf};
use rustc_hash::FxHashMap;
use serde::Deserialize;
use serde_json::from_value;
use crate::build_data::BuildDataConfig;
use crate::utf8_stdout;
@ -104,6 +106,13 @@ pub struct PackageData {
pub active_features: Vec<String>,
// String representation of package id
pub id: String,
// The contents of [package.metadata.rust-analyzer]
pub metadata: RustAnalyzerPackageMetaData,
}
#[derive(Deserialize, Default, Debug, Clone, Eq, PartialEq)]
pub struct RustAnalyzerPackageMetaData {
pub rustc_private: bool,
}
#[derive(Debug, Clone, Eq, PartialEq)]
@ -161,6 +170,13 @@ impl PackageData {
}
}
#[derive(Deserialize, Default)]
// Deserialise helper for the cargo metadata
struct PackageMetadata {
#[serde(rename = "rust-analyzer")]
rust_analyzer: Option<RustAnalyzerPackageMetaData>,
}
impl CargoWorkspace {
pub fn from_cargo_metadata(
cargo_toml: &AbsPath,
@ -244,8 +260,10 @@ impl CargoWorkspace {
meta.packages.sort_by(|a, b| a.id.cmp(&b.id));
for meta_pkg in &meta.packages {
let cargo_metadata::Package { id, edition, name, manifest_path, version, .. } =
meta_pkg;
let cargo_metadata::Package {
id, edition, name, manifest_path, version, metadata, ..
} = meta_pkg;
let meta = from_value::<PackageMetadata>(metadata.clone()).unwrap_or_default();
let is_member = ws_members.contains(&id);
let edition = edition
.parse::<Edition>()
@ -262,6 +280,7 @@ impl CargoWorkspace {
dependencies: Vec::new(),
features: meta_pkg.features.clone().into_iter().collect(),
active_features: Vec::new(),
metadata: meta.rust_analyzer.unwrap_or_default(),
});
let pkg_data = &mut packages[pkg];
pkg_by_id.insert(id, pkg);

View file

@ -2,11 +2,7 @@
//! metadata` or `rust-project.json`) into representation stored in the salsa
//! database -- `CrateGraph`.
use std::{
fmt, fs,
path::{Component, Path},
process::Command,
};
use std::{collections::VecDeque, fmt, fs, path::Path, process::Command};
use anyhow::{Context, Result};
use base_db::{CrateDisplayName, CrateGraph, CrateId, CrateName, Edition, Env, FileId, ProcMacro};
@ -60,6 +56,7 @@ impl fmt::Debug for ProjectWorkspace {
match self {
ProjectWorkspace::Cargo { cargo, sysroot, rustc, rustc_cfg } => f
.debug_struct("Cargo")
.field("root", &cargo.workspace_root().file_name())
.field("n_packages", &cargo.packages().len())
.field("n_sysroot_crates", &sysroot.crates().len())
.field(
@ -279,11 +276,8 @@ impl ProjectWorkspace {
pub fn collect_build_data_configs(&self, collector: &mut BuildDataCollector) {
match self {
ProjectWorkspace::Cargo { cargo, rustc, .. } => {
ProjectWorkspace::Cargo { cargo, .. } => {
collector.add_config(&cargo.workspace_root(), cargo.build_data_config().clone());
if let Some(rustc) = rustc {
collector.add_config(rustc.workspace_root(), rustc.build_data_config().clone());
}
}
_ => {}
}
@ -380,9 +374,11 @@ fn cargo_to_crate_graph(
cfg_options.insert_atom("debug_assertions".into());
let mut pkg_crates = FxHashMap::default();
// Does any crate signal to rust-analyzer that they need the rustc_private crates?
let mut has_private = false;
// Next, create crates for each package, target pair
for pkg in cargo.packages() {
has_private |= cargo[pkg].metadata.rustc_private;
let mut lib_tgt = None;
for &tgt in cargo[pkg].targets.iter() {
if let Some(file_id) = load(&cargo[tgt].root) {
@ -443,28 +439,66 @@ fn cargo_to_crate_graph(
}
}
let mut rustc_pkg_crates = FxHashMap::default();
if has_private {
// If the user provided a path to rustc sources, we add all the rustc_private crates
// and create dependencies on them for the crates in the current workspace
// and create dependencies on them for the crates which opt-in to that
if let Some(rustc_workspace) = rustc {
for pkg in rustc_workspace.packages() {
handle_rustc_crates(
rustc_workspace,
load,
&mut crate_graph,
rustc_build_data_map,
&cfg_options,
proc_macro_loader,
&mut pkg_to_lib_crate,
&public_deps,
cargo,
&pkg_crates,
);
}
}
crate_graph
}
fn handle_rustc_crates(
rustc_workspace: &CargoWorkspace,
load: &mut dyn FnMut(&AbsPath) -> Option<FileId>,
crate_graph: &mut CrateGraph,
rustc_build_data_map: Option<&FxHashMap<String, BuildData>>,
cfg_options: &CfgOptions,
proc_macro_loader: &dyn Fn(&Path) -> Vec<ProcMacro>,
pkg_to_lib_crate: &mut FxHashMap<la_arena::Idx<crate::PackageData>, CrateId>,
public_deps: &[(CrateName, CrateId)],
cargo: &CargoWorkspace,
pkg_crates: &FxHashMap<la_arena::Idx<crate::PackageData>, Vec<CrateId>>,
) {
let mut rustc_pkg_crates = FxHashMap::default();
// The root package of the rustc-dev component is rustc_driver, so we match that
let root_pkg =
rustc_workspace.packages().find(|package| rustc_workspace[*package].name == "rustc_driver");
// The rustc workspace might be incomplete (such as if rustc-dev is not
// installed for the current toolchain) and `rustcSource` is set to discover.
if let Some(root_pkg) = root_pkg {
// Iterate through every crate in the dependency subtree of rustc_driver using BFS
let mut queue = VecDeque::new();
queue.push_back(root_pkg);
while let Some(pkg) = queue.pop_front() {
// Don't duplicate packages if they are dependended on a diamond pattern
// N.B. if this line is ommitted, we try to analyse over 4_800_000 crates
// which is not ideal
if rustc_pkg_crates.contains_key(&pkg) {
continue;
}
for dep in &rustc_workspace[pkg].dependencies {
queue.push_back(dep.pkg);
}
for &tgt in rustc_workspace[pkg].targets.iter() {
if rustc_workspace[tgt].kind != TargetKind::Lib {
continue;
}
// Exclude alloc / core / std
if rustc_workspace[tgt]
.root
.components()
.any(|c| c == Component::Normal("library".as_ref()))
{
continue;
}
if let Some(file_id) = load(&rustc_workspace[tgt].root) {
let crate_id = add_target_crate_root(
&mut crate_graph,
crate_graph,
&rustc_workspace[pkg],
rustc_build_data_map.and_then(|it| it.get(&rustc_workspace[pkg].id)),
&cfg_options,
@ -472,44 +506,50 @@ fn cargo_to_crate_graph(
file_id,
);
pkg_to_lib_crate.insert(pkg, crate_id);
// Add dependencies on the core / std / alloc for rustc
// Add dependencies on core / std / alloc for this crate
for (name, krate) in public_deps.iter() {
add_dep(&mut crate_graph, crate_id, name.clone(), *krate);
add_dep(crate_graph, crate_id, name.clone(), *krate);
}
rustc_pkg_crates.entry(pkg).or_insert_with(Vec::new).push(crate_id);
}
}
}
}
// Now add a dep edge from all targets of upstream to the lib
// target of downstream.
for pkg in rustc_workspace.packages() {
for pkg in rustc_pkg_crates.keys().copied() {
for dep in rustc_workspace[pkg].dependencies.iter() {
let name = CrateName::new(&dep.name).unwrap();
if let Some(&to) = pkg_to_lib_crate.get(&dep.pkg) {
for &from in rustc_pkg_crates.get(&pkg).into_iter().flatten() {
add_dep(&mut crate_graph, from, name.clone(), to);
add_dep(crate_graph, from, name.clone(), to);
}
}
}
}
// Add dependencies for all the crates of the current workspace to rustc_private libraries
// Add a dependency on the rustc_private crates for all targets of each package
// which opts in
for dep in rustc_workspace.packages() {
let name = CrateName::normalize_dashes(&rustc_workspace[dep].name);
if let Some(&to) = pkg_to_lib_crate.get(&dep) {
for pkg in cargo.packages() {
if !cargo[pkg].is_member {
let package = &cargo[pkg];
if !package.metadata.rustc_private {
continue;
}
for &from in pkg_crates.get(&pkg).into_iter().flatten() {
add_dep(&mut crate_graph, from, name.clone(), to);
// Avoid creating duplicate dependencies
// This avoids the situation where `from` depends on e.g. `arrayvec`, but
// `rust_analyzer` thinks that it should use the one from the `rustcSource`
// instead of the one from `crates.io`
if !crate_graph[from].dependencies.iter().any(|d| d.name == name) {
add_dep(crate_graph, from, name.clone(), to);
}
}
}
}
}
crate_graph
}
fn add_target_crate_root(

View file

@ -181,7 +181,8 @@ config_data! {
runnables_cargoExtraArgs: Vec<String> = "[]",
/// Path to the rust compiler sources, for usage in rustc_private projects, or "discover"
/// to try to automatically find it.
/// to try to automatically find it. Any project which uses rust-analyzer with the rustcPrivate
/// crates must set `[package.metadata.rust-analyzer] rustc_private=true` to use it.
rustcSource : Option<String> = "null",
/// Additional arguments to `rustfmt`.

View file

@ -107,7 +107,7 @@
[[rust-analyzer.runnables.cargoExtraArgs]]rust-analyzer.runnables.cargoExtraArgs (default: `[]`)::
Additional arguments to be passed to cargo for runnables such as tests or binaries.\nFor example, it may be `--release`.
[[rust-analyzer.rustcSource]]rust-analyzer.rustcSource (default: `null`)::
Path to the rust compiler sources, for usage in rustc_private projects, or "discover" to try to automatically find it.
Path to the rust compiler sources, for usage in rustc_private projects, or "discover" to try to automatically find it. Any project which uses rust-analyzer with the rustcPrivate crates must set `[package.metadata.rust-analyzer] rustc_private=true` to use it.
[[rust-analyzer.rustfmt.extraArgs]]rust-analyzer.rustfmt.extraArgs (default: `[]`)::
Additional arguments to `rustfmt`.
[[rust-analyzer.rustfmt.overrideCommand]]rust-analyzer.rustfmt.overrideCommand (default: `null`)::

View file

@ -713,7 +713,7 @@
}
},
"rust-analyzer.rustcSource": {
"markdownDescription": "Path to the rust compiler sources, for usage in rustc_private projects, or \"discover\" to try to automatically find it.",
"markdownDescription": "Path to the rust compiler sources, for usage in rustc_private projects, or \"discover\" to try to automatically find it. Any project which uses rust-analyzer with the rustcPrivate crates must set `[package.metadata.rust-analyzer] rustc_private=true` to use it.",
"default": null,
"type": [
"null",