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.

11 files changed, 45 insertions, 44 deletions

diff --git a/crates/hir-def/src/data.rs b/crates/hir-def/src/data.rs
index f506864902c..d4c1db8b95b 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 18aefbf332c..f9fe6d3b903 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 b5a302dbcaf..8c7fdaaf58b 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 f1f0d8990f1..3a65bc7c6ea 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 66ceb1b7d42..a500c24ce88 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 6c72dbb5edf..9a7e1a2ae6d 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 008c63c35a6..28eae6079c5 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 90649873297..5fcda6ba136 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 e8bb9ee938f..1bb82cc191f 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 4c0a538f712..5400071c4b6 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 5b3f449eeeb..dbfab537fe5 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) {