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.

Author: bors

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(),
             ));
         }
 

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);

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 },
         }
     }
 

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

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;
         }

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}"),

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();

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)]

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,

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()
     });
 }

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) {