internal: Compute syntax validation errors on demand

This commit is contained in:
Lukas Wirth 2024-03-04 11:39:38 +01:00
parent 4303e741de
commit c3c9f5ffe1
11 changed files with 45 additions and 44 deletions

View File

@ -788,11 +788,12 @@ fn collect_macro_items(
}; };
self.diagnostics.push(diag); 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.diagnostics.push(DefDiagnostic::macro_expansion_parse_error(
self.module_id.local_id, self.module_id.local_id,
error_call_kind(), error_call_kind(),
errors, errors.into_boxed_slice(),
)); ));
} }

View File

@ -1384,7 +1384,9 @@ fn collect_macro_expansion(
// First, fetch the raw expansion result for purposes of error reporting. This goes through // 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 // `parse_macro_expansion_error` to avoid depending on the full expansion result (to improve
// incrementality). // 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 { if let Some(err) = err {
let loc: MacroCallLoc = self.db.lookup_intern_macro_call(macro_call_id); let loc: MacroCallLoc = self.db.lookup_intern_macro_call(macro_call_id);
let diag = match err { let diag = match err {
@ -1398,7 +1400,7 @@ fn collect_macro_expansion(
self.def_map.diagnostics.push(diag); 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 loc: MacroCallLoc = self.db.lookup_intern_macro_call(macro_call_id);
let diag = DefDiagnostic::macro_expansion_parse_error(module_id, loc.kind, errors); let diag = DefDiagnostic::macro_expansion_parse_error(module_id, loc.kind, errors);
self.def_map.diagnostics.push(diag); self.def_map.diagnostics.push(diag);

View File

@ -117,14 +117,11 @@ pub(crate) fn macro_error(
pub(crate) fn macro_expansion_parse_error( pub(crate) fn macro_expansion_parse_error(
container: LocalModuleId, container: LocalModuleId,
ast: MacroCallKind, ast: MacroCallKind,
errors: &[SyntaxError], errors: Box<[SyntaxError]>,
) -> Self { ) -> Self {
Self { Self {
in_module: container, in_module: container,
kind: DefDiagnosticKind::MacroExpansionParseError { kind: DefDiagnosticKind::MacroExpansionParseError { ast, errors },
ast,
errors: errors.to_vec().into_boxed_slice(),
},
} }
} }

View File

@ -303,7 +303,7 @@ fn parse_macro_expansion_error(
macro_call_id: MacroCallId, macro_call_id: MacroCallId,
) -> ExpandResult<Box<[SyntaxError]>> { ) -> ExpandResult<Box<[SyntaxError]>> {
db.parse_macro_expansion(MacroFileId { macro_call_id }) 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( pub(crate) fn parse_with_map(
@ -445,7 +445,7 @@ fn macro_arg(
if matches!(loc.def.kind, MacroDefKind::BuiltInEager(..)) { if matches!(loc.def.kind, MacroDefKind::BuiltInEager(..)) {
match parse.errors() { 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( errors => ValueResult::new(
(Arc::new(tt), undo_info), (Arc::new(tt), undo_info),
// Box::<[_]>::from(res.errors()), not stable yet // Box::<[_]>::from(res.errors()), not stable yet

View File

@ -252,7 +252,7 @@ pub fn original_syntax_node(
map_node_range_up(db, &db.expansion_span_map(file_id), self.value.text_range())?; 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 // 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() { if !ctx.is_root() {
return None; return None;
} }

View File

@ -299,7 +299,7 @@ pub fn diagnostics(
let mut res = Vec::new(); let mut res = Vec::new();
// [#34344] Only take first 128 errors to prevent slowing down editor/ide, the number 128 is chosen arbitrarily. // [#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( Diagnostic::new(
DiagnosticCode::RustcHardError("syntax-error"), DiagnosticCode::RustcHardError("syntax-error"),
format!("Syntax Error: {err}"), format!("Syntax Error: {err}"),

View File

@ -55,7 +55,7 @@ fn integrated_highlighting_benchmark() {
vfs.file_id(&path).unwrap_or_else(|| panic!("can't find virtual file for {path}")) 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"); let _it = stdx::timeit("initial");
@ -72,6 +72,8 @@ fn integrated_highlighting_benchmark() {
host.apply_change(change); host.apply_change(change);
} }
let _g = crate::tracing::hprof::init("*>50");
{ {
let _it = stdx::timeit("after change"); let _it = stdx::timeit("after change");
let _span = profile::cpu_span(); let _span = profile::cpu_span();

View File

@ -52,7 +52,7 @@
use crate::tracing::hprof; 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); 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 // 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); .with_filter(profile_filter);
let subscriber = Registry::default().with(layer); let subscriber = Registry::default().with(layer);
tracing::subscriber::set_global_default(subscriber).unwrap(); tracing::subscriber::set_default(subscriber)
} }
#[derive(Default, Debug)] #[derive(Default, Debug)]

View File

@ -97,8 +97,11 @@ fn new(green: GreenNode, errors: Vec<SyntaxError>) -> Parse<T> {
pub fn syntax_node(&self) -> SyntaxNode { pub fn syntax_node(&self) -> SyntaxNode {
SyntaxNode::new_root(self.green.clone()) 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 @@ pub fn tree(&self) -> T {
T::cast(self.syntax_node()).unwrap() T::cast(self.syntax_node()).unwrap()
} }
pub fn ok(self) -> Result<T, Arc<[SyntaxError]>> { pub fn ok(self) -> Result<T, Vec<SyntaxError>> {
match self.errors { match self.errors() {
Some(e) => Err(e), errors if !errors.is_empty() => Err(errors),
None => Ok(self.tree()), _ => Ok(self.tree()),
} }
} }
} }
@ -132,7 +135,7 @@ pub fn cast<N: AstNode>(self) -> Option<Parse<N>> {
impl Parse<SourceFile> { impl Parse<SourceFile> {
pub fn debug_dump(&self) -> String { pub fn debug_dump(&self) -> String {
let mut buf = format!("{:#?}", self.tree().syntax()); 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); format_to!(buf, "error {:?}: {}\n", err.range(), err);
} }
buf buf
@ -169,11 +172,9 @@ fn full_reparse(&self, indel: &Indel) -> Parse<SourceFile> {
impl SourceFile { impl SourceFile {
pub fn parse(text: &str) -> Parse<SourceFile> { pub fn parse(text: &str) -> Parse<SourceFile> {
let _p = tracing::span!(tracing::Level::INFO, "SourceFile::parse").entered(); 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()); let root = SyntaxNode::new_root(green.clone());
errors.extend(validation::validate(&root));
assert_eq!(root.kind(), SyntaxKind::SOURCE_FILE); assert_eq!(root.kind(), SyntaxKind::SOURCE_FILE);
Parse { Parse {
green, green,

View File

@ -39,7 +39,7 @@ fn benchmark_parser() {
let tree = { let tree = {
let _b = bench("parsing"); let _b = bench("parsing");
let p = SourceFile::parse(&data); 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()); assert_eq!(p.tree().syntax.text_range().len(), 352474.into());
p.tree() p.tree()
}; };
@ -57,7 +57,7 @@ fn validation_tests() {
dir_tests(&test_data_dir(), &["parser/validation"], "rast", |text, path| { dir_tests(&test_data_dir(), &["parser/validation"], "rast", |text, path| {
let parse = SourceFile::parse(text); let parse = SourceFile::parse(text);
let errors = parse.errors(); let errors = parse.errors();
assert_errors_are_present(errors, path); assert_errors_are_present(&errors, path);
parse.debug_dump() parse.debug_dump()
}); });
} }

View File

@ -15,34 +15,32 @@
SyntaxNode, SyntaxToken, TextSize, T, 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(); let _p = tracing::span!(tracing::Level::INFO, "parser::validate").entered();
// FIXME: // FIXME:
// * Add unescape validation of raw string literals and raw byte string literals // * Add unescape validation of raw string literals and raw byte string literals
// * Add validation of doc comments are being attached to nodes // * Add validation of doc comments are being attached to nodes
let mut errors = Vec::new();
for node in root.descendants() { for node in root.descendants() {
match_ast! { match_ast! {
match node { match node {
ast::Literal(it) => validate_literal(it, &mut errors), ast::Literal(it) => validate_literal(it, errors),
ast::Const(it) => validate_const(it, &mut errors), ast::Const(it) => validate_const(it, errors),
ast::BlockExpr(it) => block::validate_block_expr(it, &mut errors), ast::BlockExpr(it) => block::validate_block_expr(it, errors),
ast::FieldExpr(it) => validate_numeric_name(it.name_ref(), &mut errors), ast::FieldExpr(it) => validate_numeric_name(it.name_ref(), errors),
ast::RecordExprField(it) => validate_numeric_name(it.name_ref(), &mut errors), ast::RecordExprField(it) => validate_numeric_name(it.name_ref(), errors),
ast::Visibility(it) => validate_visibility(it, &mut errors), ast::Visibility(it) => validate_visibility(it, errors),
ast::RangeExpr(it) => validate_range_expr(it, &mut errors), ast::RangeExpr(it) => validate_range_expr(it, errors),
ast::PathSegment(it) => validate_path_keywords(it, &mut errors), ast::PathSegment(it) => validate_path_keywords(it, errors),
ast::RefType(it) => validate_trait_object_ref_ty(it, &mut errors), ast::RefType(it) => validate_trait_object_ref_ty(it, errors),
ast::PtrType(it) => validate_trait_object_ptr_ty(it, &mut errors), ast::PtrType(it) => validate_trait_object_ptr_ty(it, errors),
ast::FnPtrType(it) => validate_trait_object_fn_ptr_ret_ty(it, &mut errors), ast::FnPtrType(it) => validate_trait_object_fn_ptr_ret_ty(it, errors),
ast::MacroRules(it) => validate_macro_rules(it, &mut errors), ast::MacroRules(it) => validate_macro_rules(it, errors),
ast::LetExpr(it) => validate_let_expr(it, &mut errors), ast::LetExpr(it) => validate_let_expr(it, errors),
_ => (), _ => (),
} }
} }
} }
errors
} }
fn rustc_unescape_error_to_string(err: unescape::EscapeError) -> (&'static str, bool) { fn rustc_unescape_error_to_string(err: unescape::EscapeError) -> (&'static str, bool) {