Skip to content

Commit 0ef2213

Browse files
borsbors
committed
Auto merge of #17372 - Veykril:parallel-diagnostics, r=Veykril
feat: Compute native diagnostics in parallel
2 parents 5a1df7f + a800a1d commit 0ef2213

File tree

17 files changed

+150
-62
lines changed

17 files changed

+150
-62
lines changed

‎src/tools/rust-analyzer/Cargo.toml

+5-1
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,11 @@ xshell = "0.2.5"
162162
dashmap = { version = "=5.5.3", features = ["raw-api"] }
163163

164164
[workspace.lints.rust]
165-
rust_2018_idioms = "warn"
165+
bare_trait_objects = "warn"
166+
elided_lifetimes_in_paths = "warn"
167+
ellipsis_inclusive_range_patterns = "warn"
168+
explicit_outlives_requirements = "warn"
169+
unused_extern_crates = "warn"
166170
unused_lifetimes = "warn"
167171
unreachable_pub = "warn"
168172
semicolon_in_expressions_from_macros = "warn"

‎src/tools/rust-analyzer/crates/hir-ty/src/builder.rs

+1
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,7 @@ impl TyBuilder<()> {
246246
/// - yield type of coroutine ([`Coroutine::Yield`](std::ops::Coroutine::Yield))
247247
/// - return type of coroutine ([`Coroutine::Return`](std::ops::Coroutine::Return))
248248
/// - generic parameters in scope on `parent`
249+
///
249250
/// in this order.
250251
///
251252
/// This method prepopulates the builder with placeholder substitution of `parent`, so you

‎src/tools/rust-analyzer/crates/hir-ty/src/mir.rs

+12-13
Original file line numberDiff line numberDiff line change
@@ -898,20 +898,19 @@ pub enum Rvalue {
898898
Cast(CastKind, Operand, Ty),
899899

900900
// FIXME link to `pointer::offset` when it hits stable.
901-
/// * `Offset` has the same semantics as `pointer::offset`, except that the second
902-
/// parameter may be a `usize` as well.
903-
/// * The comparison operations accept `bool`s, `char`s, signed or unsigned integers, floats,
904-
/// raw pointers, or function pointers and return a `bool`. The types of the operands must be
905-
/// matching, up to the usual caveat of the lifetimes in function pointers.
906-
/// * Left and right shift operations accept signed or unsigned integers not necessarily of the
907-
/// same type and return a value of the same type as their LHS. Like in Rust, the RHS is
908-
/// truncated as needed.
909-
/// * The `Bit*` operations accept signed integers, unsigned integers, or bools with matching
910-
/// types and return a value of that type.
911-
/// * The remaining operations accept signed integers, unsigned integers, or floats with
912-
/// matching types and return a value of that type.
901+
// /// * `Offset` has the same semantics as `pointer::offset`, except that the second
902+
// /// parameter may be a `usize` as well.
903+
// /// * The comparison operations accept `bool`s, `char`s, signed or unsigned integers, floats,
904+
// /// raw pointers, or function pointers and return a `bool`. The types of the operands must be
905+
// /// matching, up to the usual caveat of the lifetimes in function pointers.
906+
// /// * Left and right shift operations accept signed or unsigned integers not necessarily of the
907+
// /// same type and return a value of the same type as their LHS. Like in Rust, the RHS is
908+
// /// truncated as needed.
909+
// /// * The `Bit*` operations accept signed integers, unsigned integers, or bools with matching
910+
// /// types and return a value of that type.
911+
// /// * The remaining operations accept signed integers, unsigned integers, or floats with
912+
// /// matching types and return a value of that type.
913913
//BinaryOp(BinOp, Box<(Operand, Operand)>),
914-
915914
/// Same as `BinaryOp`, but yields `(T, bool)` with a `bool` indicating an error condition.
916915
///
917916
/// When overflow checking is disabled and we are generating run-time code, the error condition

‎src/tools/rust-analyzer/crates/hir/src/term_search/tactics.rs

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
//! * `defs` - Set of items in scope at term search target location
66
//! * `lookup` - Lookup table for types
77
//! * `should_continue` - Function that indicates when to stop iterating
8+
//!
89
//! And they return iterator that yields type trees that unify with the `goal` type.
910
1011
use std::iter;

‎src/tools/rust-analyzer/crates/ide-assists/src/handlers/generate_function.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -393,9 +393,9 @@ impl FunctionBuilder {
393393
/// The rule for whether we focus a return type or not (and thus focus the function body),
394394
/// is rather simple:
395395
/// * If we could *not* infer what the return type should be, focus it (so the user can fill-in
396-
/// the correct return type).
396+
/// the correct return type).
397397
/// * If we could infer the return type, don't focus it (and thus focus the function body) so the
398-
/// user can change the `todo!` function body.
398+
/// user can change the `todo!` function body.
399399
fn make_return_type(
400400
ctx: &AssistContext<'_>,
401401
expr: &ast::Expr,
@@ -918,17 +918,17 @@ fn filter_generic_params(ctx: &AssistContext<'_>, node: SyntaxNode) -> Option<hi
918918
/// Say we have a trait bound `Struct<T>: Trait<U>`. Given `necessary_params`, when is it relevant
919919
/// and when not? Some observations:
920920
/// - When `necessary_params` contains `T`, it's likely that we want this bound, but now we have
921-
/// an extra param to consider: `U`.
921+
/// an extra param to consider: `U`.
922922
/// - On the other hand, when `necessary_params` contains `U` (but not `T`), then it's unlikely
923-
/// that we want this bound because it doesn't really constrain `U`.
923+
/// that we want this bound because it doesn't really constrain `U`.
924924
///
925925
/// (FIXME?: The latter clause might be overstating. We may want to include the bound if the self
926926
/// type does *not* include generic params at all - like `Option<i32>: From<U>`)
927927
///
928928
/// Can we make this a bit more formal? Let's define "dependency" between generic parameters and
929929
/// trait bounds:
930930
/// - A generic parameter `T` depends on a trait bound if `T` appears in the self type (i.e. left
931-
/// part) of the bound.
931+
/// part) of the bound.
932932
/// - A trait bound depends on a generic parameter `T` if `T` appears in the bound.
933933
///
934934
/// Using the notion, what we want is all the bounds that params in `necessary_params`

‎src/tools/rust-analyzer/crates/ide-assists/src/handlers/inline_call.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,7 @@ fn inline(
368368
_ => None,
369369
})
370370
.for_each(|usage| {
371-
ted::replace(usage, &this());
371+
ted::replace(usage, this());
372372
});
373373
}
374374
}
@@ -483,7 +483,7 @@ fn inline(
483483
cov_mark::hit!(inline_call_inline_direct_field);
484484
field.replace_expr(replacement.clone_for_update());
485485
} else {
486-
ted::replace(usage.syntax(), &replacement.syntax().clone_for_update());
486+
ted::replace(usage.syntax(), replacement.syntax().clone_for_update());
487487
}
488488
};
489489

‎src/tools/rust-analyzer/crates/ide-completion/src/context.rs

+1
Original file line numberDiff line numberDiff line change
@@ -452,6 +452,7 @@ pub(crate) struct CompletionContext<'a> {
452452
/// - crate-root
453453
/// - mod foo
454454
/// - mod bar
455+
///
455456
/// Here depth will be 2
456457
pub(crate) depth_from_crate_root: usize,
457458
}

