bendns' repos / rust-analyzer
Unnamed repository; edit this file 'description' to name the repository.
Auto merge of #16748 - Veykril:on-demand-validation-err, r=Veykril
internal: Compute syntax validation errors on demand The LRU cache causes us to re-parse trees quite often, yet we don't use the validation errors at all. With this we push calculating them off to the caller who is interested in them.
Browse Source
bors 2024-03-04
parent 99a1b8f · parent c3c9f5f · commit a7e9f12
Diffstat
-rw-r--r--crates/hir-def/src/data.rs5
-rw-r--r--crates/hir-def/src/nameres/collector.rs6
-rw-r--r--crates/hir-def/src/nameres/diagnostics.rs7
-rw-r--r--crates/hir-expand/src/db.rs4
-rw-r--r--crates/hir-expand/src/files.rs2
-rw-r--r--crates/ide-diagnostics/src/lib.rs2
-rw-r--r--crates/rust-analyzer/src/integrated_benchmarks.rs4
-rw-r--r--crates/rust-analyzer/src/tracing/hprof.rs4
-rw-r--r--crates/syntax/src/lib.rs21
-rw-r--r--crates/syntax/src/tests.rs4
-rw-r--r--crates/syntax/src/validation.rs30
11 files changed, 45 insertions, 44 deletions
diff --git a/crates/hir-def/src/data.rs b/crates/hir-def/src/data.rs
index f506864902..d4c1db8b95 100644
--- a/crates/hir-def/src/data.rs
+++ b/crates/hir-def/src/data.rs
@@ -788,11 +788,12 @@ impl<'a> AssocItemCollector<'a> {
};
self.diagnostics.push(diag);
}
- if let errors @ [_, ..] = parse.errors() {
+ let errors = parse.errors();
+ if !errors.is_empty() {
self.diagnostics.push(DefDiagnostic::macro_expansion_parse_error(
self.module_id.local_id,
error_call_kind(),
- errors,
+ errors.into_boxed_slice(),
));
}
diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs
index 18aefbf332..f9fe6d3b90 100644
--- a/crates/hir-def/src/nameres/collector.rs
+++ b/crates/hir-def/src/nameres/collector.rs
@@ -1384,7 +1384,9 @@ impl DefCollector<'_> {
// First, fetch the raw expansion result for purposes of error reporting. This goes through
// `parse_macro_expansion_error` to avoid depending on the full expansion result (to improve
// incrementality).
- let ExpandResult { value, err } = self.db.parse_macro_expansion_error(macro_call_id);
+ // FIXME: This kind of error fetching feels a bit odd?
+ let ExpandResult { value: errors, err } =
+ self.db.parse_macro_expansion_error(macro_call_id);
if let Some(err) = err {
let loc: MacroCallLoc = self.db.lookup_intern_macro_call(macro_call_id);
let diag = match err {
@@ -1398,7 +1400,7 @@ impl DefCollector<'_> {
self.def_map.diagnostics.push(diag);
}
- if let errors @ [_, ..] = &*value {
+ if !errors.is_empty() {
let loc: MacroCallLoc = self.db.lookup_intern_macro_call(macro_call_id);
let diag = DefDiagnostic::macro_expansion_parse_error(module_id, loc.kind, errors);
self.def_map.diagnostics.push(diag);
diff --git a/crates/hir-def/src/nameres/diagnostics.rs b/crates/hir-def/src/nameres/diagnostics.rs
index b5a302dbca..8c7fdaaf58 100644
--- a/crates/hir-def/src/nameres/diagnostics.rs
+++ b/crates/hir-def/src/nameres/diagnostics.rs
@@ -117,14 +117,11 @@ impl DefDiagnostic {
pub(crate) fn macro_expansion_parse_error(
container: LocalModuleId,
ast: MacroCallKind,
- errors: &[SyntaxError],
+ errors: Box<[SyntaxError]>,
) -> Self {
Self {
in_module: container,
- kind: DefDiagnosticKind::MacroExpansionParseError {
- ast,
- errors: errors.to_vec().into_boxed_slice(),
- },
+ kind: DefDiagnosticKind::MacroExpansionParseError { ast, errors },
}
}
diff --git a/crates/hir-expand/src/db.rs b/crates/hir-expand/src/db.rs
index f1f0d8990f..3a65bc7c6e 100644
--- a/crates/hir-expand/src/db.rs
+++ b/crates/hir-expand/src/db.rs
@@ -303,7 +303,7 @@ fn parse_macro_expansion_error(
macro_call_id: MacroCallId,
) -> ExpandResult<Box<[SyntaxError]>> {
db.parse_macro_expansion(MacroFileId { macro_call_id })
- .map(|it| it.0.errors().to_vec().into_boxed_slice())
+ .map(|it| it.0.errors().into_boxed_slice())
}
pub(crate) fn parse_with_map(
@@ -445,7 +445,7 @@ fn macro_arg(
if matches!(loc.def.kind, MacroDefKind::BuiltInEager(..)) {
match parse.errors() {
- [] => ValueResult::ok((Arc::new(tt), undo_info)),
+ errors if errors.is_empty() => ValueResult::ok((Arc::new(tt), undo_info)),
errors => ValueResult::new(
(Arc::new(tt), undo_info),
// Box::<[_]>::from(res.errors()), not stable yet
diff --git a/crates/hir-expand/src/files.rs b/crates/hir-expand/src/files.rs
index 66ceb1b7d4..a500c24ce8 100644
--- a/crates/hir-expand/src/files.rs
+++ b/crates/hir-expand/src/files.rs
@@ -252,7 +252,7 @@ impl InFile<&SyntaxNode> {
map_node_range_up(db, &db.expansion_span_map(file_id), self.value.text_range())?;
// FIXME: Figure out an API that makes proper use of ctx, this only exists to
- // keep pre-token map rewrite behaviour.
+ // keep pre-token map rewrite behavior.
if !ctx.is_root() {
return None;
}
diff --git a/crates/ide-diagnostics/src/lib.rs b/crates/ide-diagnostics/src/lib.rs
index 6c72dbb5ed..9a7e1a2ae6 100644
--- a/crates/ide-diagnostics/src/lib.rs
+++ b/crates/ide-diagnostics/src/lib.rs
@@ -299,7 +299,7 @@ pub fn diagnostics(
let mut res = Vec::new();
// [#34344] Only take first 128 errors to prevent slowing down editor/ide, the number 128 is chosen arbitrarily.
- res.extend(parse.errors().iter().take(128).map(|err| {
+ res.extend(parse.errors().into_iter().take(128).map(|err| {
Diagnostic::new(
DiagnosticCode::RustcHardError("syntax-error"),
format!("Syntax Error: {err}"),
diff --git a/crates/rust-analyzer/src/integrated_benchmarks.rs b/crates/rust-analyzer/src/integrated_benchmarks.rs
index 008c63c35a..28eae6079c 100644
--- a/crates/rust-analyzer/src/integrated_benchmarks.rs
+++ b/crates/rust-analyzer/src/integrated_benchmarks.rs
@@ -55,7 +55,7 @@ fn integrated_highlighting_benchmark() {
vfs.file_id(&path).unwrap_or_else(|| panic!("can't find virtual file for {path}"))
};
- crate::tracing::hprof::init("*>100");
+ let _g = crate::tracing::hprof::init("*>150");
{
let _it = stdx::timeit("initial");
@@ -72,6 +72,8 @@ fn integrated_highlighting_benchmark() {
host.apply_change(change);
}
+ let _g = crate::tracing::hprof::init("*>50");
+
{
let _it = stdx::timeit("after change");
let _span = profile::cpu_span();
diff --git a/crates/rust-analyzer/src/tracing/hprof.rs b/crates/rust-analyzer/src/tracing/hprof.rs
index 9064987329..5fcda6ba13 100644
--- a/crates/rust-analyzer/src/tracing/hprof.rs
+++ b/crates/rust-analyzer/src/tracing/hprof.rs
@@ -52,7 +52,7 @@ use tracing_subscriber::{
use crate::tracing::hprof;
-pub fn init(spec: &str) {
+pub fn init(spec: &str) -> tracing::subscriber::DefaultGuard {
let (write_filter, allowed_names) = WriteFilter::from_spec(spec);
// this filter the first pass for `tracing`: these are all the "profiling" spans, but things like
@@ -76,7 +76,7 @@ pub fn init(spec: &str) {
.with_filter(profile_filter);
let subscriber = Registry::default().with(layer);
- tracing::subscriber::set_global_default(subscriber).unwrap();
+ tracing::subscriber::set_default(subscriber)
}
#[derive(Default, Debug)]
diff --git a/crates/syntax/src/lib.rs b/crates/syntax/src/lib.rs
index e8bb9ee938..1bb82cc191 100644
--- a/crates/syntax/src/lib.rs
+++ b/crates/syntax/src/lib.rs
@@ -97,8 +97,11 @@ impl<T> Parse<T> {
pub fn syntax_node(&self) -> SyntaxNode {
SyntaxNode::new_root(self.green.clone())
}
- pub fn errors(&self) -> &[SyntaxError] {
- self.errors.as_deref().unwrap_or_default()
+
+ pub fn errors(&self) -> Vec<SyntaxError> {
+ let mut errors = if let Some(e) = self.errors.as_deref() { e.to_vec() } else { vec![] };
+ validation::validate(&self.syntax_node(), &mut errors);
+ errors
}
}
@@ -111,10 +114,10 @@ impl<T: AstNode> Parse<T> {
T::cast(self.syntax_node()).unwrap()
}
- pub fn ok(self) -> Result<T, Arc<[SyntaxError]>> {
- match self.errors {
- Some(e) => Err(e),
- None => Ok(self.tree()),
+ pub fn ok(self) -> Result<T, Vec<SyntaxError>> {
+ match self.errors() {
+ errors if !errors.is_empty() => Err(errors),
+ _ => Ok(self.tree()),
}
}
}
@@ -132,7 +135,7 @@ impl Parse<SyntaxNode> {
impl Parse<SourceFile> {
pub fn debug_dump(&self) -> String {
let mut buf = format!("{:#?}", self.tree().syntax());
- for err in self.errors.as_deref().into_iter().flat_map(<[_]>::iter) {
+ for err in self.errors() {
format_to!(buf, "error {:?}: {}\n", err.range(), err);
}
buf
@@ -169,11 +172,9 @@ pub use crate::ast::SourceFile;
impl SourceFile {
pub fn parse(text: &str) -> Parse<SourceFile> {
let _p = tracing::span!(tracing::Level::INFO, "SourceFile::parse").entered();
- let (green, mut errors) = parsing::parse_text(text);
+ let (green, errors) = parsing::parse_text(text);
let root = SyntaxNode::new_root(green.clone());
- errors.extend(validation::validate(&root));
-
assert_eq!(root.kind(), SyntaxKind::SOURCE_FILE);
Parse {
green,
diff --git a/crates/syntax/src/tests.rs b/crates/syntax/src/tests.rs
index 4c0a538f71..5400071c4b 100644
--- a/crates/syntax/src/tests.rs
+++ b/crates/syntax/src/tests.rs
@@ -39,7 +39,7 @@ fn benchmark_parser() {
let tree = {
let _b = bench("parsing");
let p = SourceFile::parse(&data);
- assert!(p.errors.is_none());
+ assert!(p.errors().is_empty());
assert_eq!(p.tree().syntax.text_range().len(), 352474.into());
p.tree()
};
@@ -57,7 +57,7 @@ fn validation_tests() {
dir_tests(&test_data_dir(), &["parser/validation"], "rast", |text, path| {
let parse = SourceFile::parse(text);
let errors = parse.errors();
- assert_errors_are_present(errors, path);
+ assert_errors_are_present(&errors, path);
parse.debug_dump()
});
}
diff --git a/crates/syntax/src/validation.rs b/crates/syntax/src/validation.rs
index 5b3f449eee..dbfab537fe 100644
--- a/crates/syntax/src/validation.rs
+++ b/crates/syntax/src/validation.rs
@@ -15,34 +15,32 @@ use crate::{
SyntaxNode, SyntaxToken, TextSize, T,
};
-pub(crate) fn validate(root: &SyntaxNode) -> Vec<SyntaxError> {
+pub(crate) fn validate(root: &SyntaxNode, errors: &mut Vec<SyntaxError>) {
let _p = tracing::span!(tracing::Level::INFO, "parser::validate").entered();
// FIXME:
// * Add unescape validation of raw string literals and raw byte string literals
// * Add validation of doc comments are being attached to nodes
- let mut errors = Vec::new();
for node in root.descendants() {
match_ast! {
match node {
- ast::Literal(it) => validate_literal(it, &mut errors),
- ast::Const(it) => validate_const(it, &mut errors),
- ast::BlockExpr(it) => block::validate_block_expr(it, &mut errors),
- ast::FieldExpr(it) => validate_numeric_name(it.name_ref(), &mut errors),
- ast::RecordExprField(it) => validate_numeric_name(it.name_ref(), &mut errors),
- ast::Visibility(it) => validate_visibility(it, &mut errors),
- ast::RangeExpr(it) => validate_range_expr(it, &mut errors),
- ast::PathSegment(it) => validate_path_keywords(it, &mut errors),
- ast::RefType(it) => validate_trait_object_ref_ty(it, &mut errors),
- ast::PtrType(it) => validate_trait_object_ptr_ty(it, &mut errors),
- ast::FnPtrType(it) => validate_trait_object_fn_ptr_ret_ty(it, &mut errors),
- ast::MacroRules(it) => validate_macro_rules(it, &mut errors),
- ast::LetExpr(it) => validate_let_expr(it, &mut errors),
+ ast::Literal(it) => validate_literal(it, errors),
+ ast::Const(it) => validate_const(it, errors),
+ ast::BlockExpr(it) => block::validate_block_expr(it, errors),
+ ast::FieldExpr(it) => validate_numeric_name(it.name_ref(), errors),
+ ast::RecordExprField(it) => validate_numeric_name(it.name_ref(), errors),
+ ast::Visibility(it) => validate_visibility(it, errors),
+ ast::RangeExpr(it) => validate_range_expr(it, errors),
+ ast::PathSegment(it) => validate_path_keywords(it, errors),
+ ast::RefType(it) => validate_trait_object_ref_ty(it, errors),
+ ast::PtrType(it) => validate_trait_object_ptr_ty(it, errors),
+ ast::FnPtrType(it) => validate_trait_object_fn_ptr_ret_ty(it, errors),
+ ast::MacroRules(it) => validate_macro_rules(it, errors),
+ ast::LetExpr(it) => validate_let_expr(it, errors),
_ => (),
}
}
}
- errors
}
fn rustc_unescape_error_to_string(err: unescape::EscapeError) -> (&'static str, bool) {