‎src/tools/rust-analyzer/crates/load-cargo/src/lib.rs

+14-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use ide_db::{
1515
};
1616
use itertools::Itertools;
1717
use proc_macro_api::{MacroDylib, ProcMacroServer};
18-
use project_model::{CargoConfig, PackageRoot, ProjectManifest, ProjectWorkspace};
18+
use project_model::{CargoConfig, ManifestPath, PackageRoot, ProjectManifest, ProjectWorkspace};
1919
use span::Span;
2020
use tracing::instrument;
2121
use vfs::{file_set::FileSetConfig, loader::Handle, AbsPath, AbsPathBuf, VfsPath};
@@ -238,6 +238,19 @@ impl ProjectFolders {
238238
fsc.add_file_set(file_set_roots)
239239
}
240240

241+
// register the workspace manifest as well, note that this currently causes duplicates for
242+
// non-virtual cargo workspaces! We ought to fix that
243+
for manifest in workspaces.iter().filter_map(|ws| ws.manifest().map(ManifestPath::as_ref)) {
244+
let file_set_roots: Vec<VfsPath> = vec![VfsPath::from(manifest.to_owned())];
245+
246+
let entry = vfs::loader::Entry::Files(vec![manifest.to_owned()]);
247+
248+
res.watch.push(res.load.len());
249+
res.load.push(entry);
250+
local_filesets.push(fsc.len() as u64);
251+
fsc.add_file_set(file_set_roots)
252+
}
253+
241254
let fsc = fsc.build();
242255
res.source_root_config = SourceRootConfig { fsc, local_filesets };
243256

‎src/tools/rust-analyzer/crates/proc-macro-api/src/version.rs

+2
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ fn read_section<'a>(dylib_binary: &'a [u8], section_name: &str) -> io::Result<&'
9393
/// means bytes from here(including this sequence) are compressed in
9494
/// snappy compression format. Version info is inside here, so decompress
9595
/// this.
96+
///
9697
/// The bytes you get after decompressing the snappy format portion has
9798
/// following layout:
9899
/// * [b'r',b'u',b's',b't',0,0,0,5] is the first 8 bytes(again)
@@ -102,6 +103,7 @@ fn read_section<'a>(dylib_binary: &'a [u8], section_name: &str) -> io::Result<&'
102103
/// for the version string's utf8 bytes
103104
/// * [version string bytes encoded in utf8] <- GET THIS BOI
104105
/// * [some more bytes that we don't really care but about still there] :-)
106+
///
105107
/// Check this issue for more about the bytes layout:
106108
/// <https://github.com/rust-lang/rust-analyzer/issues/6174>
107109
pub fn read_version(dylib_path: &AbsPath) -> io::Result<String> {

‎src/tools/rust-analyzer/crates/project-model/src/project_json.rs

+5
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,11 @@ impl ProjectJson {
167167
&self.project_root
168168
}
169169

170+
/// Returns the path to the project's manifest file, if it exists.
171+
pub fn manifest(&self) -> Option<&ManifestPath> {
172+
self.manifest.as_ref()
173+
}
174+
170175
/// Returns the path to the project's manifest or root folder, if no manifest exists.
171176
pub fn manifest_or_root(&self) -> &AbsPath {
172177
self.manifest.as_ref().map_or(&self.project_root, |manifest| manifest.as_ref())

‎src/tools/rust-analyzer/crates/project-model/src/workspace.rs

+10
Original file line numberDiff line numberDiff line change
@@ -527,6 +527,16 @@ impl ProjectWorkspace {
527527
}
528528
}
529529

530+
pub fn manifest(&self) -> Option<&ManifestPath> {
531+
match &self.kind {
532+
ProjectWorkspaceKind::Cargo { cargo, .. } => Some(cargo.manifest_path()),
533+
ProjectWorkspaceKind::Json(project) => project.manifest(),
534+
ProjectWorkspaceKind::DetachedFile { cargo, .. } => {
535+
Some(cargo.as_ref()?.0.manifest_path())
536+
}
537+
}
538+
}
539+
530540
pub fn find_sysroot_proc_macro_srv(&self) -> anyhow::Result<AbsPathBuf> {
531541
self.sysroot.discover_proc_macro_srv()
532542
}

‎src/tools/rust-analyzer/crates/rust-analyzer/src/diagnostics.rs

+36-12
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use ide_db::FxHashMap;
88
use itertools::Itertools;
99
use nohash_hasher::{IntMap, IntSet};
1010
use rustc_hash::FxHashSet;
11+
use stdx::iter_eq_by;
1112
use triomphe::Arc;
1213

1314
use crate::{global_state::GlobalStateSnapshot, lsp, lsp_ext};
@@ -22,14 +23,21 @@ pub struct DiagnosticsMapConfig {
2223
pub check_ignore: FxHashSet<String>,
2324
}
2425

26+
pub(crate) type DiagnosticsGeneration = usize;
27+
2528
#[derive(Debug, Default, Clone)]
2629
pub(crate) struct DiagnosticCollection {
2730
// FIXME: should be IntMap<FileId, Vec<ra_id::Diagnostic>>
28-
pub(crate) native: IntMap<FileId, Vec<lsp_types::Diagnostic>>,
31+
pub(crate) native: IntMap<FileId, (DiagnosticsGeneration, Vec<lsp_types::Diagnostic>)>,
2932
// FIXME: should be Vec<flycheck::Diagnostic>
3033
pub(crate) check: IntMap<usize, IntMap<FileId, Vec<lsp_types::Diagnostic>>>,
3134
pub(crate) check_fixes: CheckFixes,
3235
changes: IntSet<FileId>,
36+
/// Counter for supplying a new generation number for diagnostics.
37+
/// This is used to keep track of when to clear the diagnostics for a given file as we compute
38+
/// diagnostics on multiple worker threads simultaneously which may result in multiple diagnostics
39+
/// updates for the same file in a single generation update (due to macros affecting multiple files).
40+
generation: DiagnosticsGeneration,
3341
}
3442

3543
#[derive(Debug, Clone)]
@@ -82,29 +90,39 @@ impl DiagnosticCollection {
8290

8391
pub(crate) fn set_native_diagnostics(
8492
&mut self,
93+
generation: DiagnosticsGeneration,
8594
file_id: FileId,
86-
diagnostics: Vec<lsp_types::Diagnostic>,
95+
mut diagnostics: Vec<lsp_types::Diagnostic>,
8796
) {
88-
if let Some(existing_diagnostics) = self.native.get(&file_id) {
97+
diagnostics.sort_by_key(|it| (it.range.start, it.range.end));
98+
if let Some((old_gen, existing_diagnostics)) = self.native.get_mut(&file_id) {
8999
if existing_diagnostics.len() == diagnostics.len()
90-
&& diagnostics
91-
.iter()
92-
.zip(existing_diagnostics)
93-
.all(|(new, existing)| are_diagnostics_equal(new, existing))
100+
&& iter_eq_by(&diagnostics, &*existing_diagnostics, |new, existing| {
101+
are_diagnostics_equal(new, existing)
102+
})
94103
{
104+
// don't signal an update if the diagnostics are the same
95105
return;
96106
}
107+
if *old_gen < generation || generation == 0 {
108+
self.native.insert(file_id, (generation, diagnostics));
109+
} else {
110+
existing_diagnostics.extend(diagnostics);
111+
// FIXME: Doing the merge step of a merge sort here would be a bit more performant
112+
// but eh
113+
existing_diagnostics.sort_by_key(|it| (it.range.start, it.range.end))
114+
}
115+
} else {
116+
self.native.insert(file_id, (generation, diagnostics));
97117
}
98-
99-
self.native.insert(file_id, diagnostics);
100118
self.changes.insert(file_id);
101119
}
102120

103121
pub(crate) fn diagnostics_for(
104122
&self,
105123
file_id: FileId,
106124
) -> impl Iterator<Item = &lsp_types::Diagnostic> {
107-
let native = self.native.get(&file_id).into_iter().flatten();
125+
let native = self.native.get(&file_id).into_iter().flat_map(|(_, d)| d);
108126
let check = self.check.values().filter_map(move |it| it.get(&file_id)).flatten();
109127
native.chain(check)
110128
}
@@ -115,6 +133,11 @@ impl DiagnosticCollection {
115133
}
116134
Some(mem::take(&mut self.changes))
117135
}
136+
137+
pub(crate) fn next_generation(&mut self) -> usize {
138+
self.generation += 1;
139+
self.generation
140+
}
118141
}
119142

120143
fn are_diagnostics_equal(left: &lsp_types::Diagnostic, right: &lsp_types::Diagnostic) -> bool {
@@ -126,7 +149,8 @@ fn are_diagnostics_equal(left: &lsp_types::Diagnostic, right: &lsp_types::Diagno
126149

127150
pub(crate) fn fetch_native_diagnostics(
128151
snapshot: GlobalStateSnapshot,
129-
subscriptions: Vec<FileId>,
152+
subscriptions: std::sync::Arc<[FileId]>,
153+
slice: std::ops::Range<usize>,
130154
) -> Vec<(FileId, Vec<lsp_types::Diagnostic>)> {
131155
let _p = tracing::info_span!("fetch_native_diagnostics").entered();
132156
let _ctx = stdx::panic_context::enter("fetch_native_diagnostics".to_owned());
@@ -149,7 +173,7 @@ pub(crate) fn fetch_native_diagnostics(
149173
// the diagnostics produced may point to different files not requested by the concrete request,
150174
// put those into here and filter later
151175
let mut odd_ones = Vec::new();
152-
let mut diagnostics = subscriptions
176+
let mut diagnostics = subscriptions[slice]
153177
.iter()
154178
.copied()
155179
.filter_map(|file_id| {

‎src/tools/rust-analyzer/crates/rust-analyzer/src/global_state.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,9 @@ pub(crate) struct GlobalStateSnapshot {
163163
pub(crate) semantic_tokens_cache: Arc<Mutex<FxHashMap<Url, SemanticTokens>>>,
164164
vfs: Arc<RwLock<(vfs::Vfs, IntMap<FileId, LineEndings>)>>,
165165
pub(crate) workspaces: Arc<Vec<ProjectWorkspace>>,
166-
// used to signal semantic highlighting to fall back to syntax based highlighting until proc-macros have been loaded
166+
// used to signal semantic highlighting to fall back to syntax based highlighting until
167+
// proc-macros have been loaded
168+
// FIXME: Can we derive this from somewhere else?
167169
pub(crate) proc_macros_loaded: bool,
168170
pub(crate) flycheck: Arc<[FlycheckHandle]>,
169171
}

0 commit comments

Comments
 (0